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

Combining two type checks with and deduces Any if the first is Any #17100

Open
sth opened this issue Apr 5, 2024 · 2 comments
Open

Combining two type checks with and deduces Any if the first is Any #17100

sth opened this issue Apr 5, 2024 · 2 comments
Labels
bug mypy got something wrong

Comments

@sth
Copy link

sth commented Apr 5, 2024

Combining two isinstance checks with and where the first one deduces the type to Any ignores the second one.

To Reproduce

from typing import Any, reveal_type

class A: pass
class B(A): pass  
value: Any

if isinstance(value, A) and not isinstance(value, B):
    reveal_type(value)
if not isinstance(value, B) and isinstance(value, A):
    reveal_type(value)

https://mypy-play.net/?mypy=latest&python=3.12&gist=643066dcf62c1c2b88bb4714032fa8b5

Expected Behavior

Mypy should deduce the type as A in both cases:

main.py:8: note: Revealed type is "__main__.A"
main.py:10: note: Revealed type is "__main__.A"

Actual Behavior

main.py:8: note: Revealed type is "__main__.A"
main.py:10: note: Revealed type is "Any"

Background

This worked before mypy 1.7 and is a regression caused by #16237. The direct reason is that Any is now preferred in some circumstances in and_conditional_maps() (the isinstance in the if):

mypy/mypy/checker.py

Lines 7609 to 7618 in 8019010

# Both conditions can be true; combine the information. Anything
# we learn from either conditions' truth is valid. If the same
# expression's type is refined by both conditions, we somewhat
# arbitrarily give precedence to m2 unless m1 value is Any.
# In the future, we could use an intersection type or meet_types().
result = m2.copy()
m2_keys = {literal_hash(n2) for n2 in m2}
for n1 in m1:
if literal_hash(n1) not in m2_keys or isinstance(get_proper_type(m1[n1]), AnyType):
result[n1] = m1[n1]

I'm not sure why this is the case or why it is needed to prefer Any here. Intuitively, it should be the other way around. The additional Any doesn't add any new information but it shouldn't destroy the information already known from other sources.

When I remove the or isinstance(get_proper_type(m1[n1]), AnyType) in the above code, only one testcase fails (in testNarrowingWithAnyOps) because it explicitly tests for this Any result, like this:

class C: ...
class D(C): ...
tp: Any
...

c1: C
if isinstance(c1, tp) and isinstance(c1, D):
    reveal_type(c1)  # N: Revealed type is "Any"

With the changed code this deduces the type as D instead.

@sth sth added the bug mypy got something wrong label Apr 5, 2024
@sth
Copy link
Author

sth commented May 4, 2024

Maybe @ilevkivskyi remembers why the check was implemented this way?

@yonilerner
Copy link

I wonder if this is the same bug that causes this:

from typing import Type, Any

class Foo: pass

x: Any

if x and isinstance(x, Foo):
    reveal_type(x) # Reveals as Any instead of Foo

https://mypy-play.net/?mypy=master&python=3.11&gist=a89cc8d27cb963b8b5919595f73894b8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong
Projects
None yet
Development

No branches or pull requests

2 participants