Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not detecting UnboundLocalError #2400

Closed
tlynn opened this issue Nov 4, 2016 · 27 comments
Closed

Not detecting UnboundLocalError #2400

tlynn opened this issue Nov 4, 2016 · 27 comments
Labels
feature priority-0-high topic-runtime-semantics mypy doesn't model runtime semantics correctly

Comments

@tlynn
Copy link

tlynn commented Nov 4, 2016

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?

@JukkaL
Copy link
Collaborator

JukkaL commented Nov 4, 2016

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.

@tlynn
Copy link
Author

tlynn commented Nov 4, 2016

Ok. Thanks for the quick response. I haven't found any other linting tool that can do it either :-(

@gvanrossum
Copy link
Member

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.

@JukkaL
Copy link
Collaborator

JukkaL commented Nov 23, 2016

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.

@JukkaL
Copy link
Collaborator

JukkaL commented Feb 24, 2017

Mypy could also perhaps check if attributes of a class instance are defined, at least in some cases (see #2877 for an example).

@tlynn
Copy link
Author

tlynn commented Nov 16, 2017

For anyone stumbling across this: I've found that pytype https://github.com/google/pytype seems to be able to detect this issue.

@ilevkivskyi
Copy link
Member

This appears to be a common problem so raising priority to normal.

@lilydjwg
Copy link

lilydjwg commented Nov 6, 2018

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.

@kylebebak
Copy link

kylebebak commented Mar 17, 2019

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)

@kylebebak
Copy link

kylebebak commented Mar 17, 2019

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.

Neither does pylint.

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.

@kylebebak
Copy link

@JukkaL @ilevkivskyi
Hey guys, any thoughts on this?

@CoenraadS
Copy link

pyright catches this also

Currently have it installed literally only for this type of bug

@JukkaL
Copy link
Collaborator

JukkaL commented Sep 16, 2019

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.

@lilydjwg
Copy link

This also passes and seems to be related:

def t() -> None:
  try:
    x = int('x')
  except Exception:
    print(x)

@kylebebak
Copy link

@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.

@Arshaan-256
Copy link

@JukkaL I would like to work on this issue. Could you provide me with some pointers?

@tlynn
Copy link
Author

tlynn commented Mar 13, 2020

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.

@tlynn
Copy link
Author

tlynn commented Mar 13, 2020

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:

  • testUnusedTargetForLoop
  • testLambdaDefaultTypeErrors
  • testRedefineLocalForLoopIndexVariable
  • testCannotRedefineLocalWithinTry
  • testRedefineWithBreakAndContinue
  • testNewAnalyzerCyclicDefinitions
  • testNewAnalyzerNamedTupleCallNestedMethod

On the other hand, it introduces 48 passing tests (testUnboundLocal*). Hope it helps.

@JukkaL
Copy link
Collaborator

JukkaL commented Mar 20, 2020

@tlynn Thanks for making your code available! This will be useful for anybody who wants to work on this.

@karlicoss
Copy link

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)

@freundTech
Copy link
Contributor

Here's another related example, that passes mypy, but should obviously fail:

if condition:
    a = 1
else:
    print(a)

@JukkaL
Copy link
Collaborator

JukkaL commented Mar 13, 2021

We have a tentative plan to work on this in the next few months.

@JelleZijlstra
Copy link
Member

FYI, https://github.com/quora/pyanalyze will detect this sort of thing.

@Baukebrenninkmeijer
Copy link

@JukkaL Is there any progress with this issue? Can we track is somewhere?

@JelleZijlstra
Copy link
Member

Duplicate of #686 and/or #4019.

@inghowepang
Copy link

I think this unbounded feature is quite critical. Due lack of this feature, it will affect our team decision for using other tools like pylance rather than mypy.

@Dreamsorcerer
Copy link
Contributor

If you looked at the dupes that closed this issue, you'd see that cases for variables have already been resolved (presumably appearing in the next release, as it was closed only a few weeks ago). There is still an issue open to track unbound attributes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature priority-0-high topic-runtime-semantics mypy doesn't model runtime semantics correctly
Projects
None yet
Development

No branches or pull requests