bpo-34013: Move the Python 2 hints from the exception constructor to the parser (GH-27392)

Author: pablogsal

Grammar/python.gram

@@ -848,9 +848,10 @@ expression_without_invalid[expr_ty]:
     | disjunction
     | lambdef
 invalid_legacy_expression:
-    | a=NAME b=expression_without_invalid {
-        _PyPegen_check_legacy_stmt(p, a) ? RAISE_SYNTAX_ERROR_KNOWN_RANGE(a, b, "Missing parentheses in call to '%U'.", a->v.Name.id) : NULL}
- 
+    | a=NAME b=star_expressions {
+        _PyPegen_check_legacy_stmt(p, a) ? RAISE_SYNTAX_ERROR_KNOWN_RANGE(a, b,
+            "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"

Lib/test/test_exceptions.py

@@ -168,21 +168,19 @@ def ckmsg(src, msg, exception=SyntaxError):
                 self.fail("failed to get expected SyntaxError")
 
         s = '''print "old style"'''
-        ckmsg(s, "Missing parentheses in call to 'print'. "
-                 "Did you mean print(\"old style\")?")
+        ckmsg(s, "Missing parentheses in call to 'print'. Did you mean print(...)?")
 
         s = '''print "old style",'''
-        ckmsg(s, "Missing parentheses in call to 'print'. "
-                 "Did you mean print(\"old style\", end=\" \")?")
+        ckmsg(s, "Missing parentheses in call to 'print'. Did you mean print(...)?")
 
         s = 'print f(a+b,c)'
-        ckmsg(s, "Missing parentheses in call to 'print'.")
+        ckmsg(s, "Missing parentheses in call to 'print'. Did you mean print(...)?")
 
         s = '''exec "old style"'''
-        ckmsg(s, "Missing parentheses in call to 'exec'")
+        ckmsg(s, "Missing parentheses in call to 'exec'. Did you mean exec(...)?")
 
         s = 'exec f(a+b,c)'
-        ckmsg(s, "Missing parentheses in call to 'exec'.")
+        ckmsg(s, "Missing parentheses in call to 'exec'. Did you mean exec(...)?")
 
         # should not apply to subclasses, see issue #31161
         s = '''if True:\nprint "No indent"'''

Lib/test/test_print.py

@@ -140,21 +140,24 @@ def test_normal_string(self):
         with self.assertRaises(SyntaxError) as context:
             exec(python2_print_str)
 
-        self.assertIn('print("Hello World")', str(context.exception))
+        self.assertIn("Missing parentheses in call to 'print'. Did you mean print(...)",
+                str(context.exception))
 
     def test_string_with_soft_space(self):
         python2_print_str = 'print "Hello World",'
         with self.assertRaises(SyntaxError) as context:
             exec(python2_print_str)
 
-        self.assertIn('print("Hello World", end=" ")', str(context.exception))
+        self.assertIn("Missing parentheses in call to 'print'. Did you mean print(...)",
+                str(context.exception))
 
     def test_string_with_excessive_whitespace(self):
         python2_print_str = 'print  "Hello World", '
         with self.assertRaises(SyntaxError) as context:
             exec(python2_print_str)
 
-        self.assertIn('print("Hello World", end=" ")', str(context.exception))
+        self.assertIn("Missing parentheses in call to 'print'. Did you mean print(...)",
+                str(context.exception))
 
     def test_string_with_leading_whitespace(self):
         python2_print_str = '''if 1:
@@ -163,7 +166,8 @@ def test_string_with_leading_whitespace(self):
         with self.assertRaises(SyntaxError) as context:
             exec(python2_print_str)
 
-        self.assertIn('print("Hello World")', str(context.exception))
+        self.assertIn("Missing parentheses in call to 'print'. Did you mean print(...)",
+                str(context.exception))
 
     # bpo-32685: Suggestions for print statement should be proper when
     # it is in the same line as the header of a compound statement
@@ -173,14 +177,16 @@ def test_string_with_semicolon(self):
         with self.assertRaises(SyntaxError) as context:
             exec(python2_print_str)
 
-        self.assertIn('print(p)', str(context.exception))
+        self.assertIn("Missing parentheses in call to 'print'. Did you mean print(...)",
+                str(context.exception))
 
     def test_string_in_loop_on_same_line(self):
         python2_print_str = 'for i in s: print i'
         with self.assertRaises(SyntaxError) as context:
             exec(python2_print_str)
 
-        self.assertIn('print(i)', str(context.exception))
+        self.assertIn("Missing parentheses in call to 'print'. Did you mean print(...)",
+                str(context.exception))
 
     def test_stream_redirection_hint_for_py2_migration(self):
         # Test correct hint produced for Py2 redirection syntax

Objects/exceptions.c

@@ -1475,9 +1475,6 @@ ComplexExtendsException(PyExc_Exception, AttributeError,
  *    SyntaxError extends Exception
  */
 
-/* Helper function to customize error message for some syntax errors */
-static int _report_missing_parentheses(PySyntaxErrorObject *self);
-
 static int
 SyntaxError_init(PySyntaxErrorObject *self, PyObject *args, PyObject *kwds)
 {
@@ -1520,18 +1517,6 @@ SyntaxError_init(PySyntaxErrorObject *self, PyObject *args, PyObject *kwds)
             PyErr_SetString(PyExc_TypeError, "end_offset must be provided when end_lineno is provided");
             return -1;
         }
-
-        /*
-         * Issue #21669: Custom error for 'print' & 'exec' as statements
-         *
-         * Only applies to SyntaxError instances, not to subclasses such
-         * as TabError or IndentationError (see issue #31161)
-         */
-        if (Py_IS_TYPE(self, (PyTypeObject *)PyExc_SyntaxError) &&
-                self->text && PyUnicode_Check(self->text) &&
-                _report_missing_parentheses(self) < 0) {
-            return -1;
-        }
     }
     return 0;
 }
@@ -3033,189 +3018,3 @@ _PyErr_TrySetFromCause(const char *format, ...)
     PyErr_Restore(new_exc, new_val, new_tb);
     return new_val;
 }
-
-
-/* To help with migration from Python 2, SyntaxError.__init__ applies some
- * heuristics to try to report a more meaningful exception when print and
- * exec are used like statements.
- *
- * The heuristics are currently expected to detect the following cases:
- *   - top level statement
- *   - statement in a nested suite
- *   - trailing section of a one line complex statement
- *
- * They're currently known not to trigger:
- *   - after a semi-colon
- *
- * The error message can be a bit odd in cases where the "arguments" are
- * completely illegal syntactically, but that isn't worth the hassle of
- * fixing.
- *
- * We also can't do anything about cases that are legal Python 3 syntax
- * but mean something entirely different from what they did in Python 2
- * (omitting the arguments entirely, printing items preceded by a unary plus
- * or minus, using the stream redirection syntax).
- */
-
-
-// Static helper for setting legacy print error message
-static int
-_set_legacy_print_statement_msg(PySyntaxErrorObject *self, Py_ssize_t start)
-{
-    // PRINT_OFFSET is to remove the `print ` prefix from the data.
-    const int PRINT_OFFSET = 6;
-    const int STRIP_BOTH = 2;
-    Py_ssize_t start_pos = start + PRINT_OFFSET;
-    Py_ssize_t text_len = PyUnicode_GET_LENGTH(self->text);
-    Py_UCS4 semicolon = ';';
-    Py_ssize_t end_pos = PyUnicode_FindChar(self->text, semicolon,
-                                            start_pos, text_len, 1);
-    if (end_pos < -1) {
-      return -1;
-    } else if (end_pos == -1) {
-      end_pos = text_len;
-    }
-
-    PyObject *data = PyUnicode_Substring(self->text, start_pos, end_pos);
-    if (data == NULL) {
-        return -1;
-    }
-
-    PyObject *strip_sep_obj = PyUnicode_FromString(" \t\r\n");
-    if (strip_sep_obj == NULL) {
-        Py_DECREF(data);
-        return -1;
-    }
-
-    PyObject *new_data = _PyUnicode_XStrip(data, STRIP_BOTH, strip_sep_obj);
-    Py_DECREF(data);
-    Py_DECREF(strip_sep_obj);
-    if (new_data == NULL) {
-        return -1;
-    }
-    // gets the modified text_len after stripping `print `
-    text_len = PyUnicode_GET_LENGTH(new_data);
-    const char *maybe_end_arg = "";
-    if (text_len > 0 && PyUnicode_READ_CHAR(new_data, text_len-1) == ',') {
-        maybe_end_arg = " end=\" \"";
-    }
-    PyObject *error_msg = PyUnicode_FromFormat(
-        "Missing parentheses in call to 'print'. Did you mean print(%U%s)?",
-        new_data, maybe_end_arg
-    );
-    Py_DECREF(new_data);
-    if (error_msg == NULL)
-        return -1;
-
-    Py_XSETREF(self->msg, error_msg);
-    return 1;
-}
-
-static int
-_check_for_legacy_statements(PySyntaxErrorObject *self, Py_ssize_t start)
-{
-    /* Return values:
-     *   -1: an error occurred
-     *    0: nothing happened
-     *    1: the check triggered & the error message was changed
-     */
-    static PyObject *print_prefix = NULL;
-    static PyObject *exec_prefix = NULL;
-    Py_ssize_t text_len = PyUnicode_GET_LENGTH(self->text), match;
-    int kind = PyUnicode_KIND(self->text);
-    const void *data = PyUnicode_DATA(self->text);
-
-    /* Ignore leading whitespace */
-    while (start < text_len) {
-        Py_UCS4 ch = PyUnicode_READ(kind, data, start);
-        if (!Py_UNICODE_ISSPACE(ch))
-            break;
-        start++;
-    }
-    /* Checking against an empty or whitespace-only part of the string */
-    if (start == text_len) {
-        return 0;
-    }
-
-    /* Check for legacy print statements */
-    if (print_prefix == NULL) {
-        print_prefix = PyUnicode_InternFromString("print ");
-        if (print_prefix == NULL) {
-            return -1;
-        }
-    }
-    match = PyUnicode_Tailmatch(self->text, print_prefix,
-                                start, text_len, -1);
-    if (match == -1) {
-        return -1;
-    }
-    if (match) {
-        return _set_legacy_print_statement_msg(self, start);
-    }
-
-    /* Check for legacy exec statements */
-    if (exec_prefix == NULL) {
-        exec_prefix = PyUnicode_InternFromString("exec ");
-        if (exec_prefix == NULL) {
-            return -1;
-        }
-    }
-    match = PyUnicode_Tailmatch(self->text, exec_prefix, start, text_len, -1);
-    if (match == -1) {
-        return -1;
-    }
-    if (match) {
-        PyObject *msg = PyUnicode_FromString("Missing parentheses in call "
-                                             "to 'exec'");
-        if (msg == NULL) {
-            return -1;
-        }
-        Py_XSETREF(self->msg, msg);
-        return 1;
-    }
-    /* Fall back to the default error message */
-    return 0;
-}
-
-static int
-_report_missing_parentheses(PySyntaxErrorObject *self)
-{
-    Py_UCS4 left_paren = 40;
-    Py_ssize_t left_paren_index;
-    Py_ssize_t text_len = PyUnicode_GET_LENGTH(self->text);
-    int legacy_check_result = 0;
-
-    /* Skip entirely if there is an opening parenthesis */
-    left_paren_index = PyUnicode_FindChar(self->text, left_paren,
-                                          0, text_len, 1);
-    if (left_paren_index < -1) {
-        return -1;
-    }
-    if (left_paren_index != -1) {
-        /* Use default error message for any line with an opening paren */
-        return 0;
-    }
-    /* Handle the simple statement case */
-    legacy_check_result = _check_for_legacy_statements(self, 0);
-    if (legacy_check_result < 0) {
-        return -1;
-
-    }
-    if (legacy_check_result == 0) {
-        /* Handle the one-line complex statement case */
-        Py_UCS4 colon = 58;
-        Py_ssize_t colon_index;
-        colon_index = PyUnicode_FindChar(self->text, colon,
-                                         0, text_len, 1);
-        if (colon_index < -1) {
-            return -1;
-        }
-        if (colon_index >= 0 && colon_index < text_len) {
-            /* Check again, starting from just after the colon */
-            if (_check_for_legacy_statements(self, colon_index+1) < 0) {
-                return -1;
-            }
-        }
-    }
-    return 0;
-}

Parser/parser.c

@@ -18158,7 +18158,7 @@ expression_without_invalid_rule(Parser *p)
     return _res;
 }
 
-// invalid_legacy_expression: NAME expression_without_invalid
+// invalid_legacy_expression: NAME star_expressions
 static void *
 invalid_legacy_expression_rule(Parser *p)
 {
@@ -18169,22 +18169,22 @@ invalid_legacy_expression_rule(Parser *p)
     }
     void * _res = NULL;
     int _mark = p->mark;
-    { // NAME expression_without_invalid
+    { // NAME star_expressions
         if (p->error_indicator) {
             D(p->level--);
             return NULL;
         }
-        D(fprintf(stderr, "%*c> invalid_legacy_expression[%d-%d]: %s\n", p->level, ' ', _mark, p->mark, "NAME expression_without_invalid"));
+        D(fprintf(stderr, "%*c> invalid_legacy_expression[%d-%d]: %s\n", p->level, ' ', _mark, p->mark, "NAME star_expressions"));
         expr_ty a;
         expr_ty b;
         if (
             (a = _PyPegen_name_token(p))  // NAME
             &&
-            (b = expression_without_invalid_rule(p))  // expression_without_invalid
+            (b = star_expressions_rule(p))  // star_expressions
         )
         {
-            D(fprintf(stderr, "%*c+ invalid_legacy_expression[%d-%d]: %s succeeded!\n", p->level, ' ', _mark, p->mark, "NAME expression_without_invalid"));
-            _res = _PyPegen_check_legacy_stmt ( p , a ) ? RAISE_SYNTAX_ERROR_KNOWN_RANGE ( a , b , "Missing parentheses in call to '%U'." , a -> v . Name . id ) : NULL;
+            D(fprintf(stderr, "%*c+ invalid_legacy_expression[%d-%d]: %s succeeded!\n", p->level, ' ', _mark, p->mark, "NAME star_expressions"));
+            _res = _PyPegen_check_legacy_stmt ( p , a ) ? RAISE_SYNTAX_ERROR_KNOWN_RANGE ( a , b , "Missing parentheses in call to '%U'. Did you mean %U(...)?" , a -> v . Name . id , a -> v . Name . id ) : NULL;
             if (_res == NULL && PyErr_Occurred()) {
                 p->error_indicator = 1;
                 D(p->level--);
@@ -18194,7 +18194,7 @@ invalid_legacy_expression_rule(Parser *p)
         }
         p->mark = _mark;
         D(fprintf(stderr, "%*c%s invalid_legacy_expression[%d-%d]: %s failed!\n", p->level, ' ',
-                  p->error_indicator ? "ERROR!" : "-", _mark, p->mark, "NAME expression_without_invalid"));
+                  p->error_indicator ? "ERROR!" : "-", _mark, p->mark, "NAME star_expressions"));
     }
     _res = NULL;
   done: