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.

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

Add NoRedundantArgumentsSuperRule #154

Merged
merged 1 commit into from
Nov 3, 2020
Merged

Add NoRedundantArgumentsSuperRule #154

merged 1 commit into from
Nov 3, 2020

Conversation

mkniewallner
Copy link
Contributor

@mkniewallner mkniewallner commented Nov 3, 2020

Summary

Ensure that calls to super that use the direct parent as a first argument are reported (and autofixed when requested), according to PEP 3135.

This will turn

class Foo(Bar):
    def foo(self, bar):
        super(Foo, self).foo(bar)

into

class Foo(Bar):
    def foo(self, bar):
        super().foo(bar)

The rule also takes into account nested classes, so that this code

class Foo:
    class InnerFoo(Bar):
        def foo(self, bar):
            super(Foo.InnerFoo, self).foo(bar)

gets turned into

class Foo:
    class InnerFoo(Bar):
        def foo(self, bar):
            super().foo(bar)

Test Plan

Test cases have been added to ensure that correctly formatted code is not modified and code that matches the rule reports the issue.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 3, 2020
Copy link
Contributor

@jimmylai jimmylai left a comment

Choose a reason for hiding this comment

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

Thanks for contributing!
The test cases are comprehensive and make sense.

This rule reminds me some other conventions that can be built as Fixit rules.

  1. a method in a class should have self as the first argument.
  2. a classmethod in a class should have cls as the first argument.
  3. a staticmethod in a class should not have self nor cls as the first argument.

Feel free to build more rules for more Python conventions to help developers write better code faster.

@mkniewallner
Copy link
Contributor Author

  1. a classmethod in a class should have cls as the first argument.

This one is actually already implemented 🙂

@jimmylai
Copy link
Contributor

jimmylai commented Nov 3, 2020

  1. a classmethod in a class should have cls as the first argument.

This one is actually already implemented 🙂

That's great! I almost forget it.

@jimmylai jimmylai merged commit 326e0d2 into Instagram:master Nov 3, 2020
@mkniewallner mkniewallner deleted the add-no-redundant-arguments-super branch November 4, 2020 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants