Check if decorator returns use keyword (unexpected-keyword-arg) (#5547)

* Improve coverage

* Remove unnecessary declaration

* Change spacing

Author: DanielNoord

ChangeLog

@@ -107,6 +107,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

doc/whatsnew/2.13.rst

@@ -101,6 +101,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

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,46 @@ 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 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
+
+            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):
+                    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,

tests/functional/u/unexpected_keyword_arg.py

@@ -0,0 +1,118 @@
+"""Tests for unexpected-keyword-arg"""
+# pylint: disable=undefined-variable, too-few-public-methods, missing-function-docstring, missing-class-docstring
+
+
+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)
+
+
+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):
+    """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)
+
+
+do_something_double_decorated(internal_arg=2)  # [unexpected-keyword-arg]
+
+
+@param_decorator
+@kwargs_decorator
+def do_something_double_decorated_correct(junk=None):
+    """A decorated function. This should be passed a keyword argument"""
+    print(junk)
+
+
+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]

tests/functional/u/unexpected_keyword_arg.txt

@@ -0,0 +1,4 @@
+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