Skip to content

Commit 15bc9ab

Browse files
gvanrossumgvanrossum
authored
bpo-40612: Fix SyntaxError edge cases in traceback formatting (pythonGH-20072)
This fixes both the traceback.py module and the C code for formatting syntax errors (in Python/pythonrun.c). They now both consistently do the following: - Suppress caret if it points left of text - Allow caret pointing just past end of line - If caret points past end of line, clip to *just* past end of line The syntax error formatting code in traceback.py was mostly rewritten; small, subtle changes were applied to the C code in pythonrun.c. There's still a difference when the text contains embedded newlines. Neither handles these very well, and I don't think the case occurs in practice. Automerge-Triggered-By: @gvanrossum
1 parent 1aa8767 commit 15bc9ab

5 files changed

Lines changed: 94 additions & 39 deletions

File tree

‎Lib/test/test_cmd_line_script.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -633,7 +633,7 @@ def test_syntaxerror_multi_line_fstring(self):
633633
stderr.splitlines()[-3:],
634634
[
635635
b' foo"""',
636-
b' ^',
636+
b' ^',
637637
b'SyntaxError: f-string: empty expression not allowed',
638638
],
639639
)

‎Lib/test/test_traceback.py‎

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,13 @@ def test_caret(self):
5858
SyntaxError)
5959
self.assertIn("^", err[2]) # third line has caret
6060
self.assertEqual(err[2].count('\n'), 1) # and no additional newline
61-
self.assertEqual(err[1].find("+"), err[2].find("^")) # in the right place
61+
self.assertEqual(err[1].find("+") + 1, err[2].find("^")) # in the right place
6262

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

6969
def test_nocaret(self):
7070
exc = SyntaxError("error", ("x.py", 23, None, "bad syntax"))
@@ -78,14 +78,13 @@ def test_bad_indentation(self):
7878
self.assertEqual(len(err), 4)
7979
self.assertEqual(err[1].strip(), "print(2)")
8080
self.assertIn("^", err[2])
81-
self.assertEqual(err[1].find(")"), err[2].find("^"))
81+
self.assertEqual(err[1].find(")") + 1, err[2].find("^"))
8282

83+
# No caret for "unexpected indent"
8384
err = self.get_exception_format(self.syntax_error_bad_indentation2,
8485
IndentationError)
85-
self.assertEqual(len(err), 4)
86+
self.assertEqual(len(err), 3)
8687
self.assertEqual(err[1].strip(), "print(2)")
87-
self.assertIn("^", err[2])
88-
self.assertEqual(err[1].find("p"), err[2].find("^"))
8988

9089
def test_base_exception(self):
9190
# Test that exceptions derived from BaseException are formatted right
@@ -656,7 +655,7 @@ def outer_raise():
656655
self.assertIn('inner_raise() # Marker', blocks[2])
657656
self.check_zero_div(blocks[2])
658657

659-
@support.skip_if_new_parser("Pegen is arguably better here, so no need to fix this")
658+
@unittest.skipIf(support.use_old_parser(), "Pegen is arguably better here, so no need to fix this")
660659
def test_syntax_error_offset_at_eol(self):
661660
# See #10186.
662661
def e():
@@ -666,7 +665,7 @@ def e():
666665
def e():
667666
exec("x = 5 | 4 |")
668667
msg = self.get_report(e).splitlines()
669-
self.assertEqual(msg[-2], ' ^')
668+
self.assertEqual(msg[-2], ' ^')
670669

671670
def test_message_none(self):
672671
# A message that looks like "None" should not be treated specially
@@ -679,6 +678,25 @@ def test_message_none(self):
679678
err = self.get_report(Exception(''))
680679
self.assertIn('Exception\n', err)
681680

681+
def test_syntax_error_various_offsets(self):
682+
for offset in range(-5, 10):
683+
for add in [0, 2]:
684+
text = " "*add + "text%d" % offset
685+
expected = [' File "file.py", line 1']
686+
if offset < 1:
687+
expected.append(" %s" % text.lstrip())
688+
elif offset <= 6:
689+
expected.append(" %s" % text.lstrip())
690+
expected.append(" %s^" % (" "*(offset-1)))
691+
else:
692+
expected.append(" %s" % text.lstrip())
693+
expected.append(" %s^" % (" "*5))
694+
expected.append("SyntaxError: msg")
695+
expected.append("")
696+
err = self.get_report(SyntaxError("msg", ("file.py", 1, offset+add, text)))
697+
exp = "\n".join(expected)
698+
self.assertEqual(exp, err)
699+
682700

683701
class PyExcReportingTests(BaseExceptionReportingTests, unittest.TestCase):
684702
#

‎Lib/traceback.py‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -569,23 +569,30 @@ def format_exception_only(self):
569569

570570
if not issubclass(self.exc_type, SyntaxError):
571571
yield _format_final_exc_line(stype, self._str)
572-
return
572+
else:
573+
yield from self._format_syntax_error(stype)
573574

574-
# It was a syntax error; show exactly where the problem was found.
575+
def _format_syntax_error(self, stype):
576+
"""Format SyntaxError exceptions (internal helper)."""
577+
# Show exactly where the problem was found.
575578
filename = self.filename or "<string>"
576579
lineno = str(self.lineno) or '?'
577580
yield ' File "{}", line {}\n'.format(filename, lineno)
578581

579-
badline = self.text
580-
offset = self.offset
581-
if badline is not None:
582-
yield ' {}\n'.format(badline.strip())
583-
if offset is not None:
584-
caretspace = badline.rstrip('\n')
585-
offset = min(len(caretspace), offset) - 1
586-
caretspace = caretspace[:offset].lstrip()
582+
text = self.text
583+
if text is not None:
584+
# text = " foo\n"
585+
# rtext = " foo"
586+
# ltext = "foo"
587+
rtext = text.rstrip('\n')
588+
ltext = rtext.lstrip(' \n\f')
589+
spaces = len(rtext) - len(ltext)
590+
yield ' {}\n'.format(ltext)
591+
# Convert 1-based column offset to 0-based index into stripped text
592+
caret = (self.offset or 0) - 1 - spaces
593+
if caret >= 0:
587594
# non-space whitespace (likes tabs) must be kept for alignment
588-
caretspace = ((c.isspace() and c or ' ') for c in caretspace)
595+
caretspace = ((c if c.isspace() else ' ') for c in ltext[:caret])
589596
yield ' {}^\n'.format(''.join(caretspace))
590597
msg = self.msg or "<no detail available>"
591598
yield "{}: {}\n".format(stype, msg)

‎Misc/NEWS.d/next/Library/2020-05-13-10-23-29.bpo-40612.gOIreM.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix edge cases in SyntaxError formatting. If the offset is <= 0, no caret is printed.
2+
If the offset is > line length, the caret is printed pointing just after the last character.

‎Python/pythonrun.c‎

Lines changed: 47 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -554,37 +554,65 @@ parse_syntax_error(PyObject *err, PyObject **message, PyObject **filename,
554554
static void
555555
print_error_text(PyObject *f, int offset, PyObject *text_obj)
556556
{
557-
const char *text;
558-
const char *nl;
559-
560-
text = PyUnicode_AsUTF8(text_obj);
557+
/* Convert text to a char pointer; return if error */
558+
const char *text = PyUnicode_AsUTF8(text_obj);
561559
if (text == NULL)
562560
return;
563561

564-
if (offset >= 0) {
565-
if (offset > 0 && (size_t)offset == strlen(text) && text[offset - 1] == '\n')
566-
offset--;
567-
for (;;) {
568-
nl = strchr(text, '\n');
569-
if (nl == NULL || nl-text >= offset)
570-
break;
571-
offset -= (int)(nl+1-text);
572-
text = nl+1;
562+
/* Convert offset from 1-based to 0-based */
563+
offset--;
564+
565+
/* Strip leading whitespace from text, adjusting offset as we go */
566+
while (*text == ' ' || *text == '\t' || *text == '\f') {
567+
text++;
568+
offset--;
569+
}
570+
571+
/* Calculate text length excluding trailing newline */
572+
Py_ssize_t len = strlen(text);
573+
if (len > 0 && text[len-1] == '\n') {
574+
len--;
575+
}
576+
577+
/* Clip offset to at most len */
578+
if (offset > len) {
579+
offset = len;
580+
}
581+
582+
/* Skip past newlines embedded in text */
583+
for (;;) {
584+
const char *nl = strchr(text, '\n');
585+
if (nl == NULL) {
586+
break;
573587
}
574-
while (*text == ' ' || *text == '\t' || *text == '\f') {
575-
text++;
576-
offset--;
588+
Py_ssize_t inl = nl - text;
589+
if (inl >= (Py_ssize_t)offset) {
590+
break;
577591
}
592+
inl += 1;
593+
text += inl;
594+
len -= inl;
595+
offset -= (int)inl;
578596
}
597+
598+
/* Print text */
579599
PyFile_WriteString(" ", f);
580600
PyFile_WriteString(text, f);
581-
if (*text == '\0' || text[strlen(text)-1] != '\n')
601+
602+
/* Make sure there's a newline at the end */
603+
if (text[len] != '\n') {
582604
PyFile_WriteString("\n", f);
583-
if (offset == -1)
605+
}
606+
607+
/* Don't print caret if it points to the left of the text */
608+
if (offset < 0)
584609
return;
610+
611+
/* Write caret line */
585612
PyFile_WriteString(" ", f);
586-
while (--offset > 0)
613+
while (--offset >= 0) {
587614
PyFile_WriteString(" ", f);
615+
}
588616
PyFile_WriteString("^\n", f);
589617
}
590618

0 commit comments

Comments
 (0)