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

Narrowing inconsistency between == and "in" #17864

Closed
JukkaL opened this issue Oct 2, 2024 · 0 comments · Fixed by #17865
Closed

Narrowing inconsistency between == and "in" #17864

JukkaL opened this issue Oct 2, 2024 · 0 comments · Fixed by #17865
Labels
bug mypy got something wrong

Comments

@JukkaL
Copy link
Collaborator

JukkaL commented Oct 2, 2024

Type narrowing behaves inconsistently:

def f(x: str | int) -> None:
    if x == "x":
        reveal_type(x)  # str | int
    if x in ["x"]:
        reveal_type(x)  # str

Both of these should infer the same type. This is a regression that was introduced in #17344 (cc @Jordandev678).

Also, unlike with ==, narrowing using "in" doesn't consider the possibility of a custom __eq__ method, so it can infer the wrong type:

class C:
    def __init__(self, x: int) -> None:
        self.x = x

    def __eq__(self, other: object) -> bool:
        if isinstance(other, C) and other.x == self.x:
            return True
        return NotImplemented

class D(C):
    pass

def f(x: C) -> None:
    if x in [D(5)]:
        print(type(x))  # C
        reveal_type(x)  # D

f(C(5))

I'm going to revert to #17344 since fixing the logic may be non-trivial, and there is also another related bug (#17841).

The original PR can be submitted again with a fix that uses the same logic we use for == narrowing. If there is desire to support more narrowing use cases, this should be addressed in a separate issue/PR, and both == and "in" narrowing should remain consistent.

@JukkaL JukkaL added the bug mypy got something wrong label Oct 2, 2024
JukkaL added a commit that referenced this issue Oct 2, 2024
This reverts commit ed0cd4a.

See #17864 for context. The implementation has some issues, and I'm
reverting the PR to unblock the 1.12 release.
@JukkaL JukkaL closed this as completed in 329e38e Oct 2, 2024
hauntsaninja added a commit that referenced this issue Oct 5, 2024
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

Successfully merging a pull request may close this issue.

1 participant