mirror of
https://github.com/astral-sh/ruff
synced 2026-01-21 05:20:49 -05:00
[flake8-bugbear] Mark fix unsafe when it would remove comments (B033) (#22632)
The B033 erroneously removes comments in safe fix.
By running
```console
$ cargo run -p ruff -- check --select B033 - << 'EOF'
{
1,
# comment
1
}
EOF
```
We get:
# Before
```console
B033 [*] Sets should not contain duplicate item `1`
--> -:4:3
|
2 | 1,
3 | # comment
4 | 1
| ^
5 | }
|
help: Remove duplicate item
Found 1 error.
[*] 1 fixable with the `--fix` option.
```
# After
```console
B033 Sets should not contain duplicate item `1`
--> -:4:3
|
2 | 1,
3 | # comment
4 | 1
| ^
5 | }
|
help: Remove duplicate item
Found 1 error.
No fixes available (1 hidden fix can be enabled with the `--unsafe-fixes` option).
```
Closes #22629
This commit is contained in:
@@ -20,6 +20,12 @@ incorrect_set = {
|
||||
1,
|
||||
}
|
||||
incorrect_set = {False, 1, 0}
|
||||
incorrect_set_multiline_with_comment = {
|
||||
"value1",
|
||||
23,
|
||||
# B033
|
||||
"value1",
|
||||
}
|
||||
|
||||
###
|
||||
# Non-errors.
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
use ruff_diagnostics::Fix;
|
||||
use ruff_diagnostics::{Applicability, Fix};
|
||||
use rustc_hash::FxHashMap;
|
||||
|
||||
use ruff_macros::{ViolationMetadata, derive_message_formats};
|
||||
@@ -28,6 +28,20 @@ use crate::{FixAvailability, Violation};
|
||||
/// ```python
|
||||
/// {1, 2, 3}
|
||||
/// ```
|
||||
///
|
||||
/// ## Fix Safety
|
||||
/// This rule's fix is marked as unsafe if the replacement would remove comments attached to the
|
||||
/// original expression, potentially losing important context or documentation.
|
||||
///
|
||||
/// For example:
|
||||
/// ```python
|
||||
/// {
|
||||
/// 1,
|
||||
/// 2,
|
||||
/// # Comment
|
||||
/// 1,
|
||||
/// }
|
||||
/// ```
|
||||
#[derive(ViolationMetadata)]
|
||||
#[violation_metadata(stable_since = "v0.0.271")]
|
||||
pub(crate) struct DuplicateValue {
|
||||
@@ -68,10 +82,18 @@ pub(crate) fn duplicate_value(checker: &Checker, set: &ast::ExprSet) {
|
||||
},
|
||||
value.range(),
|
||||
);
|
||||
|
||||
diagnostic.try_set_fix(|| {
|
||||
edits::remove_member(&set.elts, index, checker.locator().contents())
|
||||
.map(Fix::safe_edit)
|
||||
edits::remove_member(&set.elts, index, checker.locator().contents()).map(
|
||||
|edit| {
|
||||
let applicability = if checker.comment_ranges().intersects(edit.range())
|
||||
{
|
||||
Applicability::Unsafe
|
||||
} else {
|
||||
Applicability::Safe
|
||||
};
|
||||
Fix::applicable_edit(edit, applicability)
|
||||
},
|
||||
)
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -157,7 +157,7 @@ help: Remove duplicate item
|
||||
- 1,
|
||||
20 | }
|
||||
21 | incorrect_set = {False, 1, 0}
|
||||
22 |
|
||||
22 | incorrect_set_multiline_with_comment = {
|
||||
|
||||
B033 [*] Sets should not contain duplicate items, but `False` and `0` has the same value
|
||||
--> B033.py:22:28
|
||||
@@ -166,8 +166,8 @@ B033 [*] Sets should not contain duplicate items, but `False` and `0` has the sa
|
||||
21 | }
|
||||
22 | incorrect_set = {False, 1, 0}
|
||||
| ^
|
||||
23 |
|
||||
24 | ###
|
||||
23 | incorrect_set_multiline_with_comment = {
|
||||
24 | "value1",
|
||||
|
|
||||
help: Remove duplicate item
|
||||
19 | 1,
|
||||
@@ -175,6 +175,26 @@ help: Remove duplicate item
|
||||
21 | }
|
||||
- incorrect_set = {False, 1, 0}
|
||||
22 + incorrect_set = {False, 1}
|
||||
23 |
|
||||
24 | ###
|
||||
25 | # Non-errors.
|
||||
23 | incorrect_set_multiline_with_comment = {
|
||||
24 | "value1",
|
||||
25 | 23,
|
||||
|
||||
B033 [*] Sets should not contain duplicate item `"value1"`
|
||||
--> B033.py:27:5
|
||||
|
|
||||
25 | 23,
|
||||
26 | # B033
|
||||
27 | "value1",
|
||||
| ^^^^^^^^
|
||||
28 | }
|
||||
|
|
||||
help: Remove duplicate item
|
||||
23 | incorrect_set_multiline_with_comment = {
|
||||
24 | "value1",
|
||||
25 | 23,
|
||||
- # B033
|
||||
- "value1",
|
||||
26 | }
|
||||
27 |
|
||||
28 | ###
|
||||
note: This is an unsafe fix and may change runtime behavior
|
||||
|
||||
Reference in New Issue
Block a user