From ca5622d23f0ab58436e7020a442672550d357615 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 26 Apr 2023 14:56:03 -0700 Subject: [PATCH 1/6] Add failing regression test --- Lib/test/test_call.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Lib/test/test_call.py b/Lib/test/test_call.py index aab7b1580eaf35..cf4931bf8cd5ed 100644 --- a/Lib/test/test_call.py +++ b/Lib/test/test_call.py @@ -10,6 +10,7 @@ import gc import contextlib import sys +import types class BadStr(str): @@ -202,6 +203,19 @@ def test_oldargs1_2_kw(self): msg = r"count\(\) takes no keyword arguments" self.assertRaisesRegex(TypeError, msg, [].count, x=2, y=2) + def test_object_not_callable(self): + msg = r"^'object' object is not callable$" + self.assertRaisesRegex(TypeError, msg, object()) + + def test_module_not_callable_no_suggestion(self): + msg = r"^'module' object is not callable$" + self.assertRaisesRegex(TypeError, msg, types.ModuleType("mod")) + + def test_module_not_callable_suggestion(self): + msg = r"^'module' object is not callable\. Did you mean: 'mod\.mod\(\.\.\.\)'\?$" + mod = types.ModuleType("mod") + mod.mod = lambda: ... + self.assertRaisesRegex(TypeError, msg, mod) class TestCallingConventions(unittest.TestCase): From 0edff3866190717c3c45566d22ca6a8f92e1e881 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 26 Apr 2023 14:56:59 -0700 Subject: [PATCH 2/6] Proide a helpful hint for failed module calls --- Objects/call.c | 40 ++++++++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/Objects/call.c b/Objects/call.c index bd027e41f8a9a5..9080a85751948b 100644 --- a/Objects/call.c +++ b/Objects/call.c @@ -167,6 +167,38 @@ PyObject_VectorcallDict(PyObject *callable, PyObject *const *args, return _PyObject_FastCallDictTstate(tstate, callable, args, nargsf, kwargs); } +static void +object_is_not_callable(PyThreadState *tstate, PyObject *callable) +{ + if (Py_IS_TYPE(callable, &PyModule_Type)) { + // >>> import pprint + // >>> pprint(thing) + // Traceback (most recent call last): + // File "", line 1, in + // TypeError: 'module' object is not callable. Did you mean: 'pprint.pprint(...)'? + PyObject *name = PyModule_GetNameObject(callable); + if (name == NULL) { + return; + } + PyObject *attr; + int res = _PyObject_LookupAttr(callable, name, &attr); + if (res > 0 && PyCallable_Check(attr)) { + _PyErr_Format(tstate, PyExc_TypeError, + "'%.200s' object is not callable. " + "Did you mean: '%U.%U(...)'?", + Py_TYPE(callable)->tp_name, name, name); + } + Py_XDECREF(attr); + Py_DECREF(name); + if (_PyErr_Occurred(tstate)) { + // Either our lookup failed, or we set the exception above: + return; + } + } + _PyErr_Format(tstate, PyExc_TypeError, "'%.200s' object is not callable", + Py_TYPE(callable)->tp_name); +} + PyObject * _PyObject_MakeTpCall(PyThreadState *tstate, PyObject *callable, @@ -181,9 +213,7 @@ _PyObject_MakeTpCall(PyThreadState *tstate, PyObject *callable, * temporary dictionary for keyword arguments (if any) */ ternaryfunc call = Py_TYPE(callable)->tp_call; if (call == NULL) { - _PyErr_Format(tstate, PyExc_TypeError, - "'%.200s' object is not callable", - Py_TYPE(callable)->tp_name); + object_is_not_callable(tstate, callable); return NULL; } @@ -332,9 +362,7 @@ _PyObject_Call(PyThreadState *tstate, PyObject *callable, else { call = Py_TYPE(callable)->tp_call; if (call == NULL) { - _PyErr_Format(tstate, PyExc_TypeError, - "'%.200s' object is not callable", - Py_TYPE(callable)->tp_name); + object_is_not_callable(tstate, callable); return NULL; } From f97d995aabca69d3117ea40e540c64b23ec0ddb4 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 26 Apr 2023 15:14:33 -0700 Subject: [PATCH 3/6] blurb add --- .../2023-04-26-15-14-23.gh-issue-103899.1pqKPF.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-04-26-15-14-23.gh-issue-103899.1pqKPF.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-04-26-15-14-23.gh-issue-103899.1pqKPF.rst b/Misc/NEWS.d/next/Core and Builtins/2023-04-26-15-14-23.gh-issue-103899.1pqKPF.rst new file mode 100644 index 00000000000000..c12a6b9cb841f2 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-04-26-15-14-23.gh-issue-103899.1pqKPF.rst @@ -0,0 +1,3 @@ +Provide a helpful hint in the :exc:`TypeError` message when accidentally +calling a :term:`module` object that has a callable attribute of the same +name (such as :func:`dis.dis` or :class:`datetime.datetime`). From 06fadfb27318f6716c18777313e082baeebe1079 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 26 Apr 2023 15:22:37 -0700 Subject: [PATCH 4/6] Add another test --- Lib/test/test_call.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_call.py b/Lib/test/test_call.py index cf4931bf8cd5ed..2a2edae6ea386e 100644 --- a/Lib/test/test_call.py +++ b/Lib/test/test_call.py @@ -207,10 +207,16 @@ def test_object_not_callable(self): msg = r"^'object' object is not callable$" self.assertRaisesRegex(TypeError, msg, object()) - def test_module_not_callable_no_suggestion(self): + def test_module_not_callable_no_suggestion_0(self): msg = r"^'module' object is not callable$" self.assertRaisesRegex(TypeError, msg, types.ModuleType("mod")) + def test_module_not_callable_not_suggestion_1(self): + msg = r"^'module' object is not callable$" + mod = types.ModuleType("mod") + mod.mod = 42 + self.assertRaisesRegex(TypeError, msg, mod) + def test_module_not_callable_suggestion(self): msg = r"^'module' object is not callable\. Did you mean: 'mod\.mod\(\.\.\.\)'\?$" mod = types.ModuleType("mod") From 833144d2997778786da70504bac9df3b411cfd50 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 26 Apr 2023 15:39:46 -0700 Subject: [PATCH 5/6] Add more tests --- Lib/test/test_call.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_call.py b/Lib/test/test_call.py index 2a2edae6ea386e..12759c53bb662c 100644 --- a/Lib/test/test_call.py +++ b/Lib/test/test_call.py @@ -211,12 +211,24 @@ def test_module_not_callable_no_suggestion_0(self): msg = r"^'module' object is not callable$" self.assertRaisesRegex(TypeError, msg, types.ModuleType("mod")) - def test_module_not_callable_not_suggestion_1(self): + def test_module_not_callable_no_suggestion_1(self): msg = r"^'module' object is not callable$" mod = types.ModuleType("mod") mod.mod = 42 self.assertRaisesRegex(TypeError, msg, mod) + def test_module_not_callable_no_suggestion_2(self): + msg = r"^'module' object is not callable$" + mod = types.ModuleType("mod") + del mod.__name__ + self.assertRaisesRegex(TypeError, msg, mod) + + def test_module_not_callable_no_suggestion_3(self): + msg = r"^'module' object is not callable$" + mod = types.ModuleType("mod") + mod.__name__ = 42 + self.assertRaisesRegex(TypeError, msg, mod) + def test_module_not_callable_suggestion(self): msg = r"^'module' object is not callable\. Did you mean: 'mod\.mod\(\.\.\.\)'\?$" mod = types.ModuleType("mod") From e52c1f772b9f7d70fdea99b05f1b28cd4937ae9d Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 26 Apr 2023 15:45:40 -0700 Subject: [PATCH 6/6] Ignore any other exceptions that pop up --- Objects/call.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/Objects/call.c b/Objects/call.c index 9080a85751948b..ae42baedba7e23 100644 --- a/Objects/call.c +++ b/Objects/call.c @@ -178,23 +178,27 @@ object_is_not_callable(PyThreadState *tstate, PyObject *callable) // TypeError: 'module' object is not callable. Did you mean: 'pprint.pprint(...)'? PyObject *name = PyModule_GetNameObject(callable); if (name == NULL) { - return; + _PyErr_Clear(tstate); + goto basic_type_error; } PyObject *attr; int res = _PyObject_LookupAttr(callable, name, &attr); - if (res > 0 && PyCallable_Check(attr)) { + if (res < 0) { + _PyErr_Clear(tstate); + } + else if (res > 0 && PyCallable_Check(attr)) { _PyErr_Format(tstate, PyExc_TypeError, "'%.200s' object is not callable. " "Did you mean: '%U.%U(...)'?", Py_TYPE(callable)->tp_name, name, name); + Py_DECREF(attr); + Py_DECREF(name); + return; } Py_XDECREF(attr); Py_DECREF(name); - if (_PyErr_Occurred(tstate)) { - // Either our lookup failed, or we set the exception above: - return; - } } +basic_type_error: _PyErr_Format(tstate, PyExc_TypeError, "'%.200s' object is not callable", Py_TYPE(callable)->tp_name); }