Fix `F841` false negative on assignment to multiple variables (#8489)

## Summary

Closes #8441 behind preview feature flag.

## Test Plan

`cargo test`
This commit is contained in:
Tom Kuson 2023-11-05 17:01:10 +00:00 committed by GitHub
parent b3c2935fa5
commit 8c0d65c98e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 136 additions and 12 deletions

View File

@ -1,5 +1,5 @@
def f(tup): def f(tup):
x, y = tup # this does NOT trigger F841 x, y = tup
def f(): def f():
@ -7,17 +7,17 @@ def f():
def f(): def f():
(x, y) = coords = 1, 2 # this does NOT trigger F841 (x, y) = coords = 1, 2
if x > 1: if x > 1:
print(coords) print(coords)
def f(): def f():
(x, y) = coords = 1, 2 # this triggers F841 on coords (x, y) = coords = 1, 2
def f(): def f():
coords = (x, y) = 1, 2 # this triggers F841 on coords coords = (x, y) = 1, 2
def f(): def f():

View File

@ -0,0 +1,32 @@
"""Test fix for issue #8441.
Ref: https://github.com/astral-sh/ruff/issues/8441
"""
def foo():
...
def bar():
a = foo()
b, c = foo()
def baz():
d, _e = foo()
print(d)
def qux():
f, _ = foo()
print(f)
def quux():
g, h = foo()
print(g, h)
def quuz():
_i, _j = foo()

View File

@ -26,6 +26,7 @@ mod tests {
use crate::linter::{check_path, LinterResult}; use crate::linter::{check_path, LinterResult};
use crate::registry::{AsRule, Linter, Rule}; use crate::registry::{AsRule, Linter, Rule};
use crate::rules::pyflakes; use crate::rules::pyflakes;
use crate::settings::types::PreviewMode;
use crate::settings::{flags, LinterSettings}; use crate::settings::{flags, LinterSettings};
use crate::source_kind::SourceKind; use crate::source_kind::SourceKind;
use crate::test::{test_path, test_snippet}; use crate::test::{test_path, test_snippet};
@ -145,6 +146,7 @@ mod tests {
#[test_case(Rule::UnusedVariable, Path::new("F841_1.py"))] #[test_case(Rule::UnusedVariable, Path::new("F841_1.py"))]
#[test_case(Rule::UnusedVariable, Path::new("F841_2.py"))] #[test_case(Rule::UnusedVariable, Path::new("F841_2.py"))]
#[test_case(Rule::UnusedVariable, Path::new("F841_3.py"))] #[test_case(Rule::UnusedVariable, Path::new("F841_3.py"))]
#[test_case(Rule::UnusedVariable, Path::new("F841_4.py"))]
#[test_case(Rule::UnusedAnnotation, Path::new("F842.py"))] #[test_case(Rule::UnusedAnnotation, Path::new("F842.py"))]
#[test_case(Rule::RaiseNotImplemented, Path::new("F901.py"))] #[test_case(Rule::RaiseNotImplemented, Path::new("F901.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> { fn rules(rule_code: Rule, path: &Path) -> Result<()> {
@ -157,6 +159,24 @@ mod tests {
Ok(()) Ok(())
} }
#[test_case(Rule::UnusedVariable, Path::new("F841_4.py"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
"preview__{}_{}",
rule_code.noqa_code(),
path.to_string_lossy()
);
let diagnostics = test_path(
Path::new("pyflakes").join(path).as_path(),
&LinterSettings {
preview: PreviewMode::Enabled,
..LinterSettings::for_rule(rule_code)
},
)?;
assert_messages!(snapshot, diagnostics);
Ok(())
}
#[test] #[test]
fn f841_dummy_variable_rgx() -> Result<()> { fn f841_dummy_variable_rgx() -> Result<()> {
let diagnostics = test_path( let diagnostics = test_path(
@ -1126,7 +1146,8 @@ mod tests {
#[test] #[test]
fn used_as_star_unpack() { fn used_as_star_unpack() {
// Star names in unpack are used if RHS is not a tuple/list literal. // In stable, starred names in unpack are used if RHS is not a tuple/list literal.
// In preview, these should be marked as unused.
flakes( flakes(
r#" r#"
def f(): def f():

View File

@ -12,6 +12,7 @@ use ruff_text_size::{Ranged, TextRange, TextSize};
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
use crate::fix::edits::delete_stmt; use crate::fix::edits::delete_stmt;
use crate::settings::types::PreviewMode;
/// ## What it does /// ## What it does
/// Checks for the presence of unused variables in function scopes. /// Checks for the presence of unused variables in function scopes.
@ -24,6 +25,9 @@ use crate::fix::edits::delete_stmt;
/// prefixed with an underscore, or some other value that adheres to the /// prefixed with an underscore, or some other value that adheres to the
/// [`dummy-variable-rgx`] pattern. /// [`dummy-variable-rgx`] pattern.
/// ///
/// Under [preview mode](https://docs.astral.sh/ruff/preview), this rule also
/// triggers on unused unpacked assignments (for example, `x, y = foo()`).
///
/// ## Example /// ## Example
/// ```python /// ```python
/// def foo(): /// def foo():
@ -318,7 +322,10 @@ pub(crate) fn unused_variable(checker: &Checker, scope: &Scope, diagnostics: &mu
.bindings() .bindings()
.map(|(name, binding_id)| (name, checker.semantic().binding(binding_id))) .map(|(name, binding_id)| (name, checker.semantic().binding(binding_id)))
.filter_map(|(name, binding)| { .filter_map(|(name, binding)| {
if (binding.kind.is_assignment() || binding.kind.is_named_expr_assignment()) if (binding.kind.is_assignment()
|| binding.kind.is_named_expr_assignment()
|| (matches!(checker.settings.preview, PreviewMode::Enabled)
&& binding.kind.is_unpacked_assignment()))
&& !binding.is_nonlocal() && !binding.is_nonlocal()
&& !binding.is_global() && !binding.is_global()
&& !binding.is_used() && !binding.is_used()

View File

@ -20,7 +20,7 @@ F841_1.py:6:8: F841 Local variable `y` is assigned to but never used
F841_1.py:16:14: F841 [*] Local variable `coords` is assigned to but never used F841_1.py:16:14: F841 [*] Local variable `coords` is assigned to but never used
| |
15 | def f(): 15 | def f():
16 | (x, y) = coords = 1, 2 # this triggers F841 on coords 16 | (x, y) = coords = 1, 2
| ^^^^^^ F841 | ^^^^^^ F841
| |
= help: Remove assignment to unused variable `coords` = help: Remove assignment to unused variable `coords`
@ -29,8 +29,8 @@ F841_1.py:16:14: F841 [*] Local variable `coords` is assigned to but never used
13 13 | 13 13 |
14 14 | 14 14 |
15 15 | def f(): 15 15 | def f():
16 |- (x, y) = coords = 1, 2 # this triggers F841 on coords 16 |- (x, y) = coords = 1, 2
16 |+ (x, y) = 1, 2 # this triggers F841 on coords 16 |+ (x, y) = 1, 2
17 17 | 17 17 |
18 18 | 18 18 |
19 19 | def f(): 19 19 | def f():
@ -38,7 +38,7 @@ F841_1.py:16:14: F841 [*] Local variable `coords` is assigned to but never used
F841_1.py:20:5: F841 [*] Local variable `coords` is assigned to but never used F841_1.py:20:5: F841 [*] Local variable `coords` is assigned to but never used
| |
19 | def f(): 19 | def f():
20 | coords = (x, y) = 1, 2 # this triggers F841 on coords 20 | coords = (x, y) = 1, 2
| ^^^^^^ F841 | ^^^^^^ F841
| |
= help: Remove assignment to unused variable `coords` = help: Remove assignment to unused variable `coords`
@ -47,8 +47,8 @@ F841_1.py:20:5: F841 [*] Local variable `coords` is assigned to but never used
17 17 | 17 17 |
18 18 | 18 18 |
19 19 | def f(): 19 19 | def f():
20 |- coords = (x, y) = 1, 2 # this triggers F841 on coords 20 |- coords = (x, y) = 1, 2
20 |+ (x, y) = 1, 2 # this triggers F841 on coords 20 |+ (x, y) = 1, 2
21 21 | 21 21 |
22 22 | 22 22 |
23 23 | def f(): 23 23 | def f():

View File

@ -0,0 +1,23 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---
F841_4.py:12:5: F841 [*] Local variable `a` is assigned to but never used
|
11 | def bar():
12 | a = foo()
| ^ F841
13 | b, c = foo()
|
= help: Remove assignment to unused variable `a`
Suggested fix
9 9 |
10 10 |
11 11 | def bar():
12 |- a = foo()
12 |+ foo()
13 13 | b, c = foo()
14 14 |
15 15 |

View File

@ -0,0 +1,41 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---
F841_4.py:12:5: F841 [*] Local variable `a` is assigned to but never used
|
11 | def bar():
12 | a = foo()
| ^ F841
13 | b, c = foo()
|
= help: Remove assignment to unused variable `a`
Suggested fix
9 9 |
10 10 |
11 11 | def bar():
12 |- a = foo()
12 |+ foo()
13 13 | b, c = foo()
14 14 |
15 15 |
F841_4.py:13:5: F841 Local variable `b` is assigned to but never used
|
11 | def bar():
12 | a = foo()
13 | b, c = foo()
| ^ F841
|
= help: Remove assignment to unused variable `b`
F841_4.py:13:8: F841 Local variable `c` is assigned to but never used
|
11 | def bar():
12 | a = foo()
13 | b, c = foo()
| ^ F841
|
= help: Remove assignment to unused variable `c`