Skip to content

Commit

Permalink
Avoid removing too many imports in redefined-while-unused (#15585)
Browse files Browse the repository at this point in the history
## Summary

Closes #15583.
  • Loading branch information
charliermarsh authored Jan 19, 2025
1 parent 444f799 commit 98fccec
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
"""Regression test for: https://github.com/astral-sh/ruff/issues/15583"""

from typing import (
List,
List,
)


def foo() -> List[int]: ...
18 changes: 14 additions & 4 deletions crates/ruff_linter/src/fix/codemods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use libcst_native::{
Codegen, CodegenState, Expression, ImportNames, NameOrAttribute, ParenthesizableWhitespace,
SmallStatement, Statement,
};
use rustc_hash::FxHashSet;
use rustc_hash::{FxHashMap, FxHashSet};
use smallvec::{smallvec, SmallVec};
use unicode_normalization::UnicodeNormalization;

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

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

// But avoid destroying any trailing comments.
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pyflakes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ mod tests {
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_29.pyi"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_30.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_31.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_32.py"))]
#[test_case(Rule::UndefinedName, Path::new("F821_0.py"))]
#[test_case(Rule::UndefinedName, Path::new("F821_1.py"))]
#[test_case(Rule::UndefinedName, Path::new("F821_2.py"))]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---
F811_32.py:5:5: F811 [*] Redefinition of unused `List` from line 4
|
3 | from typing import (
4 | List,
5 | List,
| ^^^^ F811
6 | )
|
= help: Remove definition: `List`

Safe fix
2 2 |
3 3 | from typing import (
4 4 | List,
5 |- List,
6 5 | )
7 6 |
8 7 |

0 comments on commit 98fccec

Please sign in to comment.