Unify empty context fallback in assignments (#20851)

Fixes #11455 (kind of accidentally)

As we discussed with @JukkaL before, using full type context as a
fallback will help with #19918 for
`--allow-redefinition-new`. This allowed to fix most errors on
self-check, so that we can now switch at any moment (when performance is
adequate).

Implementation is mostly straightforward, it is even smaller than it
looks. It is mostly moving blocks of code around and adding/updating
comments. Now we have just two `accept()` calls. I unify the fallback
logic for the two cases:
* Redefinition + invalid inferred type in first accept
* Union type context (this was added a while ago to help with various
common patterns)

I didn't to detailed performance measurements, but i don't see visible
difference on average of 5 runs. (Also in default mode there are should
be only very minor semantic changes in edge cases.)

Author: ilevkivskyi

mypy/binder.py

@@ -312,46 +312,51 @@ def update_from_options(self, frames: list[Frame]) -> bool:
             keys = list(set(keys))
         for key in keys:
             current_value = self._get(key)
-            resulting_values = [f.types.get(key, current_value) for f in frames]
+            all_resulting_values = [f.types.get(key, current_value) for f in frames]
             # Keys can be narrowed using two different semantics. The new semantics
             # is enabled for plain variables when bind_all is true, and it allows
             # variable types to be widened using subsequent assignments. This is
             # tricky to support for instance attributes (primarily due to deferrals),
             # so we don't use it for them.
             old_semantics = not self.bind_all or extract_var_from_literal_hash(key) is None
-            if old_semantics and any(x is None for x in resulting_values):
+            if old_semantics and any(x is None for x in all_resulting_values):
                 # We didn't know anything about key before
                 # (current_value must be None), and we still don't
                 # know anything about key in at least one possible frame.
                 continue
 
-            resulting_values = [x for x in resulting_values if x is not None]
+            resulting_values = [x for x in all_resulting_values if x is not None]
 
-            if all_reachable and all(
-                x is not None and not x.from_assignment for x in resulting_values
-            ):
+            if all_reachable and all(not x.from_assignment for x in resulting_values):
                 # Do not synthesize a new type if we encountered a conditional block
                 # (if, while or match-case) without assignments.
                 # See check-isinstance.test::testNoneCheckDoesNotMakeTypeVarOptional
                 # This is a safe assumption: the fact that we checked something with `is`
                 # or `isinstance` does not change the type of the value.
                 continue
 
-            current_type = resulting_values[0]
-            assert current_type is not None
-            type = current_type.type
+            # Remove exact duplicates to save pointless work later, this is
+            # a micro-optimization for --allow-redefinition-new.
+            seen_types = set()
+            resulting_types = []
+            for rv in resulting_values:
+                assert rv is not None
+                if rv.type in seen_types:
+                    continue
+                resulting_types.append(rv.type)
+                seen_types.add(rv.type)
+
+            type = resulting_types[0]
             declaration_type = get_proper_type(self.declarations.get(key))
             if isinstance(declaration_type, AnyType):
                 # At this point resulting values can't contain None, see continue above
-                if not all(
-                    t is not None and is_same_type(type, t.type) for t in resulting_values[1:]
-                ):
+                if not all(is_same_type(type, t) for t in resulting_types[1:]):
                     type = AnyType(TypeOfAny.from_another_any, source_any=declaration_type)
             else:
                 possible_types = []
-                for t in resulting_values:
+                for t in resulting_types:
                     assert t is not None
-                    possible_types.append(t.type)
+                    possible_types.append(t)
                 if len(possible_types) == 1:
                     # This is to avoid calling get_proper_type() unless needed, as this may
                     # interfere with our (hacky) TypeGuard support.

mypy/checker.py

@@ -4619,7 +4619,7 @@ def set_inference_error_fallback_type(self, var: Var, lvalue: Lvalue, type: Type
         """Store best known type for variable if type inference failed.
 
         If a program ignores error on type inference error, the variable should get some
-        inferred type so that it can used later on in the program. Example:
+        inferred type so that it can be used later on in the program. Example:
 
           x = []  # type: ignore
           x.append(1)   # Should be ok!
@@ -4654,6 +4654,82 @@ def simple_rvalue(self, rvalue: Expression) -> bool:
                     return not any(item.variables for item in typ.items)
         return False
 
+    def infer_rvalue_with_fallback_context(
+        self,
+        lvalue_type: Type | None,
+        rvalue: Expression,
+        preferred_context: Type | None,
+        fallback_context: Type | None,
+        inferred: Var | None,
+        always_allow_any: bool,
+    ) -> Type:
+        """Infer rvalue type in two type context and select the best one.
+
+        See comments below for supported fallback scenarios.
+        """
+        assert fallback_context is not preferred_context
+        # TODO: make assignment checking correct in presence of walrus in r.h.s.
+        # We may accept r.h.s. twice. In presence of walrus this can lead to weird
+        # false negatives and "back action". A proper solution would be to use
+        # binder.accumulate_type_assignments() and assign the types inferred for type
+        # context that is ultimately used. This is however tricky with redefinitions.
+        # For now we simply disable second accept in cases known to cause problems,
+        # see e.g. testAssignToOptionalTupleWalrus.
+        binder_version = self.binder.version
+
+        fallback_context_used = False
+        with (
+            self.msg.filter_errors(save_filtered_errors=True) as local_errors,
+            self.local_type_map as type_map,
+        ):
+            rvalue_type = self.expr_checker.accept(
+                rvalue, type_context=preferred_context, always_allow_any=always_allow_any
+            )
+
+        # There are two cases where we want to try re-inferring r.h.s. in a fallback
+        # type context. First case is when redefinitions are allowed, and we got
+        # invalid type when using the preferred (empty) type context.
+        redefinition_fallback = inferred is not None and not is_valid_inferred_type(
+            rvalue_type, self.options
+        )
+        # Try re-inferring r.h.s. in empty context for union with explicit annotation,
+        # and use it results in a narrower type. This helps with various practical
+        # examples, see e.g. testOptionalTypeNarrowedByGenericCall.
+        union_fallback = (
+            inferred is None
+            and isinstance(get_proper_type(lvalue_type), UnionType)
+            and binder_version == self.binder.version
+        )
+
+        # Skip literal types, as they have special logic (for better errors).
+        if (redefinition_fallback or union_fallback) and not is_literal_type_like(rvalue_type):
+            with (
+                self.msg.filter_errors(save_filtered_errors=True) as alt_local_errors,
+                self.local_type_map as alt_type_map,
+            ):
+                alt_rvalue_type = self.expr_checker.accept(
+                    rvalue, fallback_context, always_allow_any=always_allow_any
+                )
+            if (
+                not alt_local_errors.has_new_errors()
+                and is_valid_inferred_type(alt_rvalue_type, self.options)
+                and (
+                    # For redefinition fallback we are fine getting not a subtype.
+                    redefinition_fallback
+                    # Skip Any type, since it is special cased in binder.
+                    or not isinstance(get_proper_type(alt_rvalue_type), AnyType)
+                    and is_proper_subtype(alt_rvalue_type, rvalue_type)
+                )
+            ):
+                fallback_context_used = True
+                rvalue_type = alt_rvalue_type
+                self.store_types(alt_type_map)
+
+        if not fallback_context_used:
+            self.msg.add_errors(local_errors.filtered_errors())
+            self.store_types(type_map)
+        return rvalue_type
+
     def check_simple_assignment(
         self,
         lvalue_type: Type | None,
@@ -4674,39 +4750,45 @@ def check_simple_assignment(
             always_allow_any = lvalue_type is not None and not isinstance(
                 get_proper_type(lvalue_type), AnyType
             )
-            if inferred is None or is_typeddict_type_context(lvalue_type):
-                type_context = lvalue_type
+
+            # If redefinitions are allowed (i.e. we have --allow-redefinition-new
+            # and a variable without annotation) then we start with an empty context,
+            # since this gives somewhat more intuitive behavior. The only exception
+            # is TypedDicts, they are often useless without context.
+            try_fallback = (
+                inferred is not None or isinstance(get_proper_type(lvalue_type), UnionType)
+            ) and not self.simple_rvalue(rvalue)
+
+            if not try_fallback or lvalue_type is None or is_typeddict_type_context(lvalue_type):
+                rvalue_type = self.expr_checker.accept(
+                    rvalue, type_context=lvalue_type, always_allow_any=always_allow_any
+                )
             else:
-                type_context = None
-
-            # TODO: make assignment checking correct in presence of walrus in r.h.s.
-            # Right now we can accept the r.h.s. up to four(!) times. In presence of
-            # walrus this can result in weird false negatives and "back action". A proper
-            # solution would be to:
-            #   * Refactor the code to reduce number of times we accept the r.h.s.
-            #   (two should be enough: empty context + l.h.s. context).
-            #   * For each accept use binder.accumulate_type_assignments() and assign
-            #   the types inferred for context that is ultimately used.
-            # For now we simply disable some logic that is known to cause problems in
-            # presence of walrus, see e.g. testAssignToOptionalTupleWalrus.
-            binder_version = self.binder.version
+                if inferred is not None:
+                    preferred = None
+                    fallback = lvalue_type
+                else:
+                    preferred = lvalue_type
+                    fallback = None
+
+                rvalue_type = self.infer_rvalue_with_fallback_context(
+                    lvalue_type, rvalue, preferred, fallback, inferred, always_allow_any
+                )
 
-            rvalue_type = self.expr_checker.accept(
-                rvalue, type_context=type_context, always_allow_any=always_allow_any
-            )
             if (
-                lvalue_type is not None
-                and type_context is None
+                inferred is not None
                 and not is_valid_inferred_type(rvalue_type, self.options)
-            ):
-                # Inference in an empty type context didn't produce a valid type, so
-                # try using lvalue type as context instead.
-                rvalue_type = self.expr_checker.accept(
-                    rvalue, type_context=lvalue_type, always_allow_any=always_allow_any
+                and (
+                    not inferred.type
+                    or isinstance(inferred.type, PartialType)
+                    # This additional check is to give an error instead of inferring
+                    # a useless type like None | list[Never] in case of "double-partial"
+                    # types that are not supported yet, see issue #20257.
+                    or not is_proper_subtype(rvalue_type, inferred.type)
                 )
-                if not is_valid_inferred_type(rvalue_type, self.options) and inferred is not None:
-                    self.msg.need_annotation_for_var(inferred, context, self.options)
-                    rvalue_type = rvalue_type.accept(SetNothingToAny())
+            ):
+                self.msg.need_annotation_for_var(inferred, inferred, self.options)
+                rvalue_type = rvalue_type.accept(SetNothingToAny())
 
             if (
                 isinstance(lvalue, NameExpr)
@@ -4715,49 +4797,25 @@ def check_simple_assignment(
                 and not inferred.is_final
             ):
                 new_inferred = remove_instance_last_known_values(rvalue_type)
-                if not is_same_type(inferred.type, new_inferred):
-                    # Should we widen the inferred type or the lvalue? Variables defined
-                    # at module level or class bodies can't be widened in functions, or
-                    # in another module.
-                    if not self.refers_to_different_scope(lvalue):
-                        lvalue_type = make_simplified_union([inferred.type, new_inferred])
-                        if not is_same_type(lvalue_type, inferred.type) and not isinstance(
-                            inferred.type, PartialType
-                        ):
-                            # Widen the type to the union of original and new type.
-                            self.widened_vars.append(inferred.name)
-                            self.set_inferred_type(inferred, lvalue, lvalue_type)
-                            self.binder.put(lvalue, rvalue_type)
-                            # TODO: A bit hacky, maybe add a binder method that does put and
-                            #       updates declaration?
-                            lit = literal_hash(lvalue)
-                            if lit is not None:
-                                self.binder.declarations[lit] = lvalue_type
-            if (
-                isinstance(get_proper_type(lvalue_type), UnionType)
-                # Skip literal types, as they have special logic (for better errors).
-                and not is_literal_type_like(rvalue_type)
-                and not self.simple_rvalue(rvalue)
-                and binder_version == self.binder.version
-            ):
-                # Try re-inferring r.h.s. in empty context, and use that if it
-                # results in a narrower type. We don't do this always because this
-                # may cause some perf impact, plus we want to partially preserve
-                # the old behavior. This helps with various practical examples, see
-                # e.g. testOptionalTypeNarrowedByGenericCall.
-                with self.msg.filter_errors() as local_errors, self.local_type_map as type_map:
-                    alt_rvalue_type = self.expr_checker.accept(
-                        rvalue, None, always_allow_any=always_allow_any
-                    )
+                # Should we widen the inferred type or the lvalue? Variables defined
+                # at module level or class bodies can't be widened in functions, or
+                # in another module.
                 if (
-                    not local_errors.has_new_errors()
-                    # Skip Any type, since it is special cased in binder.
-                    and not isinstance(get_proper_type(alt_rvalue_type), AnyType)
-                    and is_valid_inferred_type(alt_rvalue_type, self.options)
-                    and is_proper_subtype(alt_rvalue_type, rvalue_type)
+                    not self.refers_to_different_scope(lvalue)
+                    and not isinstance(inferred.type, PartialType)
+                    and not is_proper_subtype(new_inferred, inferred.type)
                 ):
-                    rvalue_type = alt_rvalue_type
-                    self.store_types(type_map)
+                    lvalue_type = make_simplified_union([inferred.type, new_inferred])
+                    # Widen the type to the union of original and new type.
+                    self.widened_vars.append(inferred.name)
+                    self.set_inferred_type(inferred, lvalue, lvalue_type)
+                    self.binder.put(lvalue, rvalue_type)
+                    # TODO: A bit hacky, maybe add a binder method that does put and
+                    #       updates declaration?
+                    lit = literal_hash(lvalue)
+                    if lit is not None:
+                        self.binder.declarations[lit] = lvalue_type
+
             if isinstance(rvalue_type, DeletedType):
                 self.msg.deleted_as_rvalue(rvalue_type, context)
             if isinstance(lvalue_type, DeletedType):
@@ -6065,7 +6123,7 @@ def partition_by_callable(
 
         if isinstance(typ, UnionType):
             callables = []
-            uncallables = []
+            uncallables: list[Type] = []
             for subtype in typ.items:
                 # Use unsound_partition when handling unions in order to
                 # allow the expected type discrimination.
@@ -9479,11 +9537,15 @@ def ambiguous_enum_equality_keys(t: Type) -> set[str]:
     return result
 
 
-def is_typeddict_type_context(lvalue_type: Type | None) -> bool:
-    if lvalue_type is None:
-        return False
-    lvalue_proper = get_proper_type(lvalue_type)
-    return isinstance(lvalue_proper, TypedDictType)
+def is_typeddict_type_context(lvalue_type: Type) -> bool:
+    lvalue_type = get_proper_type(lvalue_type)
+    if isinstance(lvalue_type, TypedDictType):
+        return True
+    if isinstance(lvalue_type, UnionType):
+        for item in lvalue_type.items:
+            if is_typeddict_type_context(item):
+                return True
+    return False
 
 
 def is_method(node: SymbolNode | None) -> bool:

mypy/checkpattern.py

@@ -293,7 +293,7 @@ def visit_sequence_pattern(self, o: SequencePattern) -> PatternType:
         #
         unpack_index = None
         if isinstance(current_type, TupleType):
-            inner_types = current_type.items
+            inner_types: list[Type] = current_type.items
             unpack_index = find_unpack_in_list(inner_types)
             if unpack_index is None:
                 size_diff = len(inner_types) - required_patterns

mypy/config_parser.py

@@ -551,7 +551,7 @@ def parse_section(
                     dv = getattr(template, options_key, None)
                 else:
                     continue
-            ct = type(dv)
+            ct = type(dv) if dv is not None else None
         v: Any = None
         try:
             if ct is bool:

mypy/messages.py

@@ -796,10 +796,10 @@ def incompatible_argument(
                     )
                 expected_type = get_proper_type(expected_type)
                 if isinstance(expected_type, UnionType):
-                    expected_types = list(expected_type.items)
+                    expected_types = get_proper_types(expected_type.items)
                 else:
                     expected_types = [expected_type]
-                for type in get_proper_types(expected_types):
+                for type in expected_types:
                     if isinstance(arg_type, Instance) and isinstance(type, Instance):
                         notes = append_invariance_notes(notes, arg_type, type)
                         notes = append_numbers_notes(notes, arg_type, type)

mypy/types.py

@@ -377,7 +377,9 @@ def _expand_once(self) -> Type:
 
         # TODO: this logic duplicates the one in expand_type_by_instance().
         if self.alias.tvar_tuple_index is None:
-            mapping = {v.id: s for (v, s) in zip(self.alias.alias_tvars, self.args)}
+            mapping: dict[TypeVarId, Type] = {
+                v.id: s for (v, s) in zip(self.alias.alias_tvars, self.args)
+            }
         else:
             prefix = self.alias.tvar_tuple_index
             suffix = len(self.alias.alias_tvars) - self.alias.tvar_tuple_index - 1

test-data/unit/check-python38.test

@@ -200,11 +200,11 @@ def h(arg=0, /): ...
 i = lambda arg=0, /: arg
 
 f(1)
-g(1)
+reveal_type(g(1))  # N: Revealed type is "Any"
 h()
 h(1)
-i()
-i(1)
+reveal_type(i())  # N: Revealed type is "Any"
+reveal_type(i(1))  # N: Revealed type is "Any"
 f(arg=0)  # E: Unexpected keyword argument "arg" for "f"
 g(arg=0)  # E: Unexpected keyword argument "arg"
 h(arg=0)  # E: Unexpected keyword argument "arg" for "h"