Fix handling of NoReturn in Union

[kreathon]

Description

There are several discussions and comments describing the following problem (references can be found at the end of this post):

def func() -> str | NoReturn:
    ...

func().lower()

At the moment the code results in: "NoReturn" of "Union[str, NoReturn]" has no attribute "lower"

Union[int, NoReturn] should be equivalent to int, because in case the function returns it must be int.

Implementation

For every call expression apply make_simplified_union, which is already capable of transforming Union[int, NoReturn] to int.

ret_type = self.check_call_expr_with_callee_type(...)
...
if isinstance(ret_type, UnionType):
    ret_type = make_simplified_union(ret_type.items)

Test Plan

The following test case was added to verify the implementation is working as intended:

[case` testUnionWithNoReturn]
from typing import Union, NoReturn
def f() -> Union[int, NoReturn]: return 1
reveal_type(f()) # N: Revealed type is "builtins.int"

References

• Union[X, NoReturn] isn't properly narrowed to X #11876
• Some thoughts on `Union[X, NoReturn]` typing#994 (comment)
• Consider changing NoReturn to not be a bottom type #4116 (comment))

[github-actions]

Diff from mypy_primer, showing the effect of this PR on open source code:

steam.py (https://github.com/Gobot1234/steam.py)
- steam/state.py:842: error: Incompatible types in assignment (expression has type "Optional[User]", variable has type "CMsgClientFriendsListFriend")  [assignment]
+ steam/state.py:842: error: Incompatible types in assignment (expression has type "User", variable has type "CMsgClientFriendsListFriend")  [assignment]

spark (https://github.com/apache/spark)
+ python/pyspark/pandas/series.py:4246: error: On Python 3 formatting "b'abc'" with "{}" produces "b'abc'", not "abc"; use "{!r}" if this is desired behavior  [str-bytes-safe]

pip (https://github.com/pypa/pip)
- src/pip/_internal/cli/progress_bars.py:99: error: Incompatible types in assignment (expression has type "Callable[[int, FrameType], None]", variable has type "Union[Callable[[int, Optional[FrameType]], Any], int, Handlers, None]")
+ src/pip/_internal/cli/progress_bars.py:99: error: Incompatible types in assignment (expression has type "Callable[[int, FrameType], None]", variable has type "Union[Callable[[int, Optional[FrameType]], Any], int, None]")

[sobolevn]

@kreathon can you please analyze primer output? Some types have changed. Is it right?

[kreathon]

@sobolevn Yes, it seems that something changed. But the changes are not obvious to me. I am going to have a closer look.

[kreathon]

mypy_primer Analysis

@sobolevn

steam.py

steam.py (https://github.com/Gobot1234/steam.py)
- steam/state.py:842: error: Incompatible types in assignment (expression has type "Optional[User]", variable has type "CMsgClientFriendsListFriend")  [assignment]
+ steam/state.py:842: error: Incompatible types in assignment (expression has type "User", variable has type "CMsgClientFriendsListFriend")  [assignment]

The project uses strict_optional = False (config)

Code:

def get_user(self, id64: int) -> User | None:
    return self._users.get(id64)

...
friend = self.get_user(steam_id.id64)

make_simplified_union runs is_proper_subtype, which simplifies User | None to simply User (because without strict optional: None is a subtype of User)

spark

spark (https://github.com/apache/spark)
+ python/pyspark/pandas/series.py:4246: error: On Python 3 formatting "b'abc'" with "{}" produces "b'abc'", not "abc"; use "{!r}" if this is desired behavior  [str-bytes-safe]

Simplified version of the code:

most_value = ser_count.max()
reveal_type(most_value)  # added by me
sdf_count.filter("count == {}".format(most_value))

On the new implementation (where make_simplified_union is called) we get:

Revealed type is "Union[builtins.float, builtins.str, builtins.bytes, decimal.Decimal, datetime.date, pyspark.pandas.series.Series[Any]]

On current master we get:

Revealed type is "Union[Union[builtins.int, builtins.float, builtins.bool, builtins.str, builtins.bytes, decimal.Decimal, datetime.date, datetime.datetime, None], pyspark.pandas.series.Series[Any]]

I think the new behavior is actually correct, because bytes is a possible value for most_value. The problem is caused by check_specs_in_format_call.

If the Union is not nested, for every possible type it is checked whether there is a custom __format__, if yes no checks are run, otherwise perform_special_format_checks is called (which fails for bytes).

For a nested Union for all items of the inner Union it is checked whether any of them have a custom __format__ (which is the case here), thus the perform_special_format_checks is not evaluated for bytes.

I could try to investigate this on another branch, but I am not sure if we can get much out of it, since Unions are already simplified on a lot of other occasions (as far as I understand it).

pip

pip (https://github.com/pypa/pip)
- src/pip/_internal/cli/progress_bars.py:99: error: Incompatible types in assignment (expression has type "Callable[[int, FrameType], None]", variable has type "Union[Callable[[int, Optional[FrameType]], Any], int, Handlers, None]")
+ src/pip/_internal/cli/progress_bars.py:99: error: Incompatible types in assignment (expression has type "Callable[[int, FrameType], None]", variable has type "Union[Callable[[int, Optional[FrameType]], Any], int, None]")

signal.Handlers is simplified because we have the following type hierarchy

buildins.int -> enum.IntEnum -> signal.Handlers

[sobolevn]

For a nested Union for all items of the inner Union it is checked whether any of them have a custom format (which is the case here), thus the perform_special_format_checks is not evaluated for bytes.

This looks like a separate bug, do you want to work on it after this one? 🙂

[sobolevn]

LGTM! Thank you! Great analysis!

Going to wait for some else to double check and merge 🙂

[kreathon]

This looks like a separate bug, do you want to work on it after this one? slightly_smiling_face<<

Yes, I can do that. I am going to open a new issue as soon as I have collected more information.

[JukkaL]

For consistency, the constructor of UnionType should perhaps remove NoReturn unless it's the only item in the union. We can still get errors about NoReturn union items in other contexts, which could be argued to be false positives:

from typing import Union, NoReturn
x: Union[str, NoReturn]
x + 's'  # Unsupported left operand type for + ("NoReturn")

We could perhaps do this in flatten_nested_unions (though we may want to rename the function). However, it seems that return types are most likely to contain these unions than other contexts, so this PR seems like a worthwhile improvement.

(Additional rationale: I think make_simplified_union should not alter the semantics of the union type, which it currently does by removing NoReturn types.)

@kreathon What do you think? Would you like to try to work on the general case? We can also just create a follow-up issue where this can be discussed.