From 67ade403a2dbbb41f270afd7000febadef85a27e Mon Sep 17 00:00:00 2001 From: Kaushik Kulkarni <15399010+kaushikcfd@users.noreply.github.com> Date: Sat, 5 Nov 2022 11:31:57 -0500 Subject: gh-98284: better error message for undefined abstractmethod (#97971) --- Include/internal/pycore_global_strings.h | 1 - Include/internal/pycore_runtime_init_generated.h | 5 ----- Lib/test/test_abc.py | 12 ++++++------ Lib/test/test_dataclasses.py | 2 +- .../2022-10-15-10-43-45.gh-issue-98284.SaVHTd.rst | 3 +++ Objects/typeobject.c | 19 +++++++++++++------ 6 files changed, 23 insertions(+), 19 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2022-10-15-10-43-45.gh-issue-98284.SaVHTd.rst 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, "") STRUCT_FOR_STR(anon_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, ""), \ INIT_STR(anon_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); -- cgit v0.12