Skip to content

Commit

Permalink
Remove Optional[...] special-casing during semantic analysis (#13357)
Browse files Browse the repository at this point in the history
This is another fix for recursive aliases. Ref #13297 

Previously this special casing caused `Optional[...]` to fail with infinite recursion, where `Union[..., None]` worked. I also delete a duplicate helper, and replace it with another existing one that handles type aliases properly.

(I have no idea why some TypeGuard test started passing, but we have `xfail-strict` so I am re-enabling this test.)
  • Loading branch information
ilevkivskyi authored Aug 8, 2022
1 parent ae2fc03 commit e69bd9a
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 30 deletions.
9 changes: 5 additions & 4 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,14 +199,14 @@
UnboundType,
UninhabitedType,
UnionType,
flatten_nested_unions,
get_proper_type,
get_proper_types,
is_literal_type,
is_named_instance,
is_optional,
remove_optional,
strip_type,
union_items,
)
from mypy.typetraverser import TypeTraverserVisitor
from mypy.typevars import fill_typevars, fill_typevars_with_any, has_no_typevars
Expand Down Expand Up @@ -1464,7 +1464,8 @@ def check_overlapping_op_methods(
# inheritance. (This is consistent with how we handle overloads: we also
# do not try checking unsafe overlaps due to multiple inheritance there.)

for forward_item in union_items(forward_type):
for forward_item in flatten_nested_unions([forward_type]):
forward_item = get_proper_type(forward_item)
if isinstance(forward_item, CallableType):
if self.is_unsafe_overlapping_op(forward_item, forward_base, reverse_type):
self.msg.operator_method_signatures_overlap(
Expand Down Expand Up @@ -5320,8 +5321,8 @@ def replay_lookup(new_parent_type: ProperType) -> Optional[Type]:
# Take each element in the parent union and replay the original lookup procedure
# to figure out which parents are compatible.
new_parent_types = []
for item in union_items(parent_type):
member_type = replay_lookup(item)
for item in flatten_nested_unions(parent_type.items):
member_type = replay_lookup(get_proper_type(item))
if member_type is None:
# We were unable to obtain the member type. So, we give up on refining this
# parent type entirely and abort.
Expand Down
11 changes: 7 additions & 4 deletions mypy/plugins/ctypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
Type,
TypeOfAny,
UnionType,
flatten_nested_unions,
get_proper_type,
union_items,
)


Expand Down Expand Up @@ -54,7 +54,8 @@ def _autoconvertible_to_cdata(tp: Type, api: "mypy.plugin.CheckerPluginInterface
# items. This is not quite correct - strictly speaking, only types convertible to *all* of the
# union items should be allowed. This may be worth changing in the future, but the more
# correct algorithm could be too strict to be useful.
for t in union_items(tp):
for t in flatten_nested_unions([tp]):
t = get_proper_type(t)
# Every type can be converted from itself (obviously).
allowed_types.append(t)
if isinstance(t, Instance):
Expand Down Expand Up @@ -197,7 +198,8 @@ def array_value_callback(ctx: "mypy.plugin.AttributeContext") -> Type:
et = _get_array_element_type(ctx.type)
if et is not None:
types: List[Type] = []
for tp in union_items(et):
for tp in flatten_nested_unions([et]):
tp = get_proper_type(tp)
if isinstance(tp, AnyType):
types.append(AnyType(TypeOfAny.from_another_any, source_any=tp))
elif isinstance(tp, Instance) and tp.type.fullname == "ctypes.c_char":
Expand All @@ -219,7 +221,8 @@ def array_raw_callback(ctx: "mypy.plugin.AttributeContext") -> Type:
et = _get_array_element_type(ctx.type)
if et is not None:
types: List[Type] = []
for tp in union_items(et):
for tp in flatten_nested_unions([et]):
tp = get_proper_type(tp)
if (
isinstance(tp, AnyType)
or isinstance(tp, Instance)
Expand Down
15 changes: 10 additions & 5 deletions mypy/typeanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@
UnpackType,
bad_type_type_item,
callable_with_ellipsis,
flatten_nested_unions,
get_proper_type,
union_items,
)
from mypy.typetraverser import TypeTraverserVisitor

Expand Down Expand Up @@ -1739,11 +1739,16 @@ def make_optional_type(t: Type) -> Type:
is called during semantic analysis and simplification only works during
type checking.
"""
t = get_proper_type(t)
if isinstance(t, NoneType):
p_t = get_proper_type(t)
if isinstance(p_t, NoneType):
return t
elif isinstance(t, UnionType):
items = [item for item in union_items(t) if not isinstance(item, NoneType)]
elif isinstance(p_t, UnionType):
# Eagerly expanding aliases is not safe during semantic analysis.
items = [
item
for item in flatten_nested_unions(p_t.items, handle_type_alias_type=False)
if not isinstance(get_proper_type(item), NoneType)
]
return UnionType(items + [NoneType()], t.line, t.column)
else:
return UnionType([t, NoneType()], t.line, t.column)
Expand Down
17 changes: 1 addition & 16 deletions mypy/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -2524,7 +2524,7 @@ def relevant_items(self) -> List[Type]:
if state.strict_optional:
return self.items
else:
return [i for i in get_proper_types(self.items) if not isinstance(i, NoneType)]
return [i for i in self.items if not isinstance(get_proper_type(i), NoneType)]

def serialize(self) -> JsonDict:
return {".class": "UnionType", "items": [t.serialize() for t in self.items]}
Expand Down Expand Up @@ -3178,21 +3178,6 @@ def flatten_nested_unions(
return flat_items


def union_items(typ: Type) -> List[ProperType]:
"""Return the flattened items of a union type.
For non-union types, return a list containing just the argument.
"""
typ = get_proper_type(typ)
if isinstance(typ, UnionType):
items = []
for item in typ.items:
items.extend(union_items(item))
return items
else:
return [typ]


def invalid_recursive_alias(seen_nodes: Set[mypy.nodes.TypeAlias], target: Type) -> bool:
"""Flag aliases like A = Union[int, A] (and similar mutual aliases).
Expand Down
8 changes: 8 additions & 0 deletions test-data/unit/check-recursive-types.test
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,14 @@ reveal_type(bar(lla)) # N: Revealed type is "__main__.A"
reveal_type(bar(llla)) # N: Revealed type is "__main__.A"
[builtins fixtures/isinstancelist.pyi]

[case testRecursiveAliasesWithOptional]
# flags: --enable-recursive-aliases
from typing import Optional, Sequence

A = Sequence[Optional[A]]
x: A
y: str = x[0] # E: Incompatible types in assignment (expression has type "Optional[A]", variable has type "str")

[case testRecursiveAliasesProhibitBadAliases]
# flags: --enable-recursive-aliases
from typing import Union, Type, List, TypeVar
Expand Down
2 changes: 1 addition & 1 deletion test-data/unit/check-typeguard.test
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ def foobar(items: List[object]):
c: List[Bar] = [x for x in items if is_foo(x)] # E: List comprehension has incompatible type List[Foo]; expected List[Bar]
[builtins fixtures/tuple.pyi]

[case testTypeGuardNestedRestrictionUnionIsInstance-xfail]
[case testTypeGuardNestedRestrictionUnionIsInstance]
from typing_extensions import TypeGuard
from typing import Any, List

Expand Down

0 comments on commit e69bd9a

Please sign in to comment.