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

Empty lines in method body #2486

Closed
blablatdinov opened this issue Sep 5, 2022 · 7 comments · Fixed by #2487
Closed

Empty lines in method body #2486

blablatdinov opened this issue Sep 5, 2022 · 7 comments · Fixed by #2487
Labels
rule request Adding a new rule

Comments

@blablatdinov
Copy link
Contributor

blablatdinov commented Sep 5, 2022

Rule request

Empty lines in the method body are bad. I propose prohibit their use or limit it count in method body

Thesis

I received a PR with the same code

    def to_representation(self, data):
        result = None

        self.get_person(data)

        if self.person and self.person.use_other_veh:

            result = super().to_representation(data)

        return result

Reasoning

I think, if method have few empty lines, it not holistic, and it makes sense to divide the method into several. Yegor Bugaenko's post about empty lines

@blablatdinov blablatdinov added the rule request Adding a new rule label Sep 5, 2022
@sobolevn
Copy link
Member

sobolevn commented Sep 5, 2022

Interesting, I think we can say that there must a maximum of empty lines in a function's body.
For example: a function with 2 lines can only have 0 empty lines.
But, a functions with 4 lines, can have 1 empty line.

I think we need to come up with a ratio 🤔

@blablatdinov
Copy link
Contributor Author

@sobolevn Thank you for answer! I agree with you. Maybe made this functional configurable (like max-arguments)

@sobolevn
Copy link
Member

sobolevn commented Sep 5, 2022

Would you like to open a PR for this?

@blablatdinov
Copy link
Contributor Author

@sobolevn Yes, of course. But I don't have experience in develop plugins for flake8, and I will be glad of any support.

At the end of the month, I will make a report linter at the "X5 Group" internal meetup (greetings from Artem Kalinin). I would like to add a slide about the extension of this tool to my presentation

@sobolevn
Copy link
Member

sobolevn commented Sep 5, 2022

Awesome!

So, basically you need to take a look at visit_FuncDef or visit_any_function methods.
Inside you will see the full ast of a function defition.

Next, collect all lines that have 0 nodes on it to find empty ones. And collect the final number of lines.

Finally, decide should we raise a violation or not.
Any help I can provide - I will :)

@blablatdinov
Copy link
Contributor Author

@sobolevn I have increment: create class for checking empty lines in functions or method body. In the next commit I planning add parameter for configuration this rule. Please check my PR

@blablatdinov
Copy link
Contributor Author

@sobolevn I'm push changes that make this rule configurable, please check it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule request Adding a new rule
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants