Skip to content

Commit cf508c2

Browse files
ambvhauntsaninja
andauthored
Fix spurious possibly-undefined errors in for-else with break (#19696)
When a for loop contains branches with `break` and an `else` block, variables declared inside those branches were incorrectly discarded from further analysis, leading Mypy to incorrectly report a variable as undefined after the loop or as used before declaration. With this fix, when a for loop's `else` block is considered, variables declared in every branch of the `for` loop body that called `break` are now considered as defined within the body of the loop. Fixes #14209 Fixes #19690 --------- Co-authored-by: Shantanu Jain <hauntsaninja@gmail.com> Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
1 parent 356b889 commit cf508c2

3 files changed

Lines changed: 219 additions & 0 deletions

File tree

mypy/partially_defined.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,8 @@ def is_undefined(self, name: str) -> bool:
297297
class Loop:
298298
def __init__(self) -> None:
299299
self.has_break = False
300+
# variables defined in every loop branch with `break`
301+
self.break_vars: set[str] | None = None
300302

301303

302304
class PossiblyUndefinedVariableVisitor(ExtendedTraverserVisitor):
@@ -476,6 +478,9 @@ def visit_for_stmt(self, o: ForStmt) -> None:
476478
has_break = loop.has_break
477479
if has_break:
478480
self.tracker.start_branch_statement()
481+
if loop.break_vars is not None:
482+
for bv in loop.break_vars:
483+
self.tracker.record_definition(bv)
479484
self.tracker.next_branch()
480485
o.else_body.accept(self)
481486
if has_break:
@@ -508,6 +513,14 @@ def visit_break_stmt(self, o: BreakStmt) -> None:
508513
super().visit_break_stmt(o)
509514
if self.loops:
510515
self.loops[-1].has_break = True
516+
# Track variables that are definitely defined at the point of break
517+
if len(self.tracker._scope().branch_stmts) > 0:
518+
branch = self.tracker._scope().branch_stmts[-1].branches[-1]
519+
if self.loops[-1].break_vars is None:
520+
self.loops[-1].break_vars = set(branch.must_be_defined)
521+
else:
522+
# we only want variables that have been defined in each branch
523+
self.loops[-1].break_vars.intersection_update(branch.must_be_defined)
511524
self.tracker.skip_branch()
512525

513526
def visit_expression_stmt(self, o: ExpressionStmt) -> None:

test-data/unit/check-possibly-undefined.test

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1055,6 +1055,158 @@ def foo(x: Union[int, str]) -> None:
10551055
f # OK
10561056
[builtins fixtures/tuple.pyi]
10571057

1058+
[case testForElseWithBreakInTryExceptContinue]
1059+
# flags: --enable-error-code possibly-undefined
1060+
# Test for issue where variable defined before break in try block
1061+
# was incorrectly reported as undefined when except has continue
1062+
def random() -> float: return 0.5
1063+
1064+
if random():
1065+
for i in range(10):
1066+
try:
1067+
value = random()
1068+
break
1069+
except Exception:
1070+
continue
1071+
else:
1072+
raise RuntimeError
1073+
1074+
print(value) # Should not error - value is defined if we broke
1075+
else:
1076+
value = random()
1077+
1078+
print(value) # Should not error
1079+
[builtins fixtures/for_else_exception.pyi]
1080+
1081+
[case testForElseWithBreakInTryExceptContinueNoIf]
1082+
# flags: --enable-error-code possibly-undefined
1083+
# Simpler version without if statement
1084+
def random() -> float: return 0.5
1085+
1086+
for i in range(10):
1087+
try:
1088+
value = random()
1089+
break
1090+
except Exception:
1091+
continue
1092+
else:
1093+
raise RuntimeError
1094+
1095+
print(value) # Should not error
1096+
[builtins fixtures/for_else_exception.pyi]
1097+
1098+
[case testForElseWithBreakInTryExceptPass]
1099+
# flags: --enable-error-code possibly-undefined
1100+
# Version with pass instead of continue - should also work
1101+
def random() -> float: return 0.5
1102+
1103+
if random():
1104+
for i in range(10):
1105+
try:
1106+
value = random()
1107+
break
1108+
except Exception:
1109+
pass
1110+
else:
1111+
raise RuntimeError
1112+
1113+
print(value) # Should not error
1114+
else:
1115+
value = random()
1116+
1117+
print(value) # Should not error
1118+
[builtins fixtures/for_else_exception.pyi]
1119+
1120+
[case testForElseWithConditionalDefBeforeBreak]
1121+
# flags: --enable-error-code possibly-undefined
1122+
# Test that conditional definition before break still works correctly
1123+
def random() -> float: return 0.5
1124+
1125+
if random():
1126+
for i in range(10):
1127+
try:
1128+
if i > 10:
1129+
value = random()
1130+
break
1131+
except Exception:
1132+
continue
1133+
else:
1134+
raise RuntimeError
1135+
1136+
print(value) # Should not error (though might be undefined at runtime)
1137+
else:
1138+
value = random()
1139+
1140+
print(value) # Should not error
1141+
[builtins fixtures/for_else_exception.pyi]
1142+
1143+
[case testForElseDefineInBothBranches]
1144+
# flags: --enable-error-code possibly-undefined
1145+
# Test that variable defined in both for break and else branches is not undefined
1146+
for i in range(10):
1147+
if i:
1148+
value = i
1149+
break
1150+
else:
1151+
value = 1
1152+
1153+
print(value) # Should not error - value is defined in all paths
1154+
[builtins fixtures/for_else_exception.pyi]
1155+
1156+
[case testForElseWithWalrusInBreak]
1157+
# flags: --enable-error-code possibly-undefined
1158+
# Test with walrus operator in if condition before break
1159+
def random() -> float: return 0.5
1160+
1161+
if random():
1162+
for i in range(10):
1163+
if value := random():
1164+
break
1165+
else:
1166+
raise RuntimeError
1167+
1168+
print(value) # Should not error - value is defined if we broke
1169+
else:
1170+
value = random()
1171+
1172+
print(value) # Should not error
1173+
[builtins fixtures/for_else_exception.pyi]
1174+
1175+
[case testForElseWithWalrusInBreakConditionalRaise]
1176+
# flags: --enable-error-code possibly-undefined
1177+
# Test with walrus operator in if condition before break
1178+
def random() -> float: return 0.5
1179+
1180+
def f1() -> None:
1181+
if random():
1182+
for i in range(10):
1183+
if value := random():
1184+
break
1185+
else:
1186+
if random() > 0.5: # E: Unsupported left operand type for > ("float")
1187+
raise RuntimeError
1188+
1189+
print(value) # E: Name "value" may be undefined
1190+
else:
1191+
value = random()
1192+
1193+
print(value) # Already emitted error
1194+
1195+
1196+
def f2() -> None:
1197+
if random():
1198+
for i in range(10):
1199+
if value := random():
1200+
break
1201+
else:
1202+
if random() > 0.5: # E: Unsupported left operand type for > ("float")
1203+
raise RuntimeError
1204+
else:
1205+
value = random()
1206+
1207+
print(value) # E: Name "value" may be undefined
1208+
[builtins fixtures/for_else_exception.pyi]
1209+
10581210
[case testOmittedUnrequiredElse]
10591211
# flags: --enable-error-code possibly-undefined
10601212
from typing import Literal
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
# Fixture for for-else tests with exceptions
2+
# Combines needed elements from primitives.pyi and exception.pyi
3+
4+
from typing import Generic, Iterator, Mapping, Sequence, TypeVar
5+
6+
T = TypeVar('T')
7+
V = TypeVar('V')
8+
9+
class object:
10+
def __init__(self) -> None: pass
11+
class type:
12+
def __init__(self, x: object) -> None: pass
13+
class int:
14+
def __init__(self, x: object = ..., base: int = ...) -> None: pass
15+
def __add__(self, i: int) -> int: pass
16+
def __rmul__(self, x: int) -> int: pass
17+
def __bool__(self) -> bool: pass
18+
def __eq__(self, x: object) -> bool: pass
19+
def __ne__(self, x: object) -> bool: pass
20+
def __lt__(self, x: 'int') -> bool: pass
21+
def __le__(self, x: 'int') -> bool: pass
22+
def __gt__(self, x: 'int') -> bool: pass
23+
def __ge__(self, x: 'int') -> bool: pass
24+
class float:
25+
def __float__(self) -> float: pass
26+
def __add__(self, x: float) -> float: pass
27+
def hex(self) -> str: pass
28+
class bool(int): pass
29+
class str(Sequence[str]):
30+
def __add__(self, s: str) -> str: pass
31+
def __iter__(self) -> Iterator[str]: pass
32+
def __contains__(self, other: object) -> bool: pass
33+
def __getitem__(self, item: int) -> str: pass
34+
def format(self, *args: object, **kwargs: object) -> str: pass
35+
class dict(Mapping[T, V]):
36+
def __iter__(self) -> Iterator[T]: pass
37+
class tuple(Generic[T]):
38+
def __contains__(self, other: object) -> bool: pass
39+
class ellipsis: pass
40+
41+
class BaseException:
42+
def __init__(self, *args: object) -> None: ...
43+
class Exception(BaseException): pass
44+
class RuntimeError(Exception): pass
45+
46+
class range(Sequence[int]):
47+
def __init__(self, __x: int, __y: int = ..., __z: int = ...) -> None: pass
48+
def count(self, value: int) -> int: pass
49+
def index(self, value: int) -> int: pass
50+
def __getitem__(self, i: int) -> int: pass
51+
def __iter__(self) -> Iterator[int]: pass
52+
def __contains__(self, other: object) -> bool: pass
53+
54+
def print(x: object) -> None: pass

0 commit comments

Comments
 (0)