From ecc3c8e4216958d85385bf2467441c975128f26c Mon Sep 17 00:00:00 2001 From: Pablo Galindo Salgado Date: Tue, 27 Jul 2021 21:30:32 +0100 Subject: [PATCH] bpo-34013: Move the Python 2 hints from the exception constructor to the parser (GH-27392) --- Grammar/python.gram | 7 +- Lib/test/test_exceptions.py | 12 +-- Lib/test/test_print.py | 18 ++-- Objects/exceptions.c | 201 ------------------------------------ Parser/parser.c | 14 +-- 5 files changed, 28 insertions(+), 224 deletions(-) diff --git a/Grammar/python.gram b/Grammar/python.gram index 7b8af046ead..8219add9006 100644 --- a/Grammar/python.gram +++ b/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" diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py index 8ebed02ca4b..8ea415f45a6 100644 --- a/Lib/test/test_exceptions.py +++ b/Lib/test/test_exceptions.py @@ -168,21 +168,19 @@ class ExceptionTests(unittest.TestCase): 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"''' diff --git a/Lib/test/test_print.py b/Lib/test/test_print.py index e8381122182..5f1bfd9e30d 100644 --- a/Lib/test/test_print.py +++ b/Lib/test/test_print.py @@ -140,21 +140,24 @@ class TestPy2MigrationHint(unittest.TestCase): 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 @@ class TestPy2MigrationHint(unittest.TestCase): 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 @@ class TestPy2MigrationHint(unittest.TestCase): 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 diff --git a/Objects/exceptions.c b/Objects/exceptions.c index 20fcd4e04bf..76772fbed21 100644 --- a/Objects/exceptions.c +++ b/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; -} diff --git a/Parser/parser.c b/Parser/parser.c index 8cd9e72a43d..d01d2b8d85c 100644 --- a/Parser/parser.c +++ b/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: