Use union types instead of join in binder (python#18538)

This would be more consistent with what we already do for ternary
expressions. Note the change in match test results from match logic not
handling well the situation when initial type is a union. A possible
workaround would be to force "collapsing" union of tuples back into a
tuple with union, but it is not easy and was planning to do some cleanup
in the match handling as well (in particular it uses joins instead of
unions in a way that will be inconsistent with new binder behavior). I
want to put the switch from join to union for match statement in a
separate PR.

Note I also simplify a bunch of special-casing around `Any` in the
binder that existed mostly because `join(Any, X) == Any`.
Fixes python#3724

Author: ilevkivskyi

mypy/binder.py

@@ -7,10 +7,10 @@
 from typing_extensions import TypeAlias as _TypeAlias
 
 from mypy.erasetype import remove_instance_last_known_values
-from mypy.join import join_simple
 from mypy.literals import Key, literal, literal_hash, subkeys
 from mypy.nodes import Expression, IndexExpr, MemberExpr, NameExpr, RefExpr, TypeInfo, Var
 from mypy.subtypes import is_same_type, is_subtype
+from mypy.typeops import make_simplified_union
 from mypy.types import (
     AnyType,
     Instance,
@@ -21,6 +21,7 @@
     Type,
     TypeOfAny,
     TypeType,
+    TypeVarType,
     UnionType,
     UnpackType,
     find_unpack_in_list,
@@ -237,9 +238,21 @@ def update_from_options(self, frames: list[Frame]) -> bool:
                 ):
                     type = AnyType(TypeOfAny.from_another_any, source_any=declaration_type)
             else:
-                for other in resulting_values[1:]:
-                    assert other is not None
-                    type = join_simple(self.declarations[key], type, other.type)
+                possible_types = []
+                for t in resulting_values:
+                    assert t is not None
+                    possible_types.append(t.type)
+                if len(possible_types) == 1:
+                    # This is to avoid calling get_proper_type() unless needed, as this may
+                    # interfere with our (hacky) TypeGuard support.
+                    type = possible_types[0]
+                else:
+                    type = make_simplified_union(possible_types)
+                    # Legacy guard for corner case when the original type is TypeVarType.
+                    if isinstance(declaration_type, TypeVarType) and not is_subtype(
+                        type, declaration_type
+                    ):
+                        type = declaration_type
                     # Try simplifying resulting type for unions involving variadic tuples.
                     # Technically, everything is still valid without this step, but if we do
                     # not do this, this may create long unions after exiting an if check like:
@@ -258,7 +271,7 @@ def update_from_options(self, frames: list[Frame]) -> bool:
                         )
                         if simplified == self.declarations[key]:
                             type = simplified
-            if current_value is None or not is_same_type(type, current_value[0]):
+            if current_value is None or not is_same_type(type, current_value.type):
                 self._put(key, type, from_assignment=True)
                 changed = True
 
@@ -300,9 +313,7 @@ def accumulate_type_assignments(self) -> Iterator[Assigns]:
         yield self.type_assignments
         self.type_assignments = old_assignments
 
-    def assign_type(
-        self, expr: Expression, type: Type, declared_type: Type | None, restrict_any: bool = False
-    ) -> None:
+    def assign_type(self, expr: Expression, type: Type, declared_type: Type | None) -> None:
         # We should erase last known value in binder, because if we are using it,
         # it means that the target is not final, and therefore can't hold a literal.
         type = remove_instance_last_known_values(type)
@@ -333,41 +344,39 @@ def assign_type(
 
         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.
-            # This could be changed.
-            # Instead, since we narrowed type from Any in a recent frame (probably an
-            # isinstance check), but now it is reassigned, we broaden back
-            # to Any (which is the most recent enclosing type)
-            self.put(expr, enclosing_type)
-        # As a special case, when assigning Any to a variable with a
-        # declared Optional type that has been narrowed to None,
-        # replace all the Nones in the declared Union type with Any.
-        # This overrides the normal behavior of ignoring Any assignments to variables
-        # in order to prevent false positives.
-        # (See discussion in #3526)
-        elif (
-            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 p_declared.items
-            ]
-            self.put(expr, UnionType(new_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.
-            self.put(expr, declared_type)
+        if isinstance(p_type, AnyType):
+            # Any type requires some special casing, for both historical reasons,
+            # and to optimise user experience without sacrificing correctness too much.
+            if isinstance(expr, RefExpr) and isinstance(expr.node, Var) and expr.node.is_inferred:
+                # First case: a local/global variable without explicit annotation,
+                # in this case we just assign Any (essentially following the SSA logic).
+                self.put(expr, type)
+            elif isinstance(p_declared, UnionType) and any(
+                isinstance(get_proper_type(item), NoneType) for item in p_declared.items
+            ):
+                # Second case: explicit optional type, in this case we optimize for a common
+                # pattern when an untyped value used as a fallback replacing None.
+                new_items = [
+                    type if isinstance(get_proper_type(item), NoneType) else item
+                    for item in p_declared.items
+                ]
+                self.put(expr, UnionType(new_items))
+            elif isinstance(p_declared, UnionType) and any(
+                isinstance(get_proper_type(item), AnyType) for item in p_declared.items
+            ):
+                # Third case: a union already containing Any (most likely from an un-imported
+                # name), in this case we allow assigning Any as well.
+                self.put(expr, type)
+            else:
+                # In all other cases we don't narrow to Any to minimize false negatives.
+                self.put(expr, declared_type)
+        elif isinstance(p_declared, AnyType):
+            # Mirroring the first case above, we don't narrow to a precise type if the variable
+            # has an explicit `Any` type annotation.
+            if isinstance(expr, RefExpr) and isinstance(expr.node, Var) and expr.node.is_inferred:
+                self.put(expr, type)
+            else:
+                self.put(expr, declared_type)
         else:
             self.put(expr, type)
 
@@ -389,19 +398,6 @@ def invalidate_dependencies(self, expr: BindableExpression) -> None:
         for dep in self.dependencies.get(key, set()):
             self._cleanse_key(dep)
 
-    def most_recent_enclosing_type(self, expr: BindableExpression, type: Type) -> Type | None:
-        type = get_proper_type(type)
-        if isinstance(type, AnyType):
-            return get_declaration(expr)
-        key = literal_hash(expr)
-        assert key is not None
-        enclosers = [get_declaration(expr)] + [
-            f.types[key].type
-            for f in self.frames
-            if key in f.types and is_subtype(type, f.types[key][0])
-        ]
-        return enclosers[-1]
-
     def allow_jump(self, index: int) -> None:
         # self.frames and self.options_on_return have different lengths
         # so make sure the index is positive

mypy/checker.py

@@ -3284,7 +3284,7 @@ def check_assignment(
                 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.
                     if not (isinstance(lvalue, NameExpr) and lvalue.is_special_form):
-                        self.binder.assign_type(lvalue, rvalue_type, lvalue_type, False)
+                        self.binder.assign_type(lvalue, rvalue_type, lvalue_type)
                         if (
                             isinstance(lvalue, NameExpr)
                             and isinstance(lvalue.node, Var)
@@ -4023,7 +4023,7 @@ def check_multi_assignment_from_union(
             if isinstance(expr, StarExpr):
                 expr = expr.expr
 
-            # TODO: See todo in binder.py, ConditionalTypeBinder.assign_type
+            # TODO: See comment in binder.py, ConditionalTypeBinder.assign_type
             # It's unclear why the 'declared_type' param is sometimes 'None'
             clean_items: list[tuple[Type, Type]] = []
             for type, declared_type in items:
@@ -4035,7 +4035,6 @@ def check_multi_assignment_from_union(
                 expr,
                 make_simplified_union(list(types)),
                 make_simplified_union(list(declared_types)),
-                False,
             )
         for union, lv in zip(union_types, self.flatten_lvalues(lvalues)):
             # Properly store the inferred types.
@@ -5233,7 +5232,7 @@ def visit_del_stmt(self, s: DelStmt) -> None:
             for elt in flatten(s.expr):
                 if isinstance(elt, NameExpr):
                     self.binder.assign_type(
-                        elt, DeletedType(source=elt.name), get_declaration(elt), False
+                        elt, DeletedType(source=elt.name), get_declaration(elt)
                     )
 
     def visit_decorator(self, e: Decorator) -> None:

mypy/fastparse.py

@@ -1106,7 +1106,9 @@ def make_argument(
         if argument_elide_name(arg.arg):
             pos_only = True
 
-        argument = Argument(Var(arg.arg, arg_type), arg_type, self.visit(default), kind, pos_only)
+        var = Var(arg.arg, arg_type)
+        var.is_inferred = False
+        argument = Argument(var, arg_type, self.visit(default), kind, pos_only)
         argument.set_line(
             arg.lineno,
             arg.col_offset,

mypy/join.py

@@ -183,55 +183,6 @@ def join_instances_via_supertype(self, t: Instance, s: Instance) -> ProperType:
         return best
 
 
-def join_simple(declaration: Type | None, s: Type, t: Type) -> ProperType:
-    """Return a simple least upper bound given the declared type.
-
-    This function should be only used by binder, and should not recurse.
-    For all other uses, use `join_types()`.
-    """
-    declaration = get_proper_type(declaration)
-    s = get_proper_type(s)
-    t = get_proper_type(t)
-
-    if (s.can_be_true, s.can_be_false) != (t.can_be_true, t.can_be_false):
-        # if types are restricted in different ways, use the more general versions
-        s = mypy.typeops.true_or_false(s)
-        t = mypy.typeops.true_or_false(t)
-
-    if isinstance(s, AnyType):
-        return s
-
-    if isinstance(s, ErasedType):
-        return t
-
-    if is_proper_subtype(s, t, ignore_promotions=True):
-        return t
-
-    if is_proper_subtype(t, s, ignore_promotions=True):
-        return s
-
-    if isinstance(declaration, UnionType):
-        return mypy.typeops.make_simplified_union([s, t])
-
-    if isinstance(s, NoneType) and not isinstance(t, NoneType):
-        s, t = t, s
-
-    if isinstance(s, UninhabitedType) and not isinstance(t, UninhabitedType):
-        s, t = t, s
-
-    # Meets/joins require callable type normalization.
-    s, t = normalize_callables(s, t)
-
-    if isinstance(s, UnionType) and not isinstance(t, UnionType):
-        s, t = t, s
-
-    value = t.accept(TypeJoinVisitor(s))
-    if declaration is None or is_subtype(value, declaration):
-        return value
-
-    return declaration
-
-
 def trivial_join(s: Type, t: Type) -> Type:
     """Return one of types (expanded) if it is a supertype of other, otherwise top type."""
     if is_subtype(s, t):

mypy/test/testtypes.py

@@ -7,7 +7,7 @@
 
 from mypy.erasetype import erase_type, remove_instance_last_known_values
 from mypy.indirection import TypeIndirectionVisitor
-from mypy.join import join_simple, join_types
+from mypy.join import join_types
 from mypy.meet import meet_types, narrow_declared_type
 from mypy.nodes import (
     ARG_NAMED,
@@ -817,12 +817,12 @@ def test_any_type(self) -> None:
             self.assert_join(t, self.fx.anyt, self.fx.anyt)
 
     def test_mixed_truth_restricted_type_simple(self) -> None:
-        # join_simple against differently restricted truthiness types drops restrictions.
+        # make_simplified_union against differently restricted truthiness types drops restrictions.
         true_a = true_only(self.fx.a)
         false_o = false_only(self.fx.o)
-        j = join_simple(self.fx.o, true_a, false_o)
-        assert j.can_be_true
-        assert j.can_be_false
+        u = make_simplified_union([true_a, false_o])
+        assert u.can_be_true
+        assert u.can_be_false
 
     def test_mixed_truth_restricted_type(self) -> None:
         # join_types against differently restricted truthiness types drops restrictions.

mypyc/test-data/irbuild-any.test

@@ -37,7 +37,6 @@ def f(a: Any, n: int, c: C) -> None:
     c.n = a
     a = n
     n = a
-    a.a = n
 [out]
 def f(a, n, c):
     a :: object
@@ -49,10 +48,6 @@ def f(a, n, c):
     r3 :: bool
     r4 :: object
     r5 :: int
-    r6 :: str
-    r7 :: object
-    r8 :: i32
-    r9 :: bit
 L0:
     r0 = box(int, n)
     c.a = r0; r1 = is_error
@@ -62,10 +57,6 @@ L0:
     a = r4
     r5 = unbox(int, a)
     n = r5
-    r6 = 'a'
-    r7 = box(int, n)
-    r8 = PyObject_SetAttr(a, r6, r7)
-    r9 = r8 >= 0 :: signed
     return 1
 
 [case testCoerceAnyInOps]

test-data/unit/check-classes.test

@@ -3709,7 +3709,7 @@ def new(uc: Type[U]) -> U:
     if 1:
         u = uc(0)
         u.foo()
-        u = uc('')  # Error
+        uc('')  # Error
         u.foo(0)  # Error
         return uc()
 u = new(User)

test-data/unit/check-dynamic-typing.test

@@ -252,7 +252,7 @@ if int():
 if int():
     a = d.foo(a, a)
 d.x = a
-d.x.y.z  # E: "A" has no attribute "y"
+d.x.y.z
 
 class A: pass
 [out]
@@ -320,8 +320,10 @@ d = None # All ok
 d = t
 d = g
 d = A
-t = d
-f = d
+
+d1: Any
+t = d1
+f = d1
 [builtins fixtures/tuple.pyi]
 
 

test-data/unit/check-isinstance.test

@@ -115,8 +115,8 @@ if int():
     x = B()
     x.z
     x = foo()
-    x.z          # E: "A" has no attribute "z"
-    x.y
+    reveal_type(x)  # N: Revealed type is "Any"
+reveal_type(x)  # N: Revealed type is "__main__.A"
 
 [case testSingleMultiAssignment]
 x = 'a'
@@ -1915,17 +1915,28 @@ if isinstance(x, str, 1): # E: Too many arguments for "isinstance"
     reveal_type(x) # N: Revealed type is "builtins.int"
 [builtins fixtures/isinstancelist.pyi]
 
-[case testIsinstanceNarrowAny]
+[case testIsinstanceNarrowAnyExplicit]
 from typing import Any
 
 def narrow_any_to_str_then_reassign_to_int() -> None:
-    v = 1 # type: Any
+    v: Any = 1
 
     if isinstance(v, str):
         reveal_type(v)  # N: Revealed type is "builtins.str"
         v = 2
         reveal_type(v)  # N: Revealed type is "Any"
+[builtins fixtures/isinstance.pyi]
 
+[case testIsinstanceNarrowAnyImplicit]
+def foo(): ...
+
+def narrow_any_to_str_then_reassign_to_int() -> None:
+    v = foo()
+
+    if isinstance(v, str):
+        reveal_type(v)  # N: Revealed type is "builtins.str"
+        v = 2
+        reveal_type(v)  # N: Revealed type is "builtins.int"
 [builtins fixtures/isinstance.pyi]
 
 [case testNarrowTypeAfterInList]