Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,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
Expand Down
4 changes: 4 additions & 0 deletions doc/whatsnew/2.13.rst
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,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
Expand Down
44 changes: 44 additions & 0 deletions pylint/checkers/typecheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really related to this MR but if we have to handle confidence level, this kind of return where we do not exactly know if inference was used or not in the calling code will be a headache later. Makes me thing that implementing confidence properly will be an enormous work, maybe not even worth it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thankfully we have UNDEFINED 😄

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,
Expand Down
118 changes: 118 additions & 0 deletions tests/functional/u/unexpected_keyword_arg.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
"""Tests for unexpected-keyword-arg"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests are on point !

# 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]
4 changes: 4 additions & 0 deletions tests/functional/u/unexpected_keyword_arg.txt
Original file line number Diff line number Diff line change
@@ -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