Skip to content

Commit 98fccec

Browse files
Avoid removing too many imports in redefined-while-unused (#15585)
## Summary Closes #15583.
1 parent 444f799 commit 98fccec

File tree

4 files changed

+45
-4
lines changed

4 files changed

+45
-4
lines changed
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
"""Regression test for: https://github.com/astral-sh/ruff/issues/15583"""
2+
3+
from typing import (
4+
List,
5+
List,
6+
)
7+
8+
9+
def foo() -> List[int]: ...

crates/ruff_linter/src/fix/codemods.rs

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use libcst_native::{
77
Codegen, CodegenState, Expression, ImportNames, NameOrAttribute, ParenthesizableWhitespace,
88
SmallStatement, Statement,
99
};
10-
use rustc_hash::FxHashSet;
10+
use rustc_hash::{FxHashMap, FxHashSet};
1111
use smallvec::{smallvec, SmallVec};
1212
use unicode_normalization::UnicodeNormalization;
1313

@@ -81,10 +81,20 @@ pub(crate) fn remove_imports<'a>(
8181
// Preserve the trailing comma (or not) from the last entry.
8282
let trailing_comma = aliases.last().and_then(|alias| alias.comma.clone());
8383

84-
// Remove any imports that are specified in the `imports` iterator.
85-
let member_names = member_names.collect::<FxHashSet<_>>();
84+
// Remove any imports that are specified in the `imports` iterator (but, e.g., if the name is
85+
// provided once, only remove the first occurrence).
86+
let mut counts = member_names.fold(FxHashMap::<&str, usize>::default(), |mut map, name| {
87+
map.entry(name).and_modify(|c| *c += 1).or_insert(1);
88+
map
89+
});
8690
aliases.retain(|alias| {
87-
!member_names.contains(qualified_name_from_name_or_attribute(&alias.name).as_str())
91+
let name = qualified_name_from_name_or_attribute(&alias.name);
92+
if let Some(count) = counts.get_mut(name.as_str()).filter(|count| **count > 0) {
93+
*count -= 1;
94+
false
95+
} else {
96+
true
97+
}
8898
});
8999

90100
// But avoid destroying any trailing comments.

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ mod tests {
129129
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_29.pyi"))]
130130
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_30.py"))]
131131
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_31.py"))]
132+
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_32.py"))]
132133
#[test_case(Rule::UndefinedName, Path::new("F821_0.py"))]
133134
#[test_case(Rule::UndefinedName, Path::new("F821_1.py"))]
134135
#[test_case(Rule::UndefinedName, Path::new("F821_2.py"))]
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
3+
---
4+
F811_32.py:5:5: F811 [*] Redefinition of unused `List` from line 4
5+
|
6+
3 | from typing import (
7+
4 | List,
8+
5 | List,
9+
| ^^^^ F811
10+
6 | )
11+
|
12+
= help: Remove definition: `List`
13+
14+
Safe fix
15+
2 2 |
16+
3 3 | from typing import (
17+
4 4 | List,
18+
5 |- List,
19+
6 5 | )
20+
7 6 |
21+
8 7 |

0 commit comments

Comments
 (0)