mypy doesn't seem to warn about control flows which might lead to UnboundLocalError, e.g.:
def func(arg):
if arg:
variable = 1
print(variable)
func(0)
Is this deliberate?
This is deliberate -- mypy generally doesn't check for whether a variable is defined or not, as it's not directly related to type checking
It's somewhat difficult to implement without false positives. It could be a useful thing to have, if we can (mostly) avoid false positives.
Ok. Thanks for the quick response. I haven't found any other linting tool that can do it either :-(
Actually I wonder if the "binder" in mypy could be used to do this -- it's been quite successful in determining whether the end of a block can actually be reached or not (in order to implement --warn-no-return), so maybe it could also keep track of whether a variable is set or not in each branch.
The original example should be possible to support without many false positives -- we define a variable conditionally and later read it unconditionally in a parent block, which looks like a pretty clear error, at least if there are no return/continue/break statements in the middle.
Things like this are harder:
if foo():
var = 1
...
if foo():
print(var) # may be safe or unsafe, depending on foo()
Not sure if those are common, but I've seen examples like that.
Mypy could also perhaps check if attributes of a class instance are defined, at least in some cases (see #2877 for an example).
For anyone stumbling across this: I've found that pytype https://github.com/google/pytype seems to be able to detect this issue.
This appears to be a common problem so raising priority to normal.
I'd like to have this feature even there will be a lot of false positives. I've fixed a lot of compiler and linter warnings in other languages that had no real impact and I don't feel bad about that. If you are concerned about false positives you can implement it as an opt-in feature.
I agree with @lilydjwg , this would be great as an opt-in feature. Does anyone know of a linter other than pytype that can currently detect potential UnboundLocalError?
I.e., a linter that would complain about either of the first two functions, but not the third one? I imagine an implementation like this one would be a lot simpler. It would have false positives, like in the second case, but these false positives are easy to get around.
This one could be ignored with # type: ignore, or it could be fixed by adding b = None or something similar above the if statement.
def a():
if False:
b = 10
print(b)
def a():
if True:
b = 10
print(b)
def a():
b = 5
if True:
b = 10
print(b)
Just to document the state of affairs with catching UnboundLocalError in various Python linters for others that come along...
pyflakes (and hence flake8) will probably never do this.
It looks like the only tool that does this is pytype, but it would be nice if mypy handled it instead, so that users wouldn't have to depend on pytype just to catch UnboundLocalError.
Edit: pytype handles this, but it looks like it really parses all of the code, e.g. to try to determine if a variable under an if statement was really declared so it can be referenced later. I just saw it fail to catch an UnboundLocalError in my code.
Avoiding false negatives, which IMO are much worse than false positives, seems to be another reason to avoid an overly "smart" (and hence difficult) implementation of this feature, at least at first.
@JukkaL @ilevkivskyi
Hey guys, any thoughts on this?
pyright catches this also
Currently have it installed literally only for this type of bug
I'd actually love to have this feature. We'll likely implement this at some point. If somebody wants this badly enough to volunteer a PR, I'm happy to give some hints. It might be a separate analysis pass after type checking.
If the feature generates many false positives, it might not be enabled by default, but I suspect that we can come up with something that's not too noisy. It's also possible that there would be a "strict" mode for this feature where we try very hard to avoid false negatives, and the default mode would only catch "obvious" errors.
This also passes and seems to be related:
def t() -> None:
try:
x = int('x')
except Exception:
print(x)
@lilydjwg
Just ran into essentially this bug in my code. I'd say it's not just related, it's the same thing. The exception raised is an UnboundLocalError.
@JukkaL I would like to work on this issue. Could you provide me with some pointers?
I've looked into this a bit. It's more complicated than I expected, in part because mypy acts directly on the abstract syntax tree rather than representing the execution semantics explicitly.
Handling this perfectly involves lots of special cases, e.g. differences between assignment semantics in Python 2 and 3 (comprehensions), removal of "if 0" (as the Python compiler does), and following predictable branching on literals/predictable type-aware exceptions such as division by zero. Basically all of the standard problems that cause warnings to change between versions in optimizing compilers as their code path following abilities change.
If you want to look into it, the main relevant files are checker.py, checkexpr.py and binder.py.
I'll try to get what work I've done on it so far cleaned up enough to at least be a useful starting point, which is something I've been meaning to do for a while.
Rather than wait until I have time to clean it up, I've just put it up as-is.
https://github.com/python/mypy/compare/master...tlynn:issue2400-wip?expand=1
This is full of print statements and other debug noise, and it's breaking (at least) these tests:
On the other hand, it introduces 48 passing tests (testUnboundLocal*). Hope it helps.
@tlynn Thanks for making your code available! This will be useful for anybody who wants to work on this.
Also ran into this today, my minimal example (that I'd expect to fail mypy) is this:
y: int
print(y)
Perhaps this (i.e. pep 526 declarations) is easier to start with than more general cases? (haven't found existing issues for this, sorry if it's a duplicate)
Most helpful comment
I'd actually love to have this feature. We'll likely implement this at some point. If somebody wants this badly enough to volunteer a PR, I'm happy to give some hints. It might be a separate analysis pass after type checking.
If the feature generates many false positives, it might not be enabled by default, but I suspect that we can come up with something that's not too noisy. It's also possible that there would be a "strict" mode for this feature where we try very hard to avoid false negatives, and the default mode would only catch "obvious" errors.