From d392ea5d8a11049a275207cf18e488bf19a57c69 Mon Sep 17 00:00:00 2001 From: Mark Byrne <31762852+mbyrnepr2@users.noreply.github.com> Date: Tue, 16 May 2023 10:34:47 +0200 Subject: Add new checker `kwarg-superseded-by-positional-arg` and fix a false positive (#8644) * Fix a false positive for ``redundant-keyword-arg`` when a function, with a keyword-only-parameter and ``**kwargs``, is called with a positional argument and a keyword argument where the keyword argument has the same name as the positional-only-parameter. * Add new checker ``kwarg-superseded-by-positional-arg`` which emits a warning message for the above scenario. Closes #8558 Co-authored-by: Pierre Sassoulas --- .../k/kwarg-superseded-by-positional-arg/bad.py | 5 +++ .../k/kwarg-superseded-by-positional-arg/good.py | 5 +++ doc/whatsnew/fragments/8558.feature | 3 ++ pylint/checkers/typecheck.py | 24 +++++++++--- tests/functional/a/arguments.py | 44 +++++++++++++++++++++- tests/functional/a/arguments.txt | 2 + tests/functional/a/arguments_positional_only.py | 15 -------- tests/functional/a/arguments_positional_only.rc | 2 - tests/functional/a/arguments_positional_only.txt | 1 - .../k/kwarg_superseded_by_positional_arg.py | 39 +++++++++++++++++++ .../k/kwarg_superseded_by_positional_arg.txt | 2 + .../p/positional_only_arguments_expected.py | 2 +- 12 files changed, 119 insertions(+), 25 deletions(-) create mode 100644 doc/data/messages/k/kwarg-superseded-by-positional-arg/bad.py create mode 100644 doc/data/messages/k/kwarg-superseded-by-positional-arg/good.py create mode 100644 doc/whatsnew/fragments/8558.feature delete mode 100644 tests/functional/a/arguments_positional_only.py delete mode 100644 tests/functional/a/arguments_positional_only.rc delete mode 100644 tests/functional/a/arguments_positional_only.txt create mode 100644 tests/functional/k/kwarg_superseded_by_positional_arg.py create mode 100644 tests/functional/k/kwarg_superseded_by_positional_arg.txt diff --git a/doc/data/messages/k/kwarg-superseded-by-positional-arg/bad.py b/doc/data/messages/k/kwarg-superseded-by-positional-arg/bad.py new file mode 100644 index 000000000..87f7d8ccd --- /dev/null +++ b/doc/data/messages/k/kwarg-superseded-by-positional-arg/bad.py @@ -0,0 +1,5 @@ +def print_name(name="Sarah", /, **kwds): + print(name) + + +print_name(name="Jacob") # [kwarg-superseded-by-positional-arg] diff --git a/doc/data/messages/k/kwarg-superseded-by-positional-arg/good.py b/doc/data/messages/k/kwarg-superseded-by-positional-arg/good.py new file mode 100644 index 000000000..86dbfba07 --- /dev/null +++ b/doc/data/messages/k/kwarg-superseded-by-positional-arg/good.py @@ -0,0 +1,5 @@ +def print_name(name="Sarah", /, **kwds): + print(name) + + +print_name("Jacob") diff --git a/doc/whatsnew/fragments/8558.feature b/doc/whatsnew/fragments/8558.feature new file mode 100644 index 000000000..b5a8342f3 --- /dev/null +++ b/doc/whatsnew/fragments/8558.feature @@ -0,0 +1,3 @@ +Add a new checker ``kwarg-superseded-by-positional-arg`` to warn when a function is called with a keyword argument which shares a name with a positional-only parameter and the function contains a keyword variadic parameter dictionary. It may be surprising behaviour when the keyword argument is added to the keyword variadic parameter dictionary. + +Closes #8558 diff --git a/pylint/checkers/typecheck.py b/pylint/checkers/typecheck.py index 97a7460a8..767f4ef32 100644 --- a/pylint/checkers/typecheck.py +++ b/pylint/checkers/typecheck.py @@ -399,6 +399,13 @@ MSGS: dict[str, MessageDefinitionTuple] = { "isinstance-second-argument-not-valid-type", "Emitted when the second argument of an isinstance call is not a type.", ), + "W1117": ( + "%r will be included in %r since a positional-only parameter with this name already exists", + "kwarg-superseded-by-positional-arg", + "Emitted when a function is called with a keyword argument that has the " + "same name as a positional-only parameter and the function contains a " + "keyword variadic parameter dict.", + ), } # builtin sequence types in Python 2 and 3. @@ -1548,6 +1555,18 @@ accessed. Python regular expressions are accepted.", # 2. Match the keyword arguments. for keyword in keyword_args: + # Skip if `keyword` is the same name as a positional-only parameter + # and a `**kwargs` parameter exists. + if called.args.kwarg and keyword in [ + arg.name for arg in called.args.posonlyargs + ]: + self.add_message( + "kwarg-superseded-by-positional-arg", + node=node, + args=(keyword, f"**{called.args.kwarg}"), + confidence=HIGH, + ) + continue if keyword in parameter_name_to_index: i = parameter_name_to_index[keyword] if parameters[i][1]: @@ -1564,11 +1583,6 @@ accessed. Python regular expressions are accepted.", node=node, args=(keyword, callable_name), ) - elif ( - keyword in [arg.name for arg in called.args.posonlyargs] - and called.args.kwarg - ): - pass else: parameters[i] = (parameters[i][0], True) elif keyword in kwparams: diff --git a/tests/functional/a/arguments.py b/tests/functional/a/arguments.py index 29fdbd5ba..81a9bb73d 100644 --- a/tests/functional/a/arguments.py +++ b/tests/functional/a/arguments.py @@ -1,6 +1,6 @@ # pylint: disable=too-few-public-methods, missing-docstring,import-error,wrong-import-position # pylint: disable=wrong-import-order, unnecessary-lambda, consider-using-f-string -# pylint: disable=unnecessary-lambda-assignment, no-self-argument, unused-argument +# pylint: disable=unnecessary-lambda-assignment, no-self-argument, unused-argument, kwarg-superseded-by-positional-arg def decorator(fun): """Decorator""" @@ -278,3 +278,45 @@ class FruitPicker: picker = FruitPicker() picker.pick_apple() picker.pick_pear() + + +def name1(apple, /, **kwargs): + """ + Positional-only parameter with `**kwargs`. + Calling this function with the `apple` keyword should not emit + `redundant-keyword-arg` since it is added to `**kwargs`. + + >>> name1("Red apple", apple="Green apple") + "Red apple" + {"apple": "Green apple"} + """ + print(apple) + print(kwargs) + + +name1("Red apple", apple="Green apple") + + +def name2(apple, /, banana, **kwargs): + """ + Positional-only parameter with positional-or-keyword parameter and `**kwargs`. + """ + + +# `banana` is redundant +# +1:[redundant-keyword-arg] +name2("Red apple", "Yellow banana", apple="Green apple", banana="Green banana") + + +# Test `no-value-for-parameter` in the context of positional-only parameters + +def name3(param1, /, **kwargs): ... +def name4(param1, /, param2, **kwargs): ... +def name5(param1=True, /, **kwargs): ... +def name6(param1, **kwargs): ... + +name3(param1=43) # [no-value-for-parameter] +name3(43) +name4(1, param2=False) +name5() +name6(param1=43) diff --git a/tests/functional/a/arguments.txt b/tests/functional/a/arguments.txt index 41f7831f6..70a99e1a2 100644 --- a/tests/functional/a/arguments.txt +++ b/tests/functional/a/arguments.txt @@ -37,3 +37,5 @@ unexpected-keyword-arg:203:23:203:56:namedtuple_replace_issue_1036:Unexpected ke no-value-for-parameter:216:0:216:24::No value for argument 'third' in function call:UNDEFINED no-value-for-parameter:217:0:217:30::No value for argument 'second' in function call:UNDEFINED unexpected-keyword-arg:218:0:218:43::Unexpected keyword argument 'fourth' in function call:UNDEFINED +redundant-keyword-arg:308:0:308:79::Argument 'banana' passed by position and keyword in function call:UNDEFINED +no-value-for-parameter:318:0:318:16::No value for argument 'param1' in function call:UNDEFINED diff --git a/tests/functional/a/arguments_positional_only.py b/tests/functional/a/arguments_positional_only.py deleted file mode 100644 index eb03f1803..000000000 --- a/tests/functional/a/arguments_positional_only.py +++ /dev/null @@ -1,15 +0,0 @@ -"""Test `no-value-for-parameter` in the context of positional-only parameters""" - -# pylint: disable=missing-docstring, unused-argument - - -def name1(param1, /, **kwargs): ... -def name2(param1, /, param2, **kwargs): ... -def name3(param1=True, /, **kwargs): ... -def name4(param1, **kwargs): ... - -name1(param1=43) # [no-value-for-parameter] -name1(43) -name2(1, param2=False) -name3() -name4(param1=43) diff --git a/tests/functional/a/arguments_positional_only.rc b/tests/functional/a/arguments_positional_only.rc deleted file mode 100644 index 85fc502b3..000000000 --- a/tests/functional/a/arguments_positional_only.rc +++ /dev/null @@ -1,2 +0,0 @@ -[testoptions] -min_pyver=3.8 diff --git a/tests/functional/a/arguments_positional_only.txt b/tests/functional/a/arguments_positional_only.txt deleted file mode 100644 index 7112e6880..000000000 --- a/tests/functional/a/arguments_positional_only.txt +++ /dev/null @@ -1 +0,0 @@ -no-value-for-parameter:11:0:11:16::No value for argument 'param1' in function call:UNDEFINED diff --git a/tests/functional/k/kwarg_superseded_by_positional_arg.py b/tests/functional/k/kwarg_superseded_by_positional_arg.py new file mode 100644 index 000000000..050b2066c --- /dev/null +++ b/tests/functional/k/kwarg_superseded_by_positional_arg.py @@ -0,0 +1,39 @@ +""" +The `kwarg-superseded-by-positional-arg` warning message is emitted when a function is called with +a keyword argument which shares a name with a positional-only parameter and +the function contains a keyword variadic parameter dictionary. +It may be surprising behaviour when the keyword argument is added to the +keyword variadic parameter dictionary. +""" + + +def name1(apple, /, banana="Yellow banana", **kwargs): + """ + Positional-only parameter, positional-or-keyword parameter and `**kwargs`. + + >>> name1("Red apple", apple="Green apple", banana="Green banana") + Red apple + Green banana + {"apple": "Green apple"} + """ + + print(apple) + print(banana) + print(kwargs) + + +# +1: [kwarg-superseded-by-positional-arg] +name1("Red apple", apple="Green apple", banana="Green banana") +name1("Red apple", banana="Green banana") + + +def name2(apple="Green apple", /, **kwargs): + """ + >>> name2(apple="Red apple") + Green apple + {'apple': 'Red apple'} + """ + print(apple) + print(kwargs) + +name2(apple="Red apple") # [kwarg-superseded-by-positional-arg] diff --git a/tests/functional/k/kwarg_superseded_by_positional_arg.txt b/tests/functional/k/kwarg_superseded_by_positional_arg.txt new file mode 100644 index 000000000..15d527f7a --- /dev/null +++ b/tests/functional/k/kwarg_superseded_by_positional_arg.txt @@ -0,0 +1,2 @@ +kwarg-superseded-by-positional-arg:26:0:26:62::'apple' will be included in '**kwargs' since a positional-only parameter with this name already exists:HIGH +kwarg-superseded-by-positional-arg:39:0:39:24::'apple' will be included in '**kwargs' since a positional-only parameter with this name already exists:HIGH diff --git a/tests/functional/p/positional_only_arguments_expected.py b/tests/functional/p/positional_only_arguments_expected.py index 98a2d65f5..dde40f174 100644 --- a/tests/functional/p/positional_only_arguments_expected.py +++ b/tests/functional/p/positional_only_arguments_expected.py @@ -1,5 +1,5 @@ # pylint: disable=missing-docstring,unused-argument,pointless-statement -# pylint: disable=too-few-public-methods +# pylint: disable=too-few-public-methods, kwarg-superseded-by-positional-arg class Gateaux: def nihon(self, a, r, i, /, cheese=False): -- cgit v1.2.1