Better error message when __eq__ has unexpected signature (#6106)

This PR will add a message and an example when the arguments of the dunder method  `__eq__` do not match the expected signature. With this PR, the added message looks like this:
```
It is recommended for "__eq__" to work with arbitrary objects, for example:
    def __eq__(self, other: object) -> bool:
        if not isinstance(other, {class_name}):
            return NotImplemented
        return <logic to compare two {class_name} instances>
```

Fixes #2055

Author: David-Wobrock

mypy/checker.py

@@ -1475,6 +1475,9 @@ def check_override(self, override: FunctionLike, original: FunctionLike,
                 # are erased, then it is definitely an incompatibility.
 
                 override_ids = override.type_var_ids()
+                type_name = None
+                if isinstance(override.definition, FuncDef):
+                    type_name = override.definition.info.name()
 
                 def erase_override(t: Type) -> Type:
                     return erase_typevars(t, ids_to_erase=override_ids)
@@ -1483,7 +1486,7 @@ def erase_override(t: Type) -> Type:
                     if not is_subtype(original.arg_types[i],
                                       erase_override(override.arg_types[i])):
                         self.msg.argument_incompatible_with_supertype(
-                            i + 1, name, name_in_super, supertype, node)
+                            i + 1, name, type_name, name_in_super, supertype, node)
                         emitted_msg = True
 
                 if not is_subtype(erase_override(override.ret_type),

mypy/errors.py

@@ -405,10 +405,10 @@ def render_messages(self, errors: List[ErrorInfo]) -> List[Tuple[Optional[str],
                                                                      str, str]]:
         """Translate the messages into a sequence of tuples.
 
-        Each tuple is of form (path, line, col, message.  The rendered
-        sequence includes information about error contexts. The path
-        item may be None. If the line item is negative, the line
-        number is not defined for the tuple.
+        Each tuple is of form (path, line, col, severity, message).
+        The rendered sequence includes information about error contexts.
+        The path item may be None. If the line item is negative, the
+        line number is not defined for the tuple.
         """
         result = []  # type: List[Tuple[Optional[str], int, int, str, str]]
         # (path, line, column, severity, message)

mypy/messages.py

@@ -13,6 +13,7 @@
 from collections import OrderedDict
 import re
 import difflib
+from textwrap import dedent
 
 from typing import cast, List, Dict, Any, Sequence, Iterable, Tuple, Set, Optional, Union
 
@@ -187,7 +188,7 @@ def report(self, msg: str, context: Optional[Context], severity: str,
         if self.disable_count <= 0:
             self.errors.report(context.get_line() if context else -1,
                                context.get_column() if context else -1,
-                               msg.strip(), severity=severity, file=file, offset=offset,
+                               msg, severity=severity, file=file, offset=offset,
                                origin_line=origin.get_line() if origin else None)
 
     def fail(self, msg: str, context: Optional[Context], file: Optional[str] = None,
@@ -198,7 +199,15 @@ def fail(self, msg: str, context: Optional[Context], file: Optional[str] = None,
     def note(self, msg: str, context: Context, file: Optional[str] = None,
              origin: Optional[Context] = None, offset: int = 0) -> None:
         """Report a note (unless disabled)."""
-        self.report(msg, context, 'note', file=file, origin=origin, offset=offset)
+        self.report(msg, context, 'note', file=file, origin=origin,
+                    offset=offset)
+
+    def note_multiline(self, messages: str, context: Context, file: Optional[str] = None,
+                       origin: Optional[Context] = None, offset: int = 0) -> None:
+        """Report as many notes as lines in the message (unless disabled)."""
+        for msg in messages.splitlines():
+            self.report(msg, context, 'note', file=file, origin=origin,
+                        offset=offset)
 
     def warn(self, msg: str, context: Context, file: Optional[str] = None,
              origin: Optional[Context] = None) -> None:
@@ -854,12 +863,25 @@ def signature_incompatible_with_supertype(
             name, target), context)
 
     def argument_incompatible_with_supertype(
-            self, arg_num: int, name: str, name_in_supertype: str,
-            supertype: str, context: Context) -> None:
+            self, arg_num: int, name: str, type_name: Optional[str],
+            name_in_supertype: str, supertype: str, context: Context) -> None:
         target = self.override_target(name, name_in_supertype, supertype)
         self.fail('Argument {} of "{}" incompatible with {}'
                   .format(arg_num, name, target), context)
 
+        if name == "__eq__" and type_name:
+            multiline_msg = self.comparison_method_example_msg(class_name=type_name)
+            self.note_multiline(multiline_msg, context)
+
+    def comparison_method_example_msg(self, class_name: str) -> str:
+        return dedent('''\
+        It is recommended for "__eq__" to work with arbitrary objects, for example:
+            def __eq__(self, other: object) -> bool:
+                if not isinstance(other, {class_name}):
+                    return NotImplemented
+                return <logic to compare two {class_name} instances>
+        '''.format(class_name=class_name))
+
     def return_type_incompatible_with_supertype(
             self, name: str, name_in_supertype: str, supertype: str,
             context: Context) -> None:
@@ -1110,8 +1132,6 @@ def reveal_locals(self, type_map: Dict[str, Optional[Type]], context: Context) -
         # use an ordered dictionary sorted by variable name
         sorted_locals = OrderedDict(sorted(type_map.items(), key=lambda t: t[0]))
         self.fail("Revealed local types are:", context)
-        # Note that self.fail does a strip() on the message, so we cannot prepend with spaces
-        # for indentation
         for line in ['{}: {}'.format(k, v) for k, v in sorted_locals.items()]:
             self.fail(line, context)
 

test-data/unit/check-classes.test

@@ -282,6 +282,18 @@ class B(A):
 main:7: error: Argument 1 of "f" incompatible with supertype "A"
 main:9: error: Return type of "h" incompatible with supertype "A"
 
+[case testEqMethodsOverridingWithNonObjects]
+class A:
+  def __eq__(self, other: A) -> bool: pass  # Fail
+[builtins fixtures/attr.pyi]
+[out]
+main:2: error: Argument 1 of "__eq__" incompatible with supertype "object"
+main:2: note: It is recommended for "__eq__" to work with arbitrary objects, for example:
+main:2: note:     def __eq__(self, other: object) -> bool:
+main:2: note:         if not isinstance(other, A):
+main:2: note:             return NotImplemented
+main:2: note:         return <logic to compare two A instances>
+
 [case testMethodOverridingWithIncompatibleArgumentCount]
 import typing
 class A: