Enable use-before-def error code by default

[ilinum]

This enables the error code added in #14163.

We aren't sure if it will cause too many problems. We can see what the mypy primer outputs as well.

[ilinum]

Looks like a lot of mypy_primer failures are from partially_defined not recognizing imports. I'll try to make it handle imports soon.

[AlexWaygood]

It looks like this check is triggering some hits in .pyi files. Maybe partially-defined checks should all be skipped for stub files, since they're never actually executed at runtime?

[ilinum]

It looks like this check is triggering some hits in .pyi files. Maybe partially-defined checks should all be skipped for stub files, since they're never actually executed at runtime?

Yep, good idea!

[AlexWaygood]

It looks like sympy has a lot of code along the lines of:

for i in range(2):
    if i == 0:
        foo = 1
    else:
        bar = foo

The partially-defined check can't understand that foo will always be defined here, and I'm not sure it's reasonable to ask it to understand code like that. Seems like sympy won't thank us, though :/

[ilinum]

It looks like sympy has a lot of code along the lines of:

for i in range(2):
    if i == 0:
        foo = 1
    else:
        bar = foo

The partially-defined check can't understand that foo will always be defined here, and I'm not sure it's reasonable to ask it to understand code like that. Seems like sympy won't thank us, though :/

Yep, agreed.

We now have two checks in place: use-before-def and partially-defined. Potentially, we can call the example you listed as partially-defined and accept that it will have a higher false-positive rate. That way use-before-def will have a lower false-positive rate and makes more sense to enable it by default/have users not disable it.

[AlexWaygood]

Potentially, we can call the example you listed as partially-defined and accept that it will have a higher false-positive rate. That way use-before-def will have a lower false-positive rate and makes more sense to enable it by default/have users not disable it.

I think that would be a good idea, if it's not too difficult to do. Just the difference in error message is quite significant imo — Name "foo" is used before definition is a lot more sure of itself than Name "foo" may be undefined. It would be great to limit the "more confident" error message to cases where the chance of a false-positive is very low.

(Thanks for all the work you've been doing on this btw — this is a great feature!)

[ilinum]

#14175 has the fixes from this PR but doesn't enable use-before-def by default.

[ilinum]

After #14175 is merged, we will have the following false-positives causes for use-before-def check (roughly in the order of common they are):

1. Issues with try/except (should be fixed by [partially defined] implement support for try statements #14114).
2. Loops that execute different code on first iteration (example).
3. A version of the loop thing above but the variable is used in else, not in for (example).
4. Function uses a global scope variable but function definition comes before the variable definition. (example)

As Alex and I discussed above, solving (2) and (3) is difficult/not feasible to handle correctly in the near term. I'll probably send a PR to move detection of these to the partially-defined check, so that use-before-def has fewer false positives.

I'm not sure what we can do for (4). Most of these seem to be when using global, so we can try to treat doing global foo as a definition of foo.

@AlexWaygood @JukkaL @ilevkivskyi do you have thoughts?

[ilinum]

#14176 moves the false positives from (2) and (3) to partially defined check.

[ilinum]

Assuming tests pass, all that's remaining should be:

• fix that range false positive in pyspark
• merge [undefined vars] fix per-module error code override bug #14351 and [undefined vars] do not double report errors in class defs #14350 (both are included in the this PRs branch)

[github-actions]

Diff from mypy_primer, showing the effect of this PR on open source code:

prefect (https://github.com/PrefectHQ/prefect)
+ src/prefect/plugins.py:59: error: Name "result" is used before definition  [used-before-def]

kornia (https://github.com/kornia/kornia)
+ kornia/augmentation/container/utils.py:289: error: Name "_param" is used before definition  [used-before-def]
+ kornia/augmentation/container/utils.py:299: error: Name "geo_param" is used before definition  [used-before-def]

spark (https://github.com/apache/spark)
+ python/pyspark/pandas/namespace.py:159: error: Name "range" is used before definition  [used-before-def]

[github-actions]

Diff from mypy_primer, showing the effect of this PR on open source code:

prefect (https://github.com/PrefectHQ/prefect)
+ src/prefect/plugins.py:59: error: Name "result" is used before definition  [used-before-def]

kornia (https://github.com/kornia/kornia)
+ kornia/augmentation/container/utils.py:289: error: Name "_param" is used before definition  [used-before-def]
+ kornia/augmentation/container/utils.py:299: error: Name "geo_param" is used before definition  [used-before-def]

spark (https://github.com/apache/spark)
+ python/pyspark/pandas/namespace.py:159: error: Name "range" is used before definition  [used-before-def]

[github-actions]

Diff from mypy_primer, showing the effect of this PR on open source code:

prefect (https://github.com/PrefectHQ/prefect)
+ src/prefect/plugins.py:59: error: Name "result" is used before definition  [used-before-def]

kornia (https://github.com/kornia/kornia)
+ kornia/augmentation/container/utils.py:289: error: Name "_param" is used before definition  [used-before-def]
+ kornia/augmentation/container/utils.py:299: error: Name "geo_param" is used before definition  [used-before-def]

spark (https://github.com/apache/spark)
+ python/pyspark/pandas/namespace.py:159: error: Name "range" is used before definition  [used-before-def]

[github-actions]

Diff from mypy_primer, showing the effect of this PR on open source code:

prefect (https://github.com/PrefectHQ/prefect)
+ src/prefect/plugins.py:59: error: Name "result" is used before definition  [used-before-def]

kornia (https://github.com/kornia/kornia)
+ kornia/augmentation/container/utils.py:289: error: Name "_param" is used before definition  [used-before-def]
+ kornia/augmentation/container/utils.py:299: error: Name "geo_param" is used before definition  [used-before-def]

spark (https://github.com/apache/spark)
+ python/pyspark/pandas/namespace.py:159: error: Name "range" is used before definition  [used-before-def]

[JukkaL]

Looks good to me. It's exciting to have mypy finally detect undefined variables!

[gandhis1]

Is this not a false positive?

spark (https://github.com/apache/spark)
+ python/pyspark/pandas/namespace.py:159: error: Name "range" is used before definition  [used-before-def]

[JukkaL]

Is this not a false positive?

Yes. We decided that it is still acceptable to merge PR, since there is only one false positive and this also finds real errors. Created #14476 to track the false positive.