FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-45249: Ensure the traceback module prints correctly syntax errors with ranges by pablogsal · Pull Request #28575 · python/cpython · GitHub

/ cpython Public

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.

Sign up for GitHub

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
pablogsal merged 1 commit into from
Sep 27, 2021
Conversation 6 Commits 1 Checks 0 Files changed
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension .py  (2) All 1 file type selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
13 changes: 13 additions & 0 deletions Lib/test/test_traceback.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
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
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
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.

blueyed Oct 10, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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).

pablogsal Oct 10, 2021 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

blueyed Oct 10, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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".

pablogsal Oct 10, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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
Back | FazBrowse Home | New Git URL