Skip to content

Fail gracefully on diverging recursive type aliases #13352

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

Merged
merged 2 commits into from
Aug 9, 2022
Merged
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
5 changes: 5 additions & 0 deletions mypy/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -1236,6 +1236,7 @@ class AssignmentStmt(Statement):
"new_syntax",
"is_alias_def",
"is_final_def",
"invalid_recursive_alias",
)

lvalues: List[Lvalue]
Expand All @@ -1256,6 +1257,9 @@ class AssignmentStmt(Statement):
# a final declaration overrides another final declaration (this is checked
# during type checking when MROs are known).
is_final_def: bool
# Stop further processing of this assignment, to prevent flipping back and forth
# during semantic analysis passes.
invalid_recursive_alias: bool

def __init__(
self,
Expand All @@ -1272,6 +1276,7 @@ def __init__(
self.new_syntax = new_syntax
self.is_alias_def = False
self.is_final_def = False
self.invalid_recursive_alias = False

def accept(self, visitor: StatementVisitor[T]) -> T:
return visitor.visit_assignment_stmt(self)
Expand Down
32 changes: 26 additions & 6 deletions mypy/semanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@
TypeVarLikeQuery,
analyze_type_alias,
check_for_explicit_any,
detect_diverging_alias,
fix_instance_types,
has_any_from_unimported_type,
no_subscript_builtin_alias,
Expand Down Expand Up @@ -264,11 +265,11 @@
PlaceholderType,
ProperType,
StarType,
TrivialSyntheticTypeTranslator,
TupleType,
Type,
TypeAliasType,
TypeOfAny,
TypeTranslator,
TypeType,
TypeVarLikeType,
TypeVarType,
Expand Down Expand Up @@ -3017,6 +3018,8 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool:
Note: the resulting types for subscripted (including generic) aliases
are also stored in rvalue.analyzed.
"""
if s.invalid_recursive_alias:
return True
lvalue = s.lvalues[0]
if len(s.lvalues) > 1 or not isinstance(lvalue, NameExpr):
# First rule: Only simple assignments like Alias = ... create aliases.
Expand Down Expand Up @@ -3110,8 +3113,7 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool:
check_for_explicit_any(res, self.options, self.is_typeshed_stub_file, self.msg, context=s)
# When this type alias gets "inlined", the Any is not explicit anymore,
# so we need to replace it with non-explicit Anys.
if not has_placeholder(res):
res = make_any_non_explicit(res)
res = make_any_non_explicit(res)
# Note: with the new (lazy) type alias representation we only need to set no_args to True
# if the expected number of arguments is non-zero, so that aliases like A = List work.
# However, eagerly expanding aliases like Text = str is a nice performance optimization.
Expand All @@ -3130,8 +3132,6 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool:
no_args=no_args,
eager=eager,
)
if invalid_recursive_alias({alias_node}, alias_node.target):
self.fail("Invalid recursive alias: a union item of itself", rvalue)
if isinstance(s.rvalue, (IndexExpr, CallExpr)): # CallExpr is for `void = type(None)`
s.rvalue.analyzed = TypeAliasExpr(alias_node)
s.rvalue.analyzed.line = s.line
Expand Down Expand Up @@ -3167,8 +3167,28 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool:
self.add_symbol(lvalue.name, alias_node, s)
if isinstance(rvalue, RefExpr) and isinstance(rvalue.node, TypeAlias):
alias_node.normalized = rvalue.node.normalized
current_node = existing.node if existing else alias_node
assert isinstance(current_node, TypeAlias)
self.disable_invalid_recursive_aliases(s, current_node)
return True

def disable_invalid_recursive_aliases(
self, s: AssignmentStmt, current_node: TypeAlias
) -> None:
"""Prohibit and fix recursive type aliases that are invalid/unsupported."""
messages = []
if invalid_recursive_alias({current_node}, current_node.target):
messages.append("Invalid recursive alias: a union item of itself")
if detect_diverging_alias(
current_node, current_node.target, self.lookup_qualified, self.tvar_scope
):
messages.append("Invalid recursive alias: type variable nesting on right hand side")
if messages:
current_node.target = AnyType(TypeOfAny.from_error)
s.invalid_recursive_alias = True
for msg in messages:
self.fail(msg, s.rvalue)

def analyze_lvalue(
self,
lval: Lvalue,
Expand Down Expand Up @@ -6062,7 +6082,7 @@ def make_any_non_explicit(t: Type) -> Type:
return t.accept(MakeAnyNonExplicit())


class MakeAnyNonExplicit(TypeTranslator):
class MakeAnyNonExplicit(TrivialSyntheticTypeTranslator):
def visit_any(self, t: AnyType) -> Type:
if t.type_of_any == TypeOfAny.explicit:
return t.copy_modified(TypeOfAny.special_form)
Expand Down
12 changes: 5 additions & 7 deletions mypy/semanal_typeargs.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
UnpackType,
get_proper_type,
get_proper_types,
invalid_recursive_alias,
)


Expand Down Expand Up @@ -73,12 +72,11 @@ def visit_type_alias_type(self, t: TypeAliasType) -> None:
# types, since errors there have already been reported.
return
self.seen_aliases.add(t)
assert t.alias is not None, f"Unfixed type alias {t.type_ref}"
if invalid_recursive_alias({t.alias}, t.alias.target):
# Fix type arguments for invalid aliases (error is already reported).
t.args = []
t.alias.target = AnyType(TypeOfAny.from_error)
return
# Some recursive aliases may produce spurious args. In principle this is not very
# important, as we would simply ignore them when expanding, but it is better to keep
# correct aliases.
if t.alias and len(t.args) != len(t.alias.alias_tvars):
t.args = [AnyType(TypeOfAny.from_error) for _ in t.alias.alias_tvars]
get_proper_type(t).accept(self)

def visit_instance(self, t: Instance) -> None:
Expand Down
6 changes: 6 additions & 0 deletions mypy/type_visitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,10 @@ def __init__(self, strategy: Callable[[Iterable[T]], T]) -> None:
# Keep track of the type aliases already visited. This is needed to avoid
# infinite recursion on types like A = Union[int, List[A]].
self.seen_aliases: Set[TypeAliasType] = set()
# By default, we eagerly expand type aliases, and query also types in the
# alias target. In most cases this is a desired behavior, but we may want
# to skip targets in some cases (e.g. when collecting type variables).
self.skip_alias_target = False

def visit_unbound_type(self, t: UnboundType) -> T:
return self.query_types(t.args)
Expand Down Expand Up @@ -398,6 +402,8 @@ def visit_placeholder_type(self, t: PlaceholderType) -> T:
return self.query_types(t.args)

def visit_type_alias_type(self, t: TypeAliasType) -> T:
if self.skip_alias_target:
return self.query_types(t.args)
return get_proper_type(t).accept(self)

def query_types(self, types: Iterable[Type]) -> T:
Expand Down
74 changes: 74 additions & 0 deletions mypy/typeanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
RequiredType,
StarType,
SyntheticTypeVisitor,
TrivialSyntheticTypeTranslator,
TupleType,
Type,
TypeAliasType,
Expand Down Expand Up @@ -1617,6 +1618,10 @@ def __init__(
self.scope = scope
self.include_bound_tvars = include_bound_tvars
super().__init__(flatten_tvars)
# Only include type variables in type aliases args. This would be anyway
# that case if we expand (as target variables would be overridden with args)
# and it may cause infinite recursion on invalid (diverging) recursive aliases.
self.skip_alias_target = True

def _seems_like_callable(self, type: UnboundType) -> bool:
if not type.args:
Expand Down Expand Up @@ -1662,6 +1667,75 @@ def visit_callable_type(self, t: CallableType) -> TypeVarLikeList:
return []


class DivergingAliasDetector(TrivialSyntheticTypeTranslator):
"""See docstring of detect_diverging_alias() for details."""

# TODO: this doesn't really need to be a translator, but we don't have a trivial visitor.
def __init__(
self,
seen_nodes: Set[TypeAlias],
lookup: Callable[[str, Context], Optional[SymbolTableNode]],
scope: "TypeVarLikeScope",
) -> None:
self.seen_nodes = seen_nodes
self.lookup = lookup
self.scope = scope
self.diverging = False

def is_alias_tvar(self, t: Type) -> bool:
# Generic type aliases use unbound type variables.
if not isinstance(t, UnboundType) or t.args:
return False
node = self.lookup(t.name, t)
if (
node
and isinstance(node.node, TypeVarLikeExpr)
and self.scope.get_binding(node) is None
):
return True
return False

def visit_type_alias_type(self, t: TypeAliasType) -> Type:
assert t.alias is not None, f"Unfixed type alias {t.type_ref}"
if t.alias in self.seen_nodes:
for arg in t.args:
if not self.is_alias_tvar(arg) and bool(
arg.accept(TypeVarLikeQuery(self.lookup, self.scope))
):
self.diverging = True
return t
# All clear for this expansion chain.
return t
new_nodes = self.seen_nodes | {t.alias}
visitor = DivergingAliasDetector(new_nodes, self.lookup, self.scope)
_ = get_proper_type(t).accept(visitor)
if visitor.diverging:
self.diverging = True
return t


def detect_diverging_alias(
node: TypeAlias,
target: Type,
lookup: Callable[[str, Context], Optional[SymbolTableNode]],
scope: "TypeVarLikeScope",
) -> bool:
"""This detects type aliases that will diverge during type checking.
For example F = Something[..., F[List[T]]]. At each expansion step this will produce
*new* type aliases: e.g. F[List[int]], F[List[List[int]]], etc. So we can't detect
recursion. It is a known problem in the literature, recursive aliases and generic types
don't always go well together. It looks like there is no known systematic solution yet.
# TODO: should we handle such aliases using type_recursion counter and some large limit?
They may be handy in rare cases, e.g. to express a union of non-mixed nested lists:
Nested = Union[T, Nested[List[T]]] ~> Union[T, List[T], List[List[T]], ...]
"""
visitor = DivergingAliasDetector({node}, lookup, scope)
_ = target.accept(visitor)
return visitor.diverging


def check_for_explicit_any(
typ: Optional[Type],
options: Options,
Expand Down
18 changes: 15 additions & 3 deletions test-data/unit/check-recursive-types.test
Original file line number Diff line number Diff line change
Expand Up @@ -397,13 +397,19 @@ NR = List[int]
NR2 = Union[NR, NR]
NR3 = Union[NR, Union[NR2, NR2]]

T = TypeVar("T")
NRG = Union[int, T]
NR4 = NRG[str]
NR5 = Union[NRG[int], NR4]

A = Union[B, int] # E: Invalid recursive alias: a union item of itself
B = Union[int, A] # E: Invalid recursive alias: a union item of itself
B = Union[int, A] # Error reported above
def f() -> A: ...
reveal_type(f()) # N: Revealed type is "Union[Any, builtins.int]"
reveal_type(f()) # N: Revealed type is "Any"

T = TypeVar("T")
G = Union[T, G[T]] # E: Invalid recursive alias: a union item of itself
GL = Union[T, GL[List[T]]] # E: Invalid recursive alias: a union item of itself \
# E: Invalid recursive alias: type variable nesting on right hand side
def g() -> G[int]: ...
reveal_type(g()) # N: Revealed type is "Any"

Expand All @@ -417,4 +423,10 @@ S = Type[S] # E: Type[...] cannot contain another Type[...]
U = Type[Union[int, U]] # E: Type[...] cannot contain another Type[...]
x: U
reveal_type(x) # N: Revealed type is "Type[Any]"

D = List[F[List[T]]] # E: Invalid recursive alias: type variable nesting on right hand side
F = D[T] # Error reported above
E = List[E[E[T]]] # E: Invalid recursive alias: type variable nesting on right hand side
d: D
reveal_type(d) # N: Revealed type is "Any"
[builtins fixtures/isinstancelist.pyi]