From c5c53240148e48019d02749f4aad12b4dfe5acc6 Mon Sep 17 00:00:00 2001 From: ksaketou Date: Tue, 11 May 2021 14:21:10 +0300 Subject: [PATCH 1/7] 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. --- pylint/checkers/classes.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/pylint/checkers/classes.py b/pylint/checkers/classes.py index 4c98dbe42b..7373071f01 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", @@ -1821,6 +1830,16 @@ def _check_signature(self, method1, refmethod, class_type, cls): ), node=method1, ) + elif "renamed" in msg: + self.add_message( + "arguments-renamed", + args=( + msg, + class_type, + str(method1.parent.name) + "." + str(method1.name), + ), + node=method1, + ) else: self.add_message( "arguments-differ", From 48eee9537319d5a42dd9b73f9755e470b969999b Mon Sep 17 00:00:00 2001 From: ksaketou Date: Tue, 11 May 2021 14:26:57 +0300 Subject: [PATCH 2/7] 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. --- tests/functional/a/arguments_differ.py | 50 ++++++++++++++++++++- tests/functional/a/arguments_differ.txt | 7 ++- tests/functional/a/arguments_differ_py3.py | 47 ------------------- tests/functional/a/arguments_differ_py3.rc | 2 - tests/functional/a/arguments_differ_py3.txt | 5 --- 5 files changed, 55 insertions(+), 56 deletions(-) delete mode 100644 tests/functional/a/arguments_differ_py3.py delete mode 100644 tests/functional/a/arguments_differ_py3.rc delete mode 100644 tests/functional/a/arguments_differ_py3.txt 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 From c21d952e33c88a4a012c780acc7b6b3d493d323a Mon Sep 17 00:00:00 2001 From: ksaketou Date: Tue, 11 May 2021 14:27:53 +0300 Subject: [PATCH 3/7] Add tests for arguments-renamed error --- tests/functional/a/arguments_renamed.py | 74 ++++++++++++++++++++++++ tests/functional/a/arguments_renamed.txt | 9 +++ 2 files changed, 83 insertions(+) create mode 100644 tests/functional/a/arguments_renamed.py create mode 100644 tests/functional/a/arguments_renamed.txt 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 From 70af6f7bb15a6437cdf00c12fea3d00e0e859ebe Mon Sep 17 00:00:00 2001 From: ksaketou Date: Tue, 11 May 2021 14:49:52 +0300 Subject: [PATCH 4/7] Add new error arguments-renamed --- ChangeLog | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ChangeLog b/ChangeLog index de7a504220..3841c14995 100644 --- a/ChangeLog +++ b/ChangeLog @@ -33,6 +33,10 @@ 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 What's New in Pylint 2.8.2? =========================== From 202d57ce63051916a99a5337dca2e67c2592207f Mon Sep 17 00:00:00 2001 From: ksaketou Date: Tue, 11 May 2021 14:53:17 +0300 Subject: [PATCH 5/7] Add new error arguments-renamed --- doc/whatsnew/2.9.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/whatsnew/2.9.rst b/doc/whatsnew/2.9.rst index a0053e98f0..16f58b7a86 100644 --- a/doc/whatsnew/2.9.rst +++ b/doc/whatsnew/2.9.rst @@ -23,3 +23,6 @@ Other Changes * The output messages for ``arguments-differ`` error message have been customized based on the different error cases. * 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``. From 69bae3f173584dfe32be015736879cfc7312250b Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 11 May 2021 12:12:15 +0000 Subject: [PATCH 6/7] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- ChangeLog | 1 + 1 file changed, 1 insertion(+) diff --git a/ChangeLog b/ChangeLog index 3841c14995..3811cec787 100644 --- a/ChangeLog +++ b/ChangeLog @@ -38,6 +38,7 @@ modules are added. Closes #3536 + What's New in Pylint 2.8.2? =========================== Release date: 2021-04-26 From ca1fb8fcaab01329b806d313d266263ce4224968 Mon Sep 17 00:00:00 2001 From: ksaketou Date: Mon, 17 May 2021 15:43:24 +0300 Subject: [PATCH 7/7] Refactor function call and improve code formatting --- pylint/checkers/classes.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/pylint/checkers/classes.py b/pylint/checkers/classes.py index 2c93712821..3ffdfcdd78 100644 --- a/pylint/checkers/classes.py +++ b/pylint/checkers/classes.py @@ -1820,30 +1820,28 @@ def _check_signature(self, method1, refmethod, class_type, cls): if refmethod.args.kwonlyargs: total_args_refmethod += len(refmethod.args.kwonlyargs) error_type = "arguments-differ" - msg_args=( + 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}" + f"{method1.parent.name}.{method1.name}", ) elif "renamed" in msg: error_type = "arguments-renamed" - msg_args=( + msg_args = ( msg, class_type, - f"{method1.parent.name}.{method1.name}" + f"{method1.parent.name}.{method1.name}", ) else: error_type = "arguments-differ" - msg_args=( + msg_args = ( msg, class_type, - f"{method1.parent.name}.{method1.name}" + f"{method1.parent.name}.{method1.name}", ) - self.add_message( - error_type, args=msg_args, node=method1 - ) + self.add_message(error_type, args=msg_args, node=method1) elif ( len(method1.args.defaults) < len(refmethod.args.defaults) and not method1.args.vararg