Avoid removing shadowed imports that point to different symbols (#10387)

This ensures that we don't have incorrect, automated fixes for shadowed
names that actually point to different imports.

See: https://github.com/astral-sh/ruff/issues/10384.
This commit is contained in:
Charlie Marsh 2024-03-13 08:44:28 -07:00 committed by GitHub
parent 2bf1882398
commit d59433b12e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 46 additions and 51 deletions

View file

@ -0,0 +1,6 @@
"""Regression test for: https://github.com/astral-sh/ruff/issues/10384"""
import datetime
from datetime import datetime
datetime(1, 2, 3)

View file

@ -259,23 +259,29 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
diagnostic.set_parent(range.start()); diagnostic.set_parent(range.start());
} }
if let Some(import) = binding.as_any_import() { // Remove the import if the binding and the shadowed binding are both imports,
if let Some(source) = binding.source { // and both point to the same qualified name.
diagnostic.try_set_fix(|| { if let Some(shadowed_import) = shadowed.as_any_import() {
let statement = checker.semantic().statement(source); if let Some(import) = binding.as_any_import() {
let parent = checker.semantic().parent_statement(source); if shadowed_import.qualified_name() == import.qualified_name() {
let edit = fix::edits::remove_unused_imports( if let Some(source) = binding.source {
std::iter::once(import.member_name().as_ref()), diagnostic.try_set_fix(|| {
statement, let statement = checker.semantic().statement(source);
parent, let parent = checker.semantic().parent_statement(source);
checker.locator(), let edit = fix::edits::remove_unused_imports(
checker.stylist(), std::iter::once(import.member_name().as_ref()),
checker.indexer(), statement,
)?; parent,
Ok(Fix::safe_edit(edit).isolate(Checker::isolation( checker.locator(),
checker.semantic().parent_statement_id(source), checker.stylist(),
))) checker.indexer(),
}); )?;
Ok(Fix::safe_edit(edit).isolate(Checker::isolation(
checker.semantic().parent_statement_id(source),
)))
});
}
}
} }
} }

View file

@ -124,6 +124,7 @@ mod tests {
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_25.py"))] #[test_case(Rule::RedefinedWhileUnused, Path::new("F811_25.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_26.py"))] #[test_case(Rule::RedefinedWhileUnused, Path::new("F811_26.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_27.py"))] #[test_case(Rule::RedefinedWhileUnused, Path::new("F811_27.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_28.py"))]
#[test_case(Rule::UndefinedName, Path::new("F821_0.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_1.py"))]
#[test_case(Rule::UndefinedName, Path::new("F821_2.py"))] #[test_case(Rule::UndefinedName, Path::new("F821_2.py"))]

View file

@ -1,15 +1,9 @@
--- ---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs source: crates/ruff_linter/src/rules/pyflakes/mod.rs
--- ---
F811_1.py:1:25: F811 [*] Redefinition of unused `FU` from line 1 F811_1.py:1:25: F811 Redefinition of unused `FU` from line 1
| |
1 | import fu as FU, bar as FU 1 | import fu as FU, bar as FU
| ^^ F811 | ^^ F811
| |
= help: Remove definition: `FU` = help: Remove definition: `FU`
Safe fix
1 |-import fu as FU, bar as FU
1 |+import fu as FU

View file

@ -1,7 +1,7 @@
--- ---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs source: crates/ruff_linter/src/rules/pyflakes/mod.rs
--- ---
F811_12.py:6:20: F811 [*] Redefinition of unused `mixer` from line 2 F811_12.py:6:20: F811 Redefinition of unused `mixer` from line 2
| |
4 | pass 4 | pass
5 | else: 5 | else:
@ -10,13 +10,3 @@ F811_12.py:6:20: F811 [*] Redefinition of unused `mixer` from line 2
7 | mixer(123) 7 | mixer(123)
| |
= help: Remove definition: `mixer` = help: Remove definition: `mixer`
Safe fix
3 3 | except ImportError:
4 4 | pass
5 5 | else:
6 |- from bb import mixer
6 |+ pass
7 7 | mixer(123)

View file

@ -1,15 +1,9 @@
--- ---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs source: crates/ruff_linter/src/rules/pyflakes/mod.rs
--- ---
F811_2.py:1:34: F811 [*] Redefinition of unused `FU` from line 1 F811_2.py:1:34: F811 Redefinition of unused `FU` from line 1
| |
1 | from moo import fu as FU, bar as FU 1 | from moo import fu as FU, bar as FU
| ^^ F811 | ^^ F811
| |
= help: Remove definition: `FU` = help: Remove definition: `FU`
Safe fix
1 |-from moo import fu as FU, bar as FU
1 |+from moo import fu as FU

View file

@ -1,18 +1,10 @@
--- ---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs source: crates/ruff_linter/src/rules/pyflakes/mod.rs
--- ---
F811_23.py:4:15: F811 [*] Redefinition of unused `foo` from line 3 F811_23.py:4:15: F811 Redefinition of unused `foo` from line 3
| |
3 | import foo as foo 3 | import foo as foo
4 | import bar as foo 4 | import bar as foo
| ^^^ F811 | ^^^ F811
| |
= help: Remove definition: `foo` = help: Remove definition: `foo`
Safe fix
1 1 | """Test that shadowing an explicit re-export produces a warning."""
2 2 |
3 3 | import foo as foo
4 |-import bar as foo

View file

@ -0,0 +1,12 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---
F811_28.py:4:22: F811 Redefinition of unused `datetime` from line 3
|
3 | import datetime
4 | from datetime import datetime
| ^^^^^^^^ F811
5 |
6 | datetime(1, 2, 3)
|
= help: Remove definition: `datetime`