Tweaks to --strict-equality based on user feedback

mypy/checkexpr.py

@@ -1938,7 +1938,8 @@ def visit_comparison_expr(self, e: ComparisonExpr) -> Type:
                         self.msg.unsupported_operand_types('in', left_type, right_type, e)
                 # Only show dangerous overlap if there are no other errors.
                 elif (not local_errors.is_errors() and cont_type and
-                        self.dangerous_comparison(left_type, cont_type)):
+                        self.dangerous_comparison(left_type, cont_type,
+                                                  original_cont_type=right_type)):
                     self.msg.dangerous_comparison(left_type, cont_type, 'container', e)
                 else:
                     self.msg.add_errors(local_errors)
@@ -1951,8 +1952,13 @@ def visit_comparison_expr(self, e: ComparisonExpr) -> Type:
                 # testCustomEqCheckStrictEquality for an example.
                 if self.msg.errors.total_errors() == err_count and operator in ('==', '!='):
                     right_type = self.accept(right)
-                    if self.dangerous_comparison(left_type, right_type):
-                        self.msg.dangerous_comparison(left_type, right_type, 'equality', e)
+                    if (not self.custom_equality_method(left_type) and
+                            not self.custom_equality_method(right_type)):
+                        # We suppress the error if there is a custom __eq__() method on either
+                        # side. User defined (or even standard library) classes can define this
+                        # to return True for comparisons between non-overlapping types.
+                        if self.dangerous_comparison(left_type, right_type):
+                            self.msg.dangerous_comparison(left_type, right_type, 'equality', e)
 
             elif operator == 'is' or operator == 'is not':
                 right_type = self.accept(right)  # validate the right operand
@@ -1974,9 +1980,33 @@ def visit_comparison_expr(self, e: ComparisonExpr) -> Type:
         assert result is not None
         return result
 
-    def dangerous_comparison(self, left: Type, right: Type) -> bool:
+    def custom_equality_method(self, typ: Type) -> bool:
+        """Does this type have a custom __eq__() method?"""
+        if isinstance(typ, UnionType):
+            return any(self.custom_equality_method(t) for t in typ.items)
+        if isinstance(typ, Instance):
+            method = typ.type.get_method('__eq__')
+            if method and method.info:
+                return not method.info.fullname().startswith('builtins.')
+            return False
+        # TODO: support other types (see has_member())?
+        return False
+
+    def has_bytes_component(self, typ: Type) -> bool:
+        """Is this the builtin bytes type, or a union that contains it?"""
+        if isinstance(typ, UnionType):
+            return any(self.has_bytes_component(t) for t in typ.items)
+        if isinstance(typ, Instance) and typ.type.fullname() == 'builtins.bytes':
+            return True
+        return False
+
+    def dangerous_comparison(self, left: Type, right: Type,
+                             original_cont_type: Optional[Type] = None) -> bool:
         """Check for dangerous non-overlapping comparisons like 42 == 'no'.
 
+        The original_cont_type is the original container type for 'in' checks
+        (and None for equality checks).
+
         Rules:
             * X and None are overlapping even in strict-optional mode. This is to allow
             'assert x is not None' for x defined as 'x = None  # type: str' in class body
@@ -1985,9 +2015,7 @@ def dangerous_comparison(self, left: Type, right: Type) -> bool:
             non-overlapping, although technically None is overlap, it is most
             likely an error.
             * Any overlaps with everything, i.e. always safe.
-            * Promotions are ignored, so both 'abc' == b'abc' and 1 == 1.0
-            are errors. This is mostly needed for bytes vs unicode, and
-            int vs float are added just for consistency.
+            * Special case: b'abc' in b'cde' is safe.
         """
         if not self.chk.options.strict_equality:
             return False
@@ -1996,7 +2024,12 @@ def dangerous_comparison(self, left: Type, right: Type) -> bool:
         if isinstance(left, UnionType) and isinstance(right, UnionType):
             left = remove_optional(left)
             right = remove_optional(right)
-        return not is_overlapping_types(left, right, ignore_promotions=True)
+        if (original_cont_type and self.has_bytes_component(original_cont_type) and
+                self.has_bytes_component(left)):
+            # We need to special case bytes, because both 97 in b'abc' and b'a' in b'abc'
+            # return True (and we want to show the error only if the check can _never_ be True).
+            return False
+        return not is_overlapping_types(left, right, ignore_promotions=False)
 
     def get_operator_method(self, op: str) -> str:
         if op == '/' and self.chk.options.python_version[0] == 2:

test-data/unit/check-expressions.test

@@ -2024,7 +2024,23 @@ cb: Union[Container[A], Container[B]]
 [builtins fixtures/bool.pyi]
 [typing fixtures/typing-full.pyi]
 
-[case testStrictEqualityNoPromote]
+[case testStrictEqualityBytesSpecial]
+# flags: --strict-equality
+b'abc' in b'abcde'
+[builtins fixtures/primitives.pyi]
+[typing fixtures/typing-full.pyi]
+
+[case testStrictEqualityBytesSpecialUnion]
+# flags: --strict-equality
+from typing import Union
+x: Union[bytes, str]
+
+b'abc' in x
+x in b'abc'
+[builtins fixtures/primitives.pyi]
+[typing fixtures/typing-full.pyi]
+
+[case testStrictEqualityNoPromotePy3]
 # flags: --strict-equality
 'a' == b'a'  # E: Non-overlapping equality check (left operand type: "str", right operand type: "bytes")
 b'a' in 'abc'  # E: Non-overlapping container check (element type: "bytes", container item type: "str")
@@ -2035,6 +2051,16 @@ x != y  # E: Non-overlapping equality check (left operand type: "str", right ope
 [builtins fixtures/primitives.pyi]
 [typing fixtures/typing-full.pyi]
 
+[case testStrictEqualityOkPromote]
+# flags: --strict-equality
+from typing import Container
+c: Container[int]
+
+1 == 1.0  # OK
+1.0 in c  # OK
+[builtins fixtures/primitives.pyi]
+[typing fixtures/typing-full.pyi]
+
 [case testStrictEqualityAny]
 # flags: --strict-equality
 from typing import Any, Container
@@ -2086,6 +2112,30 @@ class B:
 A() == B()  # E: Unsupported operand types for == ("A" and "B")
 [builtins fixtures/bool.pyi]
 
+[case testCustomEqCheckStrictEqualityOKInstance]
+# flags: --strict-equality
+class A:
+    def __eq__(self, other: object) -> bool:
+        ...
+class B:
+    def __eq__(self, other: object) -> bool:
+        ...
+
+A() == int()  # OK
+int() != B()  # OK
+[builtins fixtures/bool.pyi]
+
+[case testCustomEqCheckStrictEqualityOKUnion]
+# flags: --strict-equality
+from typing import Union
+class A:
+    def __eq__(self, other: object) -> bool:
+        ...
+
+x: Union[A, str]
+x == int()
+[builtins fixtures/bool.pyi]
+
 [case testCustomContainsCheckStrictEquality]
 # flags: --strict-equality
 class A:

test-data/unit/fixtures/primitives.pyi

@@ -23,7 +23,10 @@ class str(Sequence[str]):
     def __contains__(self, other: object) -> bool: pass
     def __getitem__(self, item: int) -> str: pass
     def format(self, *args) -> str: pass
-class bytes: pass
+class bytes(Sequence[int]):
+    def __iter__(self) -> Iterator[int]: pass
+    def __contains__(self, other: object) -> bool: pass
+    def __getitem__(self, item: int) -> int: pass
 class bytearray: pass
 class tuple(Generic[T]): pass
 class function: pass