[Python-checkins] bpo-20524: adds better error message for `.format()` (GH-28310)

[ericvsmith]

https://github.com/python/cpython/commit/8d8729146f21f61af66e70d3ae9501ea6bdccd09
commit: 8d8729146f21f61af66e70d3ae9501ea6bdccd09
branch: main
author: Nikita Sobolev <mail at sobolevn.me>
committer: ericvsmith <ericvsmith at users.noreply.github.com>
date: 2021-09-24T11:18:04-04:00
summary:

bpo-20524: adds better error message for `.format()` (GH-28310)

It now lists the bad format_spec and the type of the object.

files:
A Misc/NEWS.d/next/Library/2021-09-13-14-59-01.bpo-20524.PMQ1Fh.rst
M Lib/test/test_format.py
M Python/formatter_unicode.c

diff --git a/Lib/test/test_format.py b/Lib/test/test_format.py
index ae0d4f7308e2c..16d29d1ea3d94 100644
--- a/Lib/test/test_format.py
+++ b/Lib/test/test_format.py
@@ -519,5 +519,33 @@ def test_with_an_underscore_and_a_comma_in_format_specifier(self):
         with self.assertRaisesRegex(ValueError, error_msg):
             '{:_,}'.format(1)
 
+    def test_better_error_message_format(self):
+        # https://bugs.python.org/issue20524
+        for value in [12j, 12, 12.0, "12"]:
+            with self.subTest(value=value):
+                # The format spec must be invalid for all types we're testing.
+                # '%M' will suffice.
+                bad_format_spec = '%M'
+                err = re.escape("Invalid format specifier "
+                                f"'{bad_format_spec}' for object of type "
+                                f"'{type(value).__name__}'")
+                with self.assertRaisesRegex(ValueError, err):
+                    f"xx{{value:{bad_format_spec}}}yy".format(value=value)
+
+                # Also test the builtin format() function.
+                with self.assertRaisesRegex(ValueError, err):
+                    format(value, bad_format_spec)
+
+                # Also test f-strings.
+                with self.assertRaisesRegex(ValueError, err):
+                    eval("f'xx{value:{bad_format_spec}}yy'")
+
+    def test_unicode_in_error_message(self):
+        str_err = re.escape(
+            "Invalid format specifier '%ЫйЯЧ' for object of type 'str'")
+        with self.assertRaisesRegex(ValueError, str_err):
+            "{a:%ЫйЯЧ}".format(a='a')
+
+
 if __name__ == "__main__":
     unittest.main()
diff --git a/Misc/NEWS.d/next/Library/2021-09-13-14-59-01.bpo-20524.PMQ1Fh.rst b/Misc/NEWS.d/next/Library/2021-09-13-14-59-01.bpo-20524.PMQ1Fh.rst
new file mode 100644
index 0000000000000..9a41c8e64f7e3
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2021-09-13-14-59-01.bpo-20524.PMQ1Fh.rst
@@ -0,0 +1,3 @@
+Improves error messages on ``.format()`` operation for ``str``, ``float``,
+``int``, and ``complex``. New format now shows the problematic pattern and
+the object type.
diff --git a/Python/formatter_unicode.c b/Python/formatter_unicode.c
index 7b5a7bd04eb3a..9071bf3a6589e 100644
--- a/Python/formatter_unicode.c
+++ b/Python/formatter_unicode.c
@@ -162,7 +162,8 @@ DEBUG_PRINT_FORMAT_SPEC(InternalFormatSpec *format)
   if failure, sets the exception
 */
 static int
-parse_internal_render_format_spec(PyObject *format_spec,
+parse_internal_render_format_spec(PyObject *obj,
+                                  PyObject *format_spec,
                                   Py_ssize_t start, Py_ssize_t end,
                                   InternalFormatSpec *format,
                                   char default_type,
@@ -279,8 +280,19 @@ parse_internal_render_format_spec(PyObject *format_spec,
     /* Finally, parse the type field. */
 
     if (end-pos > 1) {
-        /* More than one char remain, invalid format specifier. */
-        PyErr_Format(PyExc_ValueError, "Invalid format specifier");
+        /* More than one char remains, so this is an invalid format
+           specifier. */
+        /* Create a temporary object that contains the format spec we're
+           operating on.  It's format_spec[start:end] (in Python syntax). */
+        PyObject* actual_format_spec = PyUnicode_FromKindAndData(kind,
+                                         (char*)data + kind*start,
+                                         end-start);
+        if (actual_format_spec != NULL) {
+            PyErr_Format(PyExc_ValueError,
+                "Invalid format specifier '%U' for object of type '%.200s'",
+                actual_format_spec, Py_TYPE(obj)->tp_name);
+            Py_DECREF(actual_format_spec);
+        }
         return 0;
     }
 
@@ -1444,7 +1456,7 @@ _PyUnicode_FormatAdvancedWriter(_PyUnicodeWriter *writer,
     }
 
     /* parse the format_spec */
-    if (!parse_internal_render_format_spec(format_spec, start, end,
+    if (!parse_internal_render_format_spec(obj, format_spec, start, end,
                                            &format, 's', '<'))
         return -1;
 
@@ -1480,7 +1492,7 @@ _PyLong_FormatAdvancedWriter(_PyUnicodeWriter *writer,
     }
 
     /* parse the format_spec */
-    if (!parse_internal_render_format_spec(format_spec, start, end,
+    if (!parse_internal_render_format_spec(obj, format_spec, start, end,
                                            &format, 'd', '>'))
         goto done;
 
@@ -1536,7 +1548,7 @@ _PyFloat_FormatAdvancedWriter(_PyUnicodeWriter *writer,
         return format_obj(obj, writer);
 
     /* parse the format_spec */
-    if (!parse_internal_render_format_spec(format_spec, start, end,
+    if (!parse_internal_render_format_spec(obj, format_spec, start, end,
                                            &format, '\0', '>'))
         return -1;
 
@@ -1575,7 +1587,7 @@ _PyComplex_FormatAdvancedWriter(_PyUnicodeWriter *writer,
         return format_obj(obj, writer);
 
     /* parse the format_spec */
-    if (!parse_internal_render_format_spec(format_spec, start, end,
+    if (!parse_internal_render_format_spec(obj, format_spec, start, end,
                                            &format, '\0', '>'))
         return -1;