Skip to content

Commit

Permalink
F841: support fixing unused assignments in tuples by renaming variabl…
Browse files Browse the repository at this point in the history
…es (#9107)

## Summary

A fairly common pattern which triggers F841 is unused variables from
tuple assignments, e.g.:

    user, created = User.objects.get_or_create(...)
          ^ F841: Local variable `created` is assigned to but never used

This error is currently not auto-fixable.

This PR adds support for fixing the error automatically by renaming the
unused variable to have a leading underscore (i.e. `_created`) **iff**
the `dummy-variable-rgx` setting would match it.

I considered using `renamers::Renamer` here, but because by the nature
of the error there should be no references to it, that seemed like
overkill. Also note that the fix might break by shadowing the new name
if it is already used elsewhere in the scope. I left it as is because

1. the renamed variable matches the "unused" regex, so it should
hopefully not already be used,
2. the fix is marked as unsafe so it should be reviewed manually
anyways, and
3. I'm not actually sure how to check the scope for the new variable
name 😅
  • Loading branch information
sciyoshi committed Dec 12, 2023
1 parent b972455 commit c306f85
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,14 @@ fn remove_unused_variable(binding: &Binding, checker: &Checker) -> Option<Fix> {
Some(Fix::unsafe_edit(edit).isolate(isolation))
};
}
} else {
let name = binding.name(checker.locator());
let renamed = format!("_{name}");
if checker.settings.dummy_variable_rgx.is_match(&renamed) {
let edit = Edit::range_replacement(renamed, binding.range());

return Some(Fix::unsafe_edit(edit).isolate(isolation));
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ F841_0.py:20:5: F841 [*] Local variable `foo` is assigned to but never used
22 21 |
23 22 | bar = (1, 2)

F841_0.py:21:6: F841 Local variable `a` is assigned to but never used
F841_0.py:21:6: F841 [*] Local variable `a` is assigned to but never used
|
19 | def f():
20 | foo = (1, 2)
Expand All @@ -68,7 +68,17 @@ F841_0.py:21:6: F841 Local variable `a` is assigned to but never used
|
= help: Remove assignment to unused variable `a`

F841_0.py:21:9: F841 Local variable `b` is assigned to but never used
Unsafe fix
18 18 |
19 19 | def f():
20 20 | foo = (1, 2)
21 |- (a, b) = (1, 2)
21 |+ (_a, b) = (1, 2)
22 22 |
23 23 | bar = (1, 2)
24 24 | (c, d) = bar

F841_0.py:21:9: F841 [*] Local variable `b` is assigned to but never used
|
19 | def f():
20 | foo = (1, 2)
Expand All @@ -79,6 +89,16 @@ F841_0.py:21:9: F841 Local variable `b` is assigned to but never used
|
= help: Remove assignment to unused variable `b`

Unsafe fix
18 18 |
19 19 | def f():
20 20 | foo = (1, 2)
21 |- (a, b) = (1, 2)
21 |+ (a, _b) = (1, 2)
22 22 |
23 23 | bar = (1, 2)
24 24 | (c, d) = bar

F841_0.py:26:14: F841 [*] Local variable `baz` is assigned to but never used
|
24 | (c, d) = bar
Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,42 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---
F841_1.py:6:5: F841 Local variable `x` is assigned to but never used
F841_1.py:6:5: F841 [*] Local variable `x` is assigned to but never used
|
5 | def f():
6 | x, y = 1, 2 # this triggers F841 as it's just a simple assignment where unpacking isn't needed
| ^ F841
|
= help: Remove assignment to unused variable `x`

F841_1.py:6:8: F841 Local variable `y` is assigned to but never used
Unsafe fix
3 3 |
4 4 |
5 5 | def f():
6 |- x, y = 1, 2 # this triggers F841 as it's just a simple assignment where unpacking isn't needed
6 |+ _x, y = 1, 2 # this triggers F841 as it's just a simple assignment where unpacking isn't needed
7 7 |
8 8 |
9 9 | def f():

F841_1.py:6:8: F841 [*] Local variable `y` is assigned to but never used
|
5 | def f():
6 | x, y = 1, 2 # this triggers F841 as it's just a simple assignment where unpacking isn't needed
| ^ F841
|
= help: Remove assignment to unused variable `y`

Unsafe fix
3 3 |
4 4 |
5 5 | def f():
6 |- x, y = 1, 2 # this triggers F841 as it's just a simple assignment where unpacking isn't needed
6 |+ x, _y = 1, 2 # this triggers F841 as it's just a simple assignment where unpacking isn't needed
7 7 |
8 8 |
9 9 | def f():

F841_1.py:16:14: F841 [*] Local variable `coords` is assigned to but never used
|
15 | def f():
Expand Down Expand Up @@ -53,36 +73,64 @@ F841_1.py:20:5: F841 [*] Local variable `coords` is assigned to but never used
22 22 |
23 23 | def f():

F841_1.py:24:6: F841 Local variable `a` is assigned to but never used
F841_1.py:24:6: F841 [*] Local variable `a` is assigned to but never used
|
23 | def f():
24 | (a, b) = (x, y) = 1, 2 # this triggers F841 on everything
| ^ F841
|
= help: Remove assignment to unused variable `a`

F841_1.py:24:9: F841 Local variable `b` is assigned to but never used
Unsafe fix
21 21 |
22 22 |
23 23 | def f():
24 |- (a, b) = (x, y) = 1, 2 # this triggers F841 on everything
24 |+ (_a, b) = (x, y) = 1, 2 # this triggers F841 on everything

F841_1.py:24:9: F841 [*] Local variable `b` is assigned to but never used
|
23 | def f():
24 | (a, b) = (x, y) = 1, 2 # this triggers F841 on everything
| ^ F841
|
= help: Remove assignment to unused variable `b`

F841_1.py:24:15: F841 Local variable `x` is assigned to but never used
Unsafe fix
21 21 |
22 22 |
23 23 | def f():
24 |- (a, b) = (x, y) = 1, 2 # this triggers F841 on everything
24 |+ (a, _b) = (x, y) = 1, 2 # this triggers F841 on everything

F841_1.py:24:15: F841 [*] Local variable `x` is assigned to but never used
|
23 | def f():
24 | (a, b) = (x, y) = 1, 2 # this triggers F841 on everything
| ^ F841
|
= help: Remove assignment to unused variable `x`

F841_1.py:24:18: F841 Local variable `y` is assigned to but never used
Unsafe fix
21 21 |
22 22 |
23 23 | def f():
24 |- (a, b) = (x, y) = 1, 2 # this triggers F841 on everything
24 |+ (a, b) = (_x, y) = 1, 2 # this triggers F841 on everything

F841_1.py:24:18: F841 [*] Local variable `y` is assigned to but never used
|
23 | def f():
24 | (a, b) = (x, y) = 1, 2 # this triggers F841 on everything
| ^ F841
|
= help: Remove assignment to unused variable `y`

Unsafe fix
21 21 |
22 22 |
23 23 | def f():
24 |- (a, b) = (x, y) = 1, 2 # this triggers F841 on everything
24 |+ (a, b) = (x, _y) = 1, 2 # this triggers F841 on everything


Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ F841_3.py:27:46: F841 [*] Local variable `z3` is assigned to but never used
29 29 |
30 30 |

F841_3.py:32:6: F841 Local variable `x1` is assigned to but never used
F841_3.py:32:6: F841 [*] Local variable `x1` is assigned to but never used
|
31 | def f():
32 | (x1, y1) = (1, 2)
Expand All @@ -166,7 +166,17 @@ F841_3.py:32:6: F841 Local variable `x1` is assigned to but never used
|
= help: Remove assignment to unused variable `x1`

F841_3.py:32:10: F841 Local variable `y1` is assigned to but never used
Unsafe fix
29 29 |
30 30 |
31 31 | def f():
32 |- (x1, y1) = (1, 2)
32 |+ (_x1, y1) = (1, 2)
33 33 | (x2, y2) = coords2 = (1, 2)
34 34 | coords3 = (x3, y3) = (1, 2)
35 35 |

F841_3.py:32:10: F841 [*] Local variable `y1` is assigned to but never used
|
31 | def f():
32 | (x1, y1) = (1, 2)
Expand All @@ -176,6 +186,16 @@ F841_3.py:32:10: F841 Local variable `y1` is assigned to but never used
|
= help: Remove assignment to unused variable `y1`

Unsafe fix
29 29 |
30 30 |
31 31 | def f():
32 |- (x1, y1) = (1, 2)
32 |+ (x1, _y1) = (1, 2)
33 33 | (x2, y2) = coords2 = (1, 2)
34 34 | coords3 = (x3, y3) = (1, 2)
35 35 |

F841_3.py:33:16: F841 [*] Local variable `coords2` is assigned to but never used
|
31 | def f():
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ F841_4.py:12:5: F841 [*] Local variable `a` is assigned to but never used
14 14 |
15 15 |

F841_4.py:13:5: F841 Local variable `b` is assigned to but never used
F841_4.py:13:5: F841 [*] Local variable `b` is assigned to but never used
|
11 | def bar():
12 | a = foo()
Expand All @@ -29,7 +29,17 @@ F841_4.py:13:5: F841 Local variable `b` is assigned to but never used
|
= help: Remove assignment to unused variable `b`

F841_4.py:13:8: F841 Local variable `c` is assigned to but never used
Unsafe fix
10 10 |
11 11 | def bar():
12 12 | a = foo()
13 |- b, c = foo()
13 |+ _b, c = foo()
14 14 |
15 15 |
16 16 | def baz():

F841_4.py:13:8: F841 [*] Local variable `c` is assigned to but never used
|
11 | def bar():
12 | a = foo()
Expand All @@ -38,4 +48,14 @@ F841_4.py:13:8: F841 Local variable `c` is assigned to but never used
|
= help: Remove assignment to unused variable `c`

Unsafe fix
10 10 |
11 11 | def bar():
12 12 | a = foo()
13 |- b, c = foo()
13 |+ b, _c = foo()
14 14 |
15 15 |
16 16 | def baz():


0 comments on commit c306f85

Please sign in to comment.