feat(jans-cedarling): add custom linter for inefficient string concatenation

.github/workflows/test-cedarling.yml

@@ -29,6 +29,33 @@ 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
+    - uses: cargo-bins/cargo-binstall@main
+    - name: Install dylint
+      run: |
+        cargo binstall cargo-dylint --version 5.0.0 --locked
+        cargo binstall 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:

jans-cedarling/Cargo.toml

@@ -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"] }

jans-cedarling/cedarling/src/common/default_entities.rs

@@ -5,6 +5,8 @@
 
 use base64::prelude::*;
 use cedar_policy::Entity;
+use cedar_policy::EntityId;
+use cedar_policy::EntityTypeName;
 use cedar_policy::EntityUid;
 use cedar_policy::ExpressionConstructionError;
 use cedar_policy::RestrictedExpression;
@@ -401,16 +403,17 @@ fn parse_cedar_format<'a>(
                 parent_obj.get("id").and_then(|v| v.as_str()),
             )
         {
-            let parent_uid_str = format!("{parent_entity_type}::\"{id_v}\"");
-            match EntityUid::from_str(&parent_uid_str) {
-                Ok(parent_uid) => {
+            let entity_id = EntityId::from_str(id_v).unwrap_or_else(|e| match e {});
+            match EntityTypeName::from_str(parent_entity_type) {
+                Ok(type_name) => {
+                    let parent_uid = EntityUid::from_type_name_and_id(type_name, entity_id);
                     parents_set.insert(parent_uid);
                 },
                 Err(e) => {
                     // log warn that we could not parse uid
                     warns.push(DefaultEntityWarning::InvalidParentUid {
                         entry_id: entry_id.to_string(),
-                        parent_uid_str: parent_uid_str.clone(),
+                        parent_uid_str: format!("{parent_entity_type}::\"{id_v}\""),
                         error: e.to_string(),
                     });
                 },

jans-cedarling/custom-lints/.cargo/config.toml

@@ -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"

jans-cedarling/custom-lints/.gitignore

@@ -0,0 +1 @@
+/target

jans-cedarling/custom-lints/Cargo.toml

@@ -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

jans-cedarling/custom-lints/README.md

@@ -0,0 +1,81 @@
+# Custom Lints for Cedarling
+
+This directory contains custom Rust lints specifically designed for the Cedarling project to enforce best practices and catch common performance issues.
+
+## Overview
+
+This custom linter package detects inefficient usage patterns in Cedar policy code, specifically:
+
+- **BAD_STRING_CONCATENATION**: Detects `EntityUid::from_str(&format!(...))` patterns which are explicitly warned against in Cedar documentation for performance reasons.
+
+## Usage
+
+### Running custom lints
+
+```bash
+# Build the custom lint library
+cargo build
+
+# Run the linter in project directory
+cargo dylint --all --path custom-lints
+```
+
+## Development
+
+### Creating New Custom Lints
+
+1. **Add a new lint declaration** in `src/lib.rs`:
+
+````rust
+dylint_linting::declare_late_lint! {
+    /// ### What it does
+    /// Brief description of what the lint checks for.
+    ///
+    /// ### Why is this bad?
+    /// Explain why this pattern should be avoided.
+    ///
+    /// ### Example
+    /// ```rust
+    /// // Bad example
+    /// ```
+    ///
+    /// Use instead:
+    ///
+    /// ```rust
+    /// // Good example
+    /// ```
+    pub YOUR_LINT_NAME,
+    Warn,
+    "lint description"
+}
+````
+
+2. **Implement the lint logic** in either `LateLintPass` or `EarlyLintPass` implementation
+
+3. **Add UI tests** in the `ui/` directory:
+    - Create test cases in `ui/<your_lint_name>.rs`
+    - Update expected output in `ui/<your_lint_name>.stderr`
+
+4. **Run tests** to verify your implementation:
+
+```bash
+cargo test
+```
+
+## Resources
+
+Helpful resources for writing lints:
+
+- [Dylint Documentation](https://github.com/trailofbits/dylint#quick-start)
+- [Rustc Dev Guide](https://rustc-dev-guide.rust-lang.org/)
+- [Adding a new lint](https://github.com/rust-lang/rust-clippy/blob/master/book/src/development/adding_lints.md)
+- [Common tools for writing lints](https://github.com/rust-lang/rust-clippy/blob/master/book/src/development/common_tools_writing_lints.md)
+- [Crate `rustc_hir`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_hir/index.html)
+- [Struct `rustc_lint::LateContext`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/struct.LateContext.html)
+
+### Key Concepts
+
+- **Late Lint Pass**: Runs after type checking, allowing analysis of typed expressions
+- **AST vs HIR**: Abstract Syntax Tree vs High-Level Intermediate Representation
+- **Span**: Location information in source code
+- **Diagnostic**: Error/warning messages with suggestions

jans-cedarling/custom-lints/rust-toolchain

@@ -0,0 +1,3 @@
+[toolchain]
+channel = "nightly-2025-09-18"
+components = ["llvm-tools-preview", "rustc-dev"]

jans-cedarling/custom-lints/src/lib.rs

@@ -0,0 +1,153 @@
+// 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, span_lint_and_then},
+    source::snippet,
+    sym,
+};
+use rustc_ast::LitKind::Str;
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind, Node, QPath, TyKind, def::Res::Local};
+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 or a definition
+            match exp.kind {
+                ExprKind::Path(QPath::Resolved(_, path)) => {
+                    if let Local(id) = path.res
+                        && let Node::Pat(pattern) = cx.tcx.hir_node(id)
+                        && let Node::LetStmt(parent) = cx.tcx.parent_hir_node(pattern.hir_id)
+                        && let Some(init) = parent.init
+                        && let ExprKind::Call(_, [arg]) = &init.kind
+                        && extract_format_args(arg).is_some()
+                    {
+                        span_lint_and_then(
+                            cx,
+                            BAD_STRING_CONCATENATION,
+                            expr.span,
+                            "using `EntityUid::from_str` with format! is inefficient",
+                            |diag| {
+                                diag.span_note(parent.span, "variable defined here");
+                            },
+                        );
+                    }
+                },
+                ExprKind::Call(_, [arg]) => {
+                    // Checks if the given expression is a format macro
+                    if let Some([format_arg]) = extract_format_args(arg) {
+                        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 and returns the arguments
+fn extract_format_args<'tcx>(expr: &Expr<'tcx>) -> Option<&'tcx [Expr<'tcx>]> {
+    if let ExprKind::Block(block, _) = expr.kind
+        && let Some(bexpr) = block.expr
+        && let ExprKind::Call(_, args) = &bexpr.kind
+    {
+        return Some(args);
+    }
+
+    None
+}
+
+// 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");
+}

jans-cedarling/custom-lints/ui/main.rs

@@ -0,0 +1,43 @@
+#![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""#));
+
+    // This should be flagged as well since it also has an impact on the performance
+    let raw_eid = format!(r#"{entity_type_name}::"{entity_id}""#);
+    let eid9 = EntityUid::from_str(&raw_eid);
+}
+
+// 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)
+        }
+    }
+}