From 66c2e0c836c9b60b74768a3b2e9249ce0671b585 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Fri, 17 Dec 2021 20:38:20 +0100 Subject: [PATCH 1/4] Check if decorator returns use keyword (``unexpected-keyword-arg``) --- ChangeLog | 4 + doc/whatsnew/2.13.rst | 4 + pylint/checkers/typecheck.py | 39 ++++++++++ tests/functional/u/unexpected_keyword_arg.py | 76 +++++++++++++++++++ tests/functional/u/unexpected_keyword_arg.txt | 2 + 5 files changed, 125 insertions(+) create mode 100644 tests/functional/u/unexpected_keyword_arg.py create mode 100644 tests/functional/u/unexpected_keyword_arg.txt diff --git a/ChangeLog b/ChangeLog index 10137bdaa1..005acb17bc 100644 --- a/ChangeLog +++ b/ChangeLog @@ -92,6 +92,10 @@ Release date: TBA * The ``testutils`` for unittests now accept ``end_lineno`` and ``end_column``. Tests without these will trigger a ``DeprecationWarning``. +* Fixed false positive ``unexpected-keyword-arg`` for decorators. + + Closes #258 + * ``missing-raises-doc`` will now check the class hierarchy of the raised exceptions .. code-block:: python diff --git a/doc/whatsnew/2.13.rst b/doc/whatsnew/2.13.rst index 72f071510c..bb3b5fb582 100644 --- a/doc/whatsnew/2.13.rst +++ b/doc/whatsnew/2.13.rst @@ -86,6 +86,10 @@ Other Changes * The ``testutils`` for unittests now accept ``end_lineno`` and ``end_column``. Tests without these will trigger a ``DeprecationWarning``. +* Fixed false positive ``unexpected-keyword-arg`` for decorators. + + Closes #258 + * ``missing-raises-doc`` will now check the class hierarchy of the raised exceptions .. code-block:: python diff --git a/pylint/checkers/typecheck.py b/pylint/checkers/typecheck.py index df01cc2fe9..60aea2fe78 100644 --- a/pylint/checkers/typecheck.py +++ b/pylint/checkers/typecheck.py @@ -1457,6 +1457,10 @@ def visit_call(self, node: nodes.Call) -> None: elif called.args.kwarg is not None: # The keyword argument gets assigned to the **kwargs parameter. pass + elif isinstance( + called, nodes.FunctionDef + ) and self._keyword_argument_is_in_all_decorator_returns(called, keyword): + pass elif not overload_function: # Unexpected keyword argument. self.add_message( @@ -1496,6 +1500,41 @@ def visit_call(self, node: nodes.Call) -> None: ): self.add_message("missing-kwoa", node=node, args=(name, callable_name)) + @staticmethod + def _keyword_argument_is_in_all_decorator_returns( + func: nodes.FunctionDef, keyword: str + ) -> bool: + """Check if the keyword argument exists in all signatures of the + return values of all decorators of the function. + """ + if not func.decorators: + return False + + for decorator in func.decorators.nodes: + inferred = safe_infer(decorator) + if not isinstance(inferred, nodes.FunctionDef): + return False + return_values = list(inferred.infer_call_result()) + + # If there are not return values they also can't be consuming the keyword + if not return_values: + return False + + for return_value in return_values: + if not isinstance(return_value, nodes.FunctionDef): + return False + + # If the return value uses a kwarg the keyword will be consumed + if return_value.args.kwarg: + continue + + # Check if the keyword is another type of argument + if return_value.args.is_argument(keyword): + continue + return False + + return True + def _check_invalid_sequence_index(self, subscript: nodes.Subscript): # Look for index operations where the parent is a sequence type. # If the types can be determined, only allow indices to be int, diff --git a/tests/functional/u/unexpected_keyword_arg.py b/tests/functional/u/unexpected_keyword_arg.py new file mode 100644 index 0000000000..2cd117eed1 --- /dev/null +++ b/tests/functional/u/unexpected_keyword_arg.py @@ -0,0 +1,76 @@ +"""Tests for unexpected-keyword-arg""" + + +def non_param_decorator(func): + """Decorator without a parameter""" + + def new_func(): + func() + + return new_func + + +def param_decorator(func): + """Decorator with a parameter""" + + def new_func(internal_arg=3): + func(junk=internal_arg) + + return new_func + + +def kwargs_decorator(func): + """Decorator with kwargs. + The if ... else makes the double decoration with param_decorator valid. + """ + + def new_func(**kwargs): + if "internal_arg" in kwargs: + func(junk=kwargs["internal_arg"]) + else: + func(junk=kwargs["junk"]) + + return new_func + + +@non_param_decorator +def do_something(junk=None): + """A decorated function. This should not be passed a keyword argument""" + print(junk) + + +@param_decorator +def do_something_decorated(junk=None): + """A decorated function. This should be passed a keyword argument""" + print(junk) + + +@kwargs_decorator +def do_something_decorated_too(junk=None): + """A decorated function. This should be passed a keyword argument""" + print(junk) + + +@non_param_decorator +@kwargs_decorator +def do_something_double_decorated(junk=None): + """A decorated function. This should not be passed a keyword argument. + non_param_decorator will raise an exception if a keyword argument is passed. + """ + print(junk) + + +@param_decorator +@kwargs_decorator +def do_something_double_decorated_correct(junk=None): + """A decorated function. This should be passed a keyword argument""" + print(junk) + + +def function_caller(): + """Call the decorated functions and check if they accept keywords""" + do_something(internal_arg=2) # [unexpected-keyword-arg] + do_something_decorated(internal_arg=2) + do_something_decorated_too(internal_arg=2) + do_something_double_decorated(internal_arg=2) # [unexpected-keyword-arg] + do_something_double_decorated_correct(internal_arg=2) diff --git a/tests/functional/u/unexpected_keyword_arg.txt b/tests/functional/u/unexpected_keyword_arg.txt new file mode 100644 index 0000000000..da868f7e0f --- /dev/null +++ b/tests/functional/u/unexpected_keyword_arg.txt @@ -0,0 +1,2 @@ +unexpected-keyword-arg:72:4:72:32:function_caller:Unexpected keyword argument 'internal_arg' in function call:UNDEFINED +unexpected-keyword-arg:75:4:75:49:function_caller:Unexpected keyword argument 'internal_arg' in function call:UNDEFINED From 0fb82e8ee9a494699d79a73d10024c174deac403 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Fri, 17 Dec 2021 21:14:41 +0100 Subject: [PATCH 2/4] Improve coverage --- pylint/checkers/typecheck.py | 14 +++-- tests/functional/u/unexpected_keyword_arg.py | 56 ++++++++++++++++--- tests/functional/u/unexpected_keyword_arg.txt | 6 +- 3 files changed, 63 insertions(+), 13 deletions(-) diff --git a/pylint/checkers/typecheck.py b/pylint/checkers/typecheck.py index 60aea2fe78..7e308b6300 100644 --- a/pylint/checkers/typecheck.py +++ b/pylint/checkers/typecheck.py @@ -1512,15 +1512,21 @@ def _keyword_argument_is_in_all_decorator_returns( for decorator in func.decorators.nodes: inferred = safe_infer(decorator) + + # If we can't infer the decorator we assume it satisfies consumes + # the keyword, so we don't raise false positives + if not inferred: + return True + + # We only check arguments of function decorators if not isinstance(inferred, nodes.FunctionDef): return False - return_values = list(inferred.infer_call_result()) - # If there are not return values they also can't be consuming the keyword - if not return_values: - return False + return_values = list(inferred.infer_call_result()) for return_value in return_values: + # infer_call_result() returns nodes.Const.None for None return values + # so this also catches non-returning decorators if not isinstance(return_value, nodes.FunctionDef): return False diff --git a/tests/functional/u/unexpected_keyword_arg.py b/tests/functional/u/unexpected_keyword_arg.py index 2cd117eed1..e7b648899e 100644 --- a/tests/functional/u/unexpected_keyword_arg.py +++ b/tests/functional/u/unexpected_keyword_arg.py @@ -1,4 +1,5 @@ """Tests for unexpected-keyword-arg""" +# pylint: disable=undefined-variable, too-few-public-methods, missing-function-docstring, missing-class-docstring def non_param_decorator(func): @@ -39,18 +40,27 @@ def do_something(junk=None): print(junk) +do_something(internal_arg=2) # [unexpected-keyword-arg] + + @param_decorator def do_something_decorated(junk=None): """A decorated function. This should be passed a keyword argument""" print(junk) +do_something_decorated(internal_arg=2) + + @kwargs_decorator def do_something_decorated_too(junk=None): """A decorated function. This should be passed a keyword argument""" print(junk) +do_something_decorated_too(internal_arg=2) + + @non_param_decorator @kwargs_decorator def do_something_double_decorated(junk=None): @@ -60,6 +70,9 @@ def do_something_double_decorated(junk=None): print(junk) +do_something_double_decorated(internal_arg=2) # [unexpected-keyword-arg] + + @param_decorator @kwargs_decorator def do_something_double_decorated_correct(junk=None): @@ -67,10 +80,39 @@ def do_something_double_decorated_correct(junk=None): print(junk) -def function_caller(): - """Call the decorated functions and check if they accept keywords""" - do_something(internal_arg=2) # [unexpected-keyword-arg] - do_something_decorated(internal_arg=2) - do_something_decorated_too(internal_arg=2) - do_something_double_decorated(internal_arg=2) # [unexpected-keyword-arg] - do_something_double_decorated_correct(internal_arg=2) +do_something_double_decorated_correct(internal_arg=2) + + +# Test that we don't crash on Class decoration +class DecoratorClass: + pass + + +@DecoratorClass +def crash_test(): + pass + + +crash_test(internal_arg=2) # [unexpected-keyword-arg] + + +# Test that we don't emit a false positive for uninferable decorators +@unknown_decorator +def crash_test_two(): + pass + + +crash_test_two(internal_arg=2) + + +# Test that we don't crash on decorators that don't return anything +def no_return_decorator(func): + print(func) + + +@no_return_decorator +def test_no_return(): + pass + + +test_no_return(internal_arg=2) # [unexpected-keyword-arg] diff --git a/tests/functional/u/unexpected_keyword_arg.txt b/tests/functional/u/unexpected_keyword_arg.txt index da868f7e0f..3cc968e883 100644 --- a/tests/functional/u/unexpected_keyword_arg.txt +++ b/tests/functional/u/unexpected_keyword_arg.txt @@ -1,2 +1,4 @@ -unexpected-keyword-arg:72:4:72:32:function_caller:Unexpected keyword argument 'internal_arg' in function call:UNDEFINED -unexpected-keyword-arg:75:4:75:49:function_caller:Unexpected keyword argument 'internal_arg' in function call:UNDEFINED +unexpected-keyword-arg:43:0:43:28::Unexpected keyword argument 'internal_arg' in function call:UNDEFINED +unexpected-keyword-arg:73:0:73:45::Unexpected keyword argument 'internal_arg' in function call:UNDEFINED +unexpected-keyword-arg:96:0:96:26::Unexpected keyword argument 'internal_arg' in function call:UNDEFINED +unexpected-keyword-arg:118:0:118:30::Unexpected keyword argument 'internal_arg' in function call:UNDEFINED From d0c38cb50fe1c2885e3a0d8f8e089c7095fe633d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Fri, 17 Dec 2021 21:15:37 +0100 Subject: [PATCH 3/4] Remove unnecessary declaration --- pylint/checkers/typecheck.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pylint/checkers/typecheck.py b/pylint/checkers/typecheck.py index 7e308b6300..231aa0deef 100644 --- a/pylint/checkers/typecheck.py +++ b/pylint/checkers/typecheck.py @@ -1522,9 +1522,7 @@ def _keyword_argument_is_in_all_decorator_returns( if not isinstance(inferred, nodes.FunctionDef): return False - return_values = list(inferred.infer_call_result()) - - for return_value in return_values: + for return_value in inferred.infer_call_result(): # infer_call_result() returns nodes.Const.None for None return values # so this also catches non-returning decorators if not isinstance(return_value, nodes.FunctionDef): From c3bbef6a3723e20b62978725b99553a076f81592 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Fri, 17 Dec 2021 21:15:54 +0100 Subject: [PATCH 4/4] Change spacing --- pylint/checkers/typecheck.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pylint/checkers/typecheck.py b/pylint/checkers/typecheck.py index 231aa0deef..35ed23f794 100644 --- a/pylint/checkers/typecheck.py +++ b/pylint/checkers/typecheck.py @@ -1535,6 +1535,7 @@ def _keyword_argument_is_in_all_decorator_returns( # Check if the keyword is another type of argument if return_value.args.is_argument(keyword): continue + return False return True