Skip to content

bpo-45727: Only trigger the 'did you forgot a comma' error suggestion if inside parentheses #29757

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
Nov 24, 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
3 changes: 2 additions & 1 deletion Grammar/python.gram
Original file line number Diff line number Diff line change
Expand Up @@ -1084,7 +1084,8 @@ invalid_expression:
# !(NAME STRING) is not matched so we don't show this error with some invalid string prefixes like: kf"dsfsdf"
# Soft keywords need to also be ignored because they can be parsed as NAME NAME
| !(NAME STRING | SOFT_KEYWORD) a=disjunction b=expression_without_invalid {
_PyPegen_check_legacy_stmt(p, a) ? NULL : RAISE_SYNTAX_ERROR_KNOWN_RANGE(a, b, "invalid syntax. Perhaps you forgot a comma?") }
_PyPegen_check_legacy_stmt(p, a) ? NULL : p->tokens[p->mark-1]->level == 0 ? NULL :
RAISE_SYNTAX_ERROR_KNOWN_RANGE(a, b, "invalid syntax. Perhaps you forgot a comma?") }
| a=disjunction 'if' b=disjunction !('else'|':') { RAISE_SYNTAX_ERROR_KNOWN_RANGE(a, b, "expected 'else' after 'if' expression") }

invalid_named_expression:
Expand Down
4 changes: 2 additions & 2 deletions Lib/test/test_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,14 +226,14 @@ def testSyntaxErrorOffset(self):
check(b'Python = "\xcf\xb3\xf2\xee\xed" +', 1, 18)
check('x = "a', 1, 5)
check('lambda x: x = 2', 1, 1)
check('f{a + b + c}', 1, 1)
check('f{a + b + c}', 1, 2)
check('[file for str(file) in []\n])', 1, 11)
check('a = « hello » « world »', 1, 5)
check('[\nfile\nfor str(file)\nin\n[]\n]', 3, 5)
check('[file for\n str(file) in []]', 2, 2)
check("ages = {'Alice'=22, 'Bob'=23}", 1, 16)
check('match ...:\n case {**rest, "key": value}:\n ...', 2, 19)
check("a b c d e f", 1, 1)
check("[a b c d e f]", 1, 2)

# Errors thrown by compile.c
check('class foo:return 1', 1, 11)
Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_fstring.py
Original file line number Diff line number Diff line change
Expand Up @@ -944,7 +944,7 @@ def test_invalid_string_prefixes(self):
"Bf''",
"BF''",]
double_quote_cases = [case.replace("'", '"') for case in single_quote_cases]
self.assertAllRaise(SyntaxError, 'unexpected EOF while parsing',
self.assertAllRaise(SyntaxError, 'invalid syntax',
single_quote_cases + double_quote_cases)

def test_leading_trailing_spaces(self):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Refine the custom syntax error that suggests that a comma may be missing to
trigger only when the expressions are detected between parentheses or
brackets. Patch by Pablo Galindo
2 changes: 1 addition & 1 deletion Parser/parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -18298,7 +18298,7 @@ invalid_expression_rule(Parser *p)
)
{
D(fprintf(stderr, "%*c+ invalid_expression[%d-%d]: %s succeeded!\n", p->level, ' ', _mark, p->mark, "!(NAME STRING | SOFT_KEYWORD) disjunction expression_without_invalid"));
_res = _PyPegen_check_legacy_stmt ( p , a ) ? NULL : RAISE_SYNTAX_ERROR_KNOWN_RANGE ( a , b , "invalid syntax. Perhaps you forgot a comma?" );
_res = _PyPegen_check_legacy_stmt ( p , a ) ? NULL : p -> tokens [p -> mark - 1] -> level == 0 ? NULL : RAISE_SYNTAX_ERROR_KNOWN_RANGE ( a , b , "invalid syntax. Perhaps you forgot a comma?" );
if (_res == NULL && PyErr_Occurred()) {
p->error_indicator = 1;
D(p->level--);
Expand Down
4 changes: 3 additions & 1 deletion Parser/pegen.c
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,8 @@ initialize_token(Parser *p, Token *token, const char *start, const char *end, in
return -1;
}

token->level = p->tok->level;

const char *line_start = token_type == STRING ? p->tok->multi_line_start : p->tok->line_start;
int lineno = token_type == STRING ? p->tok->first_lineno : p->tok->lineno;
int end_lineno = p->tok->lineno;
Expand Down Expand Up @@ -946,4 +948,4 @@ _PyPegen_run_parser_from_string(const char *str, int start_rule, PyObject *filen
error:
_PyTokenizer_Free(tok);
return result;
}
}
1 change: 1 addition & 0 deletions Parser/pegen.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ typedef struct _memo {
typedef struct {
int type;
PyObject *bytes;
int level;
int lineno, col_offset, end_lineno, end_col_offset;
Memo *memo;
} Token;
Expand Down
4 changes: 2 additions & 2 deletions Parser/pegen_errors.c
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ _Pypegen_set_syntax_error(Parser* p, Token* last_token) {
RAISE_SYNTAX_ERROR("error at start before reading any input");
}
// Parser encountered EOF (End of File) unexpectedtly
if (p->tok->done == E_EOF) {
if (last_token->type == ERRORTOKEN && p->tok->done == E_EOF) {
if (p->tok->level) {
raise_unclosed_parentheses_error(p);
} else {
Expand All @@ -422,4 +422,4 @@ _Pypegen_set_syntax_error(Parser* p, Token* last_token) {
// _PyPegen_tokenize_full_source_to_check_for_errors will override the existing
// generic SyntaxError we just raised if errors are found.
_PyPegen_tokenize_full_source_to_check_for_errors(p);
}
}