Skip to content

Commit d10ae46

Browse files
Fix FURB148 false positive when loop variable is used after loop (#370)
When a for-loop variable from enumerate() is used after the loop body (e.g., `print(index)` after the for loop), FURB148 incorrectly warns that the variable is unused. This happens because the check only examines the loop body, not subsequent statements in the enclosing block. The fix registers the check for Block/MypyFile nodes (in addition to GeneratorExpr/DictionaryComprehension), allowing it to examine statements following the for loop. When checking for unused variables, it now considers both the loop body and subsequent statements, stopping at any reassignment of the variable. Fixes #339. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent e2ce817 commit d10ae46

3 files changed

Lines changed: 125 additions & 2 deletions

File tree

refurb/checks/builtin/no_ignored_enumerate.py

Lines changed: 98 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,23 @@
11
from dataclasses import dataclass
22

33
from mypy.nodes import (
4+
AssignmentStmt,
5+
Block,
46
CallExpr,
57
DictionaryComprehension,
68
Expression,
79
ForStmt,
810
GeneratorExpr,
11+
MypyFile,
912
NameExpr,
1013
Node,
14+
Statement,
1115
TupleExpr,
1216
)
1317

1418
from refurb.checks.common import (
19+
ReadCountVisitor,
20+
check_block_like,
1521
check_for_loop_like,
1622
get_mypy_type,
1723
is_name_unused_in_contexts,
@@ -57,11 +63,81 @@ class ErrorInfo(Error):
5763
categories = ("builtin",)
5864

5965

66+
def _assigns_name(stmt: Statement, name: NameExpr) -> bool:
67+
"""Check if a statement assigns to the given name."""
68+
match stmt:
69+
case ForStmt(index=index):
70+
return _node_contains_name(index, name)
71+
case AssignmentStmt(lvalues=lvalues):
72+
return any(_node_contains_name(lv, name) for lv in lvalues)
73+
return False
74+
75+
76+
def _node_contains_name(node: Node, name: NameExpr) -> bool:
77+
"""Check if a node contains a NameExpr with the same fullname."""
78+
visitor = ReadCountVisitor(name)
79+
visitor.accept(node)
80+
return bool(visitor.was_read)
81+
82+
83+
def _is_name_read_after_loop(name: NameExpr, remaining_stmts: list[Statement]) -> bool:
84+
"""Check if name is read in statements after the loop, stopping at reassignment."""
85+
for stmt in remaining_stmts:
86+
if _assigns_name(stmt, name):
87+
# Name is reassigned — reads after this point don't count
88+
# as uses of the loop variable. But check the RHS first.
89+
rhs: Expression | None = None
90+
if isinstance(stmt, AssignmentStmt):
91+
rhs = stmt.rvalue
92+
elif isinstance(stmt, ForStmt):
93+
rhs = stmt.expr
94+
95+
if rhs is not None:
96+
visitor = ReadCountVisitor(name)
97+
visitor.accept(rhs)
98+
if visitor.was_read:
99+
return True
100+
101+
return False
102+
103+
# Check if name is read in this statement
104+
visitor = ReadCountVisitor(name)
105+
visitor.accept(stmt)
106+
if visitor.was_read:
107+
return True
108+
109+
return False
110+
111+
60112
def check(
61-
node: ForStmt | GeneratorExpr | DictionaryComprehension,
113+
node: Block | MypyFile | GeneratorExpr | DictionaryComprehension,
62114
errors: list[Error],
63115
) -> None:
64-
check_for_loop_like(check_enumerate_call, node, errors)
116+
match node:
117+
case GeneratorExpr() | DictionaryComprehension():
118+
check_for_loop_like(check_enumerate_call, node, errors)
119+
120+
case Block() | MypyFile():
121+
check_block_like(check_stmts_for_enumerate, node, errors)
122+
123+
124+
def check_stmts_for_enumerate(stmts: list[Statement], errors: list[Error]) -> None:
125+
for i, stmt in enumerate(stmts):
126+
if not isinstance(stmt, ForStmt):
127+
continue
128+
129+
match stmt.index, stmt.expr:
130+
case (
131+
TupleExpr(items=[NameExpr() as index, NameExpr() as value]),
132+
CallExpr(
133+
callee=NameExpr(fullname="builtins.enumerate"),
134+
args=[enumerate_arg],
135+
),
136+
) if is_subclass(get_mypy_type(enumerate_arg), "typing.Sequence"):
137+
remaining = stmts[i + 1 :]
138+
check_unused_index_or_value_in_block(
139+
index, value, stmt.body, remaining, errors, enumerate_arg
140+
)
65141

66142

67143
def check_enumerate_call(
@@ -94,3 +170,23 @@ def check_unused_index_or_value(
94170
msg = f"Value is unused, use `for {stringify(index)} in range(len({stringify(enumerate_arg)}))` instead" # noqa: E501
95171

96172
errors.append(ErrorInfo.from_node(value, msg))
173+
174+
175+
def check_unused_index_or_value_in_block( # noqa: PLR0913, PLR0917
176+
index: NameExpr,
177+
value: NameExpr,
178+
body: Node,
179+
remaining_stmts: list[Statement],
180+
errors: list[Error],
181+
enumerate_arg: Expression,
182+
) -> None:
183+
index_unused_in_body = is_name_unused_in_contexts(index, [body])
184+
value_unused_in_body = is_name_unused_in_contexts(value, [body])
185+
186+
if index_unused_in_body and not _is_name_read_after_loop(index, remaining_stmts):
187+
msg = f"Index is unused, use `for {stringify(value)} in {stringify(enumerate_arg)}` instead" # noqa: E501
188+
errors.append(ErrorInfo.from_node(index, msg))
189+
190+
if value_unused_in_body and not _is_name_read_after_loop(value, remaining_stmts):
191+
msg = f"Value is unused, use `for {stringify(index)} in range(len({stringify(enumerate_arg)}))` instead" # noqa: E501
192+
errors.append(ErrorInfo.from_node(value, msg))

test/data/err_148.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,3 +50,29 @@ class C:
5050

5151
_ = ((index, _) for index, _ in enumerate(nums))
5252
_ = ((_, num) for _, num in enumerate(nums))
53+
54+
# index used after loop (GH-339)
55+
for index, value in enumerate(nums):
56+
print(value)
57+
print(f"Last index {index}")
58+
59+
# value used after loop
60+
for index, value in enumerate(nums):
61+
print(index)
62+
_ = value
63+
64+
# index reassigned after loop using its own value (should not warn)
65+
for index, value in enumerate(nums):
66+
print(value)
67+
index = index + 1
68+
69+
# value reassigned after loop (not reading it, should warn on index)
70+
for index, value in enumerate(nums):
71+
print(value)
72+
value = "overwritten"
73+
74+
# index reassigned by a for-loop that also reads it in the iterable (should not warn)
75+
for index, value in enumerate(nums):
76+
print(value)
77+
for index in range(index):
78+
pass

test/data/err_148.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,4 @@ test/data/err_148.py:28:5 [FURB148]: Index is unused, use `for num in nums` inst
1313
test/data/err_148.py:28:12 [FURB148]: Value is unused, use `for index in range(len(nums))` instead
1414
test/data/err_148.py:34:5 [FURB148]: Index is unused, use `for num in C().l` instead
1515
test/data/err_148.py:34:12 [FURB148]: Value is unused, use `for index in range(len(C().l))` instead
16+
test/data/err_148.py:70:5 [FURB148]: Index is unused, use `for value in nums` instead

0 commit comments

Comments
 (0)