Skip to content

Commit c433986

Browse files
authored
gh-142236: Improve error location for missing comma in string concatenations (#142330)
1 parent a78f43b commit c433986

File tree

6 files changed

+49
-3
lines changed

6 files changed

+49
-3
lines changed

Grammar/python.gram

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1251,8 +1251,7 @@ invalid_expression:
12511251
# !(NAME STRING) is not matched so we don't show this error with some invalid string prefixes like: kf"dsfsdf"
12521252
# Soft keywords need to also be ignored because they can be parsed as NAME NAME
12531253
| !(NAME STRING | SOFT_KEYWORD) a=disjunction b=expression_without_invalid {
1254-
_PyPegen_check_legacy_stmt(p, a) ? NULL : p->tokens[p->mark-1]->level == 0 ? NULL :
1255-
RAISE_SYNTAX_ERROR_KNOWN_RANGE(a, b, "invalid syntax. Perhaps you forgot a comma?") }
1254+
_PyPegen_raise_error_for_missing_comma(p, a, b) }
12561255
| a=disjunction 'if' b=disjunction !('else'|':') { RAISE_SYNTAX_ERROR_KNOWN_RANGE(a, b, "expected 'else' after 'if' expression") }
12571256
| a=disjunction 'if' b=disjunction 'else' !expression {
12581257
RAISE_SYNTAX_ERROR_ON_NEXT_TOKEN("expected expression after 'else', but statement is given") }

Lib/test/test_syntax.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3336,6 +3336,20 @@ def test_multiline_compiler_error_points_to_the_end(self):
33363336
lineno=3
33373337
)
33383338

3339+
def test_multiline_string_concat_missing_comma_points_to_last_string(self):
3340+
# gh-142236: For multi-line string concatenations with a missing comma,
3341+
# the error should point to the last string, not the first.
3342+
self._check_error(
3343+
"print(\n"
3344+
' "line1"\n'
3345+
' "line2"\n'
3346+
' "line3"\n'
3347+
" x=1\n"
3348+
")",
3349+
"Perhaps you forgot a comma",
3350+
lineno=4, # Points to "line3", the last string
3351+
)
3352+
33393353
@support.cpython_only
33403354
def test_syntax_error_on_deeply_nested_blocks(self):
33413355
# This raises a SyntaxError, it used to raise a SystemError. Context
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Improve the "Perhaps you forgot a comma?" syntax error for multi-line string
2+
concatenations to point to the last string instead of the first, making it
3+
easier to locate where the comma is missing. Patch by Pablo Galindo.

Parser/action_helpers.c

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -947,6 +947,35 @@ _PyPegen_check_legacy_stmt(Parser *p, expr_ty name) {
947947
return 0;
948948
}
949949

950+
void *
951+
_PyPegen_raise_error_for_missing_comma(Parser *p, expr_ty a, expr_ty b)
952+
{
953+
// Don't raise for legacy statements like "print x" or "exec x"
954+
if (_PyPegen_check_legacy_stmt(p, a)) {
955+
return NULL;
956+
}
957+
// Only raise inside parentheses/brackets (level > 0)
958+
if (p->tokens[p->mark - 1]->level == 0) {
959+
return NULL;
960+
}
961+
// For multi-line expressions (like string concatenations), point to the
962+
// last line instead of the first for a more helpful error message.
963+
// Use a->col_offset as the starting column since all strings in the
964+
// concatenation typically share the same indentation.
965+
if (a->end_lineno > a->lineno) {
966+
return RAISE_ERROR_KNOWN_LOCATION(
967+
p, PyExc_SyntaxError, a->end_lineno, a->col_offset,
968+
a->end_lineno, a->end_col_offset,
969+
"invalid syntax. Perhaps you forgot a comma?"
970+
);
971+
}
972+
return RAISE_ERROR_KNOWN_LOCATION(
973+
p, PyExc_SyntaxError, a->lineno, a->col_offset,
974+
b->end_lineno, b->end_col_offset,
975+
"invalid syntax. Perhaps you forgot a comma?"
976+
);
977+
}
978+
950979
static ResultTokenWithMetadata *
951980
result_token_with_metadata(Parser *p, void *result, PyObject *metadata)
952981
{

Parser/parser.c

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Parser/pegen.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,7 @@ expr_ty _PyPegen_ensure_real(Parser *p, expr_ty);
358358
asdl_seq *_PyPegen_join_sequences(Parser *, asdl_seq *, asdl_seq *);
359359
int _PyPegen_check_barry_as_flufl(Parser *, Token *);
360360
int _PyPegen_check_legacy_stmt(Parser *p, expr_ty t);
361+
void *_PyPegen_raise_error_for_missing_comma(Parser *p, expr_ty a, expr_ty b);
361362
ResultTokenWithMetadata *_PyPegen_check_fstring_conversion(Parser *p, Token *, expr_ty t);
362363
ResultTokenWithMetadata *_PyPegen_setup_full_format_spec(Parser *, Token *, asdl_expr_seq *, int, int,
363364
int, int, PyArena *);

0 commit comments

Comments
 (0)