diff options
-rw-r--r-- | CONTRIBUTORS.txt | 2 | ||||
-rw-r--r-- | ChangeLog | 2 | ||||
-rw-r--r-- | doc/whatsnew/2.12.rst | 2 | ||||
-rw-r--r-- | pylint/checkers/classes.py | 84 | ||||
-rw-r--r-- | tests/functional/u/useless/useless_super_delegation.py | 19 | ||||
-rw-r--r-- | tests/functional/u/useless/useless_super_delegation.txt | 37 |
6 files changed, 70 insertions, 76 deletions
diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index d6e2769ee..fd5d627eb 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -553,6 +553,8 @@ contributors: * Samuel Forestier: contributor +* Nick Pesce: contributor + * James DesLauriers: contributor * Youngsoo Sung: contributor @@ -49,6 +49,8 @@ Release date: TBA * Added ``using-f-string-in-unsupported-version`` checker. Issued when ``py-version`` is set to a version that does not support f-strings (< 3.6) +* Fix ``useless-super-delegation`` false positive when default keyword argument is a variable. + * Properly emit ``duplicate-key`` when Enum members are duplicate dictionary keys Closes #5150 diff --git a/doc/whatsnew/2.12.rst b/doc/whatsnew/2.12.rst index 5f9fcb976..f98406800 100644 --- a/doc/whatsnew/2.12.rst +++ b/doc/whatsnew/2.12.rst @@ -33,6 +33,8 @@ New checkers * Added ``using-f-string-in-unsupported-version`` checker. Issued when ``py-version`` is set to a version that does not support f-strings (< 3.6) +* Fix ``useless-super-delegation`` false positive when default keyword argument is a variable. + * Added new checker `use-implicit-booleanness``: Emitted when using collection literals for boolean comparisons diff --git a/pylint/checkers/classes.py b/pylint/checkers/classes.py index 489bb5063..f9a05081b 100644 --- a/pylint/checkers/classes.py +++ b/pylint/checkers/classes.py @@ -186,38 +186,6 @@ 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 @@ -244,34 +212,34 @@ def _has_different_parameters_default_value(original, overridden): overridden_default = default_missing default_list = [ - arg == default_missing for arg in (original_default, overridden_default) + 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 = { - nodes.Const: "value", - nodes.ClassDef: "name", - nodes.Tuple: "elts", - nodes.List: "elts", - nodes.Dict: "items", - } - 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 + if any(default_list): + if not all(default_list): + # Only one arg has no default value 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 + # Both args have a default value. Compare them + astroid_type_comparators = { + nodes.Const: lambda a, b: a.value == b.value, + nodes.ClassDef: lambda a, b: a.name == b.name, + nodes.Tuple: lambda a, b: a.elts == b.elts, + nodes.List: lambda a, b: a.elts == b.elts, + nodes.Dict: lambda a, b: a.items == b.items, + nodes.Name: lambda a, b: set(a.infer()) == set(b.infer()), + } + original_type = type(original_default) + if original_type in astroid_type_comparators: + # 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 astroid_type_comparators[original_type]( + original_default, overridden_default + ): + # Two args with same type but different values + return True + else: + # If the default value comparison is unhandled, assume the value is different return True return False diff --git a/tests/functional/u/useless/useless_super_delegation.py b/tests/functional/u/useless/useless_super_delegation.py index 002c7eb03..2e0384040 100644 --- a/tests/functional/u/useless/useless_super_delegation.py +++ b/tests/functional/u/useless/useless_super_delegation.py @@ -3,6 +3,7 @@ # pylint: disable=line-too-long, useless-object-inheritance, arguments-out-of-order # pylint: disable=super-with-arguments, dangerous-default-value +default_var = 1 def not_a_method(param, param2): return super(None, None).not_a_method(param, param2) @@ -52,12 +53,18 @@ class Base(SuperBase): def with_default_argument_dict(self, first, default_arg={}): pass + def with_default_argument_var(self, first, default_arg=default_var): + pass + def with_default_arg_ter(self, first, default_arg="has_been_changed"): super().with_default_arg_ter(first, default_arg) def with_default_arg_quad(self, first, default_arg="has_been_changed"): super().with_default_arg_quad(first, default_arg) + def with_default_unhandled(self, first, default_arg=lambda: True): + super().with_default_arg_quad(first, default_arg) + class NotUselessSuper(Base): def multiple_statements(self): @@ -167,6 +174,11 @@ class NotUselessSuper(Base): # Not useless because the default_arg is different from the one in the base class super(NotUselessSuper, self).with_default_argument_dict(first, default_arg) + default_var = 2 + def with_default_argument_var(self, first, default_arg=default_var): + # Not useless because the default_arg refers to a different variable from the one in the base class + super(NotUselessSuper, self).with_default_argument_var(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. @@ -192,6 +204,10 @@ class NotUselessSuper(Base): # call is different from the signature super(NotUselessSuper, self).with_default_arg_quad(first, default_arg + "_and_modified") + def with_default_unhandled(self, first, default_arg=lambda: True): + # Not useless because the default value type is not explictely handled (Lambda), so assume they are different + super(NotUselessSuper, self).with_default_unhandled(first, default_arg) + class UselessSuper(Base): @@ -236,6 +252,9 @@ class UselessSuper(Base): def with_default_argument_dict(self, first, default_arg={}): # [useless-super-delegation] super(UselessSuper, self).with_default_argument_dict(first, default_arg) + def with_default_argument_var(self, first, default_arg=default_var): # [useless-super-delegation] + super(UselessSuper, self).with_default_argument_var(first, default_arg) + def __init__(self): # [useless-super-delegation] super(UselessSuper, self).__init__() diff --git a/tests/functional/u/useless/useless_super_delegation.txt b/tests/functional/u/useless/useless_super_delegation.txt index d2dae5603..0d4b0a1fa 100644 --- a/tests/functional/u/useless/useless_super_delegation.txt +++ b/tests/functional/u/useless/useless_super_delegation.txt @@ -1,18 +1,19 @@ -useless-super-delegation:198:4:UselessSuper.equivalent_params:Useless super delegation in method 'equivalent_params' -useless-super-delegation:201:4:UselessSuper.equivalent_params_1:Useless super delegation in method 'equivalent_params_1' -useless-super-delegation:204:4:UselessSuper.equivalent_params_2:Useless super delegation in method 'equivalent_params_2' -useless-super-delegation:207:4:UselessSuper.equivalent_params_3:Useless super delegation in method 'equivalent_params_3' -useless-super-delegation:210:4:UselessSuper.equivalent_params_4:Useless super delegation in method 'equivalent_params_4' -useless-super-delegation:213:4:UselessSuper.equivalent_params_5:Useless super delegation in method 'equivalent_params_5' -useless-super-delegation:216:4:UselessSuper.equivalent_params_6:Useless super delegation in method 'equivalent_params_6' -useless-super-delegation:219:4:UselessSuper.with_default_argument:Useless super delegation in method 'with_default_argument' -useless-super-delegation:223:4:UselessSuper.without_default_argument:Useless super delegation in method 'without_default_argument' -useless-super-delegation:226:4:UselessSuper.with_default_argument_none:Useless super delegation in method 'with_default_argument_none' -useless-super-delegation:230:4:UselessSuper.with_default_argument_int:Useless super delegation in method 'with_default_argument_int' -useless-super-delegation:233:4:UselessSuper.with_default_argument_tuple:Useless super delegation in method 'with_default_argument_tuple' -useless-super-delegation:236:4:UselessSuper.with_default_argument_dict:Useless super delegation in method 'with_default_argument_dict' -useless-super-delegation:239:4:UselessSuper.__init__:Useless super delegation in method '__init__' -useless-super-delegation:242:4:UselessSuper.with_default_arg:Useless super delegation in method 'with_default_arg' -useless-super-delegation:245:4:UselessSuper.with_default_arg_bis:Useless super delegation in method 'with_default_arg_bis' -useless-super-delegation:248:4:UselessSuper.with_default_arg_ter:Useless super delegation in method 'with_default_arg_ter' -useless-super-delegation:251:4:UselessSuper.with_default_arg_quad:Useless super delegation in method 'with_default_arg_quad' +useless-super-delegation:214:4:UselessSuper.equivalent_params:Useless super delegation in method 'equivalent_params' +useless-super-delegation:217:4:UselessSuper.equivalent_params_1:Useless super delegation in method 'equivalent_params_1' +useless-super-delegation:220:4:UselessSuper.equivalent_params_2:Useless super delegation in method 'equivalent_params_2' +useless-super-delegation:223:4:UselessSuper.equivalent_params_3:Useless super delegation in method 'equivalent_params_3' +useless-super-delegation:226:4:UselessSuper.equivalent_params_4:Useless super delegation in method 'equivalent_params_4' +useless-super-delegation:229:4:UselessSuper.equivalent_params_5:Useless super delegation in method 'equivalent_params_5' +useless-super-delegation:232:4:UselessSuper.equivalent_params_6:Useless super delegation in method 'equivalent_params_6' +useless-super-delegation:235:4:UselessSuper.with_default_argument:Useless super delegation in method 'with_default_argument' +useless-super-delegation:239:4:UselessSuper.without_default_argument:Useless super delegation in method 'without_default_argument' +useless-super-delegation:242:4:UselessSuper.with_default_argument_none:Useless super delegation in method 'with_default_argument_none' +useless-super-delegation:246:4:UselessSuper.with_default_argument_int:Useless super delegation in method 'with_default_argument_int' +useless-super-delegation:249:4:UselessSuper.with_default_argument_tuple:Useless super delegation in method 'with_default_argument_tuple' +useless-super-delegation:252:4:UselessSuper.with_default_argument_dict:Useless super delegation in method 'with_default_argument_dict' +useless-super-delegation:255:4:UselessSuper.with_default_argument_var:Useless super delegation in method 'with_default_argument_var' +useless-super-delegation:258:4:UselessSuper.__init__:Useless super delegation in method '__init__' +useless-super-delegation:261:4:UselessSuper.with_default_arg:Useless super delegation in method 'with_default_arg' +useless-super-delegation:264:4:UselessSuper.with_default_arg_bis:Useless super delegation in method 'with_default_arg_bis' +useless-super-delegation:267:4:UselessSuper.with_default_arg_ter:Useless super delegation in method 'with_default_arg_ter' +useless-super-delegation:270:4:UselessSuper.with_default_arg_quad:Useless super delegation in method 'with_default_arg_quad' |