Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix false positive for useless-super-delegation for variadics #6949

Merged
merged 7 commits into from Jun 15, 2022
Merged

Fix false positive for useless-super-delegation for variadics #6949

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e03ea4a
Fix false positive for ``useless-super-delegation`` for variadics
DanielNoord Jun 14, 2022
70177c3
Update pylint/checkers/classes/class_checker.py
DanielNoord Jun 14, 2022
94ed776
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jun 14, 2022
271666b
Update
DanielNoord Jun 15, 2022
d70ea49
Update pylint/checkers/classes/class_checker.py
DanielNoord Jun 15, 2022
a3df5e8
Merge branch 'main' of https://github.com/PyCQA/pylint into super
DanielNoord Jun 15, 2022
90ec8d4
Move changelog
DanielNoord Jun 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions doc/whatsnew/2/2.14/full.rst
View file
Open in desktop
Expand Up @@ -27,6 +27,11 @@ Release date: TBA

Closes #6818

* Fixed a false positive for ``useless-super-delegation`` for subclasses that specify the number of
of parameters against a parent that uses a variadic argument.

Closes #2270

* Fixed an issue with multi-line ``init-hook`` options which did not record the line endings.

Closes #6888
Expand Down
3 changes: 3 additions & 0 deletions pylint/checkers/classes/class_checker.py
View file
Open in desktop
Expand Up @@ -1276,6 +1276,9 @@ def _check_useless_super_delegation(self, function: nodes.FunctionDef) -> None:
args = _signature_from_call(call)

if meth_node is not None:
# Detect if the super method uses varargs and the function doesn't
DanielNoord marked this conversation as resolved.
Show resolved Hide resolved
if meth_node.args.vararg and not function.args.vararg:
return

def form_annotations(arguments):
annotations = chain(
Expand Down
10 changes: 10 additions & 0 deletions tests/functional/u/useless/useless_super_delegation.py
View file
Open in desktop
Expand Up @@ -303,3 +303,13 @@ def __str__(self):

def __hash__(self): # [useless-super-delegation]
return super().__hash__()


# Reported in https://github.com/PyCQA/pylint/issues/2270
class Super:
def __init__(self, *args):
self.args = args

class Sub(Super):
def __init__(self, a, b):
super().__init__(a, b)
Copy link
Member

Choose a reason for hiding this comment

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

How about when the first argument is required? I see the useless-super-delegation for this:

class Super:
    def __init__(self, *args):
        self.args = args

class Sub(Super):
    def __init__(self, a, *args):
        super().__init__(a, *args)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So you would want this to raise whenever there is only a variadic in super() and one or more positional in child?

Copy link
Member

Choose a reason for hiding this comment

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

@DanielNoord that's exactly it. It's just a variation of the original issue which someone may encounter.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mbyrnepr2 Let me know what you think of this!

Copy link
Member

Choose a reason for hiding this comment

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

I think that will work @DanielNoord. I haven't time and haven't played with other variations but I think it's good to go. Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

Oh one thing is perhaps the comment on line 1279 is a bit inaccurate or hard to understand the underlying intention?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated!

Copy link
Member

Choose a reason for hiding this comment

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

Ah great lovely job!