Skip to content

Require two blank lines after toplevel def/class; issue #400 #402

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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion pep8.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,8 @@ def maximum_line_length(physical_line, max_line_length, multiline):


def blank_lines(logical_line, blank_lines, indent_level, line_number,
blank_before, previous_logical, previous_indent_level):
blank_before, previous_logical, previous_logical_toplevel,
previous_indent_level):
r"""Separate top-level function and class definitions with two blank lines.

Method definitions inside a class are separated by a single blank line.
Expand All @@ -253,6 +254,7 @@ def blank_lines(logical_line, blank_lines, indent_level, line_number,
E303: def a():\n pass\n\n\n\ndef b(n):\n pass
E303: def a():\n\n\n\n pass
E304: @decorator\n\ndef a():\n pass
E305: def a():\n pass\na()
"""
if line_number < 3 and not previous_logical:
return # Don't expect blank lines before the first line
Expand All @@ -268,6 +270,9 @@ def blank_lines(logical_line, blank_lines, indent_level, line_number,
yield 0, "E301 expected 1 blank line, found 0"
elif blank_before != 2:
yield 0, "E302 expected 2 blank lines, found %d" % blank_before
elif (logical_line and not indent_level and blank_before != 2 and
previous_logical_toplevel.startswith(('def', 'class'))):
yield 0, "E305 expected 2 blank lines before, found %d" % blank_before
Copy link

Choose a reason for hiding this comment

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

Shouldn't that expect 2 blank lines after the def or class? Sure it's checking for the next logical line if before it there are two lines but only because the block above was a def or class.

Maybe also mention that it expects two lines after a method-level function or class (I know E302 doesn't mention that too but that may be improved too?).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm? I'm pretty sure the condition is right: it's looking for a non-indented line that's preceded by more or less than two blank lines where the preceding non-blank non-indented line was a function or class definition.

It doesn't actually expect two lines after a method-level function or class, since those are only supposed to have one separating blank line according to PEP 8.

Copy link

Choose a reason for hiding this comment

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

The condition is right but I find the message confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, agreed! I'll defer that to another diff, though.



def extraneous_whitespace(logical_line):
Expand Down Expand Up @@ -1301,6 +1306,8 @@ def filename_match(filename, patterns, default=True):

def _is_eol_token(token):
return token[0] in NEWLINE or token[4][token[3][1]:].lstrip() == '\\\n'


if COMMENT_WITH_NL:
def _is_eol_token(token, _eol_token=_is_eol_token):
return _eol_token(token) or (token[0] == tokenize.COMMENT and
Expand Down Expand Up @@ -1340,6 +1347,8 @@ def init_checks_registry():
mod = inspect.getmodule(register_check)
for (name, function) in inspect.getmembers(mod, inspect.isfunction):
register_check(function)


init_checks_registry()


Expand Down Expand Up @@ -1497,6 +1506,8 @@ def check_logical(self):
if self.logical_line:
self.previous_indent_level = self.indent_level
self.previous_logical = self.logical_line
if not self.indent_level:
self.previous_logical_toplevel = self.logical_line
self.blank_lines = 0
self.tokens = []

Expand Down Expand Up @@ -1566,6 +1577,7 @@ def check_all(self, expected=None, line_offset=0):
self.indent_char = None
self.indent_level = self.previous_indent_level = 0
self.previous_logical = ''
self.previous_logical_toplevel = ''
self.tokens = []
self.blank_lines = self.blank_before = 0
parens = 0
Expand Down Expand Up @@ -2116,5 +2128,6 @@ def _main():
sys.stderr.write(str(report.total_errors) + '\n')
sys.exit(1)


if __name__ == '__main__':
_main()
3 changes: 3 additions & 0 deletions testsuite/E12not.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ def long_function_name(
var_four):
print(var_one)


if ((row < 0 or self.moduleCount <= row or
col < 0 or self.moduleCount <= col)):
raise Exception("%s,%s - %s" % (row, col, self.moduleCount))
Expand Down Expand Up @@ -400,6 +401,7 @@ def unicode2html(s):
.replace('"', '&#34;')
.replace('\n', '<br>\n'))


#
parser.add_option('--count', action='store_true',
help="print total number of errors and warnings "
Expand Down Expand Up @@ -616,6 +618,7 @@ def other_example():
for key, val in node.items()
))]


foo([
'bug'
])
Expand Down
1 change: 1 addition & 0 deletions testsuite/E22.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ def halves(n):
def squares(n):
return (i**2 for i in range(n))


ENG_PREFIXES = {
-6: "\u03bc", # Greek letter mu
-3: "m",
Expand Down
29 changes: 29 additions & 0 deletions testsuite/E30.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,32 @@ def function():
It gives error E303: too many blank lines (3)
"""
#:

#: E305:7:1
def a():
print

# comment

# another comment
a()
#: E305:8:1
def a():
print

# comment

# another comment

try:
a()
except:
pass
#: E305:5:1
def a():
print

# Two spaces before comments, too.
if a():
a()
#:
1 change: 1 addition & 0 deletions testsuite/support.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,5 +196,6 @@ def run_tests(style):
init_tests(style)
return style.check_files()


# nose should not collect these functions
init_tests.__test__ = run_tests.__test__ = False
1 change: 1 addition & 0 deletions testsuite/test_all.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,6 @@ def suite():
def _main():
return unittest.TextTestRunner(verbosity=2).run(suite())


if __name__ == '__main__':
sys.exit(not _main())