Skip to content

Commit db0beb1

Browse files
authored
Switch error code used to report vars defined in different branch (#14176)
We previously used `use-before-def` code here but this commit switched it to use `partially-defined`. This particular check generates a lot of false positives, in particular around loops of the form: ```python for i in range(2) if i == 0: x = 1 else: y = x ``` While in an ideal world mypy has no false positives, it's not feasible for us to handle this correctly in the short-term. Moving this to partially-defined error code makes the `use-before-def` have a much lower false positive rate, which is a plus. Unfortunately, `partially-defined` will always have a higher false positive rate. This means that if we enable it by default, lots of people will disable this check. We want to avoid the same thing happening to use-before-def check. See [this PR](#14166 (comment)) for further discussion.
1 parent 278a095 commit db0beb1

File tree

2 files changed

+53
-17
lines changed

2 files changed

+53
-17
lines changed

mypy/partially_defined.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,7 @@ class PartiallyDefinedVariableVisitor(ExtendedTraverserVisitor):
234234
def __init__(self, msg: MessageBuilder, type_map: dict[Expression, Type]) -> None:
235235
self.msg = msg
236236
self.type_map = type_map
237+
self.loop_depth = 0
237238
self.tracker = DefinedVariableTracker()
238239

239240
def process_lvalue(self, lvalue: Lvalue | None) -> None:
@@ -319,10 +320,12 @@ def visit_for_stmt(self, o: ForStmt) -> None:
319320
self.process_lvalue(o.index)
320321
o.index.accept(self)
321322
self.tracker.start_branch_statement()
323+
self.loop_depth += 1
322324
o.body.accept(self)
323325
self.tracker.next_branch()
324326
if o.else_body:
325327
o.else_body.accept(self)
328+
self.loop_depth -= 1
326329
self.tracker.end_branch_statement()
327330

328331
def visit_return_stmt(self, o: ReturnStmt) -> None:
@@ -354,7 +357,9 @@ def visit_expression_stmt(self, o: ExpressionStmt) -> None:
354357
def visit_while_stmt(self, o: WhileStmt) -> None:
355358
o.expr.accept(self)
356359
self.tracker.start_branch_statement()
360+
self.loop_depth += 1
357361
o.body.accept(self)
362+
self.loop_depth -= 1
358363
if not checker.is_true_literal(o.expr):
359364
self.tracker.next_branch()
360365
if o.else_body:
@@ -380,7 +385,10 @@ def visit_name_expr(self, o: NameExpr) -> None:
380385
self.tracker.record_definition(o.name)
381386
elif self.tracker.is_defined_in_different_branch(o.name):
382387
# A variable is defined in one branch but used in a different branch.
383-
self.msg.var_used_before_def(o.name, o)
388+
if self.loop_depth > 0:
389+
self.msg.variable_may_be_undefined(o.name, o)
390+
else:
391+
self.msg.var_used_before_def(o.name, o)
384392
elif self.tracker.is_undefined(o.name):
385393
# A variable is undefined. It could be due to two things:
386394
# 1. A variable is just totally undefined

test-data/unit/check-partially-defined.test

Lines changed: 44 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,48 @@ def f5() -> int:
206206
return 3
207207
return 1
208208

209+
[case testDefinedDifferentBranchUseBeforeDef]
210+
# flags: --enable-error-code partially-defined --enable-error-code use-before-def
211+
212+
def f0() -> None:
213+
if int():
214+
x = 0
215+
else:
216+
y = x # E: Name "x" is used before definition
217+
z = x # E: Name "x" is used before definition
218+
219+
def f1() -> None:
220+
x = 1
221+
if int():
222+
x = 0
223+
else:
224+
y = x # No error.
225+
226+
227+
[case testDefinedDifferentBranchPartiallyDefined]
228+
# flags: --enable-error-code partially-defined --enable-error-code use-before-def
229+
230+
def f0() -> None:
231+
first_iter = True
232+
for i in [0, 1]:
233+
if first_iter:
234+
first_iter = False
235+
x = 0
236+
else:
237+
# This is technically a false positive but mypy isn't smart enough for this yet.
238+
y = x # E: Name "x" may be undefined
239+
z = x # E: Name "x" may be undefined
240+
241+
242+
def f1() -> None:
243+
while True:
244+
if int():
245+
x = 0
246+
else:
247+
y = x # E: Name "x" may be undefined
248+
z = x # E: Name "x" may be undefined
249+
250+
209251
[case testAssert]
210252
# flags: --enable-error-code partially-defined
211253
def f1() -> int:
@@ -394,21 +436,7 @@ def f0() -> None:
394436
x = y # E: Name "y" is used before definition
395437
y: int = 1
396438

397-
def f1() -> None:
398-
if int():
399-
x = 0
400-
else:
401-
y = x # E: Name "x" is used before definition
402-
z = x # E: Name "x" is used before definition
403-
404439
def f2() -> None:
405-
x = 1
406-
if int():
407-
x = 0
408-
else:
409-
y = x # No error.
410-
411-
def f3() -> None:
412440
if int():
413441
pass
414442
else:
@@ -418,14 +446,14 @@ def f3() -> None:
418446
def inner2() -> None:
419447
z = 0
420448

421-
def f4() -> None:
449+
def f3() -> None:
422450
if int():
423451
pass
424452
else:
425453
y = z # E: Name "z" is used before definition
426454
z: int = 2
427455

428-
def f5() -> None:
456+
def f4() -> None:
429457
if int():
430458
pass
431459
else:

0 commit comments

Comments
 (0)