Limit empty lines in functions or methods body #2487

blablatdinov · 2022-09-06T17:14:41Z

I have made things!

Checklist

I have double checked that there are no unrelated changes in this pull request (old patches, accidental config files, etc)
I have created at least one test case for the changes I have made
I have updated the documentation for the changes I have made
I have added my changes to the CHANGELOG.md

Related issues

Format is:

Closes Empty lines in method body #2486

🙏 Please, if you or your company is finding wemake-python-styleguide valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/wemake-python-styleguide. As a thank you, your profile/company logo will be added to our main README which receives hundreds of unique visitors per day.

sobolevn

Please, add configuration and default ratio (I think that default amount of empty lines should be 1-for-2 non-empty lines).

blablatdinov · 2022-09-08T17:46:58Z

@sobolevn I don't agree or don't correctly understand you proportions. In this proportions linter don't reject next code:

def func(a, b):
    first_expression(a)

    second_expression(b)

Or you implied 2 line with expressions without gap between them?

sobolevn · 2022-09-08T18:33:52Z

We can add a lower bound of 0 empty lines for 2 lines. That's not a problem.

blablatdinov · 2022-09-08T18:41:05Z

@sobolevn I'm pushed commit with code for configurating this rule. Please, check it

sobolevn · 2022-09-12T08:56:26Z

Can you please rebase your work? I've fixed new poetry@1.2.0 build

…olation

blablatdinov · 2022-09-12T09:04:24Z

Yes, of course. My branch rebased now

sobolevn · 2022-09-12T09:10:31Z

Can you please fix tests in https://github.com/wemake-services/wemake-python-styleguide/runs/8301866244?check_suite_focus=true ?

blablatdinov · 2022-09-12T11:50:32Z

Sorry, my bad. I connect my visitor and found a problem: ast module ignore comments, so my algorithm count it like empty line. I've been looked solution of this problem on Google and I can't find it (in stackoverflow recomends using tokenize module). Do you have any recomendation for resolving this problem ?

skarzi · 2022-09-17T13:30:44Z

tests/test_visitors/test_ast/test_classes/test_wrong_empty_lines_count.py

+        lighter()
+"""
+
+class_with_allow_method = """


NIP: what do you think about using valid instead of allow?
class_with_valid_method sounds better

Yes, I'm agree. My english level is not high, so I'm happy with any edits

tests/test_visitors/test_ast/test_classes/test_wrong_empty_lines_count.py

wemake_python_styleguide/options/config.py

wemake_python_styleguide/options/defaults.py

wemake_python_styleguide/violations/best_practices.py

wemake_python_styleguide/visitors/ast/function_empty_lines.py

skarzi · 2022-09-17T13:51:11Z

tests/test_visitors/test_ast/test_classes/test_wrong_empty_lines_count.py

+    visitor = WrongEmptyLinesCountVisitor(default_options, tree=tree)
+    visitor.run()
+
+    assert_errors(visitor, [WrongEmptyLinesCountViolation])


Is this assertion correct? It seems like in this test input_ has only a valid number of empty lines

I forgot to change parse_ast_tree after parameterizing the tests, thanks

skarzi · 2022-09-17T13:55:01Z

tests/test_visitors/test_ast/test_classes/test_wrong_empty_lines_count.py

+    baz()
+"""
+
+wrong_function_with_loop = """


What about edge cases like below ones?

def foo(): return ( pekaboo['attr'] == 'dummy-value' and bar() == 'Knights that say Ni!' or ( 'attr2' in pekaboo and not some_object.attribute.method() ) )

pekaboo( foo=123, bar='abc', )

Can they be handled by the ast?
Probably this violation should be implemented by using BaseTokenVisitor

Yeh, I plan to change visitor to BaseTokenVisitor because ast ignore comments. I check these cases after changing the visitor, thank you

…es_count.py fix typo Co-authored-by: Łukasz Skarżyński <skarzynski_lukasz@protonmail.com>

fix typo Co-authored-by: Łukasz Skarżyński <skarzynski_lukasz@protonmail.com>

Co-authored-by: Łukasz Skarżyński <skarzynski_lukasz@protonmail.com>

….com/blablatdinov/wemake-python-styleguide into pr-restrict-empty-lines-in-func-body

blablatdinov · 2022-09-25T15:54:36Z

I'm change visitor to BaseTokenVisitor. Can you review it?

codecov · 2022-09-25T16:11:23Z

Codecov Report

Merging #2487 (8f366b7) into master (5bfdbdd) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master     #2487   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          119       120    +1     
  Lines         6390      6431   +41     
  Branches      1449      1458    +9     
=========================================
+ Hits          6390      6431   +41

Impacted Files	Coverage Δ
wemake_python_styleguide/options/config.py	`100.00% <ø> (ø)`
wemake_python_styleguide/presets/types/tree.py	`100.00% <ø> (ø)`
wemake_python_styleguide/options/defaults.py	`100.00% <100.00%> (ø)`
wemake_python_styleguide/options/validation.py	`100.00% <100.00%> (ø)`
...ake_python_styleguide/violations/best_practices.py	`100.00% <100.00%> (ø)`
...on_styleguide/visitors/ast/function_empty_lines.py	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

sobolevn · 2022-09-25T16:25:45Z

tests/test_visitors/test_ast/test_classes/test_wrong_empty_lines_count.py

+"""
+
+
+allow_function_with_comments = """


Please, add this case:

def test_func(): """ Its docstring has many new lines but this is totally fine we don't raise a violation for this """ return

sobolevn · 2022-09-25T16:27:38Z

tests/test_visitors/test_ast/test_classes/test_wrong_empty_lines_count.py

+    parse_tokens,
+):
+    """Testing wrong cases."""
+    file_tokens = parse_tokens(input_)


Don't forget to use mode fixture to test async def as well.

sobolevn · 2022-09-25T16:29:02Z

tests/test_visitors/test_ast/test_classes/test_wrong_empty_lines_count.py

+    print(customer.phone)
+    # printing customer company
+    print(customer.company)
+"""


We also need a test case like this:

def test_func(): # This function # # has lots # # of empty # # lines # # in comments return 0

A mix of comments and docstring is also a good idea 👍

blablatdinov · 2022-09-25T16:58:31Z

Thanks for comments. I'm add test cases with comments and docstrings. And wrapped input code snippets into mode fixture

sobolevn

Awesome! This is the last piece of feedback that I have.

wemake_python_styleguide/violations/best_practices.py

sobolevn · 2022-09-26T12:25:48Z

tests/test_visitors/test_ast/test_classes/test_wrong_empty_lines_count.py

+):
+    """Test zero configuration."""
+    file_tokens = parse_tokens(mode(allow_function))
+    visitor = WrongEmptyLinesCountVisitor(


Let's also add a test that function without any empty lines do pass with exps_for_one_empty_line=0

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>

….com/blablatdinov/wemake-python-styleguide into pr-restrict-empty-lines-in-func-body

blablatdinov · 2022-09-26T18:34:41Z

Thanks for your suggested changes! I added test with exps_for_one_empty_line=0 for method without empty lines

sobolevn

Thank you for the great idea and solid implementation!
👏

blablatdinov · 2022-09-27T06:52:04Z

Thank you too, this is my first contribution to open source with code! The "quality wall" in this repository is very high and strong, respect to the developers who build it. 🎉

I bought the course "typed python" from @f213 and @sobolevn, we'll meet there 😉

sobolevn reviewed Sep 7, 2022

View reviewed changes

blablatdinov force-pushed the pr-restrict-empty-lines-in-func-body branch 3 times, most recently from e686c19 to a00ff60 Compare September 8, 2022 18:17

blablatdinov requested a review from sobolevn September 12, 2022 08:55

blablatdinov added 7 commits September 12, 2022 12:02

add class and test for restrict all empty lines in func body

e48a050

add test case: function with comments

7136d32

add configuration param

9bf523d

fix baseline for formatting violation

cf78379

self.generic_visit(node) in visitor

8b490ab

rename WrongEmptyLinesCountVisitorViolation -> WrongEmptyLinesCountVi…

1715c0f

…olation

0 available empty lines case

9e4943e

blablatdinov force-pushed the pr-restrict-empty-lines-in-func-body branch from 58ece1e to 9e4943e Compare September 12, 2022 09:02

blablatdinov added 2 commits September 12, 2022 12:11

update CHANGELOG

b11609c

Fix tests

cb34ca8

blablatdinov force-pushed the pr-restrict-empty-lines-in-func-body branch from a30ad0b to cb34ca8 Compare September 12, 2022 09:42

blablatdinov force-pushed the pr-restrict-empty-lines-in-func-body branch 5 times, most recently from 8e7ba4c to 6f47f50 Compare September 12, 2022 17:40

Fix algorithm for calculating expression lines

55ba64c

blablatdinov force-pushed the pr-restrict-empty-lines-in-func-body branch from 6f47f50 to 55ba64c Compare September 12, 2022 17:49

skarzi reviewed Sep 17, 2022

View reviewed changes

blablatdinov and others added 13 commits September 18, 2022 16:33

Update tests/test_visitors/test_ast/test_classes/test_wrong_empty_lin…

f913f44

…es_count.py fix typo Co-authored-by: Łukasz Skarżyński <skarzynski_lukasz@protonmail.com>

Update wemake_python_styleguide/options/config.py

43a88b5

fix typo Co-authored-by: Łukasz Skarżyński <skarzynski_lukasz@protonmail.com>

Update wemake_python_styleguide/options/defaults.py

88cdb63

fix typo Co-authored-by: Łukasz Skarżyński <skarzynski_lukasz@protonmail.com>

Update wemake_python_styleguide/violations/best_practices.py

36b74d2

Co-authored-by: Łukasz Skarżyński <skarzynski_lukasz@protonmail.com>

Update wemake_python_styleguide/violations/best_practices.py

d78bb99

Co-authored-by: Łukasz Skarżyński <skarzynski_lukasz@protonmail.com>

Copying set by method

7a48d2f

Co-authored-by: Łukasz Skarżyński <skarzynski_lukasz@protonmail.com>

Optimize visitor

e409a41

Co-authored-by: Łukasz Skarżyński <skarzynski_lukasz@protonmail.com>

Update wemake_python_styleguide/visitors/ast/function_empty_lines.py

2bfc662

Co-authored-by: Łukasz Skarżyński <skarzynski_lukasz@protonmail.com>

Fix typos and parse_ast_tree argument

57d8696

Merge branch 'pr-restrict-empty-lines-in-func-body' of https://github…

e0370f9

….com/blablatdinov/wemake-python-styleguide into pr-restrict-empty-lines-in-func-body

change visitor to BaseTokenVisitor

4fda3be

Merge branch 'master' into pr-restrict-empty-lines-in-func-body

b0cfd96

test option

3de1283

sobolevn reviewed Sep 25, 2022

View reviewed changes

test cases with docstring and comments

919dc2a

blablatdinov requested a review from sobolevn September 26, 2022 09:34

sobolevn reviewed Sep 26, 2022

View reviewed changes

blablatdinov and others added 4 commits September 26, 2022 21:31

fix error template

96bf0ee

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>

expanded documentation

62ab4b9

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>

test valid configuration with allow zero empty lines

be93737

Merge branch 'pr-restrict-empty-lines-in-func-body' of https://github…

8f366b7

….com/blablatdinov/wemake-python-styleguide into pr-restrict-empty-lines-in-func-body

blablatdinov requested a review from sobolevn September 26, 2022 18:40

sobolevn approved these changes Sep 26, 2022

View reviewed changes

sobolevn merged commit 4ca670e into wemake-services:master Sep 26, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Limit empty lines in functions or methods body #2487

Limit empty lines in functions or methods body #2487

blablatdinov commented Sep 6, 2022 •

edited

sobolevn left a comment •

edited

blablatdinov commented Sep 8, 2022

sobolevn commented Sep 8, 2022

blablatdinov commented Sep 8, 2022

sobolevn commented Sep 12, 2022

blablatdinov commented Sep 12, 2022 •

edited

sobolevn commented Sep 12, 2022

blablatdinov commented Sep 12, 2022 •

edited

skarzi Sep 17, 2022

blablatdinov Sep 18, 2022

skarzi Sep 17, 2022

blablatdinov Sep 18, 2022

skarzi Sep 17, 2022

blablatdinov Sep 18, 2022

blablatdinov commented Sep 25, 2022

codecov bot commented Sep 25, 2022 •

edited

sobolevn Sep 25, 2022

sobolevn Sep 25, 2022

sobolevn Sep 25, 2022

sobolevn Sep 25, 2022

blablatdinov commented Sep 25, 2022 •

edited

sobolevn left a comment

sobolevn Sep 26, 2022

blablatdinov commented Sep 26, 2022 •

edited

sobolevn left a comment •

edited

blablatdinov commented Sep 27, 2022

Limit empty lines in functions or methods body #2487

Limit empty lines in functions or methods body #2487

Conversation

blablatdinov commented Sep 6, 2022 • edited

I have made things!

Checklist

Related issues

sobolevn left a comment • edited

Choose a reason for hiding this comment

blablatdinov commented Sep 8, 2022

sobolevn commented Sep 8, 2022

blablatdinov commented Sep 8, 2022

sobolevn commented Sep 12, 2022

blablatdinov commented Sep 12, 2022 • edited

sobolevn commented Sep 12, 2022

blablatdinov commented Sep 12, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blablatdinov commented Sep 25, 2022

codecov bot commented Sep 25, 2022 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blablatdinov commented Sep 25, 2022 • edited

sobolevn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blablatdinov commented Sep 26, 2022 • edited

sobolevn left a comment • edited

Choose a reason for hiding this comment

blablatdinov commented Sep 27, 2022

blablatdinov commented Sep 6, 2022 •

edited

sobolevn left a comment •

edited

blablatdinov commented Sep 12, 2022 •

edited

blablatdinov commented Sep 12, 2022 •

edited

codecov bot commented Sep 25, 2022 •

edited

blablatdinov commented Sep 25, 2022 •

edited

blablatdinov commented Sep 26, 2022 •

edited

sobolevn left a comment •

edited