Skip to content

Commit

Permalink
Flag comparison-with-itself on builtin calls
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Aug 4, 2023
1 parent d3aa8b4 commit 2fd16d8
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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)
59 changes: 54 additions & 5 deletions crates/ruff/src/rules/pylint/rules/comparison_with_itself.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
));
}
_ => {}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
|


0 comments on commit 2fd16d8

Please sign in to comment.