Cleaner usage of get_proper_type() (#13326)

This is a follow up for #13297

I move around some calls to `get_proper_type()` to preserve original types as much as possible (plus few related required low-risk changes). This makes error messages much more concise, before some error messages in the tests I added were truly epic:
```
Incompatible types in assignment (expression has type "Sequence[Union[B, Sequence[Union[B, Sequence[Union[B, Sequence[Union[B, Sequence[Union[B, Sequence[Union[B, NestedB]]]]]]]]]]]]", variable has type "int")
```

Author: ilevkivskyi

misc/proper_plugin.py

@@ -84,7 +84,10 @@ def is_special_target(right: ProperType) -> bool:
             return True
         if right.type_object().fullname in (
             "mypy.types.UnboundType",
+            "mypy.types.TypeVarLikeType",
             "mypy.types.TypeVarType",
+            "mypy.types.UnpackType",
+            "mypy.types.TypeVarTupleType",
             "mypy.types.ParamSpecType",
             "mypy.types.RawExpressionType",
             "mypy.types.EllipsisType",
@@ -93,6 +96,7 @@ def is_special_target(right: ProperType) -> bool:
             "mypy.types.CallableArgument",
             "mypy.types.PartialType",
             "mypy.types.ErasedType",
+            "mypy.types.DeletedType",
         ):
             # Special case: these are not valid targets for a type alias and thus safe.
             # TODO: introduce a SyntheticType base to simplify this?

mypy/applytype.py

@@ -9,20 +9,18 @@
     Parameters,
     ParamSpecType,
     PartialType,
-    ProperType,
     Type,
     TypeVarId,
     TypeVarLikeType,
     TypeVarTupleType,
     TypeVarType,
     get_proper_type,
-    get_proper_types,
 )
 
 
 def get_target_type(
     tvar: TypeVarLikeType,
-    type: ProperType,
+    type: Type,
     callable: CallableType,
     report_incompatible_typevar_value: Callable[[CallableType, Type, str, Context], None],
     context: Context,
@@ -33,14 +31,15 @@ def get_target_type(
     if isinstance(tvar, TypeVarTupleType):
         return type
     assert isinstance(tvar, TypeVarType)
-    values = get_proper_types(tvar.values)
+    values = tvar.values
+    p_type = get_proper_type(type)
     if values:
-        if isinstance(type, AnyType):
+        if isinstance(p_type, AnyType):
             return type
-        if isinstance(type, TypeVarType) and type.values:
+        if isinstance(p_type, TypeVarType) and p_type.values:
             # Allow substituting T1 for T if every allowed value of T1
             # is also a legal value of T.
-            if all(any(mypy.subtypes.is_same_type(v, v1) for v in values) for v1 in type.values):
+            if all(any(mypy.subtypes.is_same_type(v, v1) for v in values) for v1 in p_type.values):
                 return type
         matching = []
         for value in values:
@@ -86,12 +85,10 @@ def apply_generic_arguments(
     assert len(tvars) == len(orig_types)
     # Check that inferred type variable values are compatible with allowed
     # values and bounds.  Also, promote subtype values to allowed values.
-    types = get_proper_types(orig_types)
-
     # Create a map from type variable id to target type.
     id_to_type: Dict[TypeVarId, Type] = {}
 
-    for tvar, type in zip(tvars, types):
+    for tvar, type in zip(tvars, orig_types):
         assert not isinstance(type, PartialType), "Internal error: must never apply partial type"
         if type is None:
             continue

mypy/argmap.py

@@ -184,6 +184,7 @@ def expand_actual_type(
         This is supposed to be called for each formal, in order. Call multiple times per
         formal if multiple actuals map to a formal.
         """
+        original_actual = actual_type
         actual_type = get_proper_type(actual_type)
         if actual_kind == nodes.ARG_STAR:
             if isinstance(actual_type, Instance) and actual_type.args:
@@ -241,4 +242,4 @@ def expand_actual_type(
                 return AnyType(TypeOfAny.from_error)
         else:
             # No translation for other kinds -- 1:1 mapping.
-            return actual_type
+            return original_actual

mypy/binder.py

@@ -260,9 +260,6 @@ def assign_type(
         # it means that the target is not final, and therefore can't hold a literal.
         type = remove_instance_last_known_values(type)
 
-        type = get_proper_type(type)
-        declared_type = get_proper_type(declared_type)
-
         if self.type_assignments is not None:
             # We are in a multiassign from union, defer the actual binding,
             # just collect the types.
@@ -287,6 +284,8 @@ def assign_type(
             # times?
             return
 
+        p_declared = get_proper_type(declared_type)
+        p_type = get_proper_type(type)
         enclosing_type = get_proper_type(self.most_recent_enclosing_type(expr, type))
         if isinstance(enclosing_type, AnyType) and not restrict_any:
             # If x is Any and y is int, after x = y we do not infer that x is int.
@@ -302,22 +301,22 @@ def assign_type(
         # in order to prevent false positives.
         # (See discussion in #3526)
         elif (
-            isinstance(type, AnyType)
-            and isinstance(declared_type, UnionType)
-            and any(isinstance(get_proper_type(item), NoneType) for item in declared_type.items)
+            isinstance(p_type, AnyType)
+            and isinstance(p_declared, UnionType)
+            and any(isinstance(get_proper_type(item), NoneType) for item in p_declared.items)
             and isinstance(
                 get_proper_type(self.most_recent_enclosing_type(expr, NoneType())), NoneType
             )
         ):
             # Replace any Nones in the union type with Any
             new_items = [
                 type if isinstance(get_proper_type(item), NoneType) else item
-                for item in declared_type.items
+                for item in p_declared.items
             ]
             self.put(expr, UnionType(new_items))
-        elif isinstance(type, AnyType) and not (
-            isinstance(declared_type, UnionType)
-            and any(isinstance(get_proper_type(item), AnyType) for item in declared_type.items)
+        elif isinstance(p_type, AnyType) and not (
+            isinstance(p_declared, UnionType)
+            and any(isinstance(get_proper_type(item), AnyType) for item in p_declared.items)
         ):
             # Assigning an Any value doesn't affect the type to avoid false negatives, unless
             # there is an Any item in a declared union type.
@@ -444,7 +443,7 @@ def top_frame_context(self) -> Iterator[Frame]:
 
 def get_declaration(expr: BindableExpression) -> Optional[Type]:
     if isinstance(expr, RefExpr) and isinstance(expr.node, Var):
-        type = get_proper_type(expr.node.type)
-        if not isinstance(type, PartialType):
+        type = expr.node.type
+        if not isinstance(get_proper_type(type), PartialType):
             return type
     return None

mypy/checker.py

@@ -2619,20 +2619,22 @@ def check_assignment(
 
                 # Special case: only non-abstract non-protocol classes can be assigned to
                 # variables with explicit type Type[A], where A is protocol or abstract.
-                rvalue_type = get_proper_type(rvalue_type)
-                lvalue_type = get_proper_type(lvalue_type)
+                p_rvalue_type = get_proper_type(rvalue_type)
+                p_lvalue_type = get_proper_type(lvalue_type)
                 if (
-                    isinstance(rvalue_type, CallableType)
-                    and rvalue_type.is_type_obj()
+                    isinstance(p_rvalue_type, CallableType)
+                    and p_rvalue_type.is_type_obj()
                     and (
-                        rvalue_type.type_object().is_abstract
-                        or rvalue_type.type_object().is_protocol
+                        p_rvalue_type.type_object().is_abstract
+                        or p_rvalue_type.type_object().is_protocol
+                    )
+                    and isinstance(p_lvalue_type, TypeType)
+                    and isinstance(p_lvalue_type.item, Instance)
+                    and (
+                        p_lvalue_type.item.type.is_abstract or p_lvalue_type.item.type.is_protocol
                     )
-                    and isinstance(lvalue_type, TypeType)
-                    and isinstance(lvalue_type.item, Instance)
-                    and (lvalue_type.item.type.is_abstract or lvalue_type.item.type.is_protocol)
                 ):
-                    self.msg.concrete_only_assign(lvalue_type, rvalue)
+                    self.msg.concrete_only_assign(p_lvalue_type, rvalue)
                     return
                 if rvalue_type and infer_lvalue_type and not isinstance(lvalue_type, PartialType):
                     # Don't use type binder for definitions of special forms, like named tuples.
@@ -3474,7 +3476,6 @@ def infer_variable_type(
         self, name: Var, lvalue: Lvalue, init_type: Type, context: Context
     ) -> None:
         """Infer the type of initialized variables from initializer type."""
-        init_type = get_proper_type(init_type)
         if isinstance(init_type, DeletedType):
             self.msg.deleted_as_rvalue(init_type, context)
         elif not is_valid_inferred_type(init_type) and not self.no_partial_types:
@@ -3620,23 +3621,21 @@ def check_simple_assignment(
             # '...' is always a valid initializer in a stub.
             return AnyType(TypeOfAny.special_form)
         else:
-            orig_lvalue = lvalue_type
-            lvalue_type = get_proper_type(lvalue_type)
-            always_allow_any = lvalue_type is not None and not isinstance(lvalue_type, AnyType)
+            always_allow_any = lvalue_type is not None and not isinstance(
+                get_proper_type(lvalue_type), AnyType
+            )
             rvalue_type = self.expr_checker.accept(
                 rvalue, lvalue_type, always_allow_any=always_allow_any
             )
-            orig_rvalue = rvalue_type
-            rvalue_type = get_proper_type(rvalue_type)
             if isinstance(rvalue_type, DeletedType):
                 self.msg.deleted_as_rvalue(rvalue_type, context)
             if isinstance(lvalue_type, DeletedType):
                 self.msg.deleted_as_lvalue(lvalue_type, context)
             elif lvalue_type:
                 self.check_subtype(
                     # Preserve original aliases for error messages when possible.
-                    orig_rvalue,
-                    orig_lvalue or lvalue_type,
+                    rvalue_type,
+                    lvalue_type,
                     context,
                     msg,
                     f"{rvalue_name} has type",

mypy/checkexpr.py

@@ -3351,13 +3351,14 @@ def visit_index_expr(self, e: IndexExpr) -> Type:
         It may also represent type application.
         """
         result = self.visit_index_expr_helper(e)
-        result = get_proper_type(self.narrow_type_from_binder(e, result))
+        result = self.narrow_type_from_binder(e, result)
+        p_result = get_proper_type(result)
         if (
             self.is_literal_context()
-            and isinstance(result, Instance)
-            and result.last_known_value is not None
+            and isinstance(p_result, Instance)
+            and p_result.last_known_value is not None
         ):
-            result = result.last_known_value
+            result = p_result.last_known_value
         return result
 
     def visit_index_expr_helper(self, e: IndexExpr) -> Type:

mypy/erasetype.py

@@ -190,10 +190,7 @@ class LastKnownValueEraser(TypeTranslator):
     def visit_instance(self, t: Instance) -> Type:
         if not t.last_known_value and not t.args:
             return t
-        new_t = t.copy_modified(args=[a.accept(self) for a in t.args], last_known_value=None)
-        new_t.can_be_true = t.can_be_true
-        new_t.can_be_false = t.can_be_false
-        return new_t
+        return t.copy_modified(args=[a.accept(self) for a in t.args], last_known_value=None)
 
     def visit_type_alias_type(self, t: TypeAliasType) -> Type:
         # Type aliases can't contain literal values, because they are
@@ -210,12 +207,14 @@ def visit_union_type(self, t: UnionType) -> Type:
         # Avoid merge in simple cases such as optional types.
         if len(instances) > 1:
             instances_by_name: Dict[str, List[Instance]] = {}
-            new_items = get_proper_types(new.items)
-            for item in new_items:
-                if isinstance(item, Instance) and not item.args:
-                    instances_by_name.setdefault(item.type.fullname, []).append(item)
+            p_new_items = get_proper_types(new.items)
+            for p_item in p_new_items:
+                if isinstance(p_item, Instance) and not p_item.args:
+                    instances_by_name.setdefault(p_item.type.fullname, []).append(p_item)
             merged: List[Type] = []
-            for item in new_items:
+            for item in new.items:
+                orig_item = item
+                item = get_proper_type(item)
                 if isinstance(item, Instance) and not item.args:
                     types = instances_by_name.get(item.type.fullname)
                     if types is not None:
@@ -227,6 +226,6 @@ def visit_union_type(self, t: UnionType) -> Type:
                             merged.append(make_simplified_union(types))
                             del instances_by_name[item.type.fullname]
                 else:
-                    merged.append(item)
+                    merged.append(orig_item)
             return UnionType.make_union(merged)
         return new

mypy/expandtype.py

@@ -139,21 +139,19 @@ def visit_instance(self, t: Instance) -> Type:
             return args
 
     def visit_type_var(self, t: TypeVarType) -> Type:
-        repl = get_proper_type(self.variables.get(t.id, t))
-        if isinstance(repl, Instance):
-            inst = repl
-            return Instance(inst.type, inst.args, line=inst.line, column=inst.column)
-        else:
-            return repl
+        repl = self.variables.get(t.id, t)
+        if isinstance(repl, ProperType) and isinstance(repl, Instance):
+            # TODO: do we really need to do this?
+            # If I try to remove this special-casing ~40 tests fail on reveal_type().
+            return repl.copy_modified(last_known_value=None)
+        return repl
 
     def visit_param_spec(self, t: ParamSpecType) -> Type:
         repl = get_proper_type(self.variables.get(t.id, t))
         if isinstance(repl, Instance):
-            inst = repl
-            # Return copy of instance with type erasure flag on.
             # TODO: what does prefix mean in this case?
             # TODO: why does this case even happen? Instances aren't plural.
-            return Instance(inst.type, inst.args, line=inst.line, column=inst.column)
+            return repl
         elif isinstance(repl, ParamSpecType):
             return repl.copy_modified(
                 flavor=t.flavor,
@@ -199,9 +197,8 @@ def expand_unpack(self, t: UnpackType) -> Optional[Union[List[Type], Instance, A
         """May return either a list of types to unpack to, any, or a single
         variable length tuple. The latter may not be valid in all contexts.
         """
-        proper_typ = get_proper_type(t.type)
-        if isinstance(proper_typ, TypeVarTupleType):
-            repl = get_proper_type(self.variables.get(proper_typ.id, t))
+        if isinstance(t.type, TypeVarTupleType):
+            repl = get_proper_type(self.variables.get(t.type.id, t))
             if isinstance(repl, TupleType):
                 return repl.items
             if isinstance(repl, TypeList):
@@ -221,7 +218,7 @@ def expand_unpack(self, t: UnpackType) -> Optional[Union[List[Type], Instance, A
             else:
                 raise NotImplementedError(f"Invalid type replacement to expand: {repl}")
         else:
-            raise NotImplementedError(f"Invalid type to expand: {proper_typ}")
+            raise NotImplementedError(f"Invalid type to expand: {t.type}")
 
     def visit_parameters(self, t: Parameters) -> Type:
         return t.copy_modified(arg_types=self.expand_types(t.arg_types))
@@ -277,9 +274,8 @@ def expand_types_with_unpack(
         """
         items: List[Type] = []
         for item in typs:
-            proper_item = get_proper_type(item)
-            if isinstance(proper_item, UnpackType):
-                unpacked_items = self.expand_unpack(proper_item)
+            if isinstance(item, UnpackType):
+                unpacked_items = self.expand_unpack(item)
                 if unpacked_items is None:
                     # TODO: better error, something like tuple of unknown?
                     return UninhabitedType()