gh-98284: better error message for undefined abstractmethod (#97971)

6 files changed, 23 insertions, 19 deletions

diff --git a/Include/internal/pycore_global_strings.h b/Include/internal/pycore_global_strings.h
index 002d81e..e60bd4b 100644
--- a/Include/internal/pycore_global_strings.h
+++ b/Include/internal/pycore_global_strings.h
@@ -37,7 +37,6 @@ struct _Py_global_strings {
         STRUCT_FOR_STR(anon_string, "<string>")
         STRUCT_FOR_STR(anon_unknown, "<unknown>")
         STRUCT_FOR_STR(close_br, "}")
-        STRUCT_FOR_STR(comma_sep, ", ")
         STRUCT_FOR_STR(dbl_close_br, "}}")
         STRUCT_FOR_STR(dbl_open_br, "{{")
         STRUCT_FOR_STR(dbl_percent, "%%")
diff --git a/Include/internal/pycore_runtime_init_generated.h b/Include/internal/pycore_runtime_init_generated.h
index 88b84cb..8ce1037 100644
--- a/Include/internal/pycore_runtime_init_generated.h
+++ b/Include/internal/pycore_runtime_init_generated.h
@@ -547,7 +547,6 @@ extern "C" {
                 INIT_STR(anon_string, "<string>"), \
                 INIT_STR(anon_unknown, "<unknown>"), \
                 INIT_STR(close_br, "}"), \
-                INIT_STR(comma_sep, ", "), \
                 INIT_STR(dbl_close_br, "}}"), \
                 INIT_STR(dbl_open_br, "{{"), \
                 INIT_STR(dbl_percent, "%%"), \
@@ -4865,10 +4864,6 @@ _PyStaticObjects_CheckRefcnt(void) {
         _PyObject_Dump((PyObject *)&_Py_STR(close_br));
         Py_FatalError("immortal object has less refcnt than expected _PyObject_IMMORTAL_REFCNT");
     };
-    if (Py_REFCNT((PyObject *)&_Py_STR(comma_sep)) < _PyObject_IMMORTAL_REFCNT) {
-        _PyObject_Dump((PyObject *)&_Py_STR(comma_sep));
-        Py_FatalError("immortal object has less refcnt than expected _PyObject_IMMORTAL_REFCNT");
-    };
     if (Py_REFCNT((PyObject *)&_Py_STR(dbl_close_br)) < _PyObject_IMMORTAL_REFCNT) {
         _PyObject_Dump((PyObject *)&_Py_STR(dbl_close_br));
         Py_FatalError("immortal object has less refcnt than expected _PyObject_IMMORTAL_REFCNT");
diff --git a/Lib/test/test_abc.py b/Lib/test/test_abc.py
index a083236..86f31a9 100644
--- a/Lib/test/test_abc.py
+++ b/Lib/test/test_abc.py
@@ -154,7 +154,7 @@ def test_factory(abc_ABCMeta, abc_get_cache_token):
                 @abc.abstractmethod
                 def method_one(self):
                     pass
-            msg = r"class C without an implementation for abstract method method_one"
+            msg = r"class C without an implementation for abstract method 'method_one'"
             self.assertRaisesRegex(TypeError, msg, C)
 
         def test_object_new_with_many_abstractmethods(self):
@@ -165,7 +165,7 @@ def test_factory(abc_ABCMeta, abc_get_cache_token):
                 @abc.abstractmethod
                 def method_two(self):
                     pass
-            msg = r"class C without an implementation for abstract methods method_one, method_two"
+            msg = r"class C without an implementation for abstract methods 'method_one', 'method_two'"
             self.assertRaisesRegex(TypeError, msg, C)
 
         def test_abstractmethod_integration(self):
@@ -535,7 +535,7 @@ def test_factory(abc_ABCMeta, abc_get_cache_token):
             A.foo = updated_foo
             abc.update_abstractmethods(A)
             self.assertEqual(A.__abstractmethods__, {'foo', 'bar'})
-            msg = "class A without an implementation for abstract methods bar, foo"
+            msg = "class A without an implementation for abstract methods 'bar', 'foo'"
             self.assertRaisesRegex(TypeError, msg, A)
 
         def test_update_implementation(self):
@@ -547,7 +547,7 @@ def test_factory(abc_ABCMeta, abc_get_cache_token):
             class B(A):
                 pass
 
-            msg = "class B without an implementation for abstract method foo"
+            msg = "class B without an implementation for abstract method 'foo'"
             self.assertRaisesRegex(TypeError, msg, B)
             self.assertEqual(B.__abstractmethods__, {'foo'})
 
@@ -605,7 +605,7 @@ def test_factory(abc_ABCMeta, abc_get_cache_token):
 
             abc.update_abstractmethods(B)
 
-            msg = "class B without an implementation for abstract method foo"
+            msg = "class B without an implementation for abstract method 'foo'"
             self.assertRaisesRegex(TypeError, msg, B)
 
         def test_update_layered_implementation(self):
@@ -627,7 +627,7 @@ def test_factory(abc_ABCMeta, abc_get_cache_token):
 
             abc.update_abstractmethods(C)
 
-            msg = "class C without an implementation for abstract method foo"
+            msg = "class C without an implementation for abstract method 'foo'"
             self.assertRaisesRegex(TypeError, msg, C)
 
         def test_update_multi_inheritance(self):
diff --git a/Lib/test/test_dataclasses.py b/Lib/test/test_dataclasses.py
index 01e6684..a09f36c 100644
--- a/Lib/test/test_dataclasses.py
+++ b/Lib/test/test_dataclasses.py
@@ -3970,7 +3970,7 @@ class TestAbstract(unittest.TestCase):
             day: 'int'
 
         self.assertTrue(inspect.isabstract(Date))
-        msg = 'class Date without an implementation for abstract method foo'
+        msg = "class Date without an implementation for abstract method 'foo'"
         self.assertRaisesRegex(TypeError, msg, Date)
 
 
diff --git a/Misc/NEWS.d/next/Library/2022-10-15-10-43-45.gh-issue-98284.SaVHTd.rst b/Misc/NEWS.d/next/Library/2022-10-15-10-43-45.gh-issue-98284.SaVHTd.rst
new file mode 100644
index 0000000..ccabd42
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2022-10-15-10-43-45.gh-issue-98284.SaVHTd.rst
@@ -0,0 +1,3 @@
+Improved :class:`TypeError` message for undefined abstract methods of a
+:class:`abc.ABC` instance. The names of the missing methods are surrounded
+by single-quotes to highlight them.
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index 7f8f2c7..e2e40b5 100644
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -4955,9 +4955,10 @@ object_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
         PyObject *abstract_methods;
         PyObject *sorted_methods;
         PyObject *joined;
+        PyObject* comma_w_quotes_sep;
         Py_ssize_t method_count;
 
-        /* Compute ", ".join(sorted(type.__abstractmethods__))
+        /* Compute "', '".join(sorted(type.__abstractmethods__))
            into joined. */
         abstract_methods = type_abstractmethods(type, NULL);
         if (abstract_methods == NULL)
@@ -4970,22 +4971,28 @@ object_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
             Py_DECREF(sorted_methods);
             return NULL;
         }
-        _Py_DECLARE_STR(comma_sep, ", ");
-        joined = PyUnicode_Join(&_Py_STR(comma_sep), sorted_methods);
+        comma_w_quotes_sep = PyUnicode_FromString("', '");
+        joined = PyUnicode_Join(comma_w_quotes_sep, sorted_methods);
         method_count = PyObject_Length(sorted_methods);
         Py_DECREF(sorted_methods);
-        if (joined == NULL)
+        if (joined == NULL)  {
+            Py_DECREF(comma_w_quotes_sep);
             return NULL;
-        if (method_count == -1)
+        }
+        if (method_count == -1) {
+            Py_DECREF(comma_w_quotes_sep);
+            Py_DECREF(joined);
             return NULL;
+        }
 
         PyErr_Format(PyExc_TypeError,
                      "Can't instantiate abstract class %s "
-                     "without an implementation for abstract method%s %U",
+                     "without an implementation for abstract method%s '%U'",
                      type->tp_name,
                      method_count > 1 ? "s" : "",
                      joined);
         Py_DECREF(joined);
+        Py_DECREF(comma_w_quotes_sep);
         return NULL;
     }
     PyObject *obj = type->tp_alloc(type, 0);