Skip to content

Commit

Permalink
Add PLU002: blank lines before return statement
Browse files Browse the repository at this point in the history
  • Loading branch information
sorenlind committed Oct 31, 2022
1 parent 7db6361 commit ee687e9
Show file tree
Hide file tree
Showing 12 changed files with 217 additions and 20 deletions.
54 changes: 38 additions & 16 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
[![black](https://img.shields.io/badge/code%20style-black-000000.svg)](https://github.com/psf/black)

Flake8-plus is a plugin for [Flake8](https://github.com/PyCQA/flake8) that detects
incorrect amounts of vertical whitespace before the first toplevel `import` statement.
By default, the plugin issues a warning if there are blank lines immediately preceding
the first toplevel `import`. The plugin can be configured to expect any number of blank
lines.
incorrect amounts of vertical whitespace before the first toplevel `import` statement
and before `return` statements. The plugin can be configured to expect any number of
blank lines. By default, the plugin expects no blank lines before both `import` and
`return`.

## Installation

Expand All @@ -30,28 +30,32 @@ $ flake8 --version

## Configuration

You can set the required number of blank lines before the first `import`. This can be
done from the command line:
You can set the required number of blank lines before the first `import` as well as the
number of blank lines required before a `return`. This can be done from the command
line:

```shell
$ flake8 --blanks-before-imports 1
$ flake8 --blanks-before-imports 1 --blanks-before-return 1
```

Or from one of the `setup.cfg`, `tox.ini`, or `.flake8` files:

```ini
[flake8]
blanks-before-imports=1
blanks-before-return=1
```

## Why no blank lines?

Neither Black, Flake8 nor Pylint enforces a specific number of blank lines preceding the
first `import` and consequently there seems to be no consensus or standard. The table
below shows the frequency of the number of blank lines before the first toplevel
`import` statement in the code bases for [Black](https://github.com/psf/black),
[Flake8](https://github.com/PyCQA/flake8) and [Pylint](https://github.com/PyCQA/pylint)
(as of October 2022).
### Before `import`

Neither [Black](https://github.com/psf/black), [Flake8](https://github.com/PyCQA/flake8)
nor [Pylint](https://github.com/PyCQA/pylint) enforces a specific number of blank lines
preceding the first `import` and consequently there seems to be no consensus or
standard. The table below shows the frequency of the number of blank lines before the
first toplevel `import` statement in the code bases for Black, Flake8 and Pylint (as of
October 2022).

| Package | Total files | 0 blanks | 1 blank | 2 blanks | Folder |
| ------- | ----------: | -------: | ------: | -------: | ------------- |
Expand All @@ -65,8 +69,26 @@ Pylint code does not consistently include module docstrings (thereby breaking
`pylint(missing-module-docstring)`). For that reason, and also because this is a Flake8
plugin, the plugin follows the style of Flake8 as the default.

### Before `return`

Neither Black, Flake8 nor Pylint enforces a specific number of blank lines preceding
`return`. However, they all use zero blank lines more frequently than they use any
other number of blanks. The table below shows the frequency of the number of blank
lines before a `return` statement in the code bases for Black, Flake8 and Pylint (as of
October 2022).

| Package | Total `return`s | 0 blanks | 1 blank | 2 blanks | Folder |
| ------- | --------------: | -------: | ------: | -------: | ------------- |
| Black | 618 | 544 | 74 | 0 | `src` |
| Flake8 | 174 | 155 | 19 | 0 | `src/flake8/` |
| Pylint | 1941 | 1852 | 89 | 0 | `pylint` |

Since zero blank lines is the style used most frequently, Flake8-plus uses that as that
as the default.

## Reported problems

| Code |  Description |
| ------ | ------------------------------------------------------- |
| PLU001 | "expected {} blank lines before first import, found {}" |
| Code |  Description |
| ------ | ----------------------------------------------------------- |
| PLU001 | "expected {} blank lines before first import, found {}" |
| PLU002 | "expected {} blank lines before return statement, found {}" |
10 changes: 9 additions & 1 deletion flake8_plus/config.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,24 @@
"""Config class."""
# pylint: disable=too-few-public-methods
from . import defaults


class Config:
"""Plugin configuration class."""

def __init__(self, blanks_before_imports: int):
def __init__(
self,
blanks_before_imports: int = defaults.BLANKS_BEFORE_IMPORTS,
blanks_before_return: int = defaults.BLANKS_BEFORE_RETURN,
):
"""
Initialize a `Configuration` instance.
Args:
blanks_before_imports (int): Number of blanks line expected before first
import statements.
blanks_before_return (int): Number of blanks line expected before return
statement.
"""
self.blanks_before_imports = blanks_before_imports
self.blanks_before_return = blanks_before_return
1 change: 1 addition & 0 deletions flake8_plus/defaults.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
"""Default values."""

BLANKS_BEFORE_IMPORTS = 0
BLANKS_BEFORE_RETURN = 0
14 changes: 13 additions & 1 deletion flake8_plus/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from .config import Config
from .version import VERSION
from .visitors.plu001_visitor import PLU001Visitor
from .visitors.plu002_visitor import PLU002Visitor


class Plugin:
Expand All @@ -19,6 +20,7 @@ class Plugin:
version = VERSION
visitors = [
PLU001Visitor,
PLU002Visitor,
]

def __init__(self, tree: ast.AST, lines: list[str]):
Expand Down Expand Up @@ -59,7 +61,17 @@ def add_options(option_manager: OptionManager) -> None: # pragma: no cover
"(Default: %(default)s)",
)

option_manager.add_option(
"--blanks-before-return",
type="int",
metavar="n",
default=defaults.BLANKS_BEFORE_RETURN,
parse_from_config=True,
help="Expected number of blank lines before return statement. "
"(Default: %(default)s)",
)

@classmethod
def parse_options(cls, options: Namespace) -> None: # pragma: no cover
"""Parse the custom configuration options given to flake8."""
cls.config = Config(options.blanks_before_imports)
cls.config = Config(options.blanks_before_imports, options.blanks_before_return)
81 changes: 81 additions & 0 deletions flake8_plus/visitors/plu002_visitor.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
"""Exception classes raised by various operations within pylint."""
# pylint: disable=too-few-public-methods
import ast
from typing import Any

from ..config import Config
from ..exceptions import MultipleStatementsError
from ..problem import Problem
from .base_visitor import BaseVisitor


class PLU002Problem(Problem):
"""Problem 002: Number of blank lines before return statement."""

code = "PLU002"
format_ = "expected {} blank lines before return statement, found {}"

def __init__( # pylint: disable=unused-argument
self,
line_number: int,
col_offset: int,
blanks_actual: int,
blanks_expected: int,
**kwargs: dict[str, Any],
):
"""
Initialize a `PLU002Problem` instance.
Args:
line_number (int): The line number.
col_offset (int): The column offset.
blanks_actual (int): Number of actual blanks before return statement.
blanks_expected (int): Number of expected blanks before return statement.
"""
message = PLU002Problem.format_.format(blanks_expected, blanks_actual)
super().__init__(line_number, col_offset, message)


class PLU002Visitor(BaseVisitor):
"""Visitor class for the PLU002 rule."""

def __init__(self, lines: list[str], config: Config):
"""
Initialize a PLU002Visitor instance.
Args:
lines (list[str]): The physical lines.
config (Config): Configuration instance for the plugin and visitor.
"""
self._last_end: int = 0
super().__init__(lines, config)

def visit(self, node: ast.AST) -> Any:
"""
Visit the specified node.
Args:
node (ast.AST): The node to visit.
Returns:
Any: The value returned by the base class `visit` method.
"""
self._process_node(node)
return super().visit(node)

def _process_node(self, node: ast.AST):
if isinstance(node, ast.Return):
try:
actual = self.compute_blanks_before(node)
except MultipleStatementsError:
return
if actual != self.config.blanks_before_return:
problem = PLU002Problem(
node.lineno,
node.col_offset,
actual,
self.config.blanks_before_return,
)
self.problems.append(problem)
elif hasattr(node, "end_lineno") and (node.end_lineno is not None):
self._last_end = node.end_lineno
2 changes: 2 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[tool.black]
extend-exclude="case_files"
2 changes: 0 additions & 2 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,8 @@ def run(self):
def _validate_version(tag: Optional[str], version: str) -> bool:
if not tag:
return version == "0.0.0"

if tag[0] != "v":
return False

return tag[1:] == version


Expand Down
3 changes: 3 additions & 0 deletions tests/case_files/plu002/1_blank.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
def some_func() -> int:

return 0
41 changes: 41 additions & 0 deletions tests/case_files/plu002/cases.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
[
{
"filename": "1_blank.py",
"cases": [
{
"expectation": { "blanks_expected": 0 },
"problems": [{ "line_number": 3, "col_offset": 4, "blanks_actual": 1 }]
},
{
"expectation": { "blanks_expected": 1 },
"problems": []
}
]
},
{
"filename": "no_blanks.py",
"cases": [
{
"expectation": { "blanks_expected": 0 },
"problems": []
},
{
"expectation": { "blanks_expected": 1 },
"problems": [{ "line_number": 2, "col_offset": 4, "blanks_actual": 0 }]
}
]
},
{
"filename": "print_return_same_line.py",
"cases": [
{
"expectation": { "blanks_expected": 0 },
"problems": []
},
{
"expectation": { "blanks_expected": 1 },
"problems": []
}
]
}
]
2 changes: 2 additions & 0 deletions tests/case_files/plu002/no_blanks.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
def some_func() -> int:
return 0
2 changes: 2 additions & 0 deletions tests/case_files/plu002/print_return_same_line.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
def something():
print("something");return
25 changes: 25 additions & 0 deletions tests/visitors/test_plu002_visitor.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
"""Tests for the `plu002_visitor` module."""
# pylint: disable=no-self-use,too-few-public-methods
import pytest

from flake8_plus.config import Config
from flake8_plus.visitors.plu002_visitor import PLU002Problem, PLU002Visitor

from .util import generate_bulk_cases, generate_results


class TestPLU002Visitor:
"""Tests for the `PLU002Visitor` class."""

@pytest.mark.parametrize(
("source_code", "expectation", "problems_expected"),
generate_bulk_cases(PLU002Problem),
)
def test_bulk(
self, source_code: str, expectation: dict[str, int], problems_expected: set[str]
):
"""Run bulk test cases."""
blanks_expected = expectation["blanks_expected"]
config = Config(blanks_before_return=blanks_expected)
actual = generate_results(PLU002Visitor, config, source_code)
assert actual == problems_expected

0 comments on commit ee687e9

Please sign in to comment.