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 syntax error reporting from stdin (#357) #716

Merged
merged 1 commit into from Jul 19, 2022
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
9 changes: 1 addition & 8 deletions pyflakes/api.py
Expand Up @@ -55,14 +55,7 @@ def check(codeString, filename, reporter=None):
text = None
offset -= 1

# If there's an encoding problem with the file, the text is None.
if text is None:
# Avoid using msg, since for the only known case, it contains a
# bogus message that claims the encoding the file declared was
# unknown.
reporter.unexpectedError(filename, 'problem decoding source')
else:
reporter.syntaxError(filename, msg, lineno, offset, text)
reporter.syntaxError(filename, msg, lineno, offset, text)
return 1
except Exception:
reporter.unexpectedError(filename, 'problem decoding source')
Expand Down
25 changes: 18 additions & 7 deletions pyflakes/reporter.py
Expand Up @@ -51,19 +51,30 @@ def syntaxError(self, filename, msg, lineno, offset, text):
@param text: The source code containing the syntax error.
@ptype text: C{unicode}
"""
line = text.splitlines()[-1]
if text is None:
line = None
else:
line = text.splitlines()[-1]

# lineno might be 0 if the error came from stdin
lineno = max(lineno, 1)

if offset is not None:
if sys.version_info < (3, 8):
if sys.version_info < (3, 8) and text is not None:
offset = offset - (len(text) - len(line)) + 1
# some versions of python emit an offset of -1 for certain encoding errors
offset = max(offset, 1)
self._stderr.write('%s:%d:%d: %s\n' %
(filename, lineno, offset, msg))
else:
self._stderr.write('%s:%d: %s\n' % (filename, lineno, msg))
self._stderr.write(line)
self._stderr.write('\n')
if offset is not None:
self._stderr.write(re.sub(r'\S', ' ', line[:offset - 1]) +
"^\n")

if line is not None:
self._stderr.write(line)
self._stderr.write('\n')
if offset is not None:
self._stderr.write(re.sub(r'\S', ' ', line[:offset - 1]) +
"^\n")

def flake(self, message):
"""
Expand Down
52 changes: 51 additions & 1 deletion pyflakes/test/test_api.py
Expand Up @@ -15,6 +15,7 @@
from pyflakes.reporter import Reporter
from pyflakes.api import (
main,
check,
checkPath,
checkRecursive,
iterSourceCode,
Expand Down Expand Up @@ -255,6 +256,17 @@ def test_syntaxErrorNoOffset(self):
"bad line of source\n"),
err.getvalue())

def test_syntaxErrorNoText(self):
"""
C{syntaxError} doesn't include text or nonsensical offsets if C{text} is C{None}.

This typically happens when reporting syntax errors from stdin.
"""
err = io.StringIO()
reporter = Reporter(None, err)
reporter.syntaxError('<stdin>', 'a problem', 0, 0, None)
self.assertEqual(("<stdin>:1:1: a problem\n"), err.getvalue())

def test_multiLineSyntaxError(self):
"""
If there's a multi-line syntax error, then we only report the last
Expand Down Expand Up @@ -606,7 +618,8 @@ def test_misencodedFileUTF8(self):
""" % SNOWMAN).encode('utf-8')
with self.makeTempFile(source) as sourcePath:
self.assertHasErrors(
sourcePath, [f"{sourcePath}: problem decoding source\n"])
sourcePath,
[f"{sourcePath}:1:1: 'ascii' codec can't decode byte 0xe2 in position 21: ordinal not in range(128)\n"]) # noqa: E501

def test_misencodedFileUTF16(self):
"""
Expand Down Expand Up @@ -648,6 +661,43 @@ def test_checkRecursive(self):
finally:
shutil.rmtree(tempdir)

def test_stdinReportsErrors(self):
"""
L{check} reports syntax errors from stdin
"""
source = "max(1 for i in range(10), key=lambda x: x+1)\n"
err = io.StringIO()
count = withStderrTo(err, check, source, "<stdin>")
self.assertEqual(count, 1)
errlines = err.getvalue().split("\n")[:-1]
Comment on lines +664 to +672
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this actually tests the behaviour -- and we already have a SyntaxError test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The behavior is triggered when ast.parse throws a SyntaxError from code with a filepath that doesn't exist (stdin).

I'll see if I can trigger this with a different syntax error that has more consistent messaging to avoid the rats nest of expected messages by version. The offset behavior is a bit concerning though. It looks like pypy has special handling that is objectively wrong in certain situations (such as this one).

Copy link
Member

Choose a reason for hiding this comment

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

pypy is usually pretty attentive to fixes for syntax error offsets (with a little help from me they inspired the improvements to cpython 3.10's error messages)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I meant pyflakes handling of pypy's offsets as here:

pyflakes/pyflakes/api.py

Lines 46 to 56 in 44ef321

if checker.PYPY:
if text is None:
lines = codeString.splitlines()
if len(lines) >= lineno:
text = lines[lineno - 1]
if isinstance(text, bytes):
try:
text = text.decode('ascii')
except UnicodeDecodeError:
text = None
offset -= 1

It's probably something that used to be an issue with pypy and was fixed since on the other end. I tested with pypy3.7-7.3.9, but tracking down which version of pypy changed offsets in syntax errors is out of scope for here.

Copy link
Member

Choose a reason for hiding this comment

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

oh yeah we should totally kill that -- forgot pyflakes had special cases (probably from like 2015-era pypy)


if PYPY:
expected_error = [
"<stdin>:1:3: Generator expression must be parenthesized if not sole argument", # noqa: E501
"max(1 for i in range(10), key=lambda x: x+1)",
" ^",
]
elif sys.version_info >= (3, 9):
expected_error = [
"<stdin>:1:5: Generator expression must be parenthesized",
"max(1 for i in range(10), key=lambda x: x+1)",
" ^",
]
elif sys.version_info >= (3, 8):
expected_error = [
"<stdin>:1:5: Generator expression must be parenthesized",
]
elif sys.version_info >= (3, 7):
expected_error = [
"<stdin>:1:4: Generator expression must be parenthesized",
]
elif sys.version_info >= (3, 6):
expected_error = [
"<stdin>:1:4: Generator expression must be parenthesized if not sole argument", # noqa: E501
]

self.assertEqual(errlines, expected_error)


class IntegrationTests(TestCase):
"""
Expand Down