Skip to content

Commit

Permalink
add autofix for PLR1714 (#7910)
Browse files Browse the repository at this point in the history 
## Summary

Add autofix for `PLR1714` using tuples.

If added complexity is desired, we can lean into the `set` part by doing
some kind of builtin check on all of the comparator elements for
starters, since we otherwise don't know if something's hashable.

## Test Plan

`cargo test`, and manually.
  • Loading branch information
@diceroll123
diceroll123 committed Oct 11, 2023
1 parent f670f9b commit 1835d7b
Show file tree
Hide file tree
Showing 2 changed files with 200 additions and 19 deletions.
39 changes: 34 additions & 5 deletions crates/ruff_linter/src/rules/pylint/rules/repeated_equality_comparison.rs
View file
Original file line number Diff line number Diff line change
@@ -1,19 +1,21 @@
use std::hash::BuildHasherDefault;
use std::ops::Deref;

use ast::ExprContext;
use itertools::{any, Itertools};
use rustc_hash::FxHashMap;

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::comparable::ComparableExpr;
use ruff_python_ast::hashable::HashableExpr;
use ruff_python_ast::{self as ast, BoolOp, CmpOp, Expr};
use ruff_source_file::Locator;
use ruff_text_size::{Ranged, TextSize};
use ruff_text_size::{Ranged, TextRange, TextSize};

use crate::checkers::ast::Checker;
use crate::fix::snippet::SourceCodeSnippet;
use crate::registry::AsRule;

/// ## What it does
/// Checks for repeated equality comparisons that can rewritten as a membership
Expand Down Expand Up @@ -48,7 +50,7 @@ pub struct RepeatedEqualityComparison {
expression: SourceCodeSnippet,
}

impl Violation for RepeatedEqualityComparison {
impl AlwaysFixableViolation for RepeatedEqualityComparison {
#[derive_message_formats]
fn message(&self) -> String {
let RepeatedEqualityComparison { expression } = self;
Expand All @@ -62,6 +64,10 @@ impl Violation for RepeatedEqualityComparison {
)
}
}

fn fix_title(&self) -> String {
format!("Merge multiple comparisons")
}
}

/// PLR1714
Expand Down Expand Up @@ -115,7 +121,7 @@ pub(crate) fn repeated_equality_comparison(checker: &mut Checker, bool_op: &ast:
.sorted_by_key(|(_, (start, _))| *start)
{
if comparators.len() > 1 {
checker.diagnostics.push(Diagnostic::new(
let mut diagnostic = Diagnostic::new(
RepeatedEqualityComparison {
expression: SourceCodeSnippet::new(merged_membership_test(
value.as_expr(),
Expand All @@ -125,7 +131,30 @@ pub(crate) fn repeated_equality_comparison(checker: &mut Checker, bool_op: &ast:
)),
},
bool_op.range(),
));
);

if checker.patch(diagnostic.kind.rule()) {
let content = checker.generator().expr(&Expr::Compare(ast::ExprCompare {
left: Box::new(value.as_expr().clone()),
ops: match bool_op.op {
BoolOp::Or => vec![CmpOp::In],
BoolOp::And => vec![CmpOp::NotIn],
},
comparators: vec![Expr::Tuple(ast::ExprTuple {
elts: comparators.iter().copied().cloned().collect(),
range: TextRange::default(),
ctx: ExprContext::Load,
})],
range: bool_op.range(),
}));

diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
content,
bool_op.range(),
)));
}

checker.diagnostics.push(diagnostic);
}
}
}
Expand Down
180 changes: 166 additions & 14 deletions ...snapshots/ruff_linter__rules__pylint__tests__PLR1714_repeated_equality_comparison.py.snap
View file
Original file line number Diff line number Diff line change
@@ -1,16 +1,25 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
repeated_equality_comparison.py:2:1: PLR1714 Consider merging multiple comparisons: `foo in ("a", "b")`. Use a `set` if the elements are hashable.
repeated_equality_comparison.py:2:1: PLR1714 [*] Consider merging multiple comparisons: `foo in ("a", "b")`. Use a `set` if the elements are hashable.
|
1 | # Errors.
2 | foo == "a" or foo == "b"
| ^^^^^^^^^^^^^^^^^^^^^^^^ PLR1714
3 |
4 | foo != "a" and foo != "b"
|
= help: Merge multiple comparisons

repeated_equality_comparison.py:4:1: PLR1714 Consider merging multiple comparisons: `foo not in ("a", "b")`. Use a `set` if the elements are hashable.
ℹ Suggested fix
1 1 | # Errors.
2 |-foo == "a" or foo == "b"
2 |+foo in ("a", "b")
3 3 |
4 4 | foo != "a" and foo != "b"
5 5 |

repeated_equality_comparison.py:4:1: PLR1714 [*] Consider merging multiple comparisons: `foo not in ("a", "b")`. Use a `set` if the elements are hashable.
|
2 | foo == "a" or foo == "b"
3 |
Expand All @@ -19,8 +28,19 @@ repeated_equality_comparison.py:4:1: PLR1714 Consider merging multiple compariso
5 |
6 | foo == "a" or foo == "b" or foo == "c"
|
= help: Merge multiple comparisons

ℹ Suggested fix
1 1 | # Errors.
2 2 | foo == "a" or foo == "b"
3 3 |
4 |-foo != "a" and foo != "b"
4 |+foo not in ("a", "b")
5 5 |
6 6 | foo == "a" or foo == "b" or foo == "c"
7 7 |

repeated_equality_comparison.py:6:1: PLR1714 Consider merging multiple comparisons: `foo in ("a", "b", "c")`. Use a `set` if the elements are hashable.
repeated_equality_comparison.py:6:1: PLR1714 [*] Consider merging multiple comparisons: `foo in ("a", "b", "c")`. Use a `set` if the elements are hashable.
|
4 | foo != "a" and foo != "b"
5 |
Expand All @@ -29,8 +49,19 @@ repeated_equality_comparison.py:6:1: PLR1714 Consider merging multiple compariso
7 |
8 | foo != "a" and foo != "b" and foo != "c"
|
= help: Merge multiple comparisons

repeated_equality_comparison.py:8:1: PLR1714 Consider merging multiple comparisons: `foo not in ("a", "b", "c")`. Use a `set` if the elements are hashable.
ℹ Suggested fix
3 3 |
4 4 | foo != "a" and foo != "b"
5 5 |
6 |-foo == "a" or foo == "b" or foo == "c"
6 |+foo in ("a", "b", "c")
7 7 |
8 8 | foo != "a" and foo != "b" and foo != "c"
9 9 |

repeated_equality_comparison.py:8:1: PLR1714 [*] Consider merging multiple comparisons: `foo not in ("a", "b", "c")`. Use a `set` if the elements are hashable.
|
6 | foo == "a" or foo == "b" or foo == "c"
7 |
Expand All @@ -39,8 +70,19 @@ repeated_equality_comparison.py:8:1: PLR1714 Consider merging multiple compariso
9 |
10 | foo == a or foo == "b" or foo == 3 # Mixed types.
|
= help: Merge multiple comparisons

ℹ Suggested fix
5 5 |
6 6 | foo == "a" or foo == "b" or foo == "c"
7 7 |
8 |-foo != "a" and foo != "b" and foo != "c"
8 |+foo not in ("a", "b", "c")
9 9 |
10 10 | foo == a or foo == "b" or foo == 3 # Mixed types.
11 11 |

repeated_equality_comparison.py:10:1: PLR1714 Consider merging multiple comparisons: `foo in (a, "b", 3)`. Use a `set` if the elements are hashable.
repeated_equality_comparison.py:10:1: PLR1714 [*] Consider merging multiple comparisons: `foo in (a, "b", 3)`. Use a `set` if the elements are hashable.
|
8 | foo != "a" and foo != "b" and foo != "c"
9 |
Expand All @@ -49,8 +91,19 @@ repeated_equality_comparison.py:10:1: PLR1714 Consider merging multiple comparis
11 |
12 | "a" == foo or "b" == foo or "c" == foo
|
= help: Merge multiple comparisons

repeated_equality_comparison.py:12:1: PLR1714 Consider merging multiple comparisons: `foo in ("a", "b", "c")`. Use a `set` if the elements are hashable.
ℹ Suggested fix
7 7 |
8 8 | foo != "a" and foo != "b" and foo != "c"
9 9 |
10 |-foo == a or foo == "b" or foo == 3 # Mixed types.
10 |+foo in (a, "b", 3) # Mixed types.
11 11 |
12 12 | "a" == foo or "b" == foo or "c" == foo
13 13 |

repeated_equality_comparison.py:12:1: PLR1714 [*] Consider merging multiple comparisons: `foo in ("a", "b", "c")`. Use a `set` if the elements are hashable.
|
10 | foo == a or foo == "b" or foo == 3 # Mixed types.
11 |
Expand All @@ -59,8 +112,19 @@ repeated_equality_comparison.py:12:1: PLR1714 Consider merging multiple comparis
13 |
14 | "a" != foo and "b" != foo and "c" != foo
|
= help: Merge multiple comparisons

ℹ Suggested fix
9 9 |
10 10 | foo == a or foo == "b" or foo == 3 # Mixed types.
11 11 |
12 |-"a" == foo or "b" == foo or "c" == foo
12 |+foo in ("a", "b", "c")
13 13 |
14 14 | "a" != foo and "b" != foo and "c" != foo
15 15 |

repeated_equality_comparison.py:14:1: PLR1714 Consider merging multiple comparisons: `foo not in ("a", "b", "c")`. Use a `set` if the elements are hashable.
repeated_equality_comparison.py:14:1: PLR1714 [*] Consider merging multiple comparisons: `foo not in ("a", "b", "c")`. Use a `set` if the elements are hashable.
|
12 | "a" == foo or "b" == foo or "c" == foo
13 |
Expand All @@ -69,8 +133,19 @@ repeated_equality_comparison.py:14:1: PLR1714 Consider merging multiple comparis
15 |
16 | "a" == foo or foo == "b" or "c" == foo
|
= help: Merge multiple comparisons

ℹ Suggested fix
11 11 |
12 12 | "a" == foo or "b" == foo or "c" == foo
13 13 |
14 |-"a" != foo and "b" != foo and "c" != foo
14 |+foo not in ("a", "b", "c")
15 15 |
16 16 | "a" == foo or foo == "b" or "c" == foo
17 17 |

repeated_equality_comparison.py:16:1: PLR1714 Consider merging multiple comparisons: `foo in ("a", "b", "c")`. Use a `set` if the elements are hashable.
repeated_equality_comparison.py:16:1: PLR1714 [*] Consider merging multiple comparisons: `foo in ("a", "b", "c")`. Use a `set` if the elements are hashable.
|
14 | "a" != foo and "b" != foo and "c" != foo
15 |
Expand All @@ -79,8 +154,19 @@ repeated_equality_comparison.py:16:1: PLR1714 Consider merging multiple comparis
17 |
18 | foo == bar or baz == foo or qux == foo
|
= help: Merge multiple comparisons

repeated_equality_comparison.py:18:1: PLR1714 Consider merging multiple comparisons: `foo in (bar, baz, qux)`. Use a `set` if the elements are hashable.
ℹ Suggested fix
13 13 |
14 14 | "a" != foo and "b" != foo and "c" != foo
15 15 |
16 |-"a" == foo or foo == "b" or "c" == foo
16 |+foo in ("a", "b", "c")
17 17 |
18 18 | foo == bar or baz == foo or qux == foo
19 19 |

repeated_equality_comparison.py:18:1: PLR1714 [*] Consider merging multiple comparisons: `foo in (bar, baz, qux)`. Use a `set` if the elements are hashable.
|
16 | "a" == foo or foo == "b" or "c" == foo
17 |
Expand All @@ -89,8 +175,19 @@ repeated_equality_comparison.py:18:1: PLR1714 Consider merging multiple comparis
19 |
20 | foo == "a" or "b" == foo or foo == "c"
|
= help: Merge multiple comparisons

ℹ Suggested fix
15 15 |
16 16 | "a" == foo or foo == "b" or "c" == foo
17 17 |
18 |-foo == bar or baz == foo or qux == foo
18 |+foo in (bar, baz, qux)
19 19 |
20 20 | foo == "a" or "b" == foo or foo == "c"
21 21 |

repeated_equality_comparison.py:20:1: PLR1714 Consider merging multiple comparisons: `foo in ("a", "b", "c")`. Use a `set` if the elements are hashable.
repeated_equality_comparison.py:20:1: PLR1714 [*] Consider merging multiple comparisons: `foo in ("a", "b", "c")`. Use a `set` if the elements are hashable.
|
18 | foo == bar or baz == foo or qux == foo
19 |
Expand All @@ -99,8 +196,19 @@ repeated_equality_comparison.py:20:1: PLR1714 Consider merging multiple comparis
21 |
22 | foo != "a" and "b" != foo and foo != "c"
|
= help: Merge multiple comparisons

repeated_equality_comparison.py:22:1: PLR1714 Consider merging multiple comparisons: `foo not in ("a", "b", "c")`. Use a `set` if the elements are hashable.
ℹ Suggested fix
17 17 |
18 18 | foo == bar or baz == foo or qux == foo
19 19 |
20 |-foo == "a" or "b" == foo or foo == "c"
20 |+foo in ("a", "b", "c")
21 21 |
22 22 | foo != "a" and "b" != foo and foo != "c"
23 23 |

repeated_equality_comparison.py:22:1: PLR1714 [*] Consider merging multiple comparisons: `foo not in ("a", "b", "c")`. Use a `set` if the elements are hashable.
|
20 | foo == "a" or "b" == foo or foo == "c"
21 |
Expand All @@ -109,8 +217,19 @@ repeated_equality_comparison.py:22:1: PLR1714 Consider merging multiple comparis
23 |
24 | foo == "a" or foo == "b" or "c" == bar or "d" == bar # Multiple targets
|
= help: Merge multiple comparisons

ℹ Suggested fix
19 19 |
20 20 | foo == "a" or "b" == foo or foo == "c"
21 21 |
22 |-foo != "a" and "b" != foo and foo != "c"
22 |+foo not in ("a", "b", "c")
23 23 |
24 24 | foo == "a" or foo == "b" or "c" == bar or "d" == bar # Multiple targets
25 25 |

repeated_equality_comparison.py:24:1: PLR1714 Consider merging multiple comparisons: `foo in ("a", "b")`. Use a `set` if the elements are hashable.
repeated_equality_comparison.py:24:1: PLR1714 [*] Consider merging multiple comparisons: `foo in ("a", "b")`. Use a `set` if the elements are hashable.
|
22 | foo != "a" and "b" != foo and foo != "c"
23 |
Expand All @@ -119,8 +238,19 @@ repeated_equality_comparison.py:24:1: PLR1714 Consider merging multiple comparis
25 |
26 | foo.bar == "a" or foo.bar == "b" # Attributes.
|
= help: Merge multiple comparisons

repeated_equality_comparison.py:24:1: PLR1714 Consider merging multiple comparisons: `bar in ("c", "d")`. Use a `set` if the elements are hashable.
ℹ Suggested fix
21 21 |
22 22 | foo != "a" and "b" != foo and foo != "c"
23 23 |
24 |-foo == "a" or foo == "b" or "c" == bar or "d" == bar # Multiple targets
24 |+foo in ("a", "b") # Multiple targets
25 25 |
26 26 | foo.bar == "a" or foo.bar == "b" # Attributes.
27 27 |

repeated_equality_comparison.py:24:1: PLR1714 [*] Consider merging multiple comparisons: `bar in ("c", "d")`. Use a `set` if the elements are hashable.
|
22 | foo != "a" and "b" != foo and foo != "c"
23 |
Expand All @@ -129,8 +259,19 @@ repeated_equality_comparison.py:24:1: PLR1714 Consider merging multiple comparis
25 |
26 | foo.bar == "a" or foo.bar == "b" # Attributes.
|
= help: Merge multiple comparisons

ℹ Suggested fix
21 21 |
22 22 | foo != "a" and "b" != foo and foo != "c"
23 23 |
24 |-foo == "a" or foo == "b" or "c" == bar or "d" == bar # Multiple targets
24 |+bar in ("c", "d") # Multiple targets
25 25 |
26 26 | foo.bar == "a" or foo.bar == "b" # Attributes.
27 27 |

repeated_equality_comparison.py:26:1: PLR1714 Consider merging multiple comparisons: `foo.bar in ("a", "b")`. Use a `set` if the elements are hashable.
repeated_equality_comparison.py:26:1: PLR1714 [*] Consider merging multiple comparisons: `foo.bar in ("a", "b")`. Use a `set` if the elements are hashable.
|
24 | foo == "a" or foo == "b" or "c" == bar or "d" == bar # Multiple targets
25 |
Expand All @@ -139,5 +280,16 @@ repeated_equality_comparison.py:26:1: PLR1714 Consider merging multiple comparis
27 |
28 | # OK
|
= help: Merge multiple comparisons

ℹ Suggested fix
23 23 |
24 24 | foo == "a" or foo == "b" or "c" == bar or "d" == bar # Multiple targets
25 25 |
26 |-foo.bar == "a" or foo.bar == "b" # Attributes.
26 |+foo.bar in ("a", "b") # Attributes.
27 27 |
28 28 | # OK
29 29 | foo == "a" and foo == "b" and foo == "c" # `and` mixed with `==`.


0 comments on commit 1835d7b

Please sign in to comment.