Skip to content

Commit

Permalink
Make is_recursive and has_recursive_types() more consistent (#14147)
Browse files Browse the repository at this point in the history
While working on another PR I noticed that current behavior of
`has_recursive_types()` is inconsistent, it returns `False` is there is
a recursive type nested as an argument to a generic non-recursive alias.
I wasn't able to find any situation where this actually matters, but I
think it is better if this function behaves consistently.
  • Loading branch information
ilevkivskyi authored Nov 19, 2022
1 parent e814c47 commit f8d71f1
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 10 deletions.
7 changes: 7 additions & 0 deletions mypy/test/testtypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
UninhabitedType,
UnionType,
get_proper_type,
has_recursive_types,
)


Expand Down Expand Up @@ -157,6 +158,12 @@ def test_type_alias_expand_all(self) -> None:
[self.fx.a, self.fx.a], Instance(self.fx.std_tuplei, [self.fx.a])
)

def test_recursive_nested_in_non_recursive(self) -> None:
A, _ = self.fx.def_alias_1(self.fx.a)
NA = self.fx.non_rec_alias(Instance(self.fx.gi, [UnboundType("T")]), ["T"], [A])
assert not NA.is_recursive
assert has_recursive_types(NA)

def test_indirection_no_infinite_recursion(self) -> None:
A, _ = self.fx.def_alias_1(self.fx.a)
visitor = TypeIndirectionVisitor()
Expand Down
10 changes: 7 additions & 3 deletions mypy/test/typefixture.py
Original file line number Diff line number Diff line change
Expand Up @@ -339,9 +339,13 @@ def def_alias_2(self, base: Instance) -> tuple[TypeAliasType, Type]:
A.alias = AN
return A, target

def non_rec_alias(self, target: Type) -> TypeAliasType:
AN = TypeAlias(target, "__main__.A", -1, -1)
return TypeAliasType(AN, [])
def non_rec_alias(
self, target: Type, alias_tvars: list[str] | None = None, args: list[Type] | None = None
) -> TypeAliasType:
AN = TypeAlias(target, "__main__.A", -1, -1, alias_tvars=alias_tvars)
if args is None:
args = []
return TypeAliasType(AN, args)


class InterfaceTypeFixture(TypeFixture):
Expand Down
26 changes: 19 additions & 7 deletions mypy/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -278,30 +278,42 @@ def _expand_once(self) -> Type:
self.alias.target, self.alias.alias_tvars, self.args, self.line, self.column
)

def _partial_expansion(self) -> tuple[ProperType, bool]:
def _partial_expansion(self, nothing_args: bool = False) -> tuple[ProperType, bool]:
# Private method mostly for debugging and testing.
unroller = UnrollAliasVisitor(set())
unrolled = self.accept(unroller)
if nothing_args:
alias = self.copy_modified(args=[UninhabitedType()] * len(self.args))
else:
alias = self
unrolled = alias.accept(unroller)
assert isinstance(unrolled, ProperType)
return unrolled, unroller.recursed

def expand_all_if_possible(self) -> ProperType | None:
def expand_all_if_possible(self, nothing_args: bool = False) -> ProperType | None:
"""Attempt a full expansion of the type alias (including nested aliases).
If the expansion is not possible, i.e. the alias is (mutually-)recursive,
return None.
return None. If nothing_args is True, replace all type arguments with an
UninhabitedType() (used to detect recursively defined aliases).
"""
unrolled, recursed = self._partial_expansion()
unrolled, recursed = self._partial_expansion(nothing_args=nothing_args)
if recursed:
return None
return unrolled

@property
def is_recursive(self) -> bool:
"""Whether this type alias is recursive.
Note this doesn't check generic alias arguments, but only if this alias
*definition* is recursive. The property value thus can be cached on the
underlying TypeAlias node. If you want to include all nested types, use
has_recursive_types() function.
"""
assert self.alias is not None, "Unfixed type alias"
is_recursive = self.alias._is_recursive
if is_recursive is None:
is_recursive = self.expand_all_if_possible() is None
is_recursive = self.expand_all_if_possible(nothing_args=True) is None
# We cache the value on the underlying TypeAlias node as an optimization,
# since the value is the same for all instances of the same alias.
self.alias._is_recursive = is_recursive
Expand Down Expand Up @@ -3259,7 +3271,7 @@ def __init__(self) -> None:
super().__init__(any)

def visit_type_alias_type(self, t: TypeAliasType) -> bool:
return t.is_recursive
return t.is_recursive or self.query_types(t.args)


def has_recursive_types(typ: Type) -> bool:
Expand Down

0 comments on commit f8d71f1

Please sign in to comment.