bpo-45727: Make the syntax error for missing comma more consistent (p…

…ythonGH-29427)
dlaltu · Nov 19, 2021 · 546cefc · 546cefc
1 parent c8c21bd
commit 546cefc
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 24 deletions.
diff --git a/Grammar/python.gram b/Grammar/python.gram
@@ -618,6 +618,7 @@ expressions[expr_ty]:
 
 expression[expr_ty] (memo):
     | invalid_expression
+    | invalid_legacy_expression
     | a=disjunction 'if' b=disjunction 'else' c=expression { _PyAST_IfExp(b, a, c, EXTRA) }
     | disjunction
     | lambdef
@@ -1080,11 +1081,10 @@ invalid_legacy_expression:
             "Missing parentheses in call to '%U'. Did you mean %U(...)?", a->v.Name.id, a->v.Name.id) : NULL}
 
 invalid_expression:
-   | invalid_legacy_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 {
-        RAISE_SYNTAX_ERROR_KNOWN_RANGE(a, b, "invalid syntax. Perhaps you forgot a comma?") }
+        _PyPegen_check_legacy_stmt(p, a) ? 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:

diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py
@@ -233,6 +233,7 @@ def testSyntaxErrorOffset(self):
         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)
 
         # Errors thrown by compile.c
         check('class foo:return 1', 1, 11)

diff --git a/Parser/parser.c b/Parser/parser.c
@@ -9719,6 +9719,7 @@ expressions_rule(Parser *p)
 
 // expression:
 //     | invalid_expression
+//     | invalid_legacy_expression
 //     | disjunction 'if' disjunction 'else' expression
 //     | disjunction
 //     | lambdef
@@ -9764,6 +9765,25 @@ expression_rule(Parser *p)
         D(fprintf(stderr, "%*c%s expression[%d-%d]: %s failed!\n", p->level, ' ',
                   p->error_indicator ? "ERROR!" : "-", _mark, p->mark, "invalid_expression"));
     }
+    if (p->call_invalid_rules) { // invalid_legacy_expression
+        if (p->error_indicator) {
+            D(p->level--);
+            return NULL;
+        }
+        D(fprintf(stderr, "%*c> expression[%d-%d]: %s\n", p->level, ' ', _mark, p->mark, "invalid_legacy_expression"));
+        void *invalid_legacy_expression_var;
+        if (
+            (invalid_legacy_expression_var = invalid_legacy_expression_rule(p))  // invalid_legacy_expression
+        )
+        {
+            D(fprintf(stderr, "%*c+ expression[%d-%d]: %s succeeded!\n", p->level, ' ', _mark, p->mark, "invalid_legacy_expression"));
+            _res = invalid_legacy_expression_var;
+            goto done;
+        }
+        p->mark = _mark;
+        D(fprintf(stderr, "%*c%s expression[%d-%d]: %s failed!\n", p->level, ' ',
+                  p->error_indicator ? "ERROR!" : "-", _mark, p->mark, "invalid_legacy_expression"));
+    }
     { // disjunction 'if' disjunction 'else' expression
         if (p->error_indicator) {
             D(p->level--);
@@ -18239,7 +18259,6 @@ invalid_legacy_expression_rule(Parser *p)
 }
 
 // invalid_expression:
-//     | invalid_legacy_expression
 //     | !(NAME STRING | SOFT_KEYWORD) disjunction expression_without_invalid
 //     | disjunction 'if' disjunction !('else' | ':')
 static void *
@@ -18252,25 +18271,6 @@ invalid_expression_rule(Parser *p)
     }
     void * _res = NULL;
     int _mark = p->mark;
-    if (p->call_invalid_rules) { // invalid_legacy_expression
-        if (p->error_indicator) {
-            D(p->level--);
-            return NULL;
-        }
-        D(fprintf(stderr, "%*c> invalid_expression[%d-%d]: %s\n", p->level, ' ', _mark, p->mark, "invalid_legacy_expression"));
-        void *invalid_legacy_expression_var;
-        if (
-            (invalid_legacy_expression_var = invalid_legacy_expression_rule(p))  // invalid_legacy_expression
-        )
-        {
-            D(fprintf(stderr, "%*c+ invalid_expression[%d-%d]: %s succeeded!\n", p->level, ' ', _mark, p->mark, "invalid_legacy_expression"));
-            _res = invalid_legacy_expression_var;
-            goto done;
-        }
-        p->mark = _mark;
-        D(fprintf(stderr, "%*c%s invalid_expression[%d-%d]: %s failed!\n", p->level, ' ',
-                  p->error_indicator ? "ERROR!" : "-", _mark, p->mark, "invalid_legacy_expression"));
-    }
     { // !(NAME STRING | SOFT_KEYWORD) disjunction expression_without_invalid
         if (p->error_indicator) {
             D(p->level--);
@@ -18288,7 +18288,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 = RAISE_SYNTAX_ERROR_KNOWN_RANGE ( a , b , "invalid syntax. Perhaps you forgot a comma?" );
+            _res = _PyPegen_check_legacy_stmt ( p , a ) ? 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--);

diff --git a/Parser/pegen.c b/Parser/pegen.c
@@ -79,7 +79,9 @@ _PyPegen_check_barry_as_flufl(Parser *p, Token* t) {
 
 int
 _PyPegen_check_legacy_stmt(Parser *p, expr_ty name) {
-    assert(name->kind == Name_kind);
+    if (name->kind != Name_kind) {
+        return 0;
+    }
     const char* candidates[2] = {"print", "exec"};
     for (int i=0; i<2; i++) {
         if (PyUnicode_CompareWithASCIIString(name->v.Name.id, candidates[i]) == 0) {