Skip to content

Commit

Permalink
Fix F841 false negative on assignment to multiple variables (#8489)
Browse files Browse the repository at this point in the history
## Summary

Closes #8441 behind preview feature flag.

## Test Plan

`cargo test`
  • Loading branch information
tjkuson authored Nov 5, 2023
1 parent b3c2935 commit 8c0d65c
Show file tree
Hide file tree
Showing 7 changed files with 136 additions and 12 deletions.
8 changes: 4 additions & 4 deletions crates/ruff_linter/resources/test/fixtures/pyflakes/F841_1.py
Original file line number Diff line number Diff line change
@@ -1,23 +1,23 @@
def f(tup):
x, y = tup # this does NOT trigger F841
x, y = tup


def f():
x, y = 1, 2 # this triggers F841 as it's just a simple assignment where unpacking isn't needed


def f():
(x, y) = coords = 1, 2 # this does NOT trigger F841
(x, y) = coords = 1, 2
if x > 1:
print(coords)


def f():
(x, y) = coords = 1, 2 # this triggers F841 on coords
(x, y) = coords = 1, 2


def f():
coords = (x, y) = 1, 2 # this triggers F841 on coords
coords = (x, y) = 1, 2


def f():
Expand Down
32 changes: 32 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pyflakes/F841_4.py
Original file line number Diff line number Diff line change
@@ -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()
23 changes: 22 additions & 1 deletion crates/ruff_linter/src/rules/pyflakes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ mod tests {
use crate::linter::{check_path, LinterResult};
use crate::registry::{AsRule, Linter, Rule};
use crate::rules::pyflakes;
use crate::settings::types::PreviewMode;
use crate::settings::{flags, LinterSettings};
use crate::source_kind::SourceKind;
use crate::test::{test_path, test_snippet};
Expand Down Expand Up @@ -145,6 +146,7 @@ mod tests {
#[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_3.py"))]
#[test_case(Rule::UnusedVariable, Path::new("F841_4.py"))]
#[test_case(Rule::UnusedAnnotation, Path::new("F842.py"))]
#[test_case(Rule::RaiseNotImplemented, Path::new("F901.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
Expand All @@ -157,6 +159,24 @@ mod tests {
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]
fn f841_dummy_variable_rgx() -> Result<()> {
let diagnostics = test_path(
Expand Down Expand Up @@ -1126,7 +1146,8 @@ mod tests {

#[test]
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(
r#"
def f():
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use ruff_text_size::{Ranged, TextRange, TextSize};

use crate::checkers::ast::Checker;
use crate::fix::edits::delete_stmt;
use crate::settings::types::PreviewMode;

/// ## What it does
/// Checks for the presence of unused variables in function scopes.
Expand All @@ -24,6 +25,9 @@ use crate::fix::edits::delete_stmt;
/// prefixed with an underscore, or some other value that adheres to the
/// [`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
/// ```python
/// def foo():
Expand Down Expand Up @@ -318,7 +322,10 @@ pub(crate) fn unused_variable(checker: &Checker, scope: &Scope, diagnostics: &mu
.bindings()
.map(|(name, binding_id)| (name, checker.semantic().binding(binding_id)))
.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_global()
&& !binding.is_used()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
15 | def f():
16 | (x, y) = coords = 1, 2 # this triggers F841 on coords
16 | (x, y) = coords = 1, 2
| ^^^^^^ F841
|
= help: Remove assignment to unused variable `coords`
Expand All @@ -29,16 +29,16 @@ F841_1.py:16:14: F841 [*] Local variable `coords` is assigned to but never used
13 13 |
14 14 |
15 15 | def f():
16 |- (x, y) = coords = 1, 2 # this triggers F841 on coords
16 |+ (x, y) = 1, 2 # this triggers F841 on coords
16 |- (x, y) = coords = 1, 2
16 |+ (x, y) = 1, 2
17 17 |
18 18 |
19 19 | def f():

F841_1.py:20:5: F841 [*] Local variable `coords` is assigned to but never used
|
19 | def f():
20 | coords = (x, y) = 1, 2 # this triggers F841 on coords
20 | coords = (x, y) = 1, 2
| ^^^^^^ F841
|
= help: Remove assignment to unused variable `coords`
Expand All @@ -47,8 +47,8 @@ F841_1.py:20:5: F841 [*] Local variable `coords` is assigned to but never used
17 17 |
18 18 |
19 19 | def f():
20 |- coords = (x, y) = 1, 2 # this triggers F841 on coords
20 |+ (x, y) = 1, 2 # this triggers F841 on coords
20 |- coords = (x, y) = 1, 2
20 |+ (x, y) = 1, 2
21 21 |
22 22 |
23 23 | def f():
Expand Down
Original file line number Diff line number Diff line change
@@ -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 |


Original file line number Diff line number Diff line change
@@ -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`


0 comments on commit 8c0d65c

Please sign in to comment.