Enhance the protocol checker (#3259)

This commit adds multiple checks for various Python protocols

  E0304 (invalid-bool-returned): __bool__ did not return a bool
  E0305 (invalid-index-returned): __index__ did not return an integer
  E0306 (invalid-repr-returned): __repr__ did not return a string
  E0307 (invalid-str-returned): __str__ did not return a string
  E0308 (invalid-bytes-returned): __bytes__ did not return a string
  E0309 (invalid-hash-returned): __hash__ did not return an integer
  E0310 (invalid-length-hint-returned): __length_hint__ did not return a non-negative integer
  E0311 (invalid-format-returned): __format__ did not return a string
  E0312 (invalid-getnewargs-returned): __getnewargs__ did not return a tuple
  E0313 (invalid-getnewargs-ex-returned): __getnewargs_ex__ did not return a tuple of the form (tuple, dict)

Close #560

CONTRIBUTORS.txt

@@ -352,3 +352,5 @@ contributors:
 * Bastien Vallet: contributor
 
 * Pek Chhan: contributor
+
+* Craig Henriques: contributor

ChangeLog

@@ -7,6 +7,20 @@ What's New in Pylint 2.5.0?
 
 Release date: TBA
 
+* Added errors for protocol functions when invalid return types are detected.
+  E0304 (invalid-bool-returned): __bool__ did not return a bool
+  E0305 (invalid-index-returned): __index__ did not return an integer
+  E0306 (invalid-repr-returned): __repr__ did not return a string
+  E0307 (invalid-str-returned): __str__ did not return a string
+  E0308 (invalid-bytes-returned): __bytes__ did not return a string
+  E0309 (invalid-hash-returned): __hash__ did not return an integer
+  E0310 (invalid-length-hint-returned): __length_hint__ did not return a non-negative integer
+  E0311 (invalid-format-returned): __format__ did not return a string
+  E0312 (invalid-getnewargs-returned): __getnewargs__ did not return a tuple
+  E0313 (invalid-getnewargs-ex-returned): __getnewargs_ex__ did not return a tuple of the form (tuple, dict)
+
+  Close #560
+
 * ``safe_infer`` can infer a value as long as all the paths share the same type.
 
   Close #2503

doc/whatsnew/2.5.rst

@@ -19,6 +19,18 @@ New checkers
   f-string without having any interpolated values in it, which means
   that the f-string can be a normal string.
 
+* Checks for invalid return types from protocol functions:
+
+   * ``__bool__`` must return a bool
+   * ``__index__`` must return an integer
+   * ``__repr__`` must return a string
+   * ``__str__`` must return a string
+   * ``__bytes__`` must return bytes
+   * ``__hash__`` must return a string
+   * ``__length__hint__`` must return a non-negative integer
+   * ``__format__`` must return a string
+   * ``__getnewargs__`` must return a tuple
+   * ``__getnewargs_ex__`` must return a tuple of the form (tuple, dict)
 
 Other Changes
 =============

pylint/checkers/classes.py

@@ -1719,23 +1719,105 @@ class SpecialMethodsChecker(BaseChecker):
             "invalid-length-returned",
             "Used when a __len__ method returns something which is not a "
             "non-negative integer",
-            {},
+        ),
+        "E0304": (
+            "__bool__ does not return bool",
+            "invalid-bool-returned",
+            "Used when a __bool__ method returns something which is not a bool",
+        ),
+        "E0305": (
+            "__index__ does not return int",
+            "invalid-index-returned",
+            "Used when an __index__ method returns something which is not "
+            "an integer",
+        ),
+        "E0306": (
+            "__repr__ does not return str",
+            "invalid-repr-returned",
+            "Used when a __repr__ method returns something which is not a string",
+        ),
+        "E0307": (
+            "__str__ does not return str",
+            "invalid-str-returned",
+            "Used when a __str__ method returns something which is not a string",
+        ),
+        "E0308": (
+            "__bytes__ does not return bytes",
+            "invalid-bytes-returned",
+            "Used when a __bytes__ method returns something which is not bytes",
+        ),
+        "E0309": (
+            "__hash__ does not return int",
+            "invalid-hash-returned",
+            "Used when a __hash__ method returns something which is not an integer",
+        ),
+        "E0310": (
+            "__length_hint__ does not return non-negative integer",
+            "invalid-length-hint-returned",
+            "Used when a __length_hint__ method returns something which is not a "
+            "non-negative integer",
+        ),
+        "E0311": (
+            "__format__ does not return str",
+            "invalid-format-returned",
+            "Used when a __format__ method returns something which is not a string",
+        ),
+        "E0312": (
+            "__getnewargs__ does not return a tuple",
+            "invalid-getnewargs-returned",
+            "Used when a __getnewargs__ method returns something which is not "
+            "a tuple",
+        ),
+        "E0313": (
+            "__getnewargs_ex__ does not return a tuple containing (tuple, dict)",
+            "invalid-getnewargs-ex-returned",
+            "Used when a __getnewargs_ex__ method returns something which is not "
+            "of the form tuple(tuple, dict)",
         ),
     }
     priority = -2
 
+    def __init__(self, linter=None):
+        BaseChecker.__init__(self, linter)
+        self._protocol_map = {
+            "__iter__": self._check_iter,
+            "__len__": self._check_len,
+            "__bool__": self._check_bool,
+            "__index__": self._check_index,
+            "__repr__": self._check_repr,
+            "__str__": self._check_str,
+            "__bytes__": self._check_bytes,
+            "__hash__": self._check_hash,
+            "__length_hint__": self._check_length_hint,
+            "__format__": self._check_format,
+            "__getnewargs__": self._check_getnewargs,
+            "__getnewargs_ex__": self._check_getnewargs_ex,
+        }
+
     @check_messages(
         "unexpected-special-method-signature",
         "non-iterator-returned",
         "invalid-length-returned",
+        "invalid-bool-returned",
+        "invalid-index-returned",
+        "invalid-repr-returned",
+        "invalid-str-returned",
+        "invalid-bytes-returned",
+        "invalid-hash-returned",
+        "invalid-length-hint-returned",
+        "invalid-format-returned",
+        "invalid-getnewargs-returned",
+        "invalid-getnewargs-ex-returned",
     )
     def visit_functiondef(self, node):
         if not node.is_method():
             return
-        if node.name == "__iter__":
-            self._check_iter(node)
-        if node.name == "__len__":
-            self._check_len(node)
+
+        inferred = _safe_infer_call_result(node, node)
+        # Only want to check types that we are able to infer
+        if inferred and node.name in self._protocol_map:
+            self._protocol_map[node.name](node, inferred)
+
         if node.name in PYMETHODS:
             self._check_unexpected_method_signature(node)
 
@@ -1788,6 +1870,56 @@ def _check_unexpected_method_signature(self, node):
                 node=node,
             )
 
+    @staticmethod
+    def _is_wrapped_type(node, type_):
+        return (
+            isinstance(node, astroid.Instance)
+            and node.name == type_
+            and not isinstance(node, astroid.Const)
+        )
+
+    @staticmethod
+    def _is_int(node):
+        if SpecialMethodsChecker._is_wrapped_type(node, "int"):
+            return True
+
+        return isinstance(node, astroid.Const) and isinstance(node.value, int)
+
+    @staticmethod
+    def _is_str(node):
+        if SpecialMethodsChecker._is_wrapped_type(node, "str"):
+            return True
+
+        return isinstance(node, astroid.Const) and isinstance(node.value, str)
+
+    @staticmethod
+    def _is_bool(node):
+        if SpecialMethodsChecker._is_wrapped_type(node, "bool"):
+            return True
+
+        return isinstance(node, astroid.Const) and isinstance(node.value, bool)
+
+    @staticmethod
+    def _is_bytes(node):
+        if SpecialMethodsChecker._is_wrapped_type(node, "bytes"):
+            return True
+
+        return isinstance(node, astroid.Const) and isinstance(node.value, bytes)
+
+    @staticmethod
+    def _is_tuple(node):
+        if SpecialMethodsChecker._is_wrapped_type(node, "tuple"):
+            return True
+
+        return isinstance(node, astroid.Const) and isinstance(node.value, tuple)
+
+    @staticmethod
+    def _is_dict(node):
+        if SpecialMethodsChecker._is_wrapped_type(node, "dict"):
+            return True
+
+        return isinstance(node, astroid.Const) and isinstance(node.value, dict)
+
     @staticmethod
     def _is_iterator(node):
         if node is astroid.Uninferable:
@@ -1813,33 +1945,83 @@ def _is_iterator(node):
                     pass
         return False
 
-    def _check_iter(self, node):
-        inferred = _safe_infer_call_result(node, node)
-        if inferred is not None:
-            if not self._is_iterator(inferred):
-                self.add_message("non-iterator-returned", node=node)
+    def _check_iter(self, node, inferred):
+        if not self._is_iterator(inferred):
+            self.add_message("non-iterator-returned", node=node)
 
-    def _check_len(self, node):
-        inferred = _safe_infer_call_result(node, node)
-        if not inferred or inferred is astroid.Uninferable:
-            return
+    def _check_len(self, node, inferred):
+        if not self._is_int(inferred):
+            self.add_message("invalid-length-returned", node=node)
+        elif isinstance(inferred, astroid.Const) and inferred.value < 0:
+            self.add_message("invalid-length-returned", node=node)
 
-        if (
-            isinstance(inferred, astroid.Instance)
-            and inferred.name == "int"
-            and not isinstance(inferred, astroid.Const)
-        ):
-            # Assume it's good enough, since the int() call might wrap
-            # something that's uninferable for us
+    def _check_bool(self, node, inferred):
+        if not self._is_bool(inferred):
+            self.add_message("invalid-bool-returned", node=node)
+
+    def _check_index(self, node, inferred):
+        if not self._is_int(inferred):
+            self.add_message("invalid-index-returned", node=node)
+
+    def _check_repr(self, node, inferred):
+        if not self._is_str(inferred):
+            self.add_message("invalid-repr-returned", node=node)
+
+    def _check_str(self, node, inferred):
+        if not self._is_str(inferred):
+            self.add_message("invalid-str-returned", node=node)
+
+    def _check_bytes(self, node, inferred):
+        if not self._is_bytes(inferred):
+            self.add_message("invalid-bytes-returned", node=node)
+
+    def _check_hash(self, node, inferred):
+        if not self._is_int(inferred):
+            self.add_message("invalid-hash-returned", node=node)
+
+    def _check_length_hint(self, node, inferred):
+        if not self._is_int(inferred):
+            self.add_message("invalid-length-hint-returned", node=node)
+        elif isinstance(inferred, astroid.Const) and inferred.value < 0:
+            self.add_message("invalid-length-hint-returned", node=node)
+
+    def _check_format(self, node, inferred):
+        if not self._is_str(inferred):
+            self.add_message("invalid-format-returned", node=node)
+
+    def _check_getnewargs(self, node, inferred):
+        if not self._is_tuple(inferred):
+            self.add_message("invalid-getnewargs-returned", node=node)
+
+    def _check_getnewargs_ex(self, node, inferred):
+        if not self._is_tuple(inferred):
+            self.add_message("invalid-getnewargs-ex-returned", node=node)
             return
 
-        if not isinstance(inferred, astroid.Const):
-            self.add_message("invalid-length-returned", node=node)
+        if not isinstance(inferred, astroid.Tuple):
+            # If it's not an astroid.Tuple we can't analyze it further
             return
 
-        value = inferred.value
-        if not isinstance(value, int) or value < 0:
-            self.add_message("invalid-length-returned", node=node)
+        found_error = False
+
+        if len(inferred.elts) != 2:
+            found_error = True
+        else:
+            for arg, check in [
+                (inferred.elts[0], self._is_tuple),
+                (inferred.elts[1], self._is_dict),
+            ]:
+
+                if isinstance(arg, astroid.Call):
+                    arg = safe_infer(arg)
+
+                if arg and arg is not astroid.Uninferable:
+                    if not check(arg):
+                        found_error = True
+                        break
+
+        if found_error:
+            self.add_message("invalid-getnewargs-ex-returned", node=node)
 
 
 def _ancestors_to_call(klass_node, method="__init__"):

tests/functional/i/invalid_bool_returned.py

@@ -0,0 +1,62 @@
+"""Check invalid value returned by __bool__ """
+
+# pylint: disable=too-few-public-methods,missing-docstring,no-self-use,import-error, useless-object-inheritance
+import six
+
+from missing import Missing
+
+
+class FirstGoodBool(object):
+    """__bool__ returns <type 'bool'>"""
+
+    def __bool__(self):
+        return True
+
+
+class SecondGoodBool(object):
+    """__bool__ returns <type 'bool'>"""
+
+    def __bool__(self):
+        return bool(0)
+
+
+class BoolMetaclass(type):
+    def __bool__(cls):
+        return True
+
+
+@six.add_metaclass(BoolMetaclass)
+class ThirdGoodBool(object):
+    """Bool through the metaclass."""
+
+
+class FirstBadBool(object):
+    """ __bool__ returns an integer """
+
+    def __bool__(self):  # [invalid-bool-returned]
+        return 1
+
+
+class SecondBadBool(object):
+    """ __bool__ returns str """
+
+    def __bool__(self):  # [invalid-bool-returned]
+        return "True"
+
+
+class ThirdBadBool(object):
+    """ __bool__ returns node which does not have 'value' in AST """
+
+    def __bool__(self):  # [invalid-bool-returned]
+        return lambda: 3
+
+
+class AmbigousBool(object):
+    """ Uninferable return value """
+    __bool__ = lambda self: Missing
+
+
+class AnotherAmbiguousBool(object):
+    """Potential uninferable return value"""
+    def __bool__(self):
+        return bool(Missing)

tests/functional/i/invalid_bool_returned.txt

@@ -0,0 +1,3 @@
+invalid-bool-returned:36:FirstBadBool.__bool__:__bool__ does not return bool
+invalid-bool-returned:43:SecondBadBool.__bool__:__bool__ does not return bool
+invalid-bool-returned:50:ThirdBadBool.__bool__:__bool__ does not return bool