From 6a630a68a20a0f76a8b63e78de9ff3f337aace15 Mon Sep 17 00:00:00 2001 From: Matthias Schlaipfer Date: Thu, 18 Dec 2025 15:46:03 +0000 Subject: [PATCH] chore: add to_case(Case::Pascal) lint --- iam-policy-autopilot-lints/README.md | 10 ++ .../src/convert_case_pascal.rs | 97 +++++++++++++++++++ iam-policy-autopilot-lints/src/lib.rs | 12 +++ .../src/node_kind_literal.rs | 24 ++++- iam-policy-autopilot-lints/ui/main.rs | 43 +++++++- iam-policy-autopilot-lints/ui/main.stderr | 11 ++- 6 files changed, 192 insertions(+), 5 deletions(-) create mode 100644 iam-policy-autopilot-lints/src/convert_case_pascal.rs diff --git a/iam-policy-autopilot-lints/README.md b/iam-policy-autopilot-lints/README.md index dce9664..0f6ca96 100644 --- a/iam-policy-autopilot-lints/README.md +++ b/iam-policy-autopilot-lints/README.md @@ -23,6 +23,16 @@ if node.kind() == COMPOSITE_LITERAL { } ``` +### `convert_case_pascal` + +Detects calls to `convert_case::to_case` with `Case::Pascal` as an argument. It is inviting to use such case conversion to rename Python snake_case method names to operation names, which are often--**but not always**--Pascal case (see https://github.com/awslabs/iam-policy-autopilot/issues/66). + +**Bad:** +```rust +use convert_case::{Case, Casing}; +let result = some_string.to_case(Case::Pascal); +``` + ## Usage ### Install dylint diff --git a/iam-policy-autopilot-lints/src/convert_case_pascal.rs b/iam-policy-autopilot-lints/src/convert_case_pascal.rs new file mode 100644 index 0000000..1b321fb --- /dev/null +++ b/iam-policy-autopilot-lints/src/convert_case_pascal.rs @@ -0,0 +1,97 @@ +//! Lint to detect calls to convert_case's to_case with Case::Pascal argument + +use clippy_utils::diagnostics::span_lint_and_help; +use rustc_hir::{Expr, ExprKind, QPath}; +use rustc_lint::{LateContext, LateLintPass, LintPass, LintStore}; +use rustc_session::{declare_lint, Session}; + +declare_lint! { + /// ### What it does + /// Detects calls to `convert_case::to_case` with `Case::Pascal` as an argument. + /// + /// ### Why is this bad? + /// Using `Case::Pascal` with `to_case` might indicate a pattern that should be + /// avoided or replaced with a more appropriate alternative in this codebase. + /// + /// ### Example + /// ```rust + /// use convert_case::{Case, Casing}; + /// + /// // This will trigger the lint + /// let result = some_string.to_case(Case::Pascal); + /// ``` + /// + /// Consider using an alternative approach or a different case conversion. + pub CONVERT_CASE_PASCAL, + Deny, + "use of convert_case::to_case with Case::Pascal argument" +} + +pub struct ConvertCasePascal; + +/// Check if an expression is a path that matches Case::Pascal +fn is_case_pascal_path(expr: &Expr<'_>) -> bool { + if let ExprKind::Path(QPath::Resolved(_, path)) = &expr.kind { + // Check if the path ends with Pascal and has Case as a segment + let segments: Vec<&str> = path + .segments + .iter() + .map(|s| s.ident.name.as_str()) + .collect(); + + // Look for patterns like Case::Pascal or convert_case::Case::Pascal + if segments.len() >= 2 { + let last_two = &segments[segments.len() - 2..]; + return last_two == ["Case", "Pascal"]; + } + } + false +} + +/// Check if an expression is a method call to to_case +fn is_to_case_method_call<'tcx>(expr: &'tcx Expr<'_>) -> Option<&'tcx [Expr<'tcx>]> { + if let ExprKind::MethodCall(path_segment, _receiver, args, _) = &expr.kind { + if path_segment.ident.name.as_str() == "to_case" { + return Some(args); + } + } + None +} + +impl LintPass for ConvertCasePascal { + fn name(&self) -> &'static str { + "ConvertCasePascal" + } + + fn get_lints(&self) -> Vec<&'static rustc_lint::Lint> { + vec![&CONVERT_CASE_PASCAL] + } +} + +impl<'tcx> LateLintPass<'tcx> for ConvertCasePascal { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + // Check if this is a method call to to_case + if let Some(args) = is_to_case_method_call(expr) { + // Check if the first argument is Case::Pascal + if let Some(first_arg) = args.first() { + if is_case_pascal_path(first_arg) { + let help = "if you are converting a method name to an operation name, using PascalCase conversion is not what you want: Use ServiceModelIndex::method_lookup instead."; + + span_lint_and_help( + cx, + CONVERT_CASE_PASCAL, + expr.span, + "calling `to_case` with `Case::Pascal`", + None, + help, + ); + } + } + } + } +} + +pub fn register_lints(_sess: &Session, lint_store: &mut LintStore) { + lint_store.register_lints(&[&CONVERT_CASE_PASCAL]); + lint_store.register_late_pass(|_| Box::new(ConvertCasePascal)); +} diff --git a/iam-policy-autopilot-lints/src/lib.rs b/iam-policy-autopilot-lints/src/lib.rs index e6fb5b3..ae85a02 100644 --- a/iam-policy-autopilot-lints/src/lib.rs +++ b/iam-policy-autopilot-lints/src/lib.rs @@ -3,11 +3,23 @@ #![feature(rustc_private)] #![warn(unused_extern_crates)] +dylint_linting::dylint_library!(); + extern crate rustc_ast; extern crate rustc_hir; +extern crate rustc_lint; +extern crate rustc_session; +mod convert_case_pascal; mod node_kind_literal; +#[expect(clippy::no_mangle_with_rust_abi)] +#[unsafe(no_mangle)] +pub fn register_lints(sess: &rustc_session::Session, lint_store: &mut rustc_lint::LintStore) { + node_kind_literal::register_lints(sess, lint_store); + convert_case_pascal::register_lints(sess, lint_store); +} + #[test] fn ui() { dylint_testing::ui_test(env!("CARGO_PKG_NAME"), "ui"); diff --git a/iam-policy-autopilot-lints/src/node_kind_literal.rs b/iam-policy-autopilot-lints/src/node_kind_literal.rs index 44d4eb6..061e0ba 100644 --- a/iam-policy-autopilot-lints/src/node_kind_literal.rs +++ b/iam-policy-autopilot-lints/src/node_kind_literal.rs @@ -3,9 +3,10 @@ use clippy_utils::diagnostics::span_lint_and_help; use rustc_ast::LitKind; use rustc_hir::{BinOpKind, Expr, ExprKind}; -use rustc_lint::LateLintPass; +use rustc_lint::{LateContext, LateLintPass, LintPass, LintStore}; +use rustc_session::{declare_lint, Session}; -dylint_linting::declare_late_lint! { +declare_lint! { /// ### What it does /// Detects string literals used in comparisons with `.kind()` method calls, /// which typically indicate Tree-sitter node kind checks that should use constants. @@ -38,6 +39,8 @@ dylint_linting::declare_late_lint! { "use of string literals in comparisons with .kind() method calls" } +pub struct NodeKindLiteral; + /// Check if an expression is a call to the `.kind()` method on a tree-sitter Node fn is_kind_method_call<'tcx>(cx: &rustc_lint::LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool { if let ExprKind::MethodCall(path_segment, receiver, _, _) = &expr.kind { @@ -55,8 +58,18 @@ fn is_kind_method_call<'tcx>(cx: &rustc_lint::LateContext<'tcx>, expr: &'tcx Exp } } +impl LintPass for NodeKindLiteral { + fn name(&self) -> &'static str { + "NodeKindLiteral" + } + + fn get_lints(&self) -> Vec<&'static rustc_lint::Lint> { + vec![&NODE_KIND_LITERAL] + } +} + impl<'tcx> LateLintPass<'tcx> for NodeKindLiteral { - fn check_expr(&mut self, cx: &rustc_lint::LateContext<'tcx>, expr: &'tcx Expr<'_>) { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { // Check if this is a binary operation (== or !=) if let ExprKind::Binary(op, left, right) = &expr.kind { // Only check equality and inequality operations @@ -111,3 +124,8 @@ impl<'tcx> LateLintPass<'tcx> for NodeKindLiteral { } } } + +pub fn register_lints(_sess: &Session, lint_store: &mut LintStore) { + lint_store.register_lints(&[&NODE_KIND_LITERAL]); + lint_store.register_late_pass(|_| Box::new(NodeKindLiteral)); +} diff --git a/iam-policy-autopilot-lints/ui/main.rs b/iam-policy-autopilot-lints/ui/main.rs index 446a51a..bcd639e 100644 --- a/iam-policy-autopilot-lints/ui/main.rs +++ b/iam-policy-autopilot-lints/ui/main.rs @@ -1,4 +1,4 @@ -// Test cases for node_kind_literal lint +// Test cases for node_kind_literal and convert_case_pascal lints struct Node; @@ -73,8 +73,49 @@ fn test_non_node_kind() { } } +// Test cases for convert_case_pascal lint + +// Mock convert_case types and traits for testing +mod convert_case { + pub enum Case { + Pascal, + Snake, + Camel, + } +} + +trait Casing { + fn to_case(&self, case: convert_case::Case) -> String; +} + +impl Casing for str { + fn to_case(&self, _case: convert_case::Case) -> String { + self.to_string() + } +} + +fn test_convert_case_pascal() { + let text = "hello_world"; + + // This should trigger a warning - using Case::Pascal + let _result1 = text.to_case(convert_case::Case::Pascal); + + // This should NOT trigger a warning - using different case + let _result2 = text.to_case(convert_case::Case::Snake); + let _result3 = text.to_case(convert_case::Case::Camel); +} + +fn test_other_to_case_calls() { + let text = "test_string"; + + // This should NOT trigger a warning - not using Case::Pascal + let _result = text.to_case(convert_case::Case::Snake); +} + fn main() { test_kind_comparisons(); test_allowed_comparisons(); test_non_node_kind(); + test_convert_case_pascal(); + test_other_to_case_calls(); } diff --git a/iam-policy-autopilot-lints/ui/main.stderr b/iam-policy-autopilot-lints/ui/main.stderr index df075bd..d8e40c1 100644 --- a/iam-policy-autopilot-lints/ui/main.stderr +++ b/iam-policy-autopilot-lints/ui/main.stderr @@ -31,5 +31,14 @@ LL | if node.kind() == "some_new_node_type" { | = help: define and use a constant like `const SOME_NEW_NODE_TYPE: &str = "some_new_node_type";` in a node_kinds module -error: aborting due to 4 previous errors +error: calling `to_case` with `Case::Pascal` + --> $DIR/main.rs:101:20 + | +LL | let _result1 = text.to_case(convert_case::Case::Pascal); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: if you are converting a method name to an operation name, using PascalCase conversion is not what you want: Use ServiceModelIndex::method_lookup instead. + = note: `#[deny(convert_case_pascal)]` on by default + +error: aborting due to 5 previous errors