Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid removing too many imports in redefined-while-unused #15585

Merged
merged 1 commit into from
Jan 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 |
Loading