diff --git a/ChangeLog b/ChangeLog index c496e0c0b6..a78f63f0fe 100644 --- a/ChangeLog +++ b/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. diff --git a/doc/whatsnew/2.9.rst b/doc/whatsnew/2.9.rst index 0a353d68ef..fe78c6fd2d 100644 --- a/doc/whatsnew/2.9.rst +++ b/doc/whatsnew/2.9.rst @@ -27,4 +27,7 @@ Other Changes * New option ``--fail-on=`` 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 diff --git a/pylint/checkers/classes.py b/pylint/checkers/classes.py index 4c98dbe42b..3ffdfcdd78 100644 --- a/pylint/checkers/classes.py +++ b/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 diff --git a/tests/functional/a/arguments_differ.py b/tests/functional/a/arguments_differ.py index 3b5915c84d..41ca9769f7 100644 --- a/tests/functional/a/arguments_differ.py +++ b/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" diff --git a/tests/functional/a/arguments_differ.txt b/tests/functional/a/arguments_differ.txt index 2a662d1345..12b7305038 100644 --- a/tests/functional/a/arguments_differ.txt +++ b/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 diff --git a/tests/functional/a/arguments_differ_py3.py b/tests/functional/a/arguments_differ_py3.py deleted file mode 100644 index bd3956939f..0000000000 --- a/tests/functional/a/arguments_differ_py3.py +++ /dev/null @@ -1,47 +0,0 @@ -# pylint: disable=missing-docstring,too-few-public-methods -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" diff --git a/tests/functional/a/arguments_differ_py3.rc b/tests/functional/a/arguments_differ_py3.rc deleted file mode 100644 index c093be204f..0000000000 --- a/tests/functional/a/arguments_differ_py3.rc +++ /dev/null @@ -1,2 +0,0 @@ -[testoptions] -min_pyver=3.0 diff --git a/tests/functional/a/arguments_differ_py3.txt b/tests/functional/a/arguments_differ_py3.txt deleted file mode 100644 index df06d4db5c..0000000000 --- a/tests/functional/a/arguments_differ_py3.txt +++ /dev/null @@ -1,5 +0,0 @@ -arguments-differ:25: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:28: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:31: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:34: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:37:4:Foo.kwonly_5:Variadics removed in overridden 'Foo.kwonly_5' method diff --git a/tests/functional/a/arguments_renamed.py b/tests/functional/a/arguments_renamed.py new file mode 100644 index 0000000000..65b5115ca8 --- /dev/null +++ b/tests/functional/a/arguments_renamed.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}") diff --git a/tests/functional/a/arguments_renamed.txt b/tests/functional/a/arguments_renamed.txt new file mode 100644 index 0000000000..0f3b60c1fe --- /dev/null +++ b/tests/functional/a/arguments_renamed.txt @@ -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