Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
f4eb2f5
feat(jans-cedarling): add custom linter for inefficient string concat…
dagregi Feb 2, 2026
5fe54b8
feat(jans-cedarling): update warning messages and provide suggestions
dagregi Feb 4, 2026
b341ee3
ci(jans-cedarling): update CI to include custom lints
dagregi Feb 4, 2026
12d7cae
chore(jans-cedarling): add missing license header
dagregi Feb 5, 2026
0a8cf7f
chore(jans-cedarling): update test file for the lint
dagregi Feb 5, 2026
14df962
ci(jans-cedarling): pin cargo-dylint and dylint-link version to 5.0.0
dagregi Feb 5, 2026
e4b22b5
fix(jans-cedarling): update lints to check for qualified paths as well
dagregi Feb 5, 2026
3a8bb00
fix(jans-cedarling): improve custom lint to use actual type snippet
dagregi Feb 5, 2026
d713e88
chore(jans-cedarling): fix minor typo in comment
dagregi Feb 5, 2026
d0dc044
ci(jans-cedarling): update custom lint action to use `cargo-binstall`
dagregi Feb 6, 2026
f3c8b64
feat(jans-cedarling): flag format macros passed as a variable
dagregi Feb 6, 2026
8d58e5d
fix(jans-cedarling): fix existing EntityUid parsing issues
dagregi Feb 6, 2026
af016f3
ci(jans-cedarling): fix invalid workflow file
dagregi Feb 6, 2026
a41c6ba
chore(jans-cedarling): pin cargo-binstall to 1.17.4 in workflow
dagregi Feb 6, 2026
f08fba6
chore(jans-cedarling): add --no-confirm to cargo binstall commands in CI
dagregi Feb 6, 2026
7460617
Merge branch 'main' into jans-cedarling-13010
moabu Feb 10, 2026
baf4b1f
ci: update .github/workflows/test-cedarling.yml
moabu Feb 10, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .github/workflows/test-cedarling.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,32 @@ jobs:
run: |
rustup component add clippy
cargo clippy --workspace --all-targets -- -D warnings
custom_lints:
runs-on: ubuntu-latest
steps:
- name: Harden Runner
uses: step-security/harden-runner@a4aa98b93cab29d9b1101a6143fb8bce00e2eac4 # v2.7.1
with:
egress-policy: audit
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
- name: Install Rust Nightly
uses: dtolnay/rust-toolchain@1ff72ee08e3cb84d84adba594e0a297990fc1ed3
with:
toolchain: nightly-2025-09-18
components: llvm-tools-preview, rustc-dev
- name: Install dylint
run: |
cargo install cargo-dylint --version 5.0.0 --locked
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can utilize cargo-binstall to download the binary instead of building it each time?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolved in d0dc044

cargo install dylint-link --version 5.0.0 --locked
- name: Run Tests
working-directory: jans-cedarling/custom-lints
run: cargo test
- name: Install Rust Stable (for workspace)
uses: dtolnay/rust-toolchain@1ff72ee08e3cb84d84adba594e0a297990fc1ed3 # stable
- name: Run custom lints on workspace
working-directory: jans-cedarling
run: |
RUSTFLAGS="-Dwarnings" cargo dylint --all --workspace --path custom-lints
rust_benchmarks:
runs-on: ubuntu-latest
steps:
Expand Down
2 changes: 1 addition & 1 deletion jans-cedarling/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[workspace]
resolver = "2"
members = ["bindings/*", "cedarling", "http_utils", "sparkv", "test_utils"]
exclude = ["bindings/cedarling-java"]
exclude = ["bindings/cedarling-java", "custom-lints"]

[workspace.dependencies]
serde = { version = "1.0", features = ["derive"] }
Expand Down
6 changes: 6 additions & 0 deletions jans-cedarling/custom-lints/.cargo/config.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[target.'cfg(all())']
rustflags = ["-C", "linker=dylint-link"]

# For Rust versions 1.74.0 and onward, the following alternative can be used
# (see https://github.com/rust-lang/cargo/pull/12535):
# linker = "dylint-link"
1 change: 1 addition & 0 deletions jans-cedarling/custom-lints/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/target
19 changes: 19 additions & 0 deletions jans-cedarling/custom-lints/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
[package]
name = "custom_lints"
version = "0.1.0"
description = "Custom lints for the Cedarling"
edition = "2024"
publish = false

[lib]
crate-type = ["cdylib"]

[dependencies]
clippy_utils = { git = "https://github.com/rust-lang/rust-clippy", rev = "20ce69b9a63bcd2756cd906fe0964d1e901e042a" }
dylint_linting = "5.0.0"

[dev-dependencies]
dylint_testing = "5.0.0"

[package.metadata.rust-analyzer]
rustc_private = true
3 changes: 3 additions & 0 deletions jans-cedarling/custom-lints/rust-toolchain
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[toolchain]
channel = "nightly-2025-09-18"
components = ["llvm-tools-preview", "rustc-dev"]
149 changes: 149 additions & 0 deletions jans-cedarling/custom-lints/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
// This software is available under the Apache-2.0 license.
// See https://www.apache.org/licenses/LICENSE-2.0.txt for full text.
//
// Copyright (c) 2024, Gluu, Inc.

#![feature(rustc_private)]
#![warn(unused_extern_crates)]

extern crate rustc_ast;
extern crate rustc_errors;
extern crate rustc_hir;
extern crate rustc_span;

use clippy_utils::{
diagnostics::{span_lint_and_help, span_lint_and_sugg},
source::snippet,
sym,
};
use rustc_ast::LitKind::Str;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, QPath, TyKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_span::{Symbol, source_map::Spanned};

dylint_linting::declare_late_lint! {
/// ### What it does
/// Checks for `EntityUid::from_str(&format!(...))` pattern which is explicitly
/// warned against in the Cedar documentation for performance reasons.
///
/// ### Why is this bad?
/// The Cedar crate documentation specifically warns that using `format!()`
/// with `EntityUid::from_str` is inefficient. Use string literals instead.
///
/// ### Example
///
/// ```rust
/// EntityUid::from_str(&format!("{}::\"{}\"", entity_type, id))
/// ```
///
/// Use instead:
///
/// ```rust
/// let entity_type_name = EntityTypeName::from_str(entity_type).unwrap();
/// let entity_id = EntityId::from_str(id).unwrap_or_else(|e| match e {});
///
/// EntityUid::from_type_name_and_id(entity_type_name, entity_id)
/// ```
pub BAD_STRING_CONCATENATION,
Warn,
"using EntityUid::from_str with format!() is inefficient"
}

impl<'tcx> LateLintPass<'tcx> for BadStringConcatenation {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
// This checks if the expression kind is a function call extracting
// the function itself and its arguments
if let ExprKind::Call(callee, [arg]) = &expr.kind
// Checks the function's kind is a path to definition e.g., Vec::new or EntityUid::from_str
&& let ExprKind::Path(QPath::TypeRelative(ty, segment)) = &callee.kind
// Checks the function's type is a path to a type e.g., Vec<T> or EntityUid
&& let TyKind::Path(QPath::Resolved(_, path)) = &ty.kind
// Checks for the type path segments identifier is `EntityUid` or not
// e.g., cedar_policy::EntityUid::from_str will have 2 path segments cedar_policy and EntityUid
&& path.segments.last().is_some_and(|s| s.ident.name.as_str().eq("EntityUid"))
// Checks if the function name is `from_str` or not
&& segment.ident.name == sym::from_str
// Checks if the argument of `from_str` is a reference and extract the expression inside
&& let ExprKind::AddrOf(_, _, exp) = arg.kind
// Checks if the argument inside `from_str` is a function call then extract the
// argument of said function
&& let ExprKind::Call(_, [format_call]) = exp.kind
&& let ExprKind::Block(block, _) = format_call.kind
&& is_format_macro(cx, block.expr)
&& let Some(bexpr) = block.expr
&& let ExprKind::Call(_, [format_arg]) = &bexpr.kind
{
if let Some(arg) = extract_string_literal(format_arg) {
let ty_snippet = snippet(cx, ty.span, "EntityUid");
span_lint_and_sugg(
cx,
BAD_STRING_CONCATENATION,
expr.span,
"using `EntityUid::from_str` with format! is inefficient",
"try this instead",
format!("{ty_snippet}::from_str({arg:?})"),
Applicability::MachineApplicable,
);
} else {
span_lint_and_help(
cx,
BAD_STRING_CONCATENATION,
expr.span,
"using `EntityUid::from_str` with format! is inefficient",
None,
"consider using `EntityUid::from_type_name_and_id` instead",
);
}
}
}
}

/// Checks if the given expression is a format macro or not
fn is_format_macro<'tcx>(cx: &LateContext<'tcx>, expr: Option<&Expr<'tcx>>) -> bool {
if let Some(expr) = expr
&& let ExprKind::Call(callee, _) = &expr.kind
&& let ExprKind::Path(QPath::Resolved(_, path)) = &callee.kind
{
if path.segments.len() >= 2 {
let last_two: Vec<_> = path
.segments
.iter()
.rev()
.take(2)
.map(|s| s.ident.name.as_str())
.collect();

if last_two[0] == "format" && last_two[1] == "fmt" {
return true;
}
}

// Fallback: check def_id
if let Some(def_id) = path.res.opt_def_id() {
let path_str = cx.tcx.def_path_str(def_id);
return matches!(path_str.as_str(), "alloc::fmt::format" | "std::fmt::format");
}
}
false
}

// Checks if the argument of format macro is a string literal and extract it
fn extract_string_literal(expr: &Expr<'_>) -> Option<Symbol> {
if let ExprKind::Call(_, [arg]) = expr.kind
&& let ExprKind::AddrOf(_, _, expr) = arg.kind
&& let ExprKind::Array([element]) = expr.kind
&& let ExprKind::Lit(Spanned {
node: Str(item, _), ..
}) = element.kind
{
return Some(item);
}

None
}

#[test]
fn ui() {
dylint_testing::ui_test(env!("CARGO_PKG_NAME"), "ui");
}
39 changes: 39 additions & 0 deletions jans-cedarling/custom-lints/ui/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
#![allow(unused)]

use cedar_policy::EntityUid;

fn main() {
// This should trigger the lint
let eid1 = EntityUid::from_str(&format!(r#"User::"alice""#));

// This should trigger the lint
let user = "alice";
let eid2 = EntityUid::from_str(&format!(r#"User::"{}""#, user));

// This should trigger the lint - (even with std::format!)
let eid3 = EntityUid::from_str(&std::format!(r#"User::"bob""#));

// This should NOT trigger
let eid4 = EntityUid::from_str(r#"User::"alice""#);

// This should NOT trigger
let literal = String::from(r#"User::"dave""#);
let eid5 = EntityUid::from_str(&literal);

let entity_type_name = "Jans::User";
let entity_id = "user123";
let eid7 = cedar_policy::EntityUid::from_str(&format!(r#"{entity_type_name}::"{entity_id}""#));

let eid8 = cedar_policy::EntityUid::from_str(&format!(r#"Jans::User::"bob123""#));
}

// Mock EntityUid for testing (since we don't want to pull in the actual Cedar crate)
mod cedar_policy {
pub(super) struct EntityUid;

impl EntityUid {
pub(super) fn from_str(s: &str) -> Result<Self, String> {
Ok(EntityUid)
}
}
}
38 changes: 38 additions & 0 deletions jans-cedarling/custom-lints/ui/main.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
warning: using `EntityUid::from_str` with format! is inefficient
--> $DIR/main.rs:7:16
|
LL | let eid1 = EntityUid::from_str(&format!(r#"User::"alice""#));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this instead: `EntityUid::from_str("User::/"alice/"")`
|
= note: `#[warn(bad_string_concatenation)]` on by default

warning: using `EntityUid::from_str` with format! is inefficient
--> $DIR/main.rs:11:16
|
LL | let eid2 = EntityUid::from_str(&format!(r#"User::"{}""#, user));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider using `EntityUid::from_type_name_and_id` instead

warning: using `EntityUid::from_str` with format! is inefficient
--> $DIR/main.rs:14:16
|
LL | let eid3 = EntityUid::from_str(&std::format!(r#"User::"bob""#));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this instead: `EntityUid::from_str("User::/"bob/"")`

warning: using `EntityUid::from_str` with format! is inefficient
--> $DIR/main.rs:25:16
|
LL | let eid7 = cedar_policy::EntityUid::from_str(&format!(r#"{entity_type_name}::"{entity_id}""#));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider using `EntityUid::from_type_name_and_id` instead

warning: using `EntityUid::from_str` with format! is inefficient
--> $DIR/main.rs:27:16
|
LL | let eid8 = cedar_policy::EntityUid::from_str(&format!(r#"Jans::User::"bob123""#));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this instead: `cedar_policy::EntityUid::from_str("Jans::User::/"bob123/"")`

warning: 5 warnings emitted