Skip to content

Commit

Permalink
Add a fix for redefinition-while-unused (#9419)
Browse files Browse the repository at this point in the history
## Summary

This PR enables Ruff to remove redefined imports, as in:

```python
import os
import os

print(os)
```

Previously, Ruff would flag `F811` on the second `import os`, but
couldn't fix it.

For now, this fix is marked as safe, but only available in preview.

Closes #3477.
  • Loading branch information
charliermarsh committed Jan 9, 2024
1 parent 985f1d1 commit 84ab21f
Show file tree
Hide file tree
Showing 49 changed files with 391 additions and 3 deletions.
29 changes: 27 additions & 2 deletions crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use ruff_diagnostics::Diagnostic;
use ruff_diagnostics::{Diagnostic, Fix};
use ruff_python_semantic::analyze::visibility;
use ruff_python_semantic::{Binding, BindingKind, ScopeKind};
use ruff_python_semantic::{Binding, BindingKind, Imported, ScopeKind};
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
use crate::codes::Rule;
use crate::fix;
use crate::rules::{
flake8_pyi, flake8_type_checking, flake8_unused_arguments, pyflakes, pylint, ruff,
};
Expand Down Expand Up @@ -247,9 +248,33 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
},
binding.range(),
);

if let Some(range) = binding.parent_range(&checker.semantic) {
diagnostic.set_parent(range.start());
}

if checker.settings.preview.is_enabled() {
if let Some(import) = binding.as_any_import() {
if let Some(source) = binding.source {
diagnostic.try_set_fix(|| {
let statement = checker.semantic().statement(source);
let parent = checker.semantic().parent_statement(source);
let edit = fix::edits::remove_unused_imports(
std::iter::once(import.member_name().as_ref()),
statement,
parent,
checker.locator(),
checker.stylist(),
checker.indexer(),
)?;
Ok(Fix::safe_edit(edit).isolate(Checker::isolation(
checker.semantic().parent_statement_id(source),
)))
});
}
}
}

diagnostics.push(diagnostic);
}
}
Expand Down
28 changes: 28 additions & 0 deletions crates/ruff_linter/src/rules/pyflakes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,34 @@ mod tests {
Ok(())
}

#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_0.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_1.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_10.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_11.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_12.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_13.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_14.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_15.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_16.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_17.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_18.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_19.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_2.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_20.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_21.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_22.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_23.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_24.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_27.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_3.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_4.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_5.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_6.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_7.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_8.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_9.py"))]
#[test_case(Rule::UnusedVariable, Path::new("F841_4.py"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use ruff_diagnostics::Violation;
use ruff_diagnostics::{FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_source_file::SourceRow;

Expand Down Expand Up @@ -29,9 +29,16 @@ pub struct RedefinedWhileUnused {
}

impl Violation for RedefinedWhileUnused {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;

#[derive_message_formats]
fn message(&self) -> String {
let RedefinedWhileUnused { name, row } = self;
format!("Redefinition of unused `{name}` from {row}")
}

fn fix_title(&self) -> Option<String> {
let RedefinedWhileUnused { name, .. } = self;
Some(format!("Remove definition: `{name}`"))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@ F811_0.py:10:5: F811 Redefinition of unused `bar` from line 6
| ^^^ F811
11 | pass
|
= help: Remove definition: `bar`


Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,6 @@ F811_1.py:1:25: F811 Redefinition of unused `FU` from line 1
1 | import fu as FU, bar as FU
| ^^ F811
|
= help: Remove definition: `FU`


Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@ F811_12.py:6:20: F811 Redefinition of unused `mixer` from line 2
| ^^^^^ F811
7 | mixer(123)
|
= help: Remove definition: `mixer`


Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@ F811_15.py:4:5: F811 Redefinition of unused `fu` from line 1
| ^^ F811
5 | pass
|
= help: Remove definition: `fu`


Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@ F811_16.py:8:13: F811 Redefinition of unused `fu` from line 3
| ^^ F811
9 | pass
|
= help: Remove definition: `fu`


Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ F811_17.py:6:12: F811 Redefinition of unused `fu` from line 2
7 |
8 | def baz():
|
= help: Remove definition: `fu`

F811_17.py:9:13: F811 Redefinition of unused `fu` from line 6
|
Expand All @@ -17,5 +18,6 @@ F811_17.py:9:13: F811 Redefinition of unused `fu` from line 6
| ^^ F811
10 | pass
|
= help: Remove definition: `fu`


Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,6 @@ F811_2.py:1:34: F811 Redefinition of unused `FU` from line 1
1 | from moo import fu as FU, bar as FU
| ^^ F811
|
= help: Remove definition: `FU`


Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@ F811_21.py:32:5: F811 Redefinition of unused `Sequence` from line 26
| ^^^^^^^^ F811
33 | )
|
= help: Remove definition: `Sequence`


Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@ F811_23.py:4:15: F811 Redefinition of unused `foo` from line 3
4 | import bar as foo
| ^^^ F811
|
= help: Remove definition: `foo`


Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@ F811_26.py:5:9: F811 Redefinition of unused `func` from line 2
| ^^^^ F811
6 | pass
|
= help: Remove definition: `func`


Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,6 @@ F811_3.py:1:12: F811 Redefinition of unused `fu` from line 1
1 | import fu; fu = 3
| ^^ F811
|
= help: Remove definition: `fu`


Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,6 @@ F811_4.py:1:12: F811 Redefinition of unused `fu` from line 1
1 | import fu; fu, bar = 3
| ^^ F811
|
= help: Remove definition: `fu`


Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,6 @@ F811_5.py:1:13: F811 Redefinition of unused `fu` from line 1
1 | import fu; [fu, bar] = 3
| ^^ F811
|
= help: Remove definition: `fu`


Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@ F811_6.py:6:12: F811 Redefinition of unused `os` from line 5
| ^^ F811
7 | os.path
|
= help: Remove definition: `os`


Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,6 @@ F811_8.py:5:12: F811 Redefinition of unused `os` from line 4
6 | except:
7 | pass
|
= help: Remove definition: `os`


Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,6 @@ source: crates/ruff_linter/src/rules/pyflakes/mod.rs
6 |
7 | # Despite this `del`, `import os` in `f` should still be flagged as shadowing an unused
|
= help: Remove definition: `os`


Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,6 @@ source: crates/ruff_linter/src/rules/pyflakes/mod.rs
| ^^ F811
6 | print(os)
|
= help: Remove definition: `os`


Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,6 @@ source: crates/ruff_linter/src/rules/pyflakes/mod.rs
5 |
6 | # Despite this `del`, `import os` should still be flagged as shadowing an unused
|
= help: Remove definition: `os`


Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---
F811_0.py:10:5: F811 Redefinition of unused `bar` from line 6
|
10 | def bar():
| ^^^ F811
11 | pass
|
= help: Remove definition: `bar`


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

Safe fix
1 |-import fu as FU, bar as FU
1 |+import fu as FU


Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---

Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---

Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---
F811_12.py:6:20: F811 [*] Redefinition of unused `mixer` from line 2
|
4 | pass
5 | else:
6 | from bb import mixer
| ^^^^^ F811
7 | mixer(123)
|
= 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)


Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---

Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---

Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---
F811_15.py:4:5: F811 Redefinition of unused `fu` from line 1
|
4 | def fu():
| ^^ F811
5 | pass
|
= help: Remove definition: `fu`


Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---
F811_16.py:8:13: F811 Redefinition of unused `fu` from line 3
|
6 | def bar():
7 | def baz():
8 | def fu():
| ^^ F811
9 | pass
|
= help: Remove definition: `fu`


Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---
F811_17.py:6:12: F811 [*] Redefinition of unused `fu` from line 2
|
5 | def bar():
6 | import fu
| ^^ F811
7 |
8 | def baz():
|
= help: Remove definition: `fu`

Safe fix
3 3 |
4 4 |
5 5 | def bar():
6 |- import fu
7 6 |
8 7 | def baz():
9 8 | def fu():

F811_17.py:9:13: F811 Redefinition of unused `fu` from line 6
|
8 | def baz():
9 | def fu():
| ^^ F811
10 | pass
|
= help: Remove definition: `fu`


Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---

Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---

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

Safe fix
1 |-from moo import fu as FU, bar as FU
1 |+from moo import fu as FU


Loading

0 comments on commit 84ab21f

Please sign in to comment.