Create new error arguments-renamed (#4467)

* Create new error arguments-renamed

This commits creates the new error arguments-renamed based on the functionality
added on #4422 and the changes of #4456.

* Merge test files for arguments-differ

This commit merges the two kinds of test files that existed for the arguments-differ error, since now all
tests run in Python3.

* Add tests for arguments-renamed error
* Add new error arguments-renamed

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Pierre Sassoulas <[email protected]>

Author: 3 people

ChangeLog

@@ -35,6 +35,11 @@ modules are added.
 
 * Fix raising false-positive ``no-member`` on abstract properties
 
+* Created new error message called ``arguments-renamed`` which identifies any changes at the parameter
+  names of overridden functions.
+
+  Closes #3536
+
 * New checker ``consider-using-dict-items``. Emitted  when iterating over dictionary keys and then
   indexing the same dictionary with the key within loop body.
 

doc/whatsnew/2.9.rst

@@ -27,4 +27,7 @@ Other Changes
 
 * New option ``--fail-on=<msg ids>`` to return non-zero exit codes regardless of ``fail-under`` value.
 
+* A new error called ``arguments-renamed`` has been created, which identifies any changes at the parameter names
+  of overridden functions. It aims to separate the functionality of ``arguments-differ``.
+
 * Fix incompatibility with Python 3.6.0 caused by ``typing.Counter`` and ``typing.NoReturn`` usage

pylint/checkers/classes.py

@@ -342,6 +342,9 @@ def _different_parameters(
             output_messages.append("Number of parameters ")
             output_messages += different_positional[1:]
             output_messages += different_kwonly[1:]
+        else:
+            output_messages += different_positional
+            output_messages += different_kwonly
     else:
         if different_positional:
             output_messages += different_positional
@@ -627,6 +630,12 @@ def _has_same_layout_slots(slots, assigned_value):
         "that does not match its base class "
         "which could result in potential bugs at runtime.",
     ),
+    "W0237": (
+        "%s %s %r method",
+        "arguments-renamed",
+        "Used when a method parameter has a different name than in "
+        "the implemented interface or in an overridden method.",
+    ),
     "E0236": (
         "Invalid object %r in __slots__, must contain only non empty strings",
         "invalid-slots-object",
@@ -1810,27 +1819,29 @@ def _check_signature(self, method1, refmethod, class_type, cls):
                         total_args_refmethod += 1
                     if refmethod.args.kwonlyargs:
                         total_args_refmethod += len(refmethod.args.kwonlyargs)
-                    self.add_message(
-                        "arguments-differ",
-                        args=(
-                            msg
-                            + f"was {total_args_refmethod} in '{refmethod.parent.name}.{refmethod.name}' and "
-                            f"is now {total_args_method1} in",
-                            class_type,
-                            str(method1.parent.name) + "." + str(method1.name),
-                        ),
-                        node=method1,
+                    error_type = "arguments-differ"
+                    msg_args = (
+                        msg
+                        + f"was {total_args_refmethod} in '{refmethod.parent.name}.{refmethod.name}' and "
+                        f"is now {total_args_method1} in",
+                        class_type,
+                        f"{method1.parent.name}.{method1.name}",
+                    )
+                elif "renamed" in msg:
+                    error_type = "arguments-renamed"
+                    msg_args = (
+                        msg,
+                        class_type,
+                        f"{method1.parent.name}.{method1.name}",
                     )
                 else:
-                    self.add_message(
-                        "arguments-differ",
-                        args=(
-                            msg,
-                            class_type,
-                            str(method1.parent.name) + "." + str(method1.name),
-                        ),
-                        node=method1,
+                    error_type = "arguments-differ"
+                    msg_args = (
+                        msg,
+                        class_type,
+                        f"{method1.parent.name}.{method1.name}",
                     )
+                self.add_message(error_type, args=msg_args, node=method1)
         elif (
             len(method1.args.defaults) < len(refmethod.args.defaults)
             and not method1.args.vararg

tests/functional/a/arguments_differ.py

@@ -68,7 +68,7 @@ class VarargsChild(Varargs):
     def has_kwargs(self, arg):  # [arguments-differ]
         "Not okay to lose capabilities. Also, type has changed."
 
-    def no_kwargs(self, arg, **kwargs):  # [arguments-differ]
+    def no_kwargs(self, arg, **kwargs):  # [arguments-renamed]
         "Addition of kwargs does not violate LSP, but first argument's name has changed."
 
 
@@ -270,3 +270,51 @@ def func(self, user_input: FooT1) -> None:
 class ChildT3(ParentT3):
     def func(self, user_input: FooT1) -> None:
         pass
+
+# Keyword and positional overriddes
+class AbstractFoo:
+
+    def kwonly_1(self, first, *, second, third):
+        "Normal positional with two positional only params."
+
+    def kwonly_2(self, *, first, second):
+        "Two positional only parameter."
+
+    def kwonly_3(self, *, first, second):
+        "Two positional only params."
+
+    def kwonly_4(self, *, first, second=None):
+        "One positional only and another with a default."
+
+    def kwonly_5(self, *, first, **kwargs):
+        "Keyword only and keyword variadics."
+
+    def kwonly_6(self, first, second, *, third):
+        "Two positional and one keyword"
+
+
+class Foo(AbstractFoo):
+
+    def kwonly_1(self, first, *, second):  # [arguments-differ]
+        "One positional and only one positional only param."
+
+    def kwonly_2(self, *, first):  # [arguments-differ]
+        "Only one positional parameter instead of two positional only parameters."
+
+    def kwonly_3(self, first, second):  # [arguments-differ]
+        "Two positional params."
+
+    def kwonly_4(self, first, second):  # [arguments-differ]
+        "Two positional params."
+
+    def kwonly_5(self, *, first):  # [arguments-differ]
+        "Keyword only, but no variadics."
+
+    def kwonly_6(self, *args, **kwargs):  # valid override
+        "Positional and keyword variadics to pass through parent params"
+
+
+class Foo2(AbstractFoo):
+
+    def kwonly_6(self, first, *args, **kwargs):  # valid override
+        "One positional with the rest variadics to pass through parent params"

tests/functional/a/arguments_differ.txt

@@ -2,5 +2,10 @@ arguments-differ:12:4:Child.test:Number of parameters was 1 in 'Parent.test' and
 arguments-differ:23:4:ChildDefaults.test:Number of parameters was 3 in 'ParentDefaults.test' and is now 2 in overridden 'ChildDefaults.test' method
 arguments-differ:41:4:ClassmethodChild.func:Number of parameters was 2 in 'Classmethod.func' and is now 0 in overridden 'ClassmethodChild.func' method
 arguments-differ:68:4:VarargsChild.has_kwargs:Variadics removed in overridden 'VarargsChild.has_kwargs' method
-arguments-differ:71:4:VarargsChild.no_kwargs:Parameter 'args' has been renamed to 'arg' in overridden 'VarargsChild.no_kwargs' method
+arguments-renamed:71:4:VarargsChild.no_kwargs:Parameter 'args' has been renamed to 'arg' in overridden 'VarargsChild.no_kwargs' method
 arguments-differ:172:4:SecondChangesArgs.test:Number of parameters was 2 in 'FirstHasArgs.test' and is now 4 in overridden 'SecondChangesArgs.test' method
+arguments-differ:298:4:Foo.kwonly_1:Number of parameters was 4 in 'AbstractFoo.kwonly_1' and is now 3 in overridden 'Foo.kwonly_1' method
+arguments-differ:301:4:Foo.kwonly_2:Number of parameters was 3 in 'AbstractFoo.kwonly_2' and is now 2 in overridden 'Foo.kwonly_2' method
+arguments-differ:304:4:Foo.kwonly_3:Number of parameters was 3 in 'AbstractFoo.kwonly_3' and is now 3 in overridden 'Foo.kwonly_3' method
+arguments-differ:307:4:Foo.kwonly_4:Number of parameters was 3 in 'AbstractFoo.kwonly_4' and is now 3 in overridden 'Foo.kwonly_4' method
+arguments-differ:310:4:Foo.kwonly_5:Variadics removed in overridden 'Foo.kwonly_5' method

tests/functional/a/arguments_differ_py3.py

@@ -0,0 +1,74 @@
+#pylint: disable = unused-argument, missing-docstring, no-self-use, line-too-long, useless-object-inheritance, too-few-public-methods
+import enum
+
+
+class Condiment(enum.Enum):
+    CINAMON = 1
+    SUGAR = 2
+
+class Fruit:
+    def brew(self, fruit_name: str):
+        print(f"Brewing a fruit named {fruit_name}")
+
+    def eat_with_condiment(self, fruit_name:str, condiment: Condiment):
+        print(f"Eating a fruit named {fruit_name} with {condiment}")
+
+class Orange(Fruit):
+    def brew(self, orange_name: str): # [arguments-renamed]
+        print(f"Brewing an orange named {orange_name}")
+
+    def eat_with_condiment(self, orange_name: str, condiment: Condiment()): #[arguments-renamed]
+        print(f"Eating a fruit named {orange_name} with {condiment}")
+
+class Banana(Fruit):
+    def brew(self, fruit_name: bool): # No warning here
+        print(f"Brewing a banana named {fruit_name}")
+
+    def eat_with_condiment(self, fruit_name: str, condiment: Condiment, error: str): # [arguments-differ]
+        print(f"Eating a fruit named {fruit_name} with {condiment}")
+
+class Parent(object):
+
+    def test(self, arg):
+        return arg + 1
+
+    def kwargs_test(self, arg, *, var1, var2):
+        print(f"keyword parameters are {var1} and {var2}.")
+
+class Child(Parent):
+
+    def test(self, arg1): # [arguments-renamed]
+        return arg1 + 1
+
+    def kwargs_test(self, arg, *, value1, var2): #[arguments-renamed]
+        print(f"keyword parameters are {value1} and {var2}.")
+
+class Child2(Parent):
+
+    def test(self, var): # [arguments-renamed]
+        return var + 1
+
+    def kwargs_test(self, *, var1, kw2): #[arguments-renamed, arguments-differ]
+        print(f"keyword parameters are {var1} and {kw2}.")
+
+class ParentDefaults(object):
+
+    def test1(self, arg, barg):
+        print(f"Argument values are {arg} and {barg}")
+
+    def test2(self, arg, barg):
+        print(f"Argument values are {arg} and {barg}!")
+
+    def test3(self, arg1, arg2):
+        print(f"arguments: {arg1} {arg2}")
+
+class ChildDefaults(ParentDefaults):
+
+    def test1(self, arg, param2): # [arguments-renamed]
+        print(f"Argument values are {arg} and {param2}")
+
+    def test2(self, _arg, ignored_barg): # no error here
+        print(f"Argument value is {_arg}")
+
+    def test3(self, dummy_param, arg2): # no error here
+        print(f"arguments: {arg2}")

tests/functional/a/arguments_differ_py3.rc

@@ -0,0 +1,9 @@
+arguments-renamed:17:4:Orange.brew:Parameter 'fruit_name' has been renamed to 'orange_name' in overridden 'Orange.brew' method
+arguments-renamed:20:4:Orange.eat_with_condiment:Parameter 'fruit_name' has been renamed to 'orange_name' in overridden 'Orange.eat_with_condiment' method
+arguments-differ:27:4: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
+arguments-renamed:40:4:Child.test:Parameter 'arg' has been renamed to 'arg1' in overridden 'Child.test' method
+arguments-renamed:43:4:Child.kwargs_test:Parameter 'var1' has been renamed to 'value1' in overridden 'Child.kwargs_test' method
+arguments-renamed:48:4:Child2.test:Parameter 'arg' has been renamed to 'var' in overridden 'Child2.test' method
+arguments-differ:51:4:Child2.kwargs_test:Number of parameters was 4 in 'Parent.kwargs_test' and is now 3 in overridden 'Child2.kwargs_test' method
+arguments-renamed:51:4:Child2.kwargs_test:Parameter 'var2' has been renamed to 'kw2' in overridden 'Child2.kwargs_test' method
+arguments-renamed:67:4:ChildDefaults.test1:Parameter 'barg' has been renamed to 'param2' in overridden 'ChildDefaults.test1' method