Make ``arguments-differ`` check extra parameters for default values (#5539)

Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
Co-authored-by: Jacob Walls <jacobtylerwalls@gmail.com>

7 files changed, 88 insertions, 16 deletions

diff --git a/ChangeLog b/ChangeLog
index b82e345bb..b75fe9515 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -480,6 +480,11 @@ Release date: TBA
 * The ``testutils`` for unittests now accept ``end_lineno`` and ``end_column``. Tests
   without these will trigger a ``DeprecationWarning``.
 
+* ``arguments-differ`` will no longer complain about method redefinitions with extra parameters
+  that have default values.
+
+  Closes #1556, #5338
+
 * Fixed false positive ``unexpected-keyword-arg`` for decorators.
 
   Closes #258
diff --git a/doc/whatsnew/2.13.rst b/doc/whatsnew/2.13.rst
index a47516e52..47e78e520 100644
--- a/doc/whatsnew/2.13.rst
+++ b/doc/whatsnew/2.13.rst
@@ -470,6 +470,11 @@ Other Changes
 * The ``testutils`` for unittests now accept ``end_lineno`` and ``end_column``. Tests
   without these will trigger a ``DeprecationWarning``.
 
+* ``arguments-differ`` will no longer complain about method redefinitions with extra parameters
+  that have default values.
+
+  Closes #1556, #5338
+
 * Disables for ``deprecated-module`` and similar warnings for stdlib features deprecated
   in newer versions of Python no longer raise ``useless-suppression`` when linting with
   older Python interpreters where those features are not yet deprecated.
diff --git a/pylint/checkers/classes/class_checker.py b/pylint/checkers/classes/class_checker.py
index 7492c9b42..e258f6642 100644
--- a/pylint/checkers/classes/class_checker.py
+++ b/pylint/checkers/classes/class_checker.py
@@ -191,15 +191,21 @@ def _has_different_parameters(
     overridden: List[nodes.AssignName],
     dummy_parameter_regex: Pattern,
 ) -> List[str]:
-    result = []
+    result: List[str] = []
     zipped = zip_longest(original, overridden)
     for original_param, overridden_param in zipped:
-        params = (original_param, overridden_param)
-        if not all(params):
+        if not overridden_param:
             return ["Number of parameters "]
 
+        if not original_param:
+            try:
+                overridden_param.parent.default_value(overridden_param.name)
+                continue
+            except astroid.NoDefault:
+                return ["Number of parameters "]
+
         # check for the arguments' name
-        names = [param.name for param in params]
+        names = [param.name for param in (original_param, overridden_param)]
         if any(dummy_parameter_regex.match(name) for name in names):
             continue
         if original_param.name != overridden_param.name:
@@ -211,6 +217,29 @@ def _has_different_parameters(
     return result
 
 
+def _has_different_keyword_only_parameters(
+    original: List[nodes.AssignName],
+    overridden: List[nodes.AssignName],
+) -> List[str]:
+    """Determine if the two methods have different keyword only parameters."""
+    original_names = [i.name for i in original]
+    overridden_names = [i.name for i in overridden]
+
+    if any(name not in overridden_names for name in original_names):
+        return ["Number of parameters "]
+
+    for name in overridden_names:
+        if name in original_names:
+            continue
+
+        try:
+            overridden[0].parent.default_value(name)
+        except astroid.NoDefault:
+            return ["Number of parameters "]
+
+    return []
+
+
 def _different_parameters(
     original: nodes.FunctionDef,
     overridden: nodes.FunctionDef,
@@ -253,8 +282,8 @@ def _different_parameters(
     different_positional = _has_different_parameters(
         original_parameters, overridden_parameters, dummy_parameter_regex
     )
-    different_kwonly = _has_different_parameters(
-        original_kwonlyargs, overridden.args.kwonlyargs, dummy_parameter_regex
+    different_kwonly = _has_different_keyword_only_parameters(
+        original_kwonlyargs, overridden.args.kwonlyargs
     )
     if different_kwonly and different_positional:
         if "Number " in different_positional[0] and "Number " in different_kwonly[0]:
@@ -483,7 +512,8 @@ MSGS = {  # pylint: disable=consider-using-namedtuple-or-dataclass
         "%s %s %r method",
         "arguments-differ",
         "Used when a method has a different number of arguments than in "
-        "the implemented interface or in an overridden method.",
+        "the implemented interface or in an overridden method. Extra arguments "
+        "with default values are ignored.",
     ),
     "W0222": (
         "Signature differs from %s %r method",
diff --git a/tests/functional/a/arguments_differ.py b/tests/functional/a/arguments_differ.py
index d6689d920..1dbd40086 100644
--- a/tests/functional/a/arguments_differ.py
+++ b/tests/functional/a/arguments_differ.py
@@ -252,6 +252,7 @@ class ChildClass(ParentClass):
 import typing  # pylint: disable=wrong-import-position

 from typing import Dict  # pylint: disable=wrong-import-position

 

+

 class ParentT1:

     def func(self, user_input: Dict[str, int]) -> None:

         pass

@@ -326,3 +327,34 @@ class Foo2(AbstractFoo):
 

     def kwonly_6(self, first, *args, **kwargs):  # valid override

         "One positional with the rest variadics to pass through parent params"

+

+

+# Adding arguments with default values to a child class is valid

+# See:

+# https://github.com/PyCQA/pylint/issues/1556

+# https://github.com/PyCQA/pylint/issues/5338

+

+

+class BaseClass:

+    def method(self, arg: str):

+        print(self, arg)

+

+

+class DerivedClassWithAnnotation(BaseClass):

+    def method(self, arg: str, param1: int = 42, *, param2: int = 42):

+        print(arg, param1, param2)

+

+

+class DerivedClassWithoutAnnotation(BaseClass):

+    def method(self, arg, param1=42, *, param2=42):

+        print(arg, param1, param2)

+

+

+class AClass:

+    def method(self, *, arg1):

+        print(self)

+

+

+class ClassWithNewNonDefaultKeywordOnly(AClass):

+    def method(self, *, arg2, arg1=None):  # [arguments-differ]

+        ...

diff --git a/tests/functional/a/arguments_differ.txt b/tests/functional/a/arguments_differ.txt
index 07ef79a0f..bb1abb2eb 100644
--- a/tests/functional/a/arguments_differ.txt
+++ b/tests/functional/a/arguments_differ.txt
@@ -5,8 +5,9 @@ arguments-differ:68:4:68:18:VarargsChild.has_kwargs:Variadics removed in overrid
 arguments-renamed:71:4:71:17:VarargsChild.no_kwargs:Parameter 'args' has been renamed to 'arg' in overridden 'VarargsChild.no_kwargs' method:UNDEFINED
 arguments-differ:144:4:144:12:StaticmethodChild2.func:Number of parameters was 1 in 'Staticmethod.func' and is now 2 in overridden 'StaticmethodChild2.func' method:UNDEFINED
 arguments-differ:180:4:180:12:SecondChangesArgs.test:Number of parameters was 2 in 'FirstHasArgs.test' and is now 4 in overridden 'SecondChangesArgs.test' method:UNDEFINED
-arguments-differ:306:4:306:16:Foo.kwonly_1:Number of parameters was 4 in 'AbstractFoo.kwonly_1' and is now 3 in overridden 'Foo.kwonly_1' method:UNDEFINED
-arguments-differ:309:4:309:16:Foo.kwonly_2:Number of parameters was 3 in 'AbstractFoo.kwonly_2' and is now 2 in overridden 'Foo.kwonly_2' method:UNDEFINED
-arguments-differ:312:4:312:16:Foo.kwonly_3:Number of parameters was 3 in 'AbstractFoo.kwonly_3' and is now 3 in overridden 'Foo.kwonly_3' method:UNDEFINED
-arguments-differ:315:4:315:16:Foo.kwonly_4:Number of parameters was 3 in 'AbstractFoo.kwonly_4' and is now 3 in overridden 'Foo.kwonly_4' method:UNDEFINED
-arguments-differ:318:4:318:16:Foo.kwonly_5:Variadics removed in overridden 'Foo.kwonly_5' method:UNDEFINED
+arguments-differ:307:4:307:16:Foo.kwonly_1:Number of parameters was 4 in 'AbstractFoo.kwonly_1' and is now 3 in overridden 'Foo.kwonly_1' method:UNDEFINED
+arguments-differ:310:4:310:16:Foo.kwonly_2:Number of parameters was 3 in 'AbstractFoo.kwonly_2' and is now 2 in overridden 'Foo.kwonly_2' method:UNDEFINED
+arguments-differ:313:4:313:16:Foo.kwonly_3:Number of parameters was 3 in 'AbstractFoo.kwonly_3' and is now 3 in overridden 'Foo.kwonly_3' method:UNDEFINED
+arguments-differ:316:4:316:16:Foo.kwonly_4:Number of parameters was 3 in 'AbstractFoo.kwonly_4' and is now 3 in overridden 'Foo.kwonly_4' method:UNDEFINED
+arguments-differ:319:4:319:16:Foo.kwonly_5:Variadics removed in overridden 'Foo.kwonly_5' method:UNDEFINED
+arguments-differ:359:4:359:14:ClassWithNewNonDefaultKeywordOnly.method:Number of parameters was 2 in 'AClass.method' and is now 3 in overridden 'ClassWithNewNonDefaultKeywordOnly.method' method:UNDEFINED
diff --git a/tests/functional/a/arguments_renamed.py b/tests/functional/a/arguments_renamed.py
index e24b670fd..2b5474c83 100644
--- a/tests/functional/a/arguments_renamed.py
+++ b/tests/functional/a/arguments_renamed.py
@@ -40,7 +40,7 @@ class Child(Parent):
     def test(self, arg1): # [arguments-renamed]
         return arg1 + 1
 
-    def kwargs_test(self, arg, *, value1, var2): #[arguments-renamed]
+    def kwargs_test(self, arg, *, value1, var2): #[arguments-differ]
         print(f"keyword parameters are {value1} and {var2}.")
 
 class Child2(Parent):
@@ -48,7 +48,7 @@ class Child2(Parent):
     def test(self, var): # [arguments-renamed]
         return var + 1
 
-    def kwargs_test(self, *, var1, kw2): #[arguments-renamed, arguments-differ]
+    def kwargs_test(self, *, var1, kw2): #[arguments-differ]
         print(f"keyword parameters are {var1} and {kw2}.")
 
 class ParentDefaults(object):
diff --git a/tests/functional/a/arguments_renamed.txt b/tests/functional/a/arguments_renamed.txt
index 7f6466ba8..47d4188dd 100644
--- a/tests/functional/a/arguments_renamed.txt
+++ b/tests/functional/a/arguments_renamed.txt
@@ -2,10 +2,9 @@ arguments-renamed:17:4:17:12:Orange.brew:Parameter 'fruit_name' has been renamed
 arguments-renamed:20:4:20:26:Orange.eat_with_condiment:Parameter 'fruit_name' has been renamed to 'orange_name' in overridden 'Orange.eat_with_condiment' method:UNDEFINED
 arguments-differ:27:4:27:26:Banana.eat_with_condiment:Number of parameters was 3 in 'Fruit.eat_with_condiment' and is now 4 in overridden 'Banana.eat_with_condiment' method:UNDEFINED
 arguments-renamed:40:4:40:12:Child.test:Parameter 'arg' has been renamed to 'arg1' in overridden 'Child.test' method:UNDEFINED
-arguments-renamed:43:4:43:19:Child.kwargs_test:Parameter 'var1' has been renamed to 'value1' in overridden 'Child.kwargs_test' method:UNDEFINED
+arguments-differ:43:4:43:19:Child.kwargs_test:Number of parameters was 4 in 'Parent.kwargs_test' and is now 4 in overridden 'Child.kwargs_test' method:UNDEFINED
 arguments-renamed:48:4:48:12:Child2.test:Parameter 'arg' has been renamed to 'var' in overridden 'Child2.test' method:UNDEFINED
 arguments-differ:51:4:51:19:Child2.kwargs_test:Number of parameters was 4 in 'Parent.kwargs_test' and is now 3 in overridden 'Child2.kwargs_test' method:UNDEFINED
-arguments-renamed:51:4:51:19:Child2.kwargs_test:Parameter 'var2' has been renamed to 'kw2' in overridden 'Child2.kwargs_test' method:UNDEFINED
 arguments-renamed:67:4:67:13:ChildDefaults.test1:Parameter 'barg' has been renamed to 'param2' in overridden 'ChildDefaults.test1' method:UNDEFINED
 arguments-renamed:95:8:95:16:FruitOverrideConditional.brew:Parameter 'fruit_name' has been renamed to 'orange_name' in overridden 'FruitOverrideConditional.brew' method:UNDEFINED
 arguments-differ:99:12:99:34:FruitOverrideConditional.eat_with_condiment:Number of parameters was 3 in 'FruitConditional.eat_with_condiment' and is now 4 in overridden 'FruitOverrideConditional.eat_with_condiment' method:UNDEFINED