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

fix skipping of physical checks when file does not end in newline #961

Merged
merged 1 commit into from
Sep 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 14 additions & 5 deletions pycodestyle.py
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ def trailing_blank_lines(physical_line, lines, line_number, total_lines):
However the last line should end with a new line (warning W292).
"""
if line_number == total_lines:
stripped_last_line = physical_line.rstrip()
stripped_last_line = physical_line.rstrip('\r\n')
if physical_line and not stripped_last_line:
return 0, "W391 blank line at end of file"
if stripped_last_line == physical_line:
Expand Down Expand Up @@ -2125,21 +2125,30 @@ def generate_tokens(self):
self.report_error(1, 0, 'E902 %s' % self._io_error, readlines)
tokengen = tokenize.generate_tokens(self.readline)
try:
prev_physical = ''
asottile marked this conversation as resolved.
Show resolved Hide resolved
for token in tokengen:
if token[2][0] > self.total_lines:
return
self.noqa = token[4] and noqa(token[4])
self.maybe_check_physical(token)
self.maybe_check_physical(token, prev_physical)
yield token
prev_physical = token[4]
except (SyntaxError, tokenize.TokenError):
self.report_invalid_syntax()

def maybe_check_physical(self, token):
def maybe_check_physical(self, token, prev_physical):
Copy link
Member

Choose a reason for hiding this comment

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

We may need to replicate some of this logic to Flake8

Copy link
Member Author

Choose a reason for hiding this comment

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

indeed, I plan to work on that today!

Copy link
Member Author

Choose a reason for hiding this comment

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

here's the flake8 patch with essentially the same logic!

https://gitlab.com/pycqa/flake8/-/merge_requests/451

"""If appropriate for token, check current physical line(s)."""
# Called after every token, but act only on end of line.

# a newline token ends a single physical line.
if _is_eol_token(token):
# Obviously, a newline token ends a single physical line.
self.check_physical(token[4])
# if the file does not end with a newline, the NEWLINE
# token is inserted by the parser, but it does not contain
# the previous physical line in `token[4]`
if token[4] == '':
self.check_physical(prev_physical)
else:
self.check_physical(token[4])
asottile marked this conversation as resolved.
Show resolved Hide resolved
elif token[0] == tokenize.STRING and '\n' in token[1]:
# Less obviously, a string that contains newlines is a
# multiline string, either triple-quoted or with internal
Expand Down
5 changes: 5 additions & 0 deletions testsuite/W29.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ class Foo(object):
#: W291:2:35
'''multiline
string with trailing whitespace'''
#: W291 W292 noeol
x = 1
#: W191 W292 noeol
if False:
pass # indented with tabs
#: W292:1:36 noeol
# This line doesn't have a linefeed
#: W292:1:5 E225:1:2 noeol
Expand Down
2 changes: 1 addition & 1 deletion testsuite/W39.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
# Two additional empty lines


#: W391:4:1 W293:3:1 W293:4:1 noeol
#: W292:4:5 W293:3:1 W293:4:1 noeol
# The last lines contain space


Expand Down