useless-super-delegation takes into account default values for the current implementation

A method can reimplement a super method in order to provide a different default value, in which case we shouldn't emit ``useless-super-delegation``.

Author: hippo91

CONTRIBUTORS.txt

@@ -139,3 +139,5 @@ Order doesn't matter (not that much, at least ;)
 * Martin von Gagern (Google): Added 'raising-format-tuple' warning.
 
 * Ahirnish Pareek, 'keyword-arg-before-var-arg' check
+
+* Guillaume Peillex: contributor.

ChangeLog

@@ -104,6 +104,10 @@ What's New in Pylint 1.8?
       c-extensions.
       Close #1466
 
+    * Fix a false positive ``useless-super-delegation`` message when
+      parameters default values are different from those used in the base class.
+      Close #1085
+
     * Disabling 'wrong-import-order', 'wrong-import-position', or
       'ungrouped-imports' for a single line now prevents that line from
       triggering violations on subsequent lines.

doc/whatsnew/1.8.rst

@@ -314,3 +314,6 @@ Other Changes
 
 * ``missing-param-doc`` and ``missing-type-doc`` are no longer emitted when
   ``Args`` and ``Keyword Args`` are mixed in Google docstring.
+    
+* Fix of false positive ``useless-super-delegation`` message when
+  parameters default values are different from those used in the base class.

pylint/checkers/classes.py

@@ -134,6 +134,82 @@ def _positional_parameters(method):
     return positional
 
 
+def _get_node_type(node, potential_types):
+    """
+    Return the type of the node if it exists in potential_types.
+
+    Args:
+        node (astroid.node): node to get the type of.
+        potential_types (tuple): potential types of the node.
+
+    Returns:
+        type: type of the node or None.
+    """
+    for potential_type in potential_types:
+        if isinstance(node, potential_type):
+            return potential_type
+    return None
+
+
+def _check_arg_equality(node_a, node_b, attr_name):
+    """
+    Check equality of nodes based on the comparison of their attributes named attr_name.
+
+    Args:
+        node_a (astroid.node): first node to compare.
+        node_b (astroid.node): second node to compare.
+        attr_name (str): name of the nodes attribute to use for comparison.
+
+    Returns:
+        bool: True if node_a.attr_name == node_b.attr_name, False otherwise.
+    """
+    return getattr(node_a, attr_name) == getattr(node_b, attr_name)
+
+
+def _has_different_parameters_default_value(original, overridden):
+    """
+    Check if original and overridden methods arguments have different default values
+
+    Return True if one of the overridden arguments has a default
+    value different from the default value of the original argument
+    If one of the method doesn't have argument (.args is None)
+    return False
+    """
+    if original.args is None or overridden.args is None:
+        return False
+
+    original_param_names = [param.name for param in original.args]
+    default_missing = object()
+    for param_name in original_param_names:
+        try:
+            original_default = original.default_value(param_name)
+        except astroid.exceptions.NoDefault:
+            original_default = default_missing
+        try:
+            overridden_default = overridden.default_value(param_name)
+        except astroid.exceptions.NoDefault:
+            overridden_default = default_missing
+
+        default_list = [arg == default_missing for arg in (original_default, overridden_default)]
+        if any(default_list) and not all(default_list):
+            # Only one arg has no default value
+            return True
+
+        astroid_type_compared_attr = {astroid.Const: "value", astroid.ClassDef: "name",
+                                      astroid.Tuple: "elts", astroid.List: "elts"}
+        handled_types = tuple(astroid_type for astroid_type in astroid_type_compared_attr)
+        original_type = _get_node_type(original_default, handled_types)
+        if original_type:
+            # We handle only astroid types that are inside the dict astroid_type_compared_attr
+            if not isinstance(overridden_default, original_type):
+                # Two args with same name but different types
+                return True
+            if not _check_arg_equality(original_default, overridden_default,
+                                       astroid_type_compared_attr[original_type]):
+                # Two args with same type but different values
+                return True
+    return False
+
 def _has_different_parameters(original, overridden, dummy_parameter_regex):
     zipped = six.moves.zip_longest(original, overridden)
     for original_param, overridden_param in zipped:
@@ -758,6 +834,26 @@ def _check_useless_super_delegation(self, function):
         if super_call.type.name != current_scope.name:
             return
 
+        # Check values of default args
+        klass = function.parent.frame()
+        for overridden in klass.local_attr_ancestors(function.name):
+            # get astroid for the searched method
+            try:
+                meth_node = overridden[function.name]
+            except KeyError:
+                # we have found the method but it's not in the local
+                # dictionary.
+                # This may happen with astroid build from living objects
+                continue
+            if not isinstance(meth_node, astroid.FunctionDef):
+                # If the method have an ancestor which is not a function
+                # then it is legitimate to redefine it
+                return
+            if _has_different_parameters_default_value(meth_node.args, function.args):
+                return
+            else:
+                break
+
         # Detect if the parameters are the same as the call's arguments.
         params = _signature_from_arguments(function.args)
         args = _signature_from_call(call)

pylint/test/functional/useless_super_delegation.py

@@ -1,15 +1,59 @@
 # pylint: disable=missing-docstring, no-member, no-self-use, bad-super-call
-# pylint: disable=too-few-public-methods, unused-argument,invalid-name,too-many-public-methods
+# pylint: disable=too-few-public-methods, unused-argument, invalid-name, too-many-public-methods
+# pylint: disable=line-too-long
 
-def not_a_method():
-    return super(None, None).not_a_method()
 
+def not_a_method(param, param2):
+    return super(None, None).not_a_method(param, param2)
 
-class Base(object):
+
+class SuperBase(object):
+    def with_default_arg(self, first, default_arg="only_in_super_base"):
+        pass
+
+    def with_default_arg_bis(self, first, default_arg="only_in_super_base"):
+        pass
+
+    def with_default_arg_ter(self, first, default_arg="will_be_changed"):
+        pass
+
+    def with_default_arg_quad(self, first, default_arg="will_be_changed"):
+        pass
+
+
+class Base(SuperBase):
+
+    fake_method = not_a_method
 
     def something(self):
         pass
 
+    def with_default_argument(self, first, default_arg="default"):
+        pass
+
+    def with_default_argument_bis(self, first, default_arg="default"):
+        pass
+
+    def without_default_argument(self, first, second):
+        pass
+
+    def with_default_argument_none(self, first, default_arg=None):
+        pass
+
+    def without_default_argument2(self, first, second):
+        pass
+
+    def with_default_argument_int(self, first, default_arg=42):
+        pass
+
+    def with_default_argument_tuple(self, first, default_arg=()):
+        pass
+
+    def with_default_arg_ter(self, first, default_arg="has_been_changed"):
+        super(Base, self).with_default_arg_ter(first, default_arg)
+
+    def with_default_arg_quad(self, first, default_arg="has_been_changed"):
+        super(Base, self).with_default_arg_quad(first, default_arg)
 
 class NotUselessSuper(Base):
 
@@ -92,6 +136,55 @@ def extraneous_positional_args(self, **args):
         super(NotUselessSuper, self).extraneous_positional_args(
             1, 2, **args)
 
+    def with_default_argument(self, first, default_arg="other"):
+        # Not useless because the default_arg is different from the one in the base class
+        super(NotUselessSuper, self).with_default_argument(first, default_arg)
+
+    def without_default_argument(self, first, second=True):
+        # Not useless because in the base class there is not default value for second argument
+        super(NotUselessSuper, self).without_default_argument(first, second)
+
+    def with_default_argument_none(self, first, default_arg='NotNone'):
+        # Not useless because the default_arg is different from the one in the base class
+        super(NotUselessSuper, self).with_default_argument_none(first, default_arg)
+
+    def without_default_argument2(self, first, second=None):
+        # Not useless because in the base class there is not default value for second argument
+        super(NotUselessSuper, self).without_default_argument2(first, second)
+
+    def with_default_argument_int(self, first, default_arg="42"):
+        # Not useless because the default_arg is a string whereas in the base class it's an int
+        super(NotUselessSuper, self).with_default_argument_int(first, default_arg)
+
+    def with_default_argument_tuple(self, first, default_arg=("42", "a")):
+        # Not useless because the default_arg is different from the one in the base class
+        super(NotUselessSuper, self).with_default_argument_tuple(first, default_arg)
+
+    def with_default_argument_bis(self, first, default_arg="default"):
+        # Although the default_arg is the same as in the base class, the call signature
+        # differs. Thus it is not useless.
+        super(NotUselessSuper, self).with_default_argument_bis(default_arg + "_argument")
+
+    def fake_method(self, param2="other"):
+        super(NotUselessSuper, self).fake_method(param2)
+
+    def with_default_arg(self, first, default_arg="only_in_super_base"):
+        # Not useless because the call of this method is different from the function signature
+        super(NotUselessSuper, self).with_default_arg(first, default_arg + "_and_here")
+
+    def with_default_arg_bis(self, first, default_arg="default_changed"):
+        # Not useless because the default value is different from the SuperBase one
+        super(NotUselessSuper, self).with_default_arg_bis(first, default_arg)
+
+    def with_default_arg_ter(self, first, default_arg="has_been_changed_again"):
+        # Not useless because the default value is different from the Base one
+        super(NotUselessSuper, self).with_default_arg_ter(first, default_arg)
+
+    def with_default_arg_quad(self, first, default_arg="has_been_changed"):
+        # Not useless because the default value is the same as in the base but the
+        # call is different from the signature
+        super(NotUselessSuper, self).with_default_arg_quad(first, default_arg + "_and_modified")
+
 
 class UselessSuper(Base):
 
@@ -116,9 +209,38 @@ def equivalent_params_5(self, first, *args): # [useless-super-delegation]
     def equivalent_params_6(self, first, *args, **kwargs): # [useless-super-delegation]
         return super(UselessSuper, self).equivalent_params_6(first, *args, **kwargs)
 
+    def with_default_argument(self, first, default_arg="default"): # [useless-super-delegation]
+        # useless because the default value here is the same as in the base class
+        return super(UselessSuper, self).with_default_argument(first, default_arg)
+
+    def without_default_argument(self, first, second): # [useless-super-delegation]
+        return super(UselessSuper, self).without_default_argument(first, second)
+
+    def with_default_argument_none(self, first, default_arg=None): # [useless-super-delegation]
+        # useless because the default value here is the same as in the base class
+        super(UselessSuper, self).with_default_argument_none(first, default_arg)
+
+    def with_default_argument_int(self, first, default_arg=42): # [useless-super-delegation]
+        super(UselessSuper, self).with_default_argument_int(first, default_arg)
+
+    def with_default_argument_tuple(self, first, default_arg=()): # [useless-super-delegation]
+        super(UselessSuper, self).with_default_argument_tuple(first, default_arg)
+
     def __init__(self): # [useless-super-delegation]
         super(UselessSuper, self).__init__()
 
+    def with_default_arg(self, first, default_arg="only_in_super_base"): # [useless-super-delegation]
+        super(UselessSuper, self).with_default_arg(first, default_arg)
+
+    def with_default_arg_bis(self, first, default_arg="only_in_super_base"): # [useless-super-delegation]
+        super(UselessSuper, self).with_default_arg_bis(first, default_arg)
+
+    def with_default_arg_ter(self, first, default_arg="has_been_changed"): # [useless-super-delegation]
+        super(UselessSuper, self).with_default_arg_ter(first, default_arg)
+
+    def with_default_arg_quad(self, first, default_arg="has_been_changed"): # [useless-super-delegation]
+        super(UselessSuper, self).with_default_arg_quad(first, default_arg)
+
 
 def trigger_something(value_to_trigger):
     pass

pylint/test/functional/useless_super_delegation.txt

@@ -1,8 +1,17 @@
-useless-super-delegation:98:UselessSuper.equivalent_params:Useless super delegation in method 'equivalent_params'
-useless-super-delegation:101:UselessSuper.equivalent_params_1:Useless super delegation in method 'equivalent_params_1'
-useless-super-delegation:104:UselessSuper.equivalent_params_2:Useless super delegation in method 'equivalent_params_2'
-useless-super-delegation:107:UselessSuper.equivalent_params_3:Useless super delegation in method 'equivalent_params_3'
-useless-super-delegation:110:UselessSuper.equivalent_params_4:Useless super delegation in method 'equivalent_params_4'
-useless-super-delegation:113:UselessSuper.equivalent_params_5:Useless super delegation in method 'equivalent_params_5'
-useless-super-delegation:116:UselessSuper.equivalent_params_6:Useless super delegation in method 'equivalent_params_6'
-useless-super-delegation:119:UselessSuper.__init__:Useless super delegation in method '__init__'
+useless-super-delegation:191:UselessSuper.equivalent_params:Useless super delegation in method 'equivalent_params'
+useless-super-delegation:194:UselessSuper.equivalent_params_1:Useless super delegation in method 'equivalent_params_1'
+useless-super-delegation:197:UselessSuper.equivalent_params_2:Useless super delegation in method 'equivalent_params_2'
+useless-super-delegation:200:UselessSuper.equivalent_params_3:Useless super delegation in method 'equivalent_params_3'
+useless-super-delegation:203:UselessSuper.equivalent_params_4:Useless super delegation in method 'equivalent_params_4'
+useless-super-delegation:206:UselessSuper.equivalent_params_5:Useless super delegation in method 'equivalent_params_5'
+useless-super-delegation:209:UselessSuper.equivalent_params_6:Useless super delegation in method 'equivalent_params_6'
+useless-super-delegation:212:UselessSuper.with_default_argument:Useless super delegation in method 'with_default_argument'
+useless-super-delegation:216:UselessSuper.without_default_argument:Useless super delegation in method 'without_default_argument'
+useless-super-delegation:219:UselessSuper.with_default_argument_none:Useless super delegation in method 'with_default_argument_none'
+useless-super-delegation:223:UselessSuper.with_default_argument_int:Useless super delegation in method 'with_default_argument_int'
+useless-super-delegation:226:UselessSuper.with_default_argument_tuple:Useless super delegation in method 'with_default_argument_tuple'
+useless-super-delegation:229:UselessSuper.__init__:Useless super delegation in method '__init__'
+useless-super-delegation:232:UselessSuper.with_default_arg:Useless super delegation in method 'with_default_arg'
+useless-super-delegation:235:UselessSuper.with_default_arg_bis:Useless super delegation in method 'with_default_arg_bis'
+useless-super-delegation:238:UselessSuper.with_default_arg_ter:Useless super delegation in method 'with_default_arg_ter'
+useless-super-delegation:241:UselessSuper.with_default_arg_quad:Useless super delegation in method 'with_default_arg_quad'