Skip to content

Commit 0c0d3db

Browse files
authored
[flake8-bugbear] Add fix for duplicate-value (B033) (#9510)
## Summary Adds autofix for [B033](https://docs.astral.sh/ruff/rules/duplicate-value/) ## Test Plan `cargo test`
1 parent 953d48b commit 0c0d3db

File tree

6 files changed

+342
-12
lines changed

6 files changed

+342
-12
lines changed

crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B033.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,28 @@
22
# Errors.
33
###
44
incorrect_set = {"value1", 23, 5, "value1"}
5+
incorrect_set = {1, 1, 2}
6+
incorrect_set_multiline = {
7+
"value1",
8+
23,
9+
5,
10+
"value1",
11+
# B033
12+
}
513
incorrect_set = {1, 1}
14+
incorrect_set = {1, 1,}
15+
incorrect_set = {0, 1, 1,}
16+
incorrect_set = {0, 1, 1}
17+
incorrect_set = {
18+
0,
19+
1,
20+
1,
21+
}
622

723
###
824
# Non-errors.
925
###
1026
correct_set = {"value1", 23, 5}
1127
correct_set = {5, "5"}
28+
correct_set = {5}
29+
correct_set = {}

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -980,9 +980,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
980980
flake8_pie::rules::unnecessary_spread(checker, dict);
981981
}
982982
}
983-
Expr::Set(ast::ExprSet { elts, range: _ }) => {
983+
Expr::Set(set) => {
984984
if checker.enabled(Rule::DuplicateValue) {
985-
flake8_bugbear::rules::duplicate_value(checker, elts);
985+
flake8_bugbear::rules::duplicate_value(checker, set);
986986
}
987987
}
988988
Expr::Yield(_) => {

crates/ruff_linter/src/rules/flake8_bugbear/mod.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ mod tests {
1212

1313
use crate::assert_messages;
1414
use crate::registry::Rule;
15+
use crate::settings::types::PreviewMode;
1516
use crate::settings::LinterSettings;
1617
use crate::test::test_path;
1718

@@ -69,6 +70,24 @@ mod tests {
6970
Ok(())
7071
}
7172

73+
#[test_case(Rule::DuplicateValue, Path::new("B033.py"))]
74+
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
75+
let snapshot = format!(
76+
"preview__{}_{}",
77+
rule_code.noqa_code(),
78+
path.to_string_lossy()
79+
);
80+
let diagnostics = test_path(
81+
Path::new("flake8_bugbear").join(path).as_path(),
82+
&LinterSettings {
83+
preview: PreviewMode::Enabled,
84+
..LinterSettings::for_rule(rule_code)
85+
},
86+
)?;
87+
assert_messages!(snapshot, diagnostics);
88+
Ok(())
89+
}
90+
7291
#[test]
7392
fn zip_without_explicit_strict() -> Result<()> {
7493
let snapshot = "B905.py";

crates/ruff_linter/src/rules/flake8_bugbear/rules/duplicate_value.rs

Lines changed: 62 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
1-
use ruff_python_ast::Expr;
1+
use anyhow::{Context, Result};
22
use rustc_hash::FxHashSet;
33

4-
use ruff_diagnostics::{Diagnostic, Violation};
4+
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
55
use ruff_macros::{derive_message_formats, violation};
6+
use ruff_python_ast as ast;
67
use ruff_python_ast::comparable::ComparableExpr;
8+
use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
79
use ruff_text_size::Ranged;
810

911
use crate::checkers::ast::Checker;
@@ -31,28 +33,82 @@ pub struct DuplicateValue {
3133
}
3234

3335
impl Violation for DuplicateValue {
36+
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
37+
3438
#[derive_message_formats]
3539
fn message(&self) -> String {
3640
let DuplicateValue { value } = self;
3741
format!("Sets should not contain duplicate item `{value}`")
3842
}
43+
44+
fn fix_title(&self) -> Option<String> {
45+
Some(format!("Remove duplicate item"))
46+
}
3947
}
4048

4149
/// B033
42-
pub(crate) fn duplicate_value(checker: &mut Checker, elts: &Vec<Expr>) {
50+
pub(crate) fn duplicate_value(checker: &mut Checker, set: &ast::ExprSet) {
4351
let mut seen_values: FxHashSet<ComparableExpr> = FxHashSet::default();
44-
for elt in elts {
52+
for (index, elt) in set.elts.iter().enumerate() {
4553
if elt.is_literal_expr() {
4654
let comparable_value: ComparableExpr = elt.into();
4755

4856
if !seen_values.insert(comparable_value) {
49-
checker.diagnostics.push(Diagnostic::new(
57+
let mut diagnostic = Diagnostic::new(
5058
DuplicateValue {
5159
value: checker.generator().expr(elt),
5260
},
5361
elt.range(),
54-
));
62+
);
63+
64+
if checker.settings.preview.is_enabled() {
65+
diagnostic.try_set_fix(|| {
66+
remove_member(set, index, checker.locator().contents()).map(Fix::safe_edit)
67+
});
68+
}
69+
70+
checker.diagnostics.push(diagnostic);
5571
}
5672
};
5773
}
5874
}
75+
76+
/// Remove the member at the given index from the [`ast::ExprSet`].
77+
fn remove_member(set: &ast::ExprSet, index: usize, source: &str) -> Result<Edit> {
78+
if index < set.elts.len() - 1 {
79+
// Case 1: the expression is _not_ the last node, so delete from the start of the
80+
// expression to the end of the subsequent comma.
81+
// Ex) Delete `"a"` in `{"a", "b", "c"}`.
82+
let mut tokenizer = SimpleTokenizer::starts_at(set.elts[index].end(), source);
83+
84+
// Find the trailing comma.
85+
tokenizer
86+
.find(|token| token.kind == SimpleTokenKind::Comma)
87+
.context("Unable to find trailing comma")?;
88+
89+
// Find the next non-whitespace token.
90+
let next = tokenizer
91+
.find(|token| {
92+
token.kind != SimpleTokenKind::Whitespace && token.kind != SimpleTokenKind::Newline
93+
})
94+
.context("Unable to find next token")?;
95+
96+
Ok(Edit::deletion(set.elts[index].start(), next.start()))
97+
} else if index > 0 {
98+
// Case 2: the expression is the last node, but not the _only_ node, so delete from the
99+
// start of the previous comma to the end of the expression.
100+
// Ex) Delete `"c"` in `{"a", "b", "c"}`.
101+
let mut tokenizer = SimpleTokenizer::starts_at(set.elts[index - 1].end(), source);
102+
103+
// Find the trailing comma.
104+
let comma = tokenizer
105+
.find(|token| token.kind == SimpleTokenKind::Comma)
106+
.context("Unable to find trailing comma")?;
107+
108+
Ok(Edit::deletion(comma.start(), set.elts[index].end()))
109+
} else {
110+
// Case 3: expression is the only node, so delete it.
111+
// Ex) Delete `"a"` in `{"a"}`.
112+
Ok(Edit::range_deletion(set.elts[index].range()))
113+
}
114+
}

crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B033_B033.py.snap

Lines changed: 72 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,85 @@ B033.py:4:35: B033 Sets should not contain duplicate item `"value1"`
77
3 | ###
88
4 | incorrect_set = {"value1", 23, 5, "value1"}
99
| ^^^^^^^^ B033
10-
5 | incorrect_set = {1, 1}
10+
5 | incorrect_set = {1, 1, 2}
11+
6 | incorrect_set_multiline = {
1112
|
13+
= help: Remove duplicate item
1214

1315
B033.py:5:21: B033 Sets should not contain duplicate item `1`
1416
|
1517
3 | ###
1618
4 | incorrect_set = {"value1", 23, 5, "value1"}
17-
5 | incorrect_set = {1, 1}
19+
5 | incorrect_set = {1, 1, 2}
1820
| ^ B033
19-
6 |
20-
7 | ###
21+
6 | incorrect_set_multiline = {
22+
7 | "value1",
2123
|
24+
= help: Remove duplicate item
25+
26+
B033.py:10:5: B033 Sets should not contain duplicate item `"value1"`
27+
|
28+
8 | 23,
29+
9 | 5,
30+
10 | "value1",
31+
| ^^^^^^^^ B033
32+
11 | # B033
33+
12 | }
34+
|
35+
= help: Remove duplicate item
36+
37+
B033.py:13:21: B033 Sets should not contain duplicate item `1`
38+
|
39+
11 | # B033
40+
12 | }
41+
13 | incorrect_set = {1, 1}
42+
| ^ B033
43+
14 | incorrect_set = {1, 1,}
44+
15 | incorrect_set = {0, 1, 1,}
45+
|
46+
= help: Remove duplicate item
47+
48+
B033.py:14:21: B033 Sets should not contain duplicate item `1`
49+
|
50+
12 | }
51+
13 | incorrect_set = {1, 1}
52+
14 | incorrect_set = {1, 1,}
53+
| ^ B033
54+
15 | incorrect_set = {0, 1, 1,}
55+
16 | incorrect_set = {0, 1, 1}
56+
|
57+
= help: Remove duplicate item
58+
59+
B033.py:15:24: B033 Sets should not contain duplicate item `1`
60+
|
61+
13 | incorrect_set = {1, 1}
62+
14 | incorrect_set = {1, 1,}
63+
15 | incorrect_set = {0, 1, 1,}
64+
| ^ B033
65+
16 | incorrect_set = {0, 1, 1}
66+
17 | incorrect_set = {
67+
|
68+
= help: Remove duplicate item
69+
70+
B033.py:16:24: B033 Sets should not contain duplicate item `1`
71+
|
72+
14 | incorrect_set = {1, 1,}
73+
15 | incorrect_set = {0, 1, 1,}
74+
16 | incorrect_set = {0, 1, 1}
75+
| ^ B033
76+
17 | incorrect_set = {
77+
18 | 0,
78+
|
79+
= help: Remove duplicate item
80+
81+
B033.py:20:5: B033 Sets should not contain duplicate item `1`
82+
|
83+
18 | 0,
84+
19 | 1,
85+
20 | 1,
86+
| ^ B033
87+
21 | }
88+
|
89+
= help: Remove duplicate item
2290

2391

0 commit comments

Comments
 (0)