Skip to content

Commit

Permalink
Add a fix for redefinition-while-unused
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jan 7, 2024
1 parent f684175 commit 184f7a3
Show file tree
Hide file tree
Showing 20 changed files with 155 additions and 13 deletions.
27 changes: 25 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,31 @@ 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 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::unsafe_edit(edit).isolate(Checker::isolation(
checker.semantic().parent_statement_id(source),
)))
});
}
}

diagnostics.push(diagnostic);
}
}
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 All @@ -22,16 +22,40 @@ use ruff_source_file::SourceRow;
/// import foo
/// import bar
/// ```
///
/// ## Fix safety
/// This rule's fix is marked as unsafe, as removing a redefinition across
/// branches or scopes may change the behavior of the program in subtle
/// ways.
///
/// For example:
/// ```python
/// import module
///
/// x = int(input())
///
/// if x > 0:
/// from package import module
/// ```
///
/// Removing the redefinition would change the `module` binding when `x > 0`.
#[violation]
pub struct RedefinedWhileUnused {
pub name: String,
pub row: SourceRow,
}

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
@@ -1,10 +1,15 @@
---
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
| ^^ F811
|
= help: Remove definition: `FU`

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


Original file line number Diff line number Diff line change
@@ -1,13 +1,22 @@
---
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
5 | else:
6 | from bb import mixer
| ^^^^^ F811
7 | mixer(123)
|
= help: Remove definition: `mixer`

Unsafe 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
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
@@ -1,14 +1,24 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---
F811_17.py:6:12: F811 Redefinition of unused `fu` from line 2
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`

Unsafe 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
|
Expand All @@ -17,5 +27,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
@@ -1,10 +1,15 @@
---
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
| ^^ F811
|
= help: Remove definition: `FU`

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


Original file line number Diff line number Diff line change
@@ -1,13 +1,25 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---
F811_21.py:32:5: F811 Redefinition of unused `Sequence` from line 26
F811_21.py:32:5: F811 [*] Redefinition of unused `Sequence` from line 26
|
30 | from typing import (
31 | List, # noqa: F811
32 | Sequence,
| ^^^^^^^^ F811
33 | )
|
= help: Remove definition: `Sequence`

Unsafe fix
29 29 | # This should ignore the first error.
30 30 | from typing import (
31 31 | List, # noqa: F811
32 |- Sequence,
33 |-)
32 |+ )
34 33 |
35 34 | # This should ignore both errors.
36 35 | from typing import ( # noqa


Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
---
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
4 | import bar as foo
| ^^^ F811
|
= help: Remove definition: `foo`

Unsafe 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


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
@@ -1,13 +1,21 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---
F811_6.py:6:12: F811 Redefinition of unused `os` from line 5
F811_6.py:6:12: F811 [*] Redefinition of unused `os` from line 5
|
4 | if i == 1:
5 | import os
6 | import os
| ^^ F811
7 | os.path
|
= help: Remove definition: `os`

Unsafe fix
3 3 | i = 2
4 4 | if i == 1:
5 5 | import os
6 |- import os
7 6 | os.path


Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---
F811_8.py:5:12: F811 Redefinition of unused `os` from line 4
F811_8.py:5:12: F811 [*] Redefinition of unused `os` from line 4
|
3 | try:
4 | import os
Expand All @@ -10,5 +10,15 @@ F811_8.py:5:12: F811 Redefinition of unused `os` from line 4
6 | except:
7 | pass
|
= help: Remove definition: `os`

Unsafe fix
2 2 |
3 3 | try:
4 4 | import os
5 |- import os
6 5 | except:
7 6 | pass
8 7 | os.path


Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,23 @@ source: crates/ruff_linter/src/rules/pyflakes/mod.rs
4 3 | def f():
5 4 | import os

<filename>:5:12: F811 Redefinition of unused `os` from line 2
<filename>:5:12: F811 [*] Redefinition of unused `os` from line 2
|
4 | def f():
5 | import os
| ^^ F811
6 |
7 | # Despite this `del`, `import os` in `f` should still be flagged as shadowing an unused
|
= help: Remove definition: `os`

ℹ Unsafe fix
2 2 | import os
3 3 |
4 4 | def f():
5 |- import os
6 5 |
7 6 | # Despite this `del`, `import os` in `f` should still be flagged as shadowing an unused
8 7 | # import.


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
@@ -1,7 +1,7 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---
<filename>:4:12: F811 Redefinition of unused `os` from line 3
<filename>:4:12: F811 [*] Redefinition of unused `os` from line 3
|
2 | def f():
3 | import os
Expand All @@ -10,5 +10,15 @@ 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`

ℹ Unsafe fix
1 1 |
2 2 | def f():
3 3 | import os
4 |- import os
5 4 |
6 5 | # Despite this `del`, `import os` should still be flagged as shadowing an unused
7 6 | # import.


0 comments on commit 184f7a3

Please sign in to comment.