bpo-40334: Correctly identify invalid target in assignment errors

python · pablogsal · May 15, 2020 · May 13, 2020 · May 13, 2020 · May 14, 2020
commit 445c7fd4b7e5bb4ea78b445968a893a566538914
diff --git a/Grammar/python.gram b/Grammar/python.gram
@@ -641,7 +641,10 @@ invalid_assignment:
    | a=expression ':' expression ['=' annotated_rhs] {
        RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, "illegal target for annotation") }
    | a=expression ('=' | augassign) (yield_expr | star_expressions) {
-        RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, "cannot assign to %s", _PyPegen_get_expr_name(a)) }
+        RAISE_SYNTAX_ERROR_KNOWN_LOCATION( 
+            _PyPegen_get_invalid_target(a), 
+            "cannot assign to %s", _PyPegen_get_expr_name(_PyPegen_get_invalid_target(a))
+        )}
 invalid_block:
    | NEWLINE !INDENT { RAISE_INDENTATION_ERROR("expected an indented block") }
 invalid_comprehension:

diff --git a/Lib/test/test_foo.py b/Lib/test/test_foo.py
@@ -0,0 +1,16 @@
+import unittest
+import zoneinfo
+import tracemalloc
+
+class Test(unittest.TestCase):
+    def test_foo(self):
+        # snapshot1 = tracemalloc.take_snapshot()
+        zoneinfo.ZoneInfo.no_cache("America/Los_Angeles")
+        # snapshot2 = tracemalloc.take_snapshot()
+        # top_stats = snapshot2.compare_to(snapshot1, 'lineno')
+
+        # print("[ Top 10 differences ]")
+        # for stat in top_stats[:10]:
+        #     print(stat)
+
+
diff --git a/Lib/test/test_syntax.py b/Lib/test/test_syntax.py
@@ -100,30 +100,29 @@
 This test just checks a couple of cases rather than enumerating all of
 them.

-# All of the following also produce different error messages with pegen
-# >>> (a, "b", c) = (1, 2, 3)
-# Traceback (most recent call last):
-# SyntaxError: cannot assign to literal
+>>> (a, "b", c) = (1, 2, 3)
+Traceback (most recent call last):
+SyntaxError: cannot assign to literal

-# >>> (a, True, c) = (1, 2, 3)
-# Traceback (most recent call last):
-# SyntaxError: cannot assign to True
+>>> (a, True, c) = (1, 2, 3)
+Traceback (most recent call last):
+SyntaxError: cannot assign to True

 >>> (a, __debug__, c) = (1, 2, 3)
 Traceback (most recent call last):
 SyntaxError: cannot assign to __debug__

-# >>> (a, *True, c) = (1, 2, 3)
-# Traceback (most recent call last):
-# SyntaxError: cannot assign to True
+>>> (a, *True, c) = (1, 2, 3)
+Traceback (most recent call last):
+SyntaxError: cannot assign to True

 >>> (a, *__debug__, c) = (1, 2, 3)
 Traceback (most recent call last):
 SyntaxError: cannot assign to __debug__

-# >>> [a, b, c + 1] = [1, 2, 3]
-# Traceback (most recent call last):
-# SyntaxError: cannot assign to operator
+>>> [a, b, c + 1] = [1, 2, 3]
+Traceback (most recent call last):
+SyntaxError: cannot assign to operator

 >>> a if 1 else b = 1
 Traceback (most recent call last):

diff --git a/Parser/pegen/parse.c b/Parser/pegen/parse.c
@@ -10853,7 +10853,7 @@ invalid_assignment_rule(Parser *p)
            (_tmp_129_var = _tmp_129_rule(p))  // yield_expr | star_expressions
        )
        {
-            _res = RAISE_SYNTAX_ERROR_KNOWN_LOCATION ( a , "cannot assign to %s" , _PyPegen_get_expr_name ( a ) );
+            _res = RAISE_SYNTAX_ERROR_KNOWN_LOCATION ( _PyPegen_get_invalid_target ( a ) , "cannot assign to %s" , _PyPegen_get_expr_name ( _PyPegen_get_invalid_target ( a ) ) );
            if (_res == NULL && PyErr_Occurred()) {
                p->error_indicator = 1;
                return NULL;

diff --git a/Parser/pegen/pegen.c b/Parser/pegen/pegen.c
@@ -2054,3 +2054,42 @@ _PyPegen_make_module(Parser *p, asdl_seq *a) {
    }
    return Module(a, type_ignores, p->arena);
 }
+
+// Error reporting helpers
+
+expr_ty
+_PyPegen_get_invalid_target(expr_ty e)
+{
+    if (e == NULL) {
+        return NULL;
+    }
+    switch (e->kind) {
+        case List_kind: {
+            Py_ssize_t len = asdl_seq_LEN(e->v.List.elts);
+            for (Py_ssize_t i = 0; i < len; i++) {
+                expr_ty other = asdl_seq_GET(e->v.List.elts, i);
+                if (_PyPegen_get_invalid_target(other)) {
+                    return other;
+                }
+            }
+            return NULL;
+        }
+        case Tuple_kind: {
+            Py_ssize_t len = asdl_seq_LEN(e->v.Tuple.elts);
+            for (Py_ssize_t i = 0; i < len; i++) {
+                expr_ty other = asdl_seq_GET(e->v.Tuple.elts, i);
+                expr_ty child = _PyPegen_get_invalid_target(other);
+                if (child != NULL) {
+                    return child;
+                }
+            }
+            return NULL;
+        }
+        case Starred_kind:
+            return _PyPegen_get_invalid_target(e->v.Starred.value);
+        case Name_kind:
+            return NULL;
+        default:
+            return e;
+    }
+}
diff --git a/Parser/pegen/pegen.h b/Parser/pegen/pegen.h
@@ -260,6 +260,11 @@ void *_PyPegen_arguments_parsing_error(Parser *, expr_ty);
 int _PyPegen_check_barry_as_flufl(Parser *);
 mod_ty _PyPegen_make_module(Parser *, asdl_seq *);

+// Error reporting helpers
+
+expr_ty _PyPegen_get_invalid_target(expr_ty e);
+
+
 void *_PyPegen_parse(Parser *);

 #endif