Unbound variable in "for/else" statement

[Azureblade3808]

Environment data

• Language Server version: Pylance 2021.2.0
• OS and version: Windows 7
• Python version (& distribution if applicable, e.g. Anaconda): Python 3.8.5 (Anaconda)

Example

for _ in []:
    pass
else:
    a = 0

print(a)  # Error reported: "a" is possibly unbound

Expected Behavior

No error reported

Other Info

"while/else" statement works correctly.

[jakebailey]

Hm, the only way that this code can actually cause a NameError is if the list were non-empty and the for loop contained a break statement:

for _ in [0]:
    break
else:
    a = 0

print(a)

We can't know statically how many items a list contains, but the fact that we are saying "unbound" here without break is surprising.

[erictraut]

I would expect the sample above to produce this error for the reason that Jake mentioned. However, I noticed that it also produces the error if I replace pass with a = 0. That's definitely a bug — probably a regression that I introduced recently. I'll investigate and fix that.

I'm also considering making the "unbound" diagnostics part of the reportGeneralTypeIssue diagnostic rule because these are being reported too frequently by users who do not have the type checker enabled, and they perceive these as false positives.

[jakebailey]

I still think these are valuable enough to leave on, if we can figure out the existing bugs like this one at least. We can see.

[erictraut]

The problem is that we can't eliminate all false positives. The sample above is a good example. From the perspective of a developer who knows that they're iterating over an empty list, they can tell that the "for" part of the loop will never be executed, and a should always be bound. A type checker doesn't know that because iterator types don't contain any information about how many times they will iterate.

I think I'd prefer to revert my previous change, which watered down for iterator targets in for loops (i.e. it introduced false negatives) along with disabling unbound variable diagnostics when type checking is off.

[jakebailey]

I don't think this is a good idea; to me this is similar to how we flag imports even though maybe there's some condition where they would work. It's something that greatly impacts the analysis and may produce confusing issues later when the type is listed as Unbound.

I don't really think that we want to change the default if it's been working so well for the large bulk of the userbase due to a few issues that have been reported; I feel like many of them were actually revealing potential bugs anyway.

Sure, it's a shame this "obvious" code would always bind a, but I don't know where we can draw the line for that obviousness. @Azureblade3808 Is this a cut down version of code you've actually written, or is this just a case you came up with to test the code flow?

And FWIW, this code runs fine, so it's not entirely just about what's in the list:

for _ in [0]:
    pass
else:
    a = 0

print(a)

[erictraut]

I've reverted the earlier change because it not only weakened the check but also introduced a regression.

I've also changed the default severity for the "reportUnboundVariable" diagnostic rule from "warning" to "none" when typeCheckingMode is "off". There are too many complaints of false positives from users who are not interested in receiving these warnings.

[Azureblade3808]

Sorry for being late. It might be unclear in the example code, so let me explain a little further.

Just think of an "for/else" statement. Code in the "for" clause runs conditionally, as the number of iterations can be zero. It is nearly impossible to tell how many the iterations will be. Meanwhile, the "else" clause runs conditionally when there is a "break", and is determined to run if there is no "break". I think it should be possible to tell whether there is one or more "break"s and these two situations should be treated differently.

for _ in some_iterable:
    a = 0
else:
    b = 1

# `a` might be unbound but `b` should be bound.

for _ in some_iterable:
    a = 0
    if some_condition_except_literal_false:
        break
else:
    b = 1

# Both `a` and `b` might be unbound.

[erictraut]

I agree. With my latest changes, the output of pylance (when the reportUnboundVariable diagnostic rule is enabled) matches what is shown in your two examples above.

[jakebailey]

This issue has been fixed in version 2021.2.1, which we've just released. You can find the changelog here: https://github.com/microsoft/pylance-release/blob/main/CHANGELOG.md#202121-10-february-2021