Skip to content

bpo-45249: Ensure the traceback module prints correctly syntax errors with ranges #28575

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

Merged
merged 1 commit into from
Sep 27, 2021
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions Lib/test/test_traceback.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ def syntax_error_with_caret(self):
def syntax_error_with_caret_2(self):
compile("1 +\n", "?", "exec")

def syntax_error_with_caret_range(self):
compile("f(x, y for y in range(30), z)", "?", "exec")

def syntax_error_bad_indentation(self):
compile("def spam():\n print(1)\n print(2)", "?", "exec")

Expand All @@ -59,18 +62,28 @@ def test_caret(self):
self.assertTrue(err[1].strip() == "return x!")
self.assertIn("^", err[2]) # third line has caret
self.assertEqual(err[1].find("!"), err[2].find("^")) # in the right place
self.assertEqual(err[2].count("^"), 1)

err = self.get_exception_format(self.syntax_error_with_caret_2,
SyntaxError)
self.assertIn("^", err[2]) # third line has caret
self.assertEqual(err[2].count('\n'), 1) # and no additional newline
self.assertEqual(err[1].find("+") + 1, err[2].find("^")) # in the right place
self.assertEqual(err[2].count("^"), 1)

err = self.get_exception_format(self.syntax_error_with_caret_non_ascii,
SyntaxError)
self.assertIn("^", err[2]) # third line has caret
self.assertEqual(err[2].count('\n'), 1) # and no additional newline
self.assertEqual(err[1].find("+") + 1, err[2].find("^")) # in the right place
self.assertEqual(err[2].count("^"), 1)

err = self.get_exception_format(self.syntax_error_with_caret_range,
SyntaxError)
self.assertIn("^", err[2]) # third line has caret
self.assertEqual(err[2].count('\n'), 1) # and no additional newline
self.assertEqual(err[1].find("y"), err[2].find("^")) # in the right place
self.assertEqual(err[2].count("^"), len("y for y in range(30)"))

def test_nocaret(self):
exc = SyntaxError("error", ("x.py", 23, None, "bad syntax"))
Expand Down
27 changes: 21 additions & 6 deletions Lib/traceback.py
Original file line number Diff line number Diff line change
Expand Up @@ -622,10 +622,14 @@ class TracebackException:
occurred.
- :attr:`lineno` For syntax errors - the linenumber where the error
occurred.
- :attr:`end_lineno` For syntax errors - the end linenumber where the error
occurred. Can be `None` if not present.
- :attr:`text` For syntax errors - the text where the error
occurred.
- :attr:`offset` For syntax errors - the offset into the text where the
error occurred.
- :attr:`end_offset` For syntax errors - the offset into the text where the
error occurred. Can be `None` if not present.
Copy link
Contributor

Choose a reason for hiding this comment

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

It is not None, but an unset attribute for non-SyntaxErrors, since it gets set for if exc_type and issubclass(exc_type, SyntaxError) only.
I think it should either get initialized always (to None), or at least the doc should get fixed (both places above).

Copy link
Member Author

@pablogsal pablogsal Oct 10, 2021

Choose a reason for hiding this comment

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

I think it should either get initialized always (to None), or at least the doc should get fixed (both places above).

I disagree. The text is the same as for other syntax errors fields. It says:

attr:end_offset For syntax errors

the same way offset is also for syntax errors:

:attr:offset For syntax errors

The thing here is that end_offset can be None if not present even for syntax errors.

- :attr:`msg` For syntax errors - the compiler error message.
"""

Expand Down Expand Up @@ -655,8 +659,11 @@ def __init__(self, exc_type, exc_value, exc_traceback, *, limit=None,
self.filename = exc_value.filename
lno = exc_value.lineno
self.lineno = str(lno) if lno is not None else None
end_lno = exc_value.end_lineno
self.end_lineno = str(end_lno) if end_lno is not None else None
self.text = exc_value.text
self.offset = exc_value.offset
self.end_offset = exc_value.end_offset
self.msg = exc_value.msg
if lookup_lines:
self._load_lines()
Expand Down Expand Up @@ -771,12 +778,20 @@ def _format_syntax_error(self, stype):
ltext = rtext.lstrip(' \n\f')
spaces = len(rtext) - len(ltext)
yield ' {}\n'.format(ltext)
# Convert 1-based column offset to 0-based index into stripped text
caret = (self.offset or 0) - 1 - spaces
if caret >= 0:
# non-space whitespace (likes tabs) must be kept for alignment
caretspace = ((c if c.isspace() else ' ') for c in ltext[:caret])
yield ' {}^\n'.format(''.join(caretspace))

if self.offset is not None:
offset = self.offset
end_offset = self.end_offset if self.end_offset is not None else offset
if offset == end_offset or end_offset == -1:
end_offset = offset + 1
Copy link
Contributor

Choose a reason for hiding this comment

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

I've noticed that self.end_offset is 0 with https://bugs.python.org/msg403588 (https://bugs.python.org/issue45249).
This might boil down to / coming from the C code using -1 for "not given/used", and then 1 being added, and therefore 0 should also get handled as "no end offset provided, use a length of 1".

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed in #28855


# Convert 1-based column offset to 0-based index into stripped text
colno = offset - 1 - spaces
end_colno = end_offset - 1 - spaces
if colno >= 0:
# non-space whitespace (likes tabs) must be kept for alignment
caretspace = ((c if c.isspace() else ' ') for c in ltext[:colno])
yield ' {}{}'.format("".join(caretspace), ('^' * (end_colno - colno) + "\n"))
msg = self.msg or "<no detail available>"
yield "{}: {}{}\n".format(stype, msg, filename_suffix)

Expand Down