Use an alternative approach to fix more crash scenarios

Author: ilevkivskyi

mypy/expandtype.py

@@ -182,16 +182,9 @@ class ExpandTypeVisitor(TrivialSyntheticTypeTranslator):
 
     variables: Mapping[TypeVarId, Type]  # TypeVar id -> TypeVar value
 
-    def __init__(
-        self, variables: Mapping[TypeVarId, Type], skip_normalization: bool = False
-    ) -> None:
+    def __init__(self, variables: Mapping[TypeVarId, Type]) -> 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
@@ -235,7 +228,9 @@ def visit_instance(self, t: Instance) -> Type:
         if t.type.fullname == "builtins.tuple":
             # Normalize Tuple[*Tuple[X, ...], ...] -> Tuple[X, ...]
             arg = args[0]
-            if not self.skip_normalization and isinstance(arg, UnpackType):
+            if isinstance(arg, UnpackType) and not (
+                isinstance(arg.type, TypeAliasType) and arg.type.is_recursive
+            ):
                 unpacked = get_proper_type(arg.type)
                 if isinstance(unpacked, Instance):
                     assert unpacked.type.fullname == "builtins.tuple"
@@ -540,7 +535,9 @@ def visit_tuple_type(self, t: TupleType) -> Type:
         if len(items) == 1:
             # Normalize Tuple[*Tuple[X, ...]] -> Tuple[X, ...]
             item = items[0]
-            if not self.skip_normalization and isinstance(item, UnpackType):
+            if isinstance(item, UnpackType) and not (
+                isinstance(item.type, TypeAliasType) and item.type.is_recursive
+            ):
                 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_typeargs.py

@@ -108,7 +108,10 @@ def visit_type_alias_type(self, t: TypeAliasType) -> None:
             self.seen_aliases.discard(t)
 
     def visit_tuple_type(self, t: TupleType) -> None:
-        t.items = flatten_nested_tuples(t.items)
+        # Unfortunately, universal normalization of tuples is not possible in presence of
+        # recursive aliases, see testNoCrashOnNonNormalRecursiveTuple for an example.
+        # TODO: update the places where we handle tuples to always expect non-normal ones.
+        t.items = flatten_nested_tuples(t.items, handle_recursive=False)
         for i, it in enumerate(t.items):
             if self.check_non_paramspec(it, "tuple", t):
                 t.items[i] = AnyType(TypeOfAny.from_error)

mypy/typeanal.py

@@ -2503,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)
-        _ = t.expand_once(skip_normalization=True).accept(visitor)
+        _ = get_proper_type(t).accept(visitor)
         if visitor.diverging:
             self.diverging = True
         return t

mypy/types.py

@@ -350,11 +350,11 @@ def __init__(
         self.args = args
         self.type_ref: str | None = None
 
-    def expand_once(self, skip_normalization: bool = False) -> Type:
+    def _expand_once(self) -> 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 helper only when needed,
+        (or even this same) type alias. Use this internal helper only when really needed,
         its public wrapper mypy.types.get_proper_type() is preferred.
         """
         assert self.alias is not None
@@ -381,8 +381,7 @@ def expand_once(self, skip_normalization: bool = False) -> Type:
             ):
                 mapping[tvar.id] = sub
 
-        visitor = InstantiateAliasVisitor(mapping, skip_normalization=skip_normalization)
-        return self.alias.target.accept(visitor)
+        return self.alias.target.accept(InstantiateAliasVisitor(mapping))
 
     @property
     def is_recursive(self) -> bool:
@@ -3708,7 +3707,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)
 
@@ -4252,12 +4251,12 @@ def find_unpack_in_list(items: Sequence[Type]) -> int | None:
             # Funky code here avoids mypyc narrowing the type of unpack_index.
             old_index = unpack_index
             assert old_index is None
-            # Don't return so that we can also sanity check there is only one.
+            # Don't return so that we can also sanity-check there is only one.
             unpack_index = i
     return unpack_index
 
 
-def flatten_nested_tuples(types: Iterable[Type]) -> list[Type]:
+def flatten_nested_tuples(types: Iterable[Type], handle_recursive: bool = True) -> list[Type]:
     """Recursively flatten TupleTypes nested with Unpack.
 
     For example this will transform
@@ -4271,7 +4270,12 @@ def flatten_nested_tuples(types: Iterable[Type]) -> list[Type]:
             res.append(typ)
             continue
         p_type = get_proper_type(typ.type)
-        if not isinstance(p_type, TupleType):
+        if (
+            not isinstance(p_type, TupleType)
+            or not handle_recursive
+            and isinstance(typ.type, TypeAliasType)
+            and typ.type.is_recursive
+        ):
             res.append(typ)
             continue
         if isinstance(typ.type, TypeAliasType):

mypy/types_utils.py

@@ -65,9 +65,7 @@ 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}, target.expand_once(skip_normalization=True)
-        )
+        return is_invalid_recursive_alias(seen_nodes | {target.alias}, get_proper_type(target))
     assert isinstance(target, ProperType)
     if not isinstance(target, (UnionType, TupleType)):
         return False

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

@@ -893,6 +893,21 @@ A = tuple[Unpack[B[Unpack[Ts]]]]  # E: Invalid recursive alias: a tuple item of
 B = tuple[Unpack[A[Unpack[Ts]]]]
 [builtins fixtures/tuple.pyi]
 
+[case testNoCrashOnInvalidRecursiveUnpackOfUnion]
+from typing import Unpack
+
+A = tuple[int, str] | list[tuple[Unpack[A]]]  # E: "tuple[int, str] | list[tuple[Unpack[A]]]" cannot be unpacked (must be tuple or TypeVarTuple)
+[builtins fixtures/tuple.pyi]
+
+[case testNoCrashOnNonNormalRecursiveTuple]
+from typing import Unpack
+
+A = tuple[int, list[tuple[str, Unpack[A]]]]
+a: A
+x, y = a
+y[0] = 1  # E: Incompatible types in assignment (expression has type "int", target has type "tuple[str, Unpack[A]]")
+[builtins fixtures/list.pyi]
+
 [case testBanTypeVarTupleNotImmediatelyInsideUnpack]
 from typing import TypeVarTuple, Unpack