Fix spurious possibly-undefined errors in for-else with break#19696
Conversation
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 python#14209 Fixes python#19690
This comment has been minimized.
This comment has been minimized.
|
@ilevkivskyi, I asked you for review since you reviewed the original change three years back. |
ilevkivskyi
left a comment
There was a problem hiding this comment.
TBH I forget many details since I last looked at this code, bu this looks reasonable. I just have a test suggestion.
| else: | ||
| raise RuntimeError | ||
|
|
||
| print(value) # Should not error - value is defined if we broke |
There was a problem hiding this comment.
Maybe also add one two tests where it should error? Like, e.g, if raise above is conditional.
There was a problem hiding this comment.
Good idea, I will add the test.
|
It would be cool to get this landed, I just stumbled across a similar false positive in our codebase. Is there anything left to be done apart from actually merging this fix? |
|
This comment has been minimized.
This comment has been minimized.
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
When a for loop contains branches with
breakand anelseblock, 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
elseblock is considered, variables declared in every branch of theforloop body that calledbreakare now considered as defined within the body of the loop.Fixes #14209
Fixes #19690