Skip to content

Commit

Permalink
Limit empty lines in functions or methods body (#2487)
Browse files Browse the repository at this point in the history
* add class and test for restrict all empty lines in func body

* add test case: function with comments

* add configuration param

* fix baseline for formatting violation

* self.generic_visit(node) in visitor

* rename WrongEmptyLinesCountVisitorViolation -> WrongEmptyLinesCountViolation

* 0 available empty lines case

* update CHANGELOG

* Fix tests

* Fix algorithm for calculating expression lines

* Update tests/test_visitors/test_ast/test_classes/test_wrong_empty_lines_count.py

fix typo

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

* Update wemake_python_styleguide/options/config.py

fix typo

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

* Update wemake_python_styleguide/options/defaults.py

fix typo

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

* Update wemake_python_styleguide/violations/best_practices.py

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

* Update wemake_python_styleguide/violations/best_practices.py

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

* Copying set by method

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

* Optimize visitor

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

* Update wemake_python_styleguide/visitors/ast/function_empty_lines.py

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

* Fix typos and parse_ast_tree argument

* change visitor to BaseTokenVisitor

* test option

* test cases with docstring and comments

* fix error template

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

* expanded documentation

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

* test valid configuration with allow zero empty lines

Co-authored-by: Łukasz Skarżyński <skarzynski_lukasz@protonmail.com>
Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
  • Loading branch information
3 people authored Sep 26, 2022
1 parent 5bfdbdd commit 4ca670e
Show file tree
Hide file tree
Showing 10 changed files with 326 additions and 0 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ Semantic versioning in our case means:
- Adds `__init_subclass__` in the beginning of accepted methods
order as per WPS338 #2411

- Adds `WrongEmptyLinesCountViolation` #2486

### Bugfixes

- Fixes `WPS226` false positives on `|` use in `SomeType | AnotherType`
Expand Down
1 change: 1 addition & 0 deletions tests/test_checker/test_noqa.py
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@
'WPS613': 1,
'WPS614': 1,
'WPS615': 2,
'WPS473': 0,
})

# Violations which may be tweaked by `i_control_code` option:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,197 @@
"""Test method contain only allowed empty lines count."""
import pytest

from wemake_python_styleguide.violations.best_practices import (
WrongEmptyLinesCountViolation,
)
from wemake_python_styleguide.visitors.ast.function_empty_lines import (
WrongEmptyLinesCountVisitor,
)

class_with_wrong_method = """
class WrongClass(object):
def wrong_method(self):
foo()
bar()
baz()
lighter()
"""

class_with_valid_method = """
class WrongClass(object):
def wrong_method(self):
foo()
bar()
baz()
"""

wrong_function = """
def func():
foo()
a = 1 + 4
bar()
baz()
"""

wrong_function_with_loop = """
def func():
for x in range(10):
requests.get(
'https://github.com/wemake-services/wemake-python-styleguide'
)
"""

allow_function = """
def func():
foo()
if name == 'Moonflower':
print('Love')
baz()
"""

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

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

function_with_docstring_and_comments = """
def test_func():
\"""
Its docstring
has many new lines
but this is
totally fine
we don't raise a violation for this
\"""
# This function
#
# has lots
#
# of empty
#
# lines
#
# in comments
return 0
"""


@pytest.mark.parametrize('input_', [
class_with_wrong_method,
wrong_function,
wrong_function_with_loop,
])
def test_wrong(
input_,
default_options,
assert_errors,
parse_tokens,
mode,
):
"""Testing wrong cases."""
file_tokens = parse_tokens(mode(input_))

visitor = WrongEmptyLinesCountVisitor(
default_options, file_tokens=file_tokens,
)
visitor.run()

assert_errors(visitor, [WrongEmptyLinesCountViolation])


@pytest.mark.parametrize('input_', [
class_with_valid_method,
allow_function,
allow_function_with_comments,
function_with_docstring,
function_with_docstring_and_comments,
])
def test_success(
input_,
parse_tokens,
default_options,
assert_errors,
mode,
):
"""Testing available cases."""
file_tokens = parse_tokens(mode(input_))

visitor = WrongEmptyLinesCountVisitor(
default_options, file_tokens=file_tokens,
)
visitor.run()

assert_errors(visitor, [])


def test_zero_option(
parse_tokens,
default_options,
assert_errors,
options,
mode,
):
"""Test zero configuration."""
file_tokens = parse_tokens(mode(allow_function))
visitor = WrongEmptyLinesCountVisitor(
options(exps_for_one_empty_line=0), file_tokens=file_tokens,
)
visitor.run()
assert_errors(visitor, [WrongEmptyLinesCountViolation])


def test_zero_option_with_valid_method(
parse_tokens,
default_options,
assert_errors,
options,
mode,
):
"""Test zero configuration with valid method."""
file_tokens = parse_tokens(mode(class_with_valid_method))
visitor = WrongEmptyLinesCountVisitor(
options(exps_for_one_empty_line=0), file_tokens=file_tokens,
)
visitor.run()
assert_errors(visitor, [])
8 changes: 8 additions & 0 deletions wemake_python_styleguide/options/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@
- ``forbidden-inline-ignore`` - list of codes of violations or
class of violations that are forbidden to ignore inline, defaults to
:str:`wemake_python_styleguide.options.defaults.FORBIDDEN_INLINE_IGNORE`
- ``exps-for-one-empty-line`` - number of expressions in
function or method body for available empty line (without code)
:str:`wemake_python_styleguide.options.defaults.EXPS_FOR_ONE_EMPTY_LINE`
.. rubric:: Complexity options
Expand Down Expand Up @@ -262,6 +265,11 @@ class Configuration(object):
type=String,
comma_separated_list=True,
),
_Option(
'--exps-for-one-empty-line',
defaults.EXPS_FOR_ONE_EMPTY_LINE,
'Count of expressions for one empty line in a function body.',
),

# Complexity:

Expand Down
3 changes: 3 additions & 0 deletions wemake_python_styleguide/options/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@
#: Violation codes that are forbidden to use.
FORBIDDEN_INLINE_IGNORE: Final = ()

#: Count of available expressions for one empty line in function or method body.
EXPS_FOR_ONE_EMPTY_LINE: Final = 2

# ===========
# Complexity:
# ===========
Expand Down
1 change: 1 addition & 0 deletions wemake_python_styleguide/options/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ class _ValidatedOptions(object):
max_import_from_members: int = attr.ib(validator=[_min_max(min=1)])
max_tuple_unpack_length: int = attr.ib(validator=[_min_max(min=1)])
show_violation_links: bool
exps_for_one_empty_line: int


def validate_options(options: ConfigurationOptions) -> _ValidatedOptions:
Expand Down
3 changes: 3 additions & 0 deletions wemake_python_styleguide/presets/types/tree.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
conditions,
decorators,
exceptions,
function_empty_lines,
functions,
imports,
iterables,
Expand Down Expand Up @@ -107,6 +108,8 @@

redundancy.RedundantEnumerateVisitor,

function_empty_lines.WrongEmptyLinesCountVisitor,

# Modules:
modules.EmptyModuleContentsVisitor,
modules.MagicModuleFunctionsVisitor,
Expand Down
4 changes: 4 additions & 0 deletions wemake_python_styleguide/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,3 +273,7 @@ def max_tuple_unpack_length(self) -> int:
@property
def show_violation_links(self) -> bool:
...

@property
def exps_for_one_empty_line(self) -> int:
...
47 changes: 47 additions & 0 deletions wemake_python_styleguide/violations/best_practices.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@
KwargsUnpackingInClassDefinitionViolation
ConsecutiveSlicesViolation
GettingElementByUnpackingViolation
WrongEmptyLinesCountViolation
Best practices
--------------
Expand Down Expand Up @@ -166,6 +167,7 @@
.. autoclass:: KwargsUnpackingInClassDefinitionViolation
.. autoclass:: ConsecutiveSlicesViolation
.. autoclass:: GettingElementByUnpackingViolation
.. autoclass:: WrongEmptyLinesCountViolation
"""

Expand Down Expand Up @@ -2770,3 +2772,48 @@ class GettingElementByUnpackingViolation(ASTViolation):
'Found unpacking used to get a single element from a collection'
)
code = 472


@final
class WrongEmptyLinesCountViolation(TokenizeViolation):
"""
Limit empty lines in functions or methods body.
Reasoning:
It's not holistic to have functions or methods that contain many
empty lines, and it makes sense to divide the method into several
ones.
Solution:
Limit count of empty lines of the function or method body
By default, we allow 1 empty line for 2 non-empty lines.
Example::
# Correct:
def func(name):
foo()
if name == 'Moonflower':
print('Love')
baz()
# Wrong:
def func(name):
foo()
if name == 'Moonflower':
print('Love')
baz()
Configuration:
This rule is configurable with ``--exps-for-one-empty-line``.
Default:
:str:`wemake_python_styleguide.options.defaults.EXPS_FOR_ONE_EMPTY_LINE`
.. versionadded:: 0.17.0
"""

error_template = 'Found too many empty lines in `def`: {0}'
code = 473
Loading

0 comments on commit 4ca670e

Please sign in to comment.