diff --git a/crates/ruff/resources/test/fixtures/pylint/comparison_with_itself.py b/crates/ruff/resources/test/fixtures/pylint/comparison_with_itself.py index e6505608a03ee0..066ddb7c846cec 100644 --- a/crates/ruff/resources/test/fixtures/pylint/comparison_with_itself.py +++ b/crates/ruff/resources/test/fixtures/pylint/comparison_with_itself.py @@ -19,6 +19,10 @@ foo not in foo +id(foo) == id(foo) + +len(foo) == len(foo) + # Non-errors. "foo" == "foo" # This is flagged by `comparison-of-constant` instead. @@ -43,3 +47,11 @@ foo in bar foo not in bar + +x(foo) == y(foo) + +id(foo) == id(bar) + +id(foo, bar) == id(foo, bar) + +id(foo, bar=1) == id(foo, bar=1) diff --git a/crates/ruff/src/rules/pylint/rules/comparison_with_itself.rs b/crates/ruff/src/rules/pylint/rules/comparison_with_itself.rs index 66bdad7880be32..67335faa4746aa 100644 --- a/crates/ruff/src/rules/pylint/rules/comparison_with_itself.rs +++ b/crates/ruff/src/rules/pylint/rules/comparison_with_itself.rs @@ -51,17 +51,66 @@ pub(crate) fn comparison_with_itself( .tuple_windows() .zip(ops) { - if let (Expr::Name(left), Expr::Name(right)) = (left, right) { - if left.id == right.id { + match (left, right) { + // Ex) `foo == foo` + (Expr::Name(left_name), Expr::Name(right_name)) => { + if left_name.id != right_name.id { + continue; + } + + checker.diagnostics.push(Diagnostic::new( + ComparisonWithItself { + left: checker.generator().expr(left), + op: *op, + right: checker.generator().expr(right), + }, + left_name.range(), + )); + } + // Ex) `id(foo) == id(foo)` + (Expr::Call(left_call), Expr::Call(right_call)) => { + // Both calls must take a single argument, of the same name. + if !left_call.arguments.keywords.is_empty() + || !right_call.arguments.keywords.is_empty() + { + continue; + } + let [Expr::Name(left_arg)] = left_call.arguments.args.as_slice() else { + continue; + }; + let [Expr::Name(right_right)] = right_call.arguments.args.as_slice() else { + continue; + }; + if left_arg.id != right_right.id { + continue; + } + + // Both calls must be to the same function. + let Expr::Name(left_func) = left_call.func.as_ref() else { + continue; + }; + let Expr::Name(right_func) = right_call.func.as_ref() else { + continue; + }; + if left_func.id != right_func.id { + continue; + } + + // The call must be a builtin (e.g., `id`). + if !checker.semantic().is_builtin(&left_func.id) { + continue; + } + checker.diagnostics.push(Diagnostic::new( ComparisonWithItself { - left: left.id.to_string(), + left: checker.generator().expr(left), op: *op, - right: right.id.to_string(), + right: checker.generator().expr(right), }, - left.range(), + left_call.range(), )); } + _ => {} } } } diff --git a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR0124_comparison_with_itself.py.snap b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR0124_comparison_with_itself.py.snap index 3fffdeb481f631..711834227e64a8 100644 --- a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR0124_comparison_with_itself.py.snap +++ b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR0124_comparison_with_itself.py.snap @@ -97,7 +97,27 @@ comparison_with_itself.py:20:1: PLR0124 Name compared with itself, consider repl 20 | foo not in foo | ^^^ PLR0124 21 | -22 | # Non-errors. +22 | id(foo) == id(foo) + | + +comparison_with_itself.py:22:1: PLR0124 Name compared with itself, consider replacing `id(foo) == id(foo)` + | +20 | foo not in foo +21 | +22 | id(foo) == id(foo) + | ^^^^^^^ PLR0124 +23 | +24 | len(foo) == len(foo) + | + +comparison_with_itself.py:24:1: PLR0124 Name compared with itself, consider replacing `len(foo) == len(foo)` + | +22 | id(foo) == id(foo) +23 | +24 | len(foo) == len(foo) + | ^^^^^^^^ PLR0124 +25 | +26 | # Non-errors. |