Skip to content

Commit d5a52c3

Browse files
committed
[ruff] Fix B033 removing comments in safe fix
1 parent 717d024 commit d5a52c3

3 files changed

Lines changed: 59 additions & 10 deletions

File tree

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,12 @@
2020
1,
2121
}
2222
incorrect_set = {False, 1, 0}
23+
incorrect_set_multiline_with_comment = {
24+
"value1",
25+
23,
26+
# B033
27+
"value1",
28+
}
2329

2430
###
2531
# Non-errors.

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

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use ruff_diagnostics::Fix;
1+
use ruff_diagnostics::{Applicability, Fix};
22
use rustc_hash::FxHashMap;
33

44
use ruff_macros::{ViolationMetadata, derive_message_formats};
@@ -28,6 +28,21 @@ use crate::{FixAvailability, Violation};
2828
/// ```python
2929
/// {1, 2, 3}
3030
/// ```
31+
///
32+
/// ## Fix Safety
33+
/// This rule's fix is marked as unsafe if the replacement would remove comments attached to the
34+
/// original expression, potentially losing important context or documentation.
35+
///
36+
/// For example:
37+
/// ```python
38+
/// {
39+
/// 1,
40+
/// 2,
41+
/// # Comment
42+
/// 1,
43+
/// }
44+
/// ```
45+
3146
#[derive(ViolationMetadata)]
3247
#[violation_metadata(stable_since = "v0.0.271")]
3348
pub(crate) struct DuplicateValue {
@@ -68,10 +83,18 @@ pub(crate) fn duplicate_value(checker: &Checker, set: &ast::ExprSet) {
6883
},
6984
value.range(),
7085
);
71-
7286
diagnostic.try_set_fix(|| {
73-
edits::remove_member(&set.elts, index, checker.locator().contents())
74-
.map(Fix::safe_edit)
87+
edits::remove_member(&set.elts, index, checker.locator().contents()).map(
88+
|edit| {
89+
let applicability = if checker.comment_ranges().intersects(edit.range())
90+
{
91+
Applicability::Unsafe
92+
} else {
93+
Applicability::Safe
94+
};
95+
Fix::applicable_edit(edit, applicability)
96+
},
97+
)
7598
});
7699
}
77100
}

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

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ help: Remove duplicate item
157157
- 1,
158158
20 | }
159159
21 | incorrect_set = {False, 1, 0}
160-
22 |
160+
22 | incorrect_set_multiline_with_comment = {
161161

162162
B033 [*] Sets should not contain duplicate items, but `False` and `0` has the same value
163163
--> B033.py:22:28
@@ -166,15 +166,35 @@ B033 [*] Sets should not contain duplicate items, but `False` and `0` has the sa
166166
21 | }
167167
22 | incorrect_set = {False, 1, 0}
168168
| ^
169-
23 |
170-
24 | ###
169+
23 | incorrect_set_multiline_with_comment = {
170+
24 | "value1",
171171
|
172172
help: Remove duplicate item
173173
19 | 1,
174174
20 | 1,
175175
21 | }
176176
- incorrect_set = {False, 1, 0}
177177
22 + incorrect_set = {False, 1}
178-
23 |
179-
24 | ###
180-
25 | # Non-errors.
178+
23 | incorrect_set_multiline_with_comment = {
179+
24 | "value1",
180+
25 | 23,
181+
182+
B033 [*] Sets should not contain duplicate item `"value1"`
183+
--> B033.py:27:5
184+
|
185+
25 | 23,
186+
26 | # B033
187+
27 | "value1",
188+
| ^^^^^^^^
189+
28 | }
190+
|
191+
help: Remove duplicate item
192+
23 | incorrect_set_multiline_with_comment = {
193+
24 | "value1",
194+
25 | 23,
195+
- # B033
196+
- "value1",
197+
26 | }
198+
27 |
199+
28 | ###
200+
note: This is an unsafe fix and may change runtime behavior

0 commit comments

Comments
 (0)