Fix type narrowing of == None and in (None,) conditions

[intgr]

Fixes #15759.

The type narrowing logic internally used is_optional() to check whether an operand type might be None. is_optional() only considers UnionType, and ignored an immediate NoneType type, which clearly is None-able.

I have created a new helper can_be_none(), which does also consider immediate NoneType.

The narrowing logic here could be further improved, but that's out of scope for the bugfix.

# Old behavior
def func(val: str | None) -> None:
    if val == None:
        reveal_type(val)  # ❌ Revealed type is "builtins.str"
    if val in (None,):
        reveal_type(val)  # ❌ Revealed type is "builtins.str"

# New behavior
def func(val: str | None) -> None:
    if val == None:
        reveal_type(val)  # ✅ Revealed type is "Union[builtins.str, None]"
    if val in (None,):
        reveal_type(val)  # ✅ Revealed type is "Union[builtins.str, None]"

TODO:

• Add tests

[intgr]

It seems to me that this logic should also use the new can_be_none(), but I'm not sure how to test this code path.

[ikonst]

meant to fail?

[intgr]

I'm not sure what you're asking. This test passes.

Mypy doesn't support type narrowing for these cases yet, I'm only fixing its buggy behavior here in this PR.

[ikonst]

Ahh, got it now! 👌

[ikonst]

(I assume actually narrowing is much more work, or simply unrelated and could be done incrementally?)

[intgr]

It's more tricky for sure, and quite rare in Python code. Not as easy as supporting is None, since classes (even a subclas of str) can override __eq__ and return True when compared to None.

I looked into this code because I want to solve #9718, I'd rather put my effort there for now :)

[hauntsaninja]

Thanks for fixing this... Can we just fold this logic into is_optional? I think everything works out (although maybe remove_optional should return UninhabitedType if fed None), and it seems less footgun-y

[sobolevn]

What about the difference between classes with and without custom __eq__?

In the original issue str instances can never be equal to None.

But, if some type has custom __eq__ - we cannot be sure about this anymore.

[intgr]

This comment is quite open-ended, there are so many nuances here that I'm left guessing what I should do.

This particular bug or fix isn't currently affected by __eq__ method.

Are you asking that I should add tests with str also? And add __eq__ method to A? It miiiight be helpful in the future when narrowing logic changes.

In the original issue str intances can never be equal to None.

One could possibly have a subclass instance of str that overrides __eq__. But is that considered unsound usage? I dunno.

---

There is another slightly related unsoundness case, when objects consider themselves equal to None. But that would be a separate bug and a harder discussion whether a fix would reduce the usefulness of narrowing.

https://mypy-play.net/?mypy=1.4.1&python=3.11&gist=d6199eb69f69c933ced7da5c51c25a50

[sobolevn]

This comment is good enough for me, thanks! :)

[intgr]

Opened another issue for the described case: #15764

[intgr]

@hauntsaninja

Can we just fold this logic into is_optional?

Makes sense, but the name seems potentially misleading. IMO "optional" implies "Union[T, None]". But NoneType by itself doesn't seem to fit the description "optional".

I could merge them under another name. So bikeshed time: can_be_none()? is_noneable()? is_possibly_none()? intersects_none()? overlaps_none()... None of these sound very good :)

I'm OK with keeping is_optional() too, I'd rather leave this decision up to mypy maintainers.

although maybe remove_optional should return UninhabitedType if fed None

👍 Makes sense and seems quite straightforward. I'm new to mypy development and I'd prefer moving in baby steps, I will do that in a follow-up PR.

[intgr]

I merged the two functions as suggested by hauntsaninja.

Due to the logic change, I opted to rename is_optional() -> is_overlapping_none(), because there is already precedent for similarly named functions (is_overlapping_types, is_overlapping_erased_types, is_enum_overlapping_union etc).

[github-actions]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅