Clean-up and move operator access to checkmember.py (#19250)

Fixes #5136
Fixes #5491

This is a fifth "major" PR toward
#7724. Although it would be
impractical to move all the operator special-casing to `checkmember.py`,
this does two things:
* Removes known inconsistencies in operator handling
* Adds a much more complete `has_operator()` helper that can be a
starting point for future performance optimizations

Author: ilevkivskyi

mypy/checkexpr.py

@@ -16,7 +16,7 @@
 from mypy import applytype, erasetype, join, message_registry, nodes, operators, types
 from mypy.argmap import ArgTypeExpander, map_actuals_to_formals, map_formals_to_actuals
 from mypy.checker_shared import ExpressionCheckerSharedApi
-from mypy.checkmember import analyze_member_access
+from mypy.checkmember import analyze_member_access, has_operator
 from mypy.checkstrformat import StringFormatterChecker
 from mypy.erasetype import erase_type, remove_instance_last_known_values, replace_meta_vars
 from mypy.errors import ErrorWatcher, report_internal_error
@@ -3834,13 +3834,16 @@ def check_method_call_by_name(
         arg_kinds: list[ArgKind],
         context: Context,
         original_type: Type | None = None,
+        self_type: Type | None = None,
     ) -> tuple[Type, Type]:
         """Type check a call to a named method on an object.
 
         Return tuple (result type, inferred method type). The 'original_type'
-        is used for error messages.
+        is used for error messages. The self_type is to bind self in methods
+        (see analyze_member_access for more details).
         """
         original_type = original_type or base_type
+        self_type = self_type or base_type
         # Unions are special-cased to allow plugins to act on each element of the union.
         base_type = get_proper_type(base_type)
         if isinstance(base_type, UnionType):
@@ -3856,7 +3859,7 @@ def check_method_call_by_name(
             is_super=False,
             is_operator=True,
             original_type=original_type,
-            self_type=base_type,
+            self_type=self_type,
             chk=self.chk,
             in_literal_context=self.is_literal_context(),
         )
@@ -3933,11 +3936,8 @@ def lookup_operator(op_name: str, base_type: Type) -> Type | None:
             """Looks up the given operator and returns the corresponding type,
             if it exists."""
 
-            # This check is an important performance optimization,
-            # even though it is mostly a subset of
-            # analyze_member_access.
-            # TODO: Find a way to remove this call without performance implications.
-            if not self.has_member(base_type, op_name):
+            # This check is an important performance optimization.
+            if not has_operator(base_type, op_name, self.named_type):
                 return None
 
             with self.msg.filter_errors() as w:
@@ -4097,14 +4097,8 @@ def lookup_definer(typ: Instance, attr_name: str) -> str | None:
                 errors.append(local_errors.filtered_errors())
                 results.append(result)
             else:
-                # In theory, we should never enter this case, but it seems
-                # we sometimes do, when dealing with Type[...]? E.g. see
-                # check-classes.testTypeTypeComparisonWorks.
-                #
-                # This is probably related to the TODO in lookup_operator(...)
-                # up above.
-                #
-                # TODO: Remove this extra case
+                # Although we should not need this case anymore, we keep it just in case, as
+                # otherwise we will get a crash if we introduce inconsistency in checkmember.py
                 return result
 
         self.msg.add_errors(errors[0])
@@ -4365,13 +4359,19 @@ def visit_index_expr_helper(self, e: IndexExpr) -> Type:
         return self.visit_index_with_type(left_type, e)
 
     def visit_index_with_type(
-        self, left_type: Type, e: IndexExpr, original_type: ProperType | None = None
+        self,
+        left_type: Type,
+        e: IndexExpr,
+        original_type: ProperType | None = None,
+        self_type: Type | None = None,
     ) -> Type:
         """Analyze type of an index expression for a given type of base expression.
 
-        The 'original_type' is used for error messages (currently used for union types).
+        The 'original_type' is used for error messages (currently used for union types). The
+        'self_type' is to bind self in methods (see analyze_member_access for more details).
         """
         index = e.index
+        self_type = self_type or left_type
         left_type = get_proper_type(left_type)
 
         # Visit the index, just to make sure we have a type for it available
@@ -4426,16 +4426,22 @@ def visit_index_with_type(
             ):
                 return self.named_type("types.GenericAlias")
 
-        if isinstance(left_type, TypeVarType) and not self.has_member(
-            left_type.upper_bound, "__getitem__"
-        ):
-            return self.visit_index_with_type(left_type.upper_bound, e, original_type)
+        if isinstance(left_type, TypeVarType):
+            return self.visit_index_with_type(
+                left_type.values_or_bound(), e, original_type, left_type
+            )
         elif isinstance(left_type, Instance) and left_type.type.fullname == "typing._SpecialForm":
             # Allow special forms to be indexed and used to create union types
             return self.named_type("typing._SpecialForm")
         else:
             result, method_type = self.check_method_call_by_name(
-                "__getitem__", left_type, [e.index], [ARG_POS], e, original_type=original_type
+                "__getitem__",
+                left_type,
+                [e.index],
+                [ARG_POS],
+                e,
+                original_type=original_type,
+                self_type=self_type,
             )
             e.method_type = method_type
             return result
@@ -5995,45 +6001,6 @@ def is_valid_keyword_var_arg(self, typ: Type) -> bool:
             or isinstance(typ, ParamSpecType)
         )
 
-    def has_member(self, typ: Type, member: str) -> bool:
-        """Does type have member with the given name?"""
-        # TODO: refactor this to use checkmember.analyze_member_access, otherwise
-        # these two should be carefully kept in sync.
-        # This is much faster than analyze_member_access, though, and so using
-        # it first as a filter is important for performance.
-        typ = get_proper_type(typ)
-
-        if isinstance(typ, TypeVarType):
-            typ = get_proper_type(typ.upper_bound)
-        if isinstance(typ, TupleType):
-            typ = tuple_fallback(typ)
-        if isinstance(typ, LiteralType):
-            typ = typ.fallback
-        if isinstance(typ, Instance):
-            return typ.type.has_readable_member(member)
-        if isinstance(typ, FunctionLike) and typ.is_type_obj():
-            return typ.fallback.type.has_readable_member(member)
-        elif isinstance(typ, AnyType):
-            return True
-        elif isinstance(typ, UnionType):
-            result = all(self.has_member(x, member) for x in typ.relevant_items())
-            return result
-        elif isinstance(typ, TypeType):
-            # Type[Union[X, ...]] is always normalized to Union[Type[X], ...],
-            # so we don't need to care about unions here.
-            item = typ.item
-            if isinstance(item, TypeVarType):
-                item = get_proper_type(item.upper_bound)
-            if isinstance(item, TupleType):
-                item = tuple_fallback(item)
-            if isinstance(item, Instance) and item.type.metaclass_type is not None:
-                return self.has_member(item.type.metaclass_type, member)
-            if isinstance(item, AnyType):
-                return True
-            return False
-        else:
-            return False
-
     def not_ready_callback(self, name: str, context: Context) -> None:
         """Called when we can't infer the type of a variable because it's not ready yet.
 

mypy/checkmember.py

@@ -1501,3 +1501,57 @@ def bind_self_fast(method: F, original_type: Type | None = None) -> F:
         is_bound=True,
     )
     return cast(F, res)
+
+
+def has_operator(typ: Type, op_method: str, named_type: Callable[[str], Instance]) -> bool:
+    """Does type have operator with the given name?
+
+    Note: this follows the rules for operator access, in particular:
+    * __getattr__ is not considered
+    * for class objects we only look in metaclass
+    * instance level attributes (i.e. extra_attrs) are not considered
+    """
+    # This is much faster than analyze_member_access, and so using
+    # it first as a filter is important for performance. This is mostly relevant
+    # in situations where we can't expect that method is likely present,
+    # e.g. for __OP__ vs __rOP__.
+    typ = get_proper_type(typ)
+
+    if isinstance(typ, TypeVarLikeType):
+        typ = typ.values_or_bound()
+    if isinstance(typ, AnyType):
+        return True
+    if isinstance(typ, UnionType):
+        return all(has_operator(x, op_method, named_type) for x in typ.relevant_items())
+    if isinstance(typ, FunctionLike) and typ.is_type_obj():
+        return typ.fallback.type.has_readable_member(op_method)
+    if isinstance(typ, TypeType):
+        # Type[Union[X, ...]] is always normalized to Union[Type[X], ...],
+        # so we don't need to care about unions here, but we need to care about
+        # Type[T], where upper bound of T is a union.
+        item = typ.item
+        if isinstance(item, TypeVarType):
+            item = item.values_or_bound()
+        if isinstance(item, UnionType):
+            return all(meta_has_operator(x, op_method, named_type) for x in item.relevant_items())
+        return meta_has_operator(item, op_method, named_type)
+    return instance_fallback(typ, named_type).type.has_readable_member(op_method)
+
+
+def instance_fallback(typ: ProperType, named_type: Callable[[str], Instance]) -> Instance:
+    if isinstance(typ, Instance):
+        return typ
+    if isinstance(typ, TupleType):
+        return tuple_fallback(typ)
+    if isinstance(typ, (LiteralType, TypedDictType)):
+        return typ.fallback
+    return named_type("builtins.object")
+
+
+def meta_has_operator(item: Type, op_method: str, named_type: Callable[[str], Instance]) -> bool:
+    item = get_proper_type(item)
+    if isinstance(item, AnyType):
+        return True
+    item = instance_fallback(item, named_type)
+    meta = item.type.metaclass_type or named_type("builtins.type")
+    return meta.type.has_readable_member(op_method)

mypy/types.py

@@ -615,6 +615,11 @@ def has_default(self) -> bool:
         t = get_proper_type(self.default)
         return not (isinstance(t, AnyType) and t.type_of_any == TypeOfAny.from_omitted_generics)
 
+    def values_or_bound(self) -> ProperType:
+        if isinstance(self, TypeVarType) and self.values:
+            return UnionType(self.values)
+        return get_proper_type(self.upper_bound)
+
 
 class TypeVarType(TypeVarLikeType):
     """Type that refers to a type variable."""

test-data/unit/check-typeddict.test

@@ -3358,16 +3358,13 @@ foo: Foo = {'key': 1}
 foo | 1
 
 class SubDict(dict): ...
-foo | SubDict()
+reveal_type(foo | SubDict())
 [out]
 main:7: error: No overload variant of "__or__" of "TypedDict" matches argument type "int"
 main:7: note: Possible overload variants:
 main:7: note:     def __or__(self, TypedDict({'key'?: int}), /) -> Foo
 main:7: note:     def __or__(self, dict[str, Any], /) -> dict[str, object]
-main:10: error: No overload variant of "__ror__" of "dict" matches argument type "Foo"
-main:10: note: Possible overload variants:
-main:10: note:     def __ror__(self, dict[Any, Any], /) -> dict[Any, Any]
-main:10: note:     def [T, T2] __ror__(self, dict[T, T2], /) -> dict[Union[Any, T], Union[Any, T2]]
+main:10: note: Revealed type is "builtins.dict[builtins.str, builtins.object]"
 [builtins fixtures/dict-full.pyi]
 [typing fixtures/typing-typeddict-iror.pyi]
 
@@ -3389,8 +3386,10 @@ d2: Dict[int, str]
 
 reveal_type(d1 | foo)  # N: Revealed type is "builtins.dict[builtins.str, builtins.object]"
 d2 | foo  # E: Unsupported operand types for | ("dict[int, str]" and "Foo")
-1 | foo  # E: Unsupported left operand type for | ("int")
-
+1 | foo  # E: No overload variant of "__ror__" of "TypedDict" matches argument type "int" \
+         # N: Possible overload variants: \
+         # N:     def __ror__(self, TypedDict({'key'?: int}), /) -> Foo \
+         # N:     def __ror__(self, dict[str, Any], /) -> dict[str, object]
 
 class Bar(TypedDict):
     key: int