Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add autofix for PLR1714 #7910

Merged
merged 4 commits into from
Oct 11, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
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!("Consider merging multiple comparisons. Use a `set` if the elements are hashable.")
}
}

/// 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,33 @@ 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()
.map(|comparator| comparator.to_owned().clone())
.collect(),
diceroll123 marked this conversation as resolved.
Show resolved Hide resolved
range: TextRange::default(),
ctx: ExprContext::Load,
})],
range: bool_op.range(),
}));

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

checker.diagnostics.push(diagnostic);
}
}
}
Expand Down
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: Consider merging multiple comparisons. Use a `set` if the elements are hashable.

repeated_equality_comparison.py:4:1: PLR1714 Consider merging multiple comparisons: `foo not in ("a", "b")`. Use a `set` if the elements are hashable.
ℹ 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: Consider merging multiple comparisons. Use a `set` if the elements are hashable.

ℹ 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: Consider merging multiple comparisons. Use a `set` if the elements are hashable.

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.
ℹ 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: Consider merging multiple comparisons. Use a `set` if the elements are hashable.

ℹ 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: Consider merging multiple comparisons. Use a `set` if the elements are hashable.

repeated_equality_comparison.py:12:1: PLR1714 Consider merging multiple comparisons: `foo in ("a", "b", "c")`. Use a `set` if the elements are hashable.
ℹ 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: Consider merging multiple comparisons. Use a `set` if the elements are hashable.

ℹ 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: Consider merging multiple comparisons. Use a `set` if the elements are hashable.

ℹ 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: Consider merging multiple comparisons. Use a `set` if the elements are hashable.

repeated_equality_comparison.py:18:1: PLR1714 Consider merging multiple comparisons: `foo in (bar, baz, qux)`. Use a `set` if the elements are hashable.
ℹ 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: Consider merging multiple comparisons. Use a `set` if the elements are hashable.

ℹ 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: Consider merging multiple comparisons. Use a `set` if the elements are hashable.

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.
ℹ 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: Consider merging multiple comparisons. Use a `set` if the elements are hashable.

ℹ 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: Consider merging multiple comparisons. Use a `set` if the elements are hashable.

repeated_equality_comparison.py:24:1: PLR1714 Consider merging multiple comparisons: `bar in ("c", "d")`. Use a `set` if the elements are hashable.
ℹ 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: Consider merging multiple comparisons. Use a `set` if the elements are hashable.

ℹ 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: Consider merging multiple comparisons. Use a `set` if the elements are hashable.

ℹ 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 `==`.