From 1239c80ba19d8e6304bd029d6071e0459220504d Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Wed, 20 Jan 2021 15:40:47 +0000 Subject: [PATCH 1/5] bpo-42978: Improve error message when 'self' is missing from the method definition --- Lib/test/test_call.py | 63 ++++++++++++++++++++++++++++++++++++++++++- Python/ceval.c | 46 ++++++++++++++++++++++++++++++- 2 files changed, 107 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_call.py b/Lib/test/test_call.py index 3f459222748b3a..962e7862083e53 100644 --- a/Lib/test/test_call.py +++ b/Lib/test/test_call.py @@ -713,5 +713,66 @@ def test_multiple_values(self): A().method_two_args("x", "y", x="oops") +class NoSelfClass: + def foo(): + pass + + def foo1(x, y): + pass + + def foo2(self, y): + pass + + @classmethod + def clsmethod(x, y): + pass + + @staticmethod + def stmethod(x, y): + pass + + +@cpython_only +class TestErrorMessagesInMethodsGiveHintAboutSelf(unittest.TestCase): + @contextlib.contextmanager + def check_raises_type_error(self, message): + with self.assertRaises(TypeError) as cm: + yield + self.assertEqual(str(cm.exception), message) + + + def test_method_with_missing_self_no_args(self): + + msg = ("NoSelfClass.foo() takes 0 positional arguments but 1 was given. " + "Did you forget 'self' in the method definition?") + with self.check_raises_type_error(msg): + NoSelfClass().foo() + + def test_method_with_missing_self_args(self): + + msg = ("NoSelfClass.foo1() takes 2 positional arguments but 3 were given. " + "Did you forget 'self' in the method definition?") + with self.check_raises_type_error(msg): + NoSelfClass().foo1(1, 2) + + def test_method_with_self_args(self): + + msg = "NoSelfClass.foo2() takes 2 positional arguments but 3 were given" + with self.check_raises_type_error(msg): + NoSelfClass().foo2(1, 2) + + def test_method_with_classmethod(self): + + msg = "NoSelfClass.clsmethod() takes 2 positional arguments but 3 were given" + with self.check_raises_type_error(msg): + NoSelfClass().clsmethod(1, 2) + + def test_method_with_staticmethod(self): + + msg = "NoSelfClass.stmethod() takes 2 positional arguments but 3 were given" + with self.check_raises_type_error(msg): + NoSelfClass().stmethod(1, 2, 3) + + if __name__ == "__main__": - unittest.main() + unittest.main() diff --git a/Python/ceval.c b/Python/ceval.c index 5e2a160f0ed35c..28da555ef237e8 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -86,6 +86,7 @@ static PyObject * special_lookup(PyThreadState *, PyObject *, _Py_Identifier *); static int check_args_iterable(PyThreadState *, PyObject *func, PyObject *vararg); static void format_kwargs_error(PyThreadState *, PyObject *func, PyObject *kwargs); static void format_awaitable_error(PyThreadState *, PyTypeObject *, int, int); +static void improve_missing_self_error(PyThreadState* tstate, PyCodeObject* co, Py_ssize_t nargs); #define NAME_ERROR_MSG \ "name '%.200s' is not defined" @@ -3786,13 +3787,21 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, PyFrameObject *f, int throwflag) We'll be passing `oparg + 1` to call_function, to make it accept the `self` as a first argument. */ + + PyObject **pfunc = (*(&sp)) - (oparg + 1) - 1; + PyObject *func = *pfunc; + PyCodeObject *co = (PyCodeObject *)PyFunction_GET_CODE(func); res = call_function(tstate, &bounds, &sp, oparg + 1, NULL); + if (res == NULL && PyErr_ExceptionMatches(PyExc_TypeError)) { + improve_missing_self_error(tstate, co, oparg - 1); + } stack_pointer = sp; } PUSH(res); - if (res == NULL) + if (res == NULL) { goto error; + } DISPATCH(); } @@ -5998,3 +6007,38 @@ void Py_LeaveRecursiveCall(void) { _Py_LeaveRecursiveCall_inline(); } + +static void improve_missing_self_error(PyThreadState* tstate, PyCodeObject* co, Py_ssize_t nargs) { + + if (nargs + 1 != co->co_argcount) { + return; + } + PyObject *type, *val, *tb; + _PyErr_Fetch(tstate, &type, &val, &tb); + + if (co->co_argcount > 0 && co->co_varnames && PyTuple_GET_SIZE(co->co_varnames) > 0) { + PyObject* first_arg = PyTuple_GET_ITEM(co->co_varnames, 0); + int comp = PyUnicode_CompareWithASCIIString(first_arg, "self"); + if (comp == 0 || comp == -1) { + PyErr_Clear(); + goto exit; + } + } + + if (PyUnicode_CheckExact(val)) { + PyObject* trailing = PyUnicode_FromString(". Did you forget 'self' in the method definition?"); + if (trailing == NULL) { + PyErr_Clear(); + goto exit; + } + PyObject* new_val = PyUnicode_Concat(val, trailing); + if (new_val == NULL) { + PyErr_Clear(); + goto exit; + } + Py_DECREF(val); + val = new_val; + } +exit: + PyErr_Restore(type, val, tb); +} From 58d5c99d67f6453d1e2d52bb4946c4118e6e8c28 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Wed, 20 Jan 2021 23:03:16 +0000 Subject: [PATCH 2/5] Update Python/ceval.c Co-authored-by: Serhiy Storchaka --- Python/ceval.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Python/ceval.c b/Python/ceval.c index 28da555ef237e8..54be784990a347 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -6008,7 +6008,9 @@ void Py_LeaveRecursiveCall(void) _Py_LeaveRecursiveCall_inline(); } -static void improve_missing_self_error(PyThreadState* tstate, PyCodeObject* co, Py_ssize_t nargs) { +static void +improve_missing_self_error(PyThreadState* tstate, PyCodeObject* co, Py_ssize_t nargs) +{ if (nargs + 1 != co->co_argcount) { return; From 219342afd605430f8ad043824a72a2ffbd69c899 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Wed, 20 Jan 2021 23:12:27 +0000 Subject: [PATCH 3/5] fixup! Update Python/ceval.c --- Python/ceval.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 54be784990a347..dd24834b36b96d 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -6015,32 +6015,30 @@ improve_missing_self_error(PyThreadState* tstate, PyCodeObject* co, Py_ssize_t n if (nargs + 1 != co->co_argcount) { return; } + PyObject *type, *val, *tb; _PyErr_Fetch(tstate, &type, &val, &tb); + if (!PyUnicode_CheckExact(val)) { + goto exit; + } + if (co->co_argcount > 0 && co->co_varnames && PyTuple_GET_SIZE(co->co_varnames) > 0) { PyObject* first_arg = PyTuple_GET_ITEM(co->co_varnames, 0); - int comp = PyUnicode_CompareWithASCIIString(first_arg, "self"); - if (comp == 0 || comp == -1) { + if (_PyUnicode_EqualToASCIIString(first_arg, "self")) { PyErr_Clear(); goto exit; } } - if (PyUnicode_CheckExact(val)) { - PyObject* trailing = PyUnicode_FromString(". Did you forget 'self' in the method definition?"); - if (trailing == NULL) { - PyErr_Clear(); - goto exit; - } - PyObject* new_val = PyUnicode_Concat(val, trailing); - if (new_val == NULL) { - PyErr_Clear(); - goto exit; - } - Py_DECREF(val); - val = new_val; + PyObject* new_val = PyUnicode_FromFormat("%U. Did you forget 'self' in the method" + " definition?", val); + if (new_val == NULL) { + PyErr_Clear(); + goto exit; } + Py_DECREF(val); + val = new_val; exit: PyErr_Restore(type, val, tb); } From 1deb282d5c4543182aeb485f46774e5e83317673 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Wed, 20 Jan 2021 23:15:43 +0000 Subject: [PATCH 4/5] Move the check for incorrect arguments up the stack --- Python/ceval.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index dd24834b36b96d..037a90633cda3b 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -86,7 +86,7 @@ static PyObject * special_lookup(PyThreadState *, PyObject *, _Py_Identifier *); static int check_args_iterable(PyThreadState *, PyObject *func, PyObject *vararg); static void format_kwargs_error(PyThreadState *, PyObject *func, PyObject *kwargs); static void format_awaitable_error(PyThreadState *, PyTypeObject *, int, int); -static void improve_missing_self_error(PyThreadState* tstate, PyCodeObject* co, Py_ssize_t nargs); +static void improve_missing_self_error(PyThreadState* tstate, PyCodeObject* co); #define NAME_ERROR_MSG \ "name '%.200s' is not defined" @@ -3792,8 +3792,9 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, PyFrameObject *f, int throwflag) PyObject *func = *pfunc; PyCodeObject *co = (PyCodeObject *)PyFunction_GET_CODE(func); res = call_function(tstate, &bounds, &sp, oparg + 1, NULL); - if (res == NULL && PyErr_ExceptionMatches(PyExc_TypeError)) { - improve_missing_self_error(tstate, co, oparg - 1); + if (res == NULL && oparg == co->co_argcount && + PyErr_ExceptionMatches(PyExc_TypeError)) { + improve_missing_self_error(tstate, co); } stack_pointer = sp; } @@ -6009,13 +6010,9 @@ void Py_LeaveRecursiveCall(void) } static void -improve_missing_self_error(PyThreadState* tstate, PyCodeObject* co, Py_ssize_t nargs) +improve_missing_self_error(PyThreadState* tstate, PyCodeObject* co) { - if (nargs + 1 != co->co_argcount) { - return; - } - PyObject *type, *val, *tb; _PyErr_Fetch(tstate, &type, &val, &tb); From f5074c378b3b0f74f1b1dbfc7992a0c9d3760cb1 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Wed, 20 Jan 2021 23:23:10 +0000 Subject: [PATCH 5/5] Fix whitespace --- Lib/test/test_call.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_call.py b/Lib/test/test_call.py index 962e7862083e53..6d16c0da984241 100644 --- a/Lib/test/test_call.py +++ b/Lib/test/test_call.py @@ -775,4 +775,4 @@ def test_method_with_staticmethod(self): if __name__ == "__main__": - unittest.main() + unittest.main()