From 1ad19d4744530bf20eab1d2977c63255ec0c3b44 Mon Sep 17 00:00:00 2001 From: Mark Byrne Date: Wed, 22 May 2024 14:28:10 +0200 Subject: [PATCH 1/7] Fix a false negative for ``duplicate-argument-name`` by including ``positional-only``, ``*args`` and ``**kwargs`` arguments in the check. Closes #9669 --- doc/whatsnew/fragments/9669.false_negative | 3 +++ pylint/checkers/base/basic_error_checker.py | 6 +++--- .../d/duplicate/duplicate_argument_name.py | 17 +++++++++++++---- .../d/duplicate/duplicate_argument_name.txt | 14 ++++++++++---- .../d/duplicate/duplicate_argument_name_py3.py | 5 ----- .../d/duplicate/duplicate_argument_name_py3.txt | 1 - 6 files changed, 29 insertions(+), 17 deletions(-) create mode 100644 doc/whatsnew/fragments/9669.false_negative delete mode 100644 tests/functional/d/duplicate/duplicate_argument_name_py3.py delete mode 100644 tests/functional/d/duplicate/duplicate_argument_name_py3.txt diff --git a/doc/whatsnew/fragments/9669.false_negative b/doc/whatsnew/fragments/9669.false_negative new file mode 100644 index 0000000000..5c16ef7bd9 --- /dev/null +++ b/doc/whatsnew/fragments/9669.false_negative @@ -0,0 +1,3 @@ +Fix a false negative for ``duplicate-argument-name`` by including ``positional-only``, ``*args`` and ``**kwargs`` arguments in the check. + +Closes #9669 diff --git a/pylint/checkers/base/basic_error_checker.py b/pylint/checkers/base/basic_error_checker.py index e58be23096..daa71f9557 100644 --- a/pylint/checkers/base/basic_error_checker.py +++ b/pylint/checkers/base/basic_error_checker.py @@ -145,7 +145,7 @@ class BasicErrorChecker(_BasicChecker): "pre-decrement operator -- and ++, which doesn't exist in Python.", ), "E0108": ( - "Duplicate argument name %s in function definition", + "Duplicate argument name %r in function definition", "duplicate-argument-name", "Duplicate argument names in function definitions are syntax errors.", ), @@ -285,8 +285,8 @@ def visit_functiondef(self, node: nodes.FunctionDef) -> None: self.add_message("return-in-init", node=node) # Check for duplicate names by clustering args with same name for detailed report arg_clusters = {} - arguments: Iterator[Any] = filter(None, [node.args.args, node.args.kwonlyargs]) - for arg in itertools.chain.from_iterable(arguments): + arguments: Iterator[Any] = node.args.arguments + for arg in arguments: if arg.name in arg_clusters: self.add_message( "duplicate-argument-name", diff --git a/tests/functional/d/duplicate/duplicate_argument_name.py b/tests/functional/d/duplicate/duplicate_argument_name.py index c0c68b43bb..7dbc113754 100644 --- a/tests/functional/d/duplicate/duplicate_argument_name.py +++ b/tests/functional/d/duplicate/duplicate_argument_name.py @@ -1,14 +1,23 @@ """Check for duplicate function arguments.""" +# pylint: disable=missing-docstring, line-too-long + def foo1(_, _): # [duplicate-argument-name] - """Function with duplicate argument name.""" + ... def foo2(_abc, *, _abc): # [duplicate-argument-name] - """Function with duplicate argument name.""" + ... def foo3(_, _=3): # [duplicate-argument-name] - """Function with duplicate argument name.""" + ... def foo4(_, *, _): # [duplicate-argument-name] - """Function with duplicate argument name.""" + ... + +def foo5(_, *_, _=3): # [duplicate-argument-name, duplicate-argument-name] + ... + +# +1: [duplicate-argument-name, duplicate-argument-name, duplicate-argument-name, duplicate-argument-name] +def foo6(_, /, _, *_, _="_", **_): + ... diff --git a/tests/functional/d/duplicate/duplicate_argument_name.txt b/tests/functional/d/duplicate/duplicate_argument_name.txt index 2925c5ac40..e80f2211e8 100644 --- a/tests/functional/d/duplicate/duplicate_argument_name.txt +++ b/tests/functional/d/duplicate/duplicate_argument_name.txt @@ -1,4 +1,10 @@ -duplicate-argument-name:4:12:4:13:foo1:Duplicate argument name _ in function definition:HIGH -duplicate-argument-name:7:18:7:22:foo2:Duplicate argument name _abc in function definition:HIGH -duplicate-argument-name:10:12:10:13:foo3:Duplicate argument name _ in function definition:HIGH -duplicate-argument-name:13:15:13:16:foo4:Duplicate argument name _ in function definition:HIGH +duplicate-argument-name:6:12:6:13:foo1:Duplicate argument name '_' in function definition:HIGH +duplicate-argument-name:9:18:9:22:foo2:Duplicate argument name '_abc' in function definition:HIGH +duplicate-argument-name:12:12:12:13:foo3:Duplicate argument name '_' in function definition:HIGH +duplicate-argument-name:15:15:15:16:foo4:Duplicate argument name '_' in function definition:HIGH +duplicate-argument-name:18:13:18:14:foo5:Duplicate argument name '_' in function definition:HIGH +duplicate-argument-name:18:16:18:17:foo5:Duplicate argument name '_' in function definition:HIGH +duplicate-argument-name:22:15:22:16:foo6:Duplicate argument name '_' in function definition:HIGH +duplicate-argument-name:22:19:22:20:foo6:Duplicate argument name '_' in function definition:HIGH +duplicate-argument-name:22:22:22:23:foo6:Duplicate argument name '_' in function definition:HIGH +duplicate-argument-name:22:31:22:32:foo6:Duplicate argument name '_' in function definition:HIGH diff --git a/tests/functional/d/duplicate/duplicate_argument_name_py3.py b/tests/functional/d/duplicate/duplicate_argument_name_py3.py deleted file mode 100644 index 4751c6f2d3..0000000000 --- a/tests/functional/d/duplicate/duplicate_argument_name_py3.py +++ /dev/null @@ -1,5 +0,0 @@ -"""Check for duplicate function keywordonly arguments.""" - - -def foo1(_, *_, _=3): # [duplicate-argument-name] - """Function with duplicate argument name.""" diff --git a/tests/functional/d/duplicate/duplicate_argument_name_py3.txt b/tests/functional/d/duplicate/duplicate_argument_name_py3.txt deleted file mode 100644 index 3d6f6f8d9d..0000000000 --- a/tests/functional/d/duplicate/duplicate_argument_name_py3.txt +++ /dev/null @@ -1 +0,0 @@ -duplicate-argument-name:4:16:4:17:foo1:Duplicate argument name _ in function definition:HIGH From 156212b78bcc553e33c70b4cb5d0ebeb4f3e88c4 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 22 May 2024 13:14:51 +0000 Subject: [PATCH 2/7] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- doc/whatsnew/fragments/9669.false_negative | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/whatsnew/fragments/9669.false_negative b/doc/whatsnew/fragments/9669.false_negative index 5c16ef7bd9..483de2a77d 100644 --- a/doc/whatsnew/fragments/9669.false_negative +++ b/doc/whatsnew/fragments/9669.false_negative @@ -1,3 +1,3 @@ -Fix a false negative for ``duplicate-argument-name`` by including ``positional-only``, ``*args`` and ``**kwargs`` arguments in the check. +Fix a false negative for ``duplicate-argument-name`` by including ``positional-only``, ``*args`` and ``**kwargs`` arguments in the check. Closes #9669 From a4d10365c3cd1d47e5c544add7cba5f680ade4c8 Mon Sep 17 00:00:00 2001 From: Mark Byrne Date: Wed, 22 May 2024 15:24:43 +0200 Subject: [PATCH 3/7] Update functional test output by adding quotes to the duplicate argument name. --- tests/functional/a/async_functions.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/a/async_functions.txt b/tests/functional/a/async_functions.txt index 985fddec2d..bfb9e52021 100644 --- a/tests/functional/a/async_functions.txt +++ b/tests/functional/a/async_functions.txt @@ -5,6 +5,6 @@ too-many-arguments:26:0:26:26:complex_function:Too many arguments (10/5):UNDEFIN too-many-branches:26:0:26:26:complex_function:Too many branches (13/12):UNDEFINED too-many-return-statements:26:0:26:26:complex_function:Too many return statements (10/6):UNDEFINED dangerous-default-value:59:0:59:14:func:Dangerous default value [] as argument:UNDEFINED -duplicate-argument-name:59:18:59:19:func:Duplicate argument name a in function definition:HIGH +duplicate-argument-name:59:18:59:19:func:Duplicate argument name 'a' in function definition:HIGH disallowed-name:64:0:64:13:foo:"Disallowed name ""foo""":HIGH empty-docstring:64:0:64:13:foo:Empty function docstring:HIGH From 9fd5946a05909543b7d601893e719b973514533b Mon Sep 17 00:00:00 2001 From: Mark Byrne Date: Wed, 22 May 2024 15:52:50 +0200 Subject: [PATCH 4/7] Regenerate the docs. --- doc/user_guide/checkers/features.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/user_guide/checkers/features.rst b/doc/user_guide/checkers/features.rst index cb63930a01..786495754f 100644 --- a/doc/user_guide/checkers/features.rst +++ b/doc/user_guide/checkers/features.rst @@ -65,7 +65,7 @@ Basic checker Messages methods and is instantiated. :star-needs-assignment-target (E0114): *Can use starred expression only in assignment target* Emitted when a star expression is not used in an assignment target. -:duplicate-argument-name (E0108): *Duplicate argument name %s in function definition* +:duplicate-argument-name (E0108): *Duplicate argument name %r in function definition* Duplicate argument names in function definitions are syntax errors. :return-in-init (E0101): *Explicit return in __init__* Used when the special class method __init__ has an explicit return value. From a2700ff8343490756eeb9c44e8cd3b1b8625c18b Mon Sep 17 00:00:00 2001 From: Mark Byrne <31762852+mbyrnepr2@users.noreply.github.com> Date: Wed, 22 May 2024 20:46:38 +0200 Subject: [PATCH 5/7] Update pylint/checkers/base/basic_error_checker.py Co-authored-by: Pierre Sassoulas --- pylint/checkers/base/basic_error_checker.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pylint/checkers/base/basic_error_checker.py b/pylint/checkers/base/basic_error_checker.py index daa71f9557..28c39a9255 100644 --- a/pylint/checkers/base/basic_error_checker.py +++ b/pylint/checkers/base/basic_error_checker.py @@ -285,8 +285,7 @@ def visit_functiondef(self, node: nodes.FunctionDef) -> None: self.add_message("return-in-init", node=node) # Check for duplicate names by clustering args with same name for detailed report arg_clusters = {} - arguments: Iterator[Any] = node.args.arguments - for arg in arguments: + for arg in node.args.arguments: if arg.name in arg_clusters: self.add_message( "duplicate-argument-name", From cbd20f19f7cf6bd92bd435e7ed927a38e7386eaf Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 22 May 2024 18:47:25 +0000 Subject: [PATCH 6/7] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- pylint/checkers/base/basic_error_checker.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/pylint/checkers/base/basic_error_checker.py b/pylint/checkers/base/basic_error_checker.py index 28c39a9255..d6e10f31d0 100644 --- a/pylint/checkers/base/basic_error_checker.py +++ b/pylint/checkers/base/basic_error_checker.py @@ -7,8 +7,6 @@ from __future__ import annotations import itertools -from collections.abc import Iterator -from typing import Any import astroid from astroid import nodes From aab51bb3ea2277e94ac0bf7b2200dddac49cbea0 Mon Sep 17 00:00:00 2001 From: Mark Byrne Date: Wed, 22 May 2024 20:56:51 +0200 Subject: [PATCH 7/7] Refactor the tests for readability. --- .../functional/d/duplicate/duplicate_argument_name.py | 11 ++++++++--- .../d/duplicate/duplicate_argument_name.txt | 7 +++---- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/tests/functional/d/duplicate/duplicate_argument_name.py b/tests/functional/d/duplicate/duplicate_argument_name.py index 7dbc113754..a6654e6bb0 100644 --- a/tests/functional/d/duplicate/duplicate_argument_name.py +++ b/tests/functional/d/duplicate/duplicate_argument_name.py @@ -1,6 +1,6 @@ """Check for duplicate function arguments.""" -# pylint: disable=missing-docstring, line-too-long +# pylint: disable=missing-docstring, line-too-long, unused-argument def foo1(_, _): # [duplicate-argument-name] @@ -18,6 +18,11 @@ def foo4(_, *, _): # [duplicate-argument-name] def foo5(_, *_, _=3): # [duplicate-argument-name, duplicate-argument-name] ... -# +1: [duplicate-argument-name, duplicate-argument-name, duplicate-argument-name, duplicate-argument-name] -def foo6(_, /, _, *_, _="_", **_): +def foo6(a, *a): # [duplicate-argument-name] + ... + +def foo7(a, /, a): # [duplicate-argument-name] + ... + +def foo8(a, **a): # [duplicate-argument-name] ... diff --git a/tests/functional/d/duplicate/duplicate_argument_name.txt b/tests/functional/d/duplicate/duplicate_argument_name.txt index e80f2211e8..c565e88f40 100644 --- a/tests/functional/d/duplicate/duplicate_argument_name.txt +++ b/tests/functional/d/duplicate/duplicate_argument_name.txt @@ -4,7 +4,6 @@ duplicate-argument-name:12:12:12:13:foo3:Duplicate argument name '_' in function duplicate-argument-name:15:15:15:16:foo4:Duplicate argument name '_' in function definition:HIGH duplicate-argument-name:18:13:18:14:foo5:Duplicate argument name '_' in function definition:HIGH duplicate-argument-name:18:16:18:17:foo5:Duplicate argument name '_' in function definition:HIGH -duplicate-argument-name:22:15:22:16:foo6:Duplicate argument name '_' in function definition:HIGH -duplicate-argument-name:22:19:22:20:foo6:Duplicate argument name '_' in function definition:HIGH -duplicate-argument-name:22:22:22:23:foo6:Duplicate argument name '_' in function definition:HIGH -duplicate-argument-name:22:31:22:32:foo6:Duplicate argument name '_' in function definition:HIGH +duplicate-argument-name:21:13:21:14:foo6:Duplicate argument name 'a' in function definition:HIGH +duplicate-argument-name:24:15:24:16:foo7:Duplicate argument name 'a' in function definition:HIGH +duplicate-argument-name:27:14:27:15:foo8:Duplicate argument name 'a' in function definition:HIGH