--warn-unreachable false positive with minimal example without types explicitly set

[LefterisJP]

Hello,

I just downloaded the new mypy v0.720 and am playing with the --warn-unreachable-flag in my projects.

I noticed one occasion where there is something that seems like a false positive, so I am making an issue for it with a minimal reproducible example.

Version

mypy==0.720
mypy-extensions==0.4.1

source code example

import sys
from typing import Optional

found_id = None

symbols_list = [{'symbol': 'A', 'id': 1}, {'symbol': 'B', 'id': 2},  {'symbol': 'B', 'id': 3}]

for entry in symbols_list:
    if entry['symbol'] == 'B':
        if found_id:
            print('error, symbol found multiple times!')
            sys.exit(1)

        found_id = entry['id']

This when tested with mypy test.py --warn-unreachable returns:

test.py:10: error: Statement is unreachable

So essentially telling us that if found_id is never entered due to the found_id type being None. But in reality the type should be Optional[int] since you can see that it's set to entry['id'] later.

If you explicitly set the types mypy no longer complains:

import sys
from typing import Optional

found_id: Optional[int] = None

symbols_list = [{'symbol': 'A', 'id': 1}, {'symbol': 'B', 'id': 2},  {'symbol': 'B', 'id': 3}]

for entry in symbols_list:
    if entry['symbol'] == 'B':
        if found_id:
            print('error, symbol found multiple times!')
            sys.exit(1)

        assert isinstance(entry['id'], int)
        found_id = entry['id']

Bug or not

Essentially I am not sure if this is intended behaviour or not. After writing this issue down I realize it all boils down to the matter of mypy not correctly inferring type of found_id.

[JelleZijlstra]

I encountered something similar in our codebase. I agree that it's a bug.

[ilevkivskyi]

This actually may be not easy to fix.

The problem here is how binder interacts with partial types. We have this nice thing called conditional type binder that does our version of abstract interpretation (thanks to Pyre team for the official term for this). In particular, it repeatedly checks loop bodies until no types have changed. Here is a problem however: it looks like it doesn't detect a change of type if it happens because of partial types (partial None type in this example).

We have an item in our roadmap to refactor/rethink the binder. Maybe we should actually do this, maybe this should be the next big refactoring after we are done with semantic analyzer?

My ideal large scale view of this is like we have fine grained targets (module top-levels and top level functions). These targets can be deferred (currently only functions), but unlike in semantic analyzer we process them in mixed order because some attributes inferred in constructor may be needed at a top level. In every target we have an "abstract interpreter" that interprets expressions in domain of types instead of domain of values. It combines the functionality currently divided between partial types and type binder.

It seems to me such model may be also more reasonable if we are going to move --allow-redefinition to something more SSA-like, plus we will be able to generalize partial types to being able infer union types after a conditional definition.

A downside however is that we might need to make --local-partial-types the default and only supported way.

[JukkaL]

I think that we should first try to come up with a simple fix. I suspect that we don't need a big redesign to fix this, but I'm not sure.

@ilevkivskyi Can you create a separate issue about the binder refactoring? I agree that it's a good candidate for our next big refactoring. The first step might be a pure refactor that just cleans up and documents the existing behavior.

[ilevkivskyi]

@JukkaL

Can you create a separate issue about the binder refactoring?

Opened https://github.com/python/mypy/issues/7324

[Herst]

Another testcase.

This one causes mypy 0.720 the show false positive warning:

from typing import Optional, Tuple

def f() -> Tuple[str, str]:
    return ("a", "b")

a = None

while a is None or a == "a":
    a, b = f()

"Workaround" by making the variable explicitly optional:

from typing import Optional, Tuple

def f() -> Tuple[str, str]:
    return ("a", "b")

a = None # type: Optional[str]

while a is None or a == "a":
    a, b = f()

Even more minimal:

a = None
while a is None or a == "a":
    a = "a"

[patrickelectric]

I have the same scenario described by @Herst, it looks indeed a bug.