mirror of https://github.com/astral-sh/ruff
Don't flag `redefined-while-unused` in if branches (#9418)
## Summary
On `main`, we flag redefinitions in cases like:
```python
import os
x = 1
if x > 0:
import os
```
That is, we consider these to be in the "same branch", since they're not
in disjoint branches. This matches Flake8's behavior, but it seems to
lead to false positives.
This commit is contained in:
parent
f419af494f
commit
985f1d10f6
|
|
@ -0,0 +1,6 @@
|
||||||
|
import os
|
||||||
|
|
||||||
|
x = 1
|
||||||
|
|
||||||
|
if x > 0:
|
||||||
|
import os
|
||||||
|
|
@ -144,9 +144,9 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
|
||||||
|
|
||||||
// If the bindings are in different forks, abort.
|
// If the bindings are in different forks, abort.
|
||||||
if shadowed.source.map_or(true, |left| {
|
if shadowed.source.map_or(true, |left| {
|
||||||
binding.source.map_or(true, |right| {
|
binding
|
||||||
checker.semantic.different_branches(left, right)
|
.source
|
||||||
})
|
.map_or(true, |right| !checker.semantic.same_branch(left, right))
|
||||||
}) {
|
}) {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
@ -233,9 +233,9 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
|
||||||
|
|
||||||
// If the bindings are in different forks, abort.
|
// If the bindings are in different forks, abort.
|
||||||
if shadowed.source.map_or(true, |left| {
|
if shadowed.source.map_or(true, |left| {
|
||||||
binding.source.map_or(true, |right| {
|
binding
|
||||||
checker.semantic.different_branches(left, right)
|
.source
|
||||||
})
|
.map_or(true, |right| !checker.semantic.same_branch(left, right))
|
||||||
}) {
|
}) {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -120,6 +120,7 @@ mod tests {
|
||||||
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_24.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_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::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"))]
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,4 @@
|
||||||
|
---
|
||||||
|
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
|
||||||
|
---
|
||||||
|
|
||||||
|
|
@ -1163,11 +1163,11 @@ impl<'a> SemanticModel<'a> {
|
||||||
false
|
false
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Returns `true` if `left` and `right` are on different branches of an `if`, `match`, or
|
/// Returns `true` if `left` and `right` are in the same branches of an `if`, `match`, or
|
||||||
/// `try` statement.
|
/// `try` statement.
|
||||||
///
|
///
|
||||||
/// This implementation assumes that the statements are in the same scope.
|
/// This implementation assumes that the statements are in the same scope.
|
||||||
pub fn different_branches(&self, left: NodeId, right: NodeId) -> bool {
|
pub fn same_branch(&self, left: NodeId, right: NodeId) -> bool {
|
||||||
// Collect the branch path for the left statement.
|
// Collect the branch path for the left statement.
|
||||||
let left = self
|
let left = self
|
||||||
.nodes
|
.nodes
|
||||||
|
|
@ -1184,10 +1184,7 @@ impl<'a> SemanticModel<'a> {
|
||||||
.flat_map(|branch_id| self.branches.ancestor_ids(*branch_id))
|
.flat_map(|branch_id| self.branches.ancestor_ids(*branch_id))
|
||||||
.collect::<Vec<_>>();
|
.collect::<Vec<_>>();
|
||||||
|
|
||||||
!left
|
left == right
|
||||||
.iter()
|
|
||||||
.zip(right.iter())
|
|
||||||
.all(|(left, right)| left == right)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Returns `true` if the given expression is an unused variable, or consists solely of
|
/// Returns `true` if the given expression is an unused variable, or consists solely of
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue