integrate patch from Peter Hammond to check protocol completness and avoid false R0903. Closes #104420

5 files changed, 108 insertions, 2 deletions

diff --git a/ChangeLog b/ChangeLog
index aa0840cc8..80a4ad594 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -16,6 +16,9 @@ ChangeLog for PyLint
 
     * #105337: allow custom reporter in output-format (patch by Kevin Jing Qiu)
 
+    * #104420: check for protocol completness and avoid false R0903
+      (patch by Peter Hammond)
+
     * #100654: fix grammatical error for W0332 message (using 'l' as
       long int identifier)
 
diff --git a/checkers/design_analysis.py b/checkers/design_analysis.py
index 2d40e8d17..daa550a71 100644
--- a/checkers/design_analysis.py
+++ b/checkers/design_analysis.py
@@ -30,6 +30,44 @@ import re
 # regexp for ignored argument name
 IGNORED_ARGUMENT_NAMES = re.compile('_.*')
 
+SPECIAL_METHODS = [('Context manager', set(('__enter__',
+                                            '__exit__',))),
+                   ('Container', set(('__len__',
+                                      '__getitem__',
+                                      '__setitem__',
+                                      '__delitem__',))),
+                   ('Callable', set(('__call__',))),
+                   ]
+
+class SpecialMethodChecker(object):
+    """A functor that checks for consistency of a set of special methods"""
+    def __init__(self, methods_found, on_error):
+        """Stores the set of __x__ method names that were found in the
+        class and a callable that will be called with args to R0024 if
+        the check fails
+        """
+        self.methods_found = methods_found
+        self.on_error = on_error
+
+    def __call__(self, methods_required, protocol):
+        """Checks the set of method names given to __init__ against the set
+        required.
+
+        If they are all present, returns true.
+        If they are all absent, returns false.
+        If some are present, reports the error and returns false.
+        """
+        required_methods_found = methods_required & self.methods_found
+        if required_methods_found == methods_required:
+            return True
+        if required_methods_found != set():
+            required_methods_missing  = methods_required - self.methods_found
+            self.on_error((protocol,
+                           ', '.join(sorted(required_methods_found)),
+                           ', '.join(sorted(required_methods_missing))))
+        return False
+
+
 def class_is_abstract(klass):
     """return true if the given class node should be considered as an abstract
     class
@@ -88,6 +126,10 @@ MSGS = {
     'R0923': ('Interface not implemented',
               'interface-not-implemented',
               'Used when an interface class is not implemented anywhere.'),
+    'R0924': ('Badly implemented %s, implements %s but not %s',
+              'incomplete-protocol',
+              'A class implements some of the special methods for a particular \
+               protocol, but not all of them')
     }
 
 
@@ -235,9 +277,12 @@ class MisdesignChecker(BaseChecker):
     def leave_class(self, node):
         """check number of public methods"""
         nb_public_methods = 0
+        special_methods = set()
         for method in node.methods():
             if not method.name.startswith('_'):
                 nb_public_methods += 1
+            if method.name.startswith("__"):
+                special_methods.add(method.name)
         # Does the class contain less than 20 public methods ?
         if nb_public_methods > self.config.max_public_methods:
             self.add_message('R0904', node=node,
@@ -246,13 +291,19 @@ class MisdesignChecker(BaseChecker):
         # stop here for exception, metaclass and interface classes
         if node.type != 'class':
             return
+        # Does the class implement special methods consitently?
+        # If so, don't enforce minimum public methods.
+        check_special = SpecialMethodChecker(
+            special_methods, lambda args: self.add_message('R0924', node=node, args=args))
+        protocols = [check_special(pmethods, pname) for pname, pmethods in SPECIAL_METHODS]
+        if True in protocols:
+            return
         # Does the class contain more than 5 public methods ?
         if nb_public_methods < self.config.min_public_methods:
             self.add_message('R0903', node=node,
                              args=(nb_public_methods,
                                    self.config.min_public_methods))
 
-
     def visit_function(self, node):
         """check function name, docstring, arguments, redefinition,
         variable names, max locals
diff --git a/test/input/func_method_could_be_function.py b/test/input/func_method_could_be_function.py
index c9630427a..1a3cc40a4 100644
--- a/test/input/func_method_could_be_function.py
+++ b/test/input/func_method_could_be_function.py
@@ -1,4 +1,4 @@
-# pylint: disable=R0903,R0922,W0232
+# pylint: disable=R0903,R0922,W0232,R0924
 """test detection of method which could be a function"""
 
 __revision__ = None
diff --git a/test/input/func_special_methods.py b/test/input/func_special_methods.py
new file mode 100644
index 000000000..699efd144
--- /dev/null
+++ b/test/input/func_special_methods.py
@@ -0,0 +1,47 @@
+#pylint: disable=C0111
+__revision__ = None
+
+class ContextManager:
+    def __enter__(self):
+        pass
+    def __exit__(self, *args):
+        pass
+    def __init__(self):
+        pass
+
+class BadContextManager:
+    def __enter__(self):
+        pass
+    def __init__(self):
+        pass
+
+class Container:
+    def __init__(self):
+        pass
+    def __len__(self):
+        return 0
+    def __getitem__(self, key):
+        pass
+    def __setitem__(self, key, value):
+        pass
+    def __delitem__(self, key, value):
+        pass
+    def __iter__(self):
+        pass
+
+class BadContainer:
+    def __init__(self):
+        pass
+    def __len__(self):
+        return 0
+    def __setitem__(self, key, value):
+        pass
+    def __iter__(self):
+        pass
+
+class Callable:
+    def __call__(self):
+        pass
+    def __init__(self):
+        pass
+
diff --git a/test/messages/func_special_methods.txt b/test/messages/func_special_methods.txt
new file mode 100644
index 000000000..3add213b7
--- /dev/null
+++ b/test/messages/func_special_methods.txt
@@ -0,0 +1,5 @@
+R: 12:BadContextManager: Badly implemented Context manager, implements __enter__ but not __exit__
+R: 12:BadContextManager: Too few public methods (0/2)
+R: 32:BadContainer: Badly implemented Container, implements __len__, __setitem__ but not __delitem__, __getitem__
+R: 32:BadContainer: Too few public methods (0/2)
+