Skip to content

Improve consistency of NoReturn #12043

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions mypy/checkexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -378,8 +378,6 @@ def visit_call_expr_inner(self, e: CallExpr, allow_none_return: bool = False) ->
if isinstance(e.callee, MemberExpr) and e.callee.name == 'format':
self.check_str_format_call(e)
ret_type = get_proper_type(ret_type)
if isinstance(ret_type, UnionType):
ret_type = make_simplified_union(ret_type.items)
if isinstance(ret_type, UninhabitedType) and not ret_type.ambiguous:
self.chk.binder.unreachable()
# Warn on calls to functions that always return None. The check
Expand Down
6 changes: 6 additions & 0 deletions mypy/test/testtypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,12 @@ def test_simplified_union(self) -> None:
self.assert_simplified_union([fx.lit1, fx.lit2_inst], UnionType([fx.lit1, fx.lit2_inst]))
self.assert_simplified_union([fx.lit1, fx.lit3_inst], UnionType([fx.lit1, fx.lit3_inst]))

self.assert_simplified_union([fx.noreturn], fx.noreturn)
self.assert_simplified_union([fx.noreturn, fx.noreturn], fx.noreturn)
self.assert_simplified_union([fx.a, fx.noreturn], UnionType([fx.a, fx.noreturn]))
self.assert_simplified_union([fx.lit1_inst, fx.noreturn],
UnionType([fx.lit1_inst, fx.noreturn]))

def assert_simplified_union(self, original: List[Type], union: Type) -> None:
assert_equal(make_simplified_union(original), union)
assert_equal(make_simplified_union(list(reversed(original))), union)
Expand Down
1 change: 1 addition & 0 deletions mypy/test/typefixture.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ def make_type_var(name: str, id: int, values: List[Type], upper_bound: Type,
self.anyt = AnyType(TypeOfAny.special_form)
self.nonet = NoneType()
self.uninhabited = UninhabitedType()
self.noreturn = UninhabitedType(is_noreturn=True)

# Abstract class TypeInfos

Expand Down
12 changes: 9 additions & 3 deletions mypy/typeops.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@
import sys

from mypy.types import (
TupleType, Instance, FunctionLike, Type, CallableType, TypeVarLikeType, Overloaded,
TupleType, Instance, FunctionLike, Type, CallableType, TypeVarLikeType,
Overloaded,
TypeVarType, UninhabitedType, FormalArgument, UnionType, NoneType,
AnyType, TypeOfAny, TypeType, ProperType, LiteralType, get_proper_type, get_proper_types,
copy_type, TypeAliasType, TypeQuery, ParamSpecType
AnyType, TypeOfAny, TypeType, ProperType, LiteralType, get_proper_type,
get_proper_types,
copy_type, TypeAliasType, TypeQuery, ParamSpecType, is_proper_noreturn_type
)
from mypy.nodes import (
FuncBase, FuncItem, FuncDef, OverloadedFuncDef, TypeInfo, ARG_STAR, ARG_STAR2, ARG_POS,
Expand Down Expand Up @@ -320,6 +322,9 @@ def make_simplified_union(items: Sequence[Type],
* [int, int] -> int
* [int, Any] -> Union[int, Any] (Any types are not simplified away!)
* [Any, Any] -> Any
* [NoReturn] -> NoReturn
* [int, NoReturn] -> Union[int, NoReturn] (NoReturn types are not simplified
if other types are present)

Note: This must NOT be used during semantic analysis, since TypeInfos may not
be fully initialized.
Expand Down Expand Up @@ -377,6 +382,7 @@ def make_simplified_union(items: Sequence[Type],
and not is_simple_literal(tj)
and is_proper_subtype(tj, item, keep_erased_types=keep_erased)
and is_redundant_literal_instance(item, tj) # XXX?
and (not is_proper_noreturn_type(tj) or is_proper_noreturn_type(item))
):
# We found a redundant item in the union.
removed.add(j)
Expand Down
19 changes: 18 additions & 1 deletion mypy/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -1952,7 +1952,8 @@ class UnionType(ProperType):
def __init__(self, items: Sequence[Type], line: int = -1, column: int = -1,
is_evaluated: bool = True, uses_pep604_syntax: bool = False) -> None:
super().__init__(line, column)
self.items = flatten_nested_unions(items)
items = flatten_nested_unions(items)
self.items = reduce_proper_noreturn_types(items)
self.can_be_true = any(item.can_be_true for item in items)
self.can_be_false = any(item.can_be_false for item in items)
# is_evaluated should be set to false for type comments and string literals
Expand Down Expand Up @@ -2575,6 +2576,17 @@ def flatten_nested_unions(types: Iterable[Type],
return flat_items


def reduce_proper_noreturn_types(types: Iterable[Type]) -> List[Type]:
"""Remove one or multiple NoReturn types if there is at least one other type.

Assumes the union was flattened (except for possible union type aliases). The
implementation CANNOT handle type aliases, because they might not yet be expanded.
"""
filtered: List[Type] = [tp for tp in types if not is_proper_noreturn_type(tp)]
# If nothing is left, we know there was at least one NoReturn.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or we can have an empty types[] iterable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assumed this cannot happen, because in the call context of this method (constructor of Union) there must be at least one item in the Union.

So are you fine if I just add an assert len(types) >= 1 and chaning the type of types to List (instead of Iterable)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be Sequence, the same as __init__ has

return filtered if filtered else [UninhabitedType(is_noreturn=True)]


def union_items(typ: Type) -> List[ProperType]:
"""Return the flattened items of a union type.

Expand Down Expand Up @@ -2611,6 +2623,11 @@ def is_optional(t: Type) -> bool:
for e in t.items)


def is_proper_noreturn_type(t: Type) -> bool:
return isinstance(t, ProperType) and \
isinstance(t, UninhabitedType) and t.is_noreturn
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

t is always ProperType if it is UninhabitedType.

I guess what we should do here is:

t = get_proper_type(t)
return isinstance(t, UninhabitedType) and t.is_noreturn

Copy link
Contributor Author

@kreathon kreathon Jan 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is related to the problem of the "Limitation" section.

t = get_proper_type(t)
return isinstance(t, UninhabitedType) and t.is_noreturn

here get_proper_type(t) fails in _expand_once() on the following assertion assert self.alias is not None

Therefore, I limited the implementation to not handle type aliases and added the isinstance(t, ProperType) check instead.



def remove_optional(typ: Type) -> Type:
typ = get_proper_type(typ)
if isinstance(typ, UnionType):
Expand Down
2 changes: 1 addition & 1 deletion test-data/unit/check-flags.test
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ from mypy_extensions import NoReturn
def no_return() -> NoReturn: pass
def f() -> int:
return 0
reveal_type(f() or no_return()) # N: Revealed type is "builtins.int"
reveal_type(f() or no_return()) # N: Revealed type is "Union[builtins.int]"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not happen, Union of a single item should be simplified to just this item.

[builtins fixtures/dict.pyi]

[case testNoReturnVariable]
Expand Down
25 changes: 23 additions & 2 deletions test-data/unit/check-unions.test
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,29 @@ f('') # E: Argument 1 to "f" has incompatible type "str"; expected "Optional[int

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

def func() -> Union[int, NoReturn]: ...
reveal_type(func()) # N: Revealed type is "Union[builtins.int]"

a: Union[NoReturn]
reveal_type(a) # N: Revealed type is "<nothing>"

b: Union[NoReturn, NoReturn]
reveal_type(b) # N: Revealed type is "Union[<nothing>]"

c: Union[str, NoReturn]
reveal_type(c) # N: Revealed type is "Union[builtins.str]"

A = Union[str, NoReturn]
d: A
reveal_type(d) # N: Revealed type is "Union[builtins.str]"

e: Union[str, Union[NoReturn, int]]
reveal_type(e) # N: Revealed type is "Union[builtins.str, builtins.int]"

B = Union[str, int]
f: Union[NoReturn, B]
reveal_type(f) # N: Revealed type is "Union[Union[builtins.str, builtins.int]]"

[case testUnionSimplificationGenericFunction]
from typing import TypeVar, Union, List
Expand Down