Fail gracefully on diverging recursive type aliases (#13352)

This is another follow up on #13297.

We can't support aliases like `Nested = Union[T, Nested[List[T]]]` (and it looks like no-one can, without hacks like fixed type recursion limit). I would propose to just ban them for now. We can reconsider if people will ask for this.

mypy/nodes.py

@@ -1238,6 +1238,7 @@ class AssignmentStmt(Statement):
         "new_syntax",
         "is_alias_def",
         "is_final_def",
+        "invalid_recursive_alias",
     )
 
     lvalues: List[Lvalue]
@@ -1258,6 +1259,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,
@@ -1274,6 +1278,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)

mypy/semanal.py

@@ -232,6 +232,7 @@
     TypeVarLikeQuery,
     analyze_type_alias,
     check_for_explicit_any,
+    detect_diverging_alias,
     fix_instance_types,
     has_any_from_unimported_type,
     no_subscript_builtin_alias,
@@ -263,11 +264,11 @@
     PlaceholderType,
     ProperType,
     StarType,
+    TrivialSyntheticTypeTranslator,
     TupleType,
     Type,
     TypeAliasType,
     TypeOfAny,
-    TypeTranslator,
     TypeType,
     TypeVarLikeType,
     TypeVarType,
@@ -3014,6 +3015,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.
@@ -3107,8 +3110,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.
@@ -3127,8 +3129,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
@@ -3164,8 +3164,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,
@@ -6056,7 +6076,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)

mypy/semanal_typeargs.py

@@ -30,7 +30,6 @@
     UnpackType,
     get_proper_type,
     get_proper_types,
-    invalid_recursive_alias,
 )
 
 
@@ -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:

mypy/type_visitor.py

@@ -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)
@@ -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:

mypy/typeanal.py

@@ -65,6 +65,7 @@
     RequiredType,
     StarType,
     SyntheticTypeVisitor,
+    TrivialSyntheticTypeTranslator,
     TupleType,
     Type,
     TypeAliasType,
@@ -1611,6 +1612,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:
@@ -1656,6 +1661,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,

test-data/unit/check-recursive-types.test

@@ -405,13 +405,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"
 
@@ -425,4 +431,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]