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 e6505608a03ee..066ddb7c846ce 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 66bdad7880be3..f6d94f67fa9c0 100644 --- a/crates/ruff/src/rules/pylint/rules/comparison_with_itself.rs +++ b/crates/ruff/src/rules/pylint/rules/comparison_with_itself.rs @@ -1,8 +1,8 @@ use itertools::Itertools; -use ruff_python_ast::{CmpOp, Expr, Ranged}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{CmpOp, Expr, Ranged}; use crate::checkers::ast::Checker; use crate::rules::pylint::helpers::CmpOpExt; @@ -51,17 +51,64 @@ 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 => { 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_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 to pure function, like `id`. + if matches!( + left_func.id.as_str(), + "id" | "len" | "type" | "int" | "bool" | "str" | "repr" | "bytes" + ) && checker.semantic().is_builtin(&left_func.id) + { + checker.diagnostics.push(Diagnostic::new( + ComparisonWithItself { + left: checker.generator().expr(left), + op: *op, + right: checker.generator().expr(right), + }, + 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 3fffdeb481f63..711834227e64a 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. |