Skip to content

Conversation

@Alphadelta14
Copy link
Contributor

@Alphadelta14 Alphadelta14 commented Jul 12, 2021

Steps

  • For new features or bug fixes, add a ChangeLog entry describing what your PR does.
  • Write a good description on what the PR does.

Description

While working on pylint-dev/pylint#4357, I happened to notice an issue with partial functions where a given scope had both the original function and some remote partial usage returned by the same inference call.

I tracked the underlying issue to partials having their parents set the original function parent.

So in the [abbreviated] code from pylint/checkers/utils.py below, func (partial(error_of_type, ...)) gets Module as the parent (and since it has the same name, PartialFunction(name=error_of_type)), both of these values are locals in Module with the same name. This lead to the extremely strange case of error_of_type(handler, exception) inferred as both error_of_type and partial(error_of_type, ...).
The proper parent for partial(error_of_type, ...) should just be the normal assignment to func.

def error_of_type(handler: astroid.ExceptHandler, error_type) -> bool:
    pass

def _except_handlers_ignores_exception(
    handlers: astroid.ExceptHandler, exception
) -> bool:
    func = partial(error_of_type, error_type=(exception,))
    return any(func(handler) for handler in handlers)

def get_exception_handlers(
    node: astroid.node_classes.NodeNG, exception=Exception
) -> Optional[List[astroid.ExceptHandler]]:
    context = find_try_except_wrapper_node(node)
    if isinstance(context, astroid.TryExcept):
        return [
            handler for handler in context.handlers if error_of_type(handler, exception)
        ]
    return []

This also fixes the underlying issue seen in pylint-dev/pylint#2588 / 87b55a6e without a special case.

Type of Changes

Type
🐛 Bug fix

Related Issue

pylint-dev/pylint#4357

Alphadelta14 added a commit to Alphadelta14/pylint that referenced this pull request Jul 12, 2021
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.6.3 milestone Jul 12, 2021
@Pierre-Sassoulas
Copy link
Member

I tested in pylint and I got:

___________________________________________________________________________________ test_functional[too_many_arguments] ___________________________________________________________________________________

self = <pylint.testutils.lint_module_test.LintModuleTest object at 0x7f37c98c7730>

    def runTest(self):
>       self._runTest()
E       AssertionError: Wrong results for file "too_many_arguments":
E       
E       Unexpected in testdata:
E         32: too-many-function-args

On :

def func_call():
    """Test we don't emit a FP for https://github.com/PyCQA/pylint/issues/2588"""
    partial_func = partial(root_function, 1, 2, 3)
    partial_func()
    return root_function(1, 2, 3) # false positive too-many-function-args

This is the test case for #2588 (#632 )

@Pierre-Sassoulas Pierre-Sassoulas added the pylint-tested PRs that don't cause major regressions with pylint label Jul 12, 2021
@Alphadelta14
Copy link
Contributor Author

okay. I've figured out how to reproduce that test locally. Will make sure that is addressed!

Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

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

It's working in pylint now 👍

@Pierre-Sassoulas Pierre-Sassoulas requested a review from cdce8p July 13, 2021 20:17
@Pierre-Sassoulas Pierre-Sassoulas modified the milestones: 2.6.3, 2.6.4, 2.6.5, 2.6.6 Jul 19, 2021
@Pierre-Sassoulas Pierre-Sassoulas merged commit ac95965 into pylint-dev:main Aug 1, 2021
Pierre-Sassoulas pushed a commit to Alphadelta14/pylint that referenced this pull request Apr 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug 🪳 pylint-tested PRs that don't cause major regressions with pylint

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants