Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 10 additions & 0 deletions iam-policy-autopilot-lints/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
97 changes: 97 additions & 0 deletions iam-policy-autopilot-lints/src/convert_case_pascal.rs
Original file line number Diff line number Diff line change
@@ -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));
}
12 changes: 12 additions & 0 deletions iam-policy-autopilot-lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
24 changes: 21 additions & 3 deletions iam-policy-autopilot-lints/src/node_kind_literal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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));
}
43 changes: 42 additions & 1 deletion iam-policy-autopilot-lints/ui/main.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Test cases for node_kind_literal lint
// Test cases for node_kind_literal and convert_case_pascal lints

struct Node;

Expand Down Expand Up @@ -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();
}
11 changes: 10 additions & 1 deletion iam-policy-autopilot-lints/ui/main.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Loading