Fix crash on invalid recursive variadic alias

Author: ilevkivskyi

mypy/expandtype.py

@@ -182,9 +182,16 @@ class ExpandTypeVisitor(TrivialSyntheticTypeTranslator):
 
     variables: Mapping[TypeVarId, Type]  # TypeVar id -> TypeVar value
 
-    def __init__(self, variables: Mapping[TypeVarId, Type]) -> None:
+    def __init__(
+        self, variables: Mapping[TypeVarId, Type], skip_normalization: bool = False
+    ) -> None:
         super().__init__()
         self.variables = variables
+        # This flag will skip normalizations that are semantically not needed, but
+        # require calling get_proper_type(). This is needed during very early stages
+        # to avoid infinite recursion when detecting invalid/divergent recursive
+        # type aliases.
+        self.skip_normalization = skip_normalization
 
     def visit_unbound_type(self, t: UnboundType) -> Type:
         return t
@@ -228,11 +235,9 @@ def visit_instance(self, t: Instance) -> Type:
         if t.type.fullname == "builtins.tuple":
             # Normalize Tuple[*Tuple[X, ...], ...] -> Tuple[X, ...]
             arg = args[0]
-            if isinstance(arg, UnpackType):
+            if not self.skip_normalization and isinstance(arg, UnpackType):
                 unpacked = get_proper_type(arg.type)
                 if isinstance(unpacked, Instance):
-                    # TODO: this and similar asserts below may be unsafe because get_proper_type()
-                    # may be called during semantic analysis before all invalid types are removed.
                     assert unpacked.type.fullname == "builtins.tuple"
                     args = list(unpacked.args)
         return t.copy_modified(args=args)
@@ -535,7 +540,7 @@ def visit_tuple_type(self, t: TupleType) -> Type:
         if len(items) == 1:
             # Normalize Tuple[*Tuple[X, ...]] -> Tuple[X, ...]
             item = items[0]
-            if isinstance(item, UnpackType):
+            if not self.skip_normalization and isinstance(item, UnpackType):
                 unpacked = get_proper_type(item.type)
                 if isinstance(unpacked, Instance):
                     # expand_type() may be called during semantic analysis, before invalid unpacks are fixed.

mypy/semanal.py

@@ -4263,6 +4263,8 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool:
             # An alias gets updated.
             updated = False
             if isinstance(existing.node, TypeAlias):
+                # Invalidate recursive status cache in case it was previously set.
+                existing.node._is_recursive = None
                 if existing.node.target != res:
                     # Copy expansion to the existing alias, this matches how we update base classes
                     # for a TypeInfo _in place_ if there are nested placeholders.
@@ -4271,8 +4273,6 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> bool:
                     existing.node.alias_tvars = alias_tvars
                     existing.node.no_args = no_args
                     updated = True
-                    # Invalidate recursive status cache in case it was previously set.
-                    existing.node._is_recursive = None
             else:
                 # Otherwise just replace existing placeholder with type alias *in place*.
                 existing._node = alias_node
@@ -5830,6 +5830,8 @@ def visit_type_alias_stmt(self, s: TypeAliasStmt) -> None:
             ):
                 updated = False
                 if isinstance(existing.node, TypeAlias):
+                    # Invalidate recursive status cache in case it was previously set.
+                    existing.node._is_recursive = None
                     if (
                         existing.node.target != res
                         or existing.node.alias_tvars != alias_node.alias_tvars
@@ -5840,8 +5842,6 @@ def visit_type_alias_stmt(self, s: TypeAliasStmt) -> None:
                         existing.node.default_depends = default_depends
                         existing.node.alias_tvars = alias_tvars
                         updated = True
-                        # Invalidate recursive status cache in case it was previously set.
-                        existing.node._is_recursive = None
                 else:
                     # Otherwise just replace existing placeholder with type alias *in place*.
                     existing._node = alias_node

mypy/server/astmerge.py

@@ -340,6 +340,8 @@ def visit_var(self, node: Var) -> None:
         super().visit_var(node)
 
     def visit_type_alias(self, node: TypeAlias) -> None:
+        # Updating alias target can invalidate its recursive status.
+        node._is_recursive = None
         self.fixup_type(node.target)
         for v in node.alias_tvars:
             self.fixup_type(v)

mypy/typeanal.py

@@ -75,6 +75,7 @@
     BoolTypeQuery,
     CallableArgument,
     CallableType,
+    CollectAliasesVisitor,
     DeletedType,
     EllipsisType,
     ErasedType,
@@ -275,7 +276,9 @@ def __init__(
         self.prohibit_special_class_field_types = prohibit_special_class_field_types
         # Allow variables typed as Type[Any] and type (useful for base classes).
         self.allow_type_any = allow_type_any
-        self.allow_type_var_tuple = False
+        # Level of nesting at which a TypeVarTuple is allowed. Note we specify exact level
+        # to prohibit things like Unpack[list[Ts]], which are not supported.
+        self.allow_type_var_tuple = -1
         self.allow_unpack = allow_unpack
         # Set when we are analyzing a default of a type variable.
         self.analyzing_tvar_def = analyzing_tvar_def
@@ -453,7 +456,7 @@ def visit_unbound_type_nonoptional(self, t: UnboundType, defining_literal: bool)
                     self.fail(msg, t, code=codes.VALID_TYPE)
                     return AnyType(TypeOfAny.from_error)
                 assert isinstance(tvar_def, TypeVarTupleType)
-                if not self.allow_type_var_tuple:
+                if self.allow_type_var_tuple != self.nesting_level:
                     self.fail(
                         f'TypeVarTuple "{t.name}" is only valid with an unpack',
                         t,
@@ -808,9 +811,9 @@ def try_analyze_special_unbound_type(self, t: UnboundType, fullname: str) -> Typ
             if not self.allow_unpack:
                 self.fail(message_registry.INVALID_UNPACK_POSITION, t, code=codes.VALID_TYPE)
                 return AnyType(TypeOfAny.from_error)
-            self.allow_type_var_tuple = True
+            self.allow_type_var_tuple = self.nesting_level + 1
             result = UnpackType(self.anal_type(t.args[0]), line=t.line, column=t.column)
-            self.allow_type_var_tuple = False
+            self.allow_type_var_tuple = -1
             return result
         elif fullname in SELF_TYPE_NAMES:
             if t.args:
@@ -1161,9 +1164,9 @@ def visit_unpack_type(self, t: UnpackType) -> Type:
         if not self.allow_unpack:
             self.fail(message_registry.INVALID_UNPACK_POSITION, t.type, code=codes.VALID_TYPE)
             return AnyType(TypeOfAny.from_error)
-        self.allow_type_var_tuple = True
+        self.allow_type_var_tuple = self.nesting_level + 1
         result = UnpackType(self.anal_type(t.type), from_star_syntax=t.from_star_syntax)
-        self.allow_type_var_tuple = False
+        self.allow_type_var_tuple = -1
         return result
 
     def visit_parameters(self, t: Parameters) -> Type:
@@ -2500,7 +2503,7 @@ def visit_type_alias_type(self, t: TypeAliasType) -> Type:
             return t
         new_nodes = self.seen_nodes | {t.alias}
         visitor = DivergingAliasDetector(new_nodes)
-        _ = get_proper_type(t).accept(visitor)
+        _ = t.expand_once(skip_normalization=True).accept(visitor)
         if visitor.diverging:
             self.diverging = True
         return t
@@ -2518,6 +2521,15 @@ def detect_diverging_alias(node: TypeAlias, target: Type) -> bool:
     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]], ...]
     """
+    is_recursive = node._is_recursive
+    if is_recursive is None:
+        is_recursive = node in node.target.accept(CollectAliasesVisitor())
+    if not is_recursive:
+        # Fast path: this is not a recursive alias at all.
+        return False
+    # Note we only cache positive case, caching negative case is risky, as this type alias
+    # (or importantly any other alias it uses) may be not ready yet.
+    node._is_recursive = True
     visitor = DivergingAliasDetector({node})
     _ = target.accept(visitor)
     return visitor.diverging

mypy/types.py

@@ -350,11 +350,11 @@ def __init__(
         self.args = args
         self.type_ref: str | None = None
 
-    def _expand_once(self) -> Type:
+    def expand_once(self, skip_normalization: bool = False) -> Type:
         """Expand to the target type exactly once.
 
         This doesn't do full expansion, i.e. the result can contain another
-        (or even this same) type alias. Use this internal helper only when really needed,
+        (or even this same) type alias. Use this helper only when needed,
         its public wrapper mypy.types.get_proper_type() is preferred.
         """
         assert self.alias is not None
@@ -381,7 +381,8 @@ def _expand_once(self) -> Type:
             ):
                 mapping[tvar.id] = sub
 
-        return self.alias.target.accept(InstantiateAliasVisitor(mapping))
+        visitor = InstantiateAliasVisitor(mapping, skip_normalization=skip_normalization)
+        return self.alias.target.accept(visitor)
 
     @property
     def is_recursive(self) -> bool:
@@ -3707,7 +3708,7 @@ def get_proper_type(typ: Type | None) -> ProperType | None:
     if isinstance(typ, TypeGuardedType):
         typ = typ.type_guard
     while isinstance(typ, TypeAliasType):
-        typ = typ._expand_once()
+        typ = typ.expand_once()
     # TODO: store the name of original type alias on this type, so we can show it in errors.
     return cast(ProperType, typ)
 

mypy/types_utils.py

@@ -65,7 +65,9 @@ def is_invalid_recursive_alias(seen_nodes: set[TypeAlias], target: Type) -> bool
         if target.alias in seen_nodes:
             return True
         assert target.alias, f"Unfixed type alias {target.type_ref}"
-        return is_invalid_recursive_alias(seen_nodes | {target.alias}, get_proper_type(target))
+        return is_invalid_recursive_alias(
+            seen_nodes | {target.alias}, target.expand_once(skip_normalization=True)
+        )
     assert isinstance(target, ProperType)
     if not isinstance(target, (UnionType, TupleType)):
         return False

test-data/unit/check-typevar-tuple.test

@@ -882,7 +882,24 @@ z: C
 reveal_type(x)  # N: Revealed type is "Any"
 reveal_type(y)  # N: Revealed type is "Any"
 reveal_type(z)  # N: Revealed type is "tuple[builtins.int, Unpack[builtins.tuple[Any, ...]]]"
+[builtins fixtures/tuple.pyi]
+
+[case testBanPathologicalRecursiveTuplesGeneric]
+from typing import TypeVarTuple, Unpack
+
+Ts = TypeVarTuple("Ts")
+A = tuple[Unpack[B[Unpack[Ts]]]]  # E: Invalid recursive alias: a tuple item of itself \
+                                  # E: Name "B" is used before definition
+B = tuple[Unpack[A[Unpack[Ts]]]]
+[builtins fixtures/tuple.pyi]
+
+[case testBanTypeVarTupleNotImmediatelyInsideUnpack]
+from typing import TypeVarTuple, Unpack
 
+Ts = TypeVarTuple("Ts")
+A = tuple[Unpack[tuple[Ts]]]  # E: TypeVarTuple "Ts" is only valid with an unpack
+x: A[int, str]
+reveal_type(x)  # N: Revealed type is "tuple[Any]"
 [builtins fixtures/tuple.pyi]
 
 [case testInferenceAgainstGenericVariadicWithBadType]