From 5eb788bf7f54a8e04429e18fc332db858edd64b6 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 6 Jun 2017 18:45:22 +0300 Subject: [PATCH] bpo-30534: Fixed error messages when pass keyword arguments (#1901) to functions implemented in C that don't support this. Also unified error messages for functions that don't take positional or keyword arguments. --- Lib/test/test_call.py | 60 ++++++++++++++++++++++++++++++++++++-- Lib/test/test_itertools.py | 2 +- Objects/call.c | 49 ++++++++++++++++--------------- Objects/descrobject.c | 2 +- Python/getargs.c | 48 ++++++++++++++++++++---------- 5 files changed, 116 insertions(+), 45 deletions(-) diff --git a/Lib/test/test_call.py b/Lib/test/test_call.py index e2b8e0fd123..0992a0e3612 100644 --- a/Lib/test/test_call.py +++ b/Lib/test/test_call.py @@ -1,4 +1,5 @@ import unittest +from test.support import cpython_only # The test cases here cover several paths through the function calling # code. They depend on the METH_XXX flag that is used to define a C @@ -33,9 +34,6 @@ def test_varargs2_ext(self): else: raise RuntimeError - def test_varargs0_kw(self): - self.assertRaises(TypeError, {}.__contains__, x=2) - def test_varargs1_kw(self): self.assertRaises(TypeError, {}.__contains__, x=2) @@ -122,5 +120,61 @@ def test_oldargs1_2_kw(self): self.assertRaises(TypeError, [].count, x=2, y=2) +@cpython_only +class CFunctionCallsErrorMessages(unittest.TestCase): + + def test_varargs0(self): + msg = r"__contains__\(\) takes exactly one argument \(0 given\)" + self.assertRaisesRegex(TypeError, msg, {}.__contains__) + + def test_varargs2(self): + msg = r"__contains__\(\) takes exactly one argument \(2 given\)" + self.assertRaisesRegex(TypeError, msg, {}.__contains__, 0, 1) + + def test_varargs1_kw(self): + msg = r"__contains__\(\) takes no keyword arguments" + self.assertRaisesRegex(TypeError, msg, {}.__contains__, x=2) + + def test_varargs2_kw(self): + msg = r"__contains__\(\) takes no keyword arguments" + self.assertRaisesRegex(TypeError, msg, {}.__contains__, x=2, y=2) + + def test_oldargs0_1(self): + msg = r"keys\(\) takes no arguments \(1 given\)" + self.assertRaisesRegex(TypeError, msg, {}.keys, 0) + + def test_oldargs0_2(self): + msg = r"keys\(\) takes no arguments \(2 given\)" + self.assertRaisesRegex(TypeError, msg, {}.keys, 0, 1) + + def test_oldargs0_1_kw(self): + msg = r"keys\(\) takes no keyword arguments" + self.assertRaisesRegex(TypeError, msg, {}.keys, x=2) + + def test_oldargs0_2_kw(self): + msg = r"keys\(\) takes no keyword arguments" + self.assertRaisesRegex(TypeError, msg, {}.keys, x=2, y=2) + + def test_oldargs1_0(self): + msg = r"count\(\) takes exactly one argument \(0 given\)" + self.assertRaisesRegex(TypeError, msg, [].count) + + def test_oldargs1_2(self): + msg = r"count\(\) takes exactly one argument \(2 given\)" + self.assertRaisesRegex(TypeError, msg, [].count, 1, 2) + + def test_oldargs1_0_kw(self): + msg = r"count\(\) takes no keyword arguments" + self.assertRaisesRegex(TypeError, msg, [].count, x=2) + + def test_oldargs1_1_kw(self): + msg = r"count\(\) takes no keyword arguments" + self.assertRaisesRegex(TypeError, msg, [].count, {}, x=2) + + def test_oldargs1_2_kw(self): + msg = r"count\(\) takes no keyword arguments" + self.assertRaisesRegex(TypeError, msg, [].count, x=2, y=2) + + if __name__ == "__main__": unittest.main() diff --git a/Lib/test/test_itertools.py b/Lib/test/test_itertools.py index c431f0dc6e1..f5252550114 100644 --- a/Lib/test/test_itertools.py +++ b/Lib/test/test_itertools.py @@ -1996,7 +1996,7 @@ def __init__(self, newarg=None, *args): Subclass(newarg=1) except TypeError as err: # we expect type errors because of wrong argument count - self.assertNotIn("does not take keyword arguments", err.args[0]) + self.assertNotIn("keyword arguments", err.args[0]) @support.cpython_only class SizeofTest(unittest.TestCase): diff --git a/Objects/call.c b/Objects/call.c index 4c74eab44f5..6c8a640cc59 100644 --- a/Objects/call.c +++ b/Objects/call.c @@ -466,6 +466,10 @@ _PyMethodDef_RawFastCallDict(PyMethodDef *method, PyObject *self, PyObject **arg switch (flags) { case METH_NOARGS: + if (kwargs != NULL && PyDict_GET_SIZE(kwargs) != 0) { + goto no_keyword_error; + } + if (nargs != 0) { PyErr_Format(PyExc_TypeError, "%.200s() takes no arguments (%zd given)", @@ -473,14 +477,14 @@ _PyMethodDef_RawFastCallDict(PyMethodDef *method, PyObject *self, PyObject **arg goto exit; } - if (kwargs != NULL && PyDict_GET_SIZE(kwargs) != 0) { - goto no_keyword_error; - } - result = (*meth) (self, NULL); break; case METH_O: + if (kwargs != NULL && PyDict_GET_SIZE(kwargs) != 0) { + goto no_keyword_error; + } + if (nargs != 1) { PyErr_Format(PyExc_TypeError, "%.200s() takes exactly one argument (%zd given)", @@ -488,16 +492,11 @@ _PyMethodDef_RawFastCallDict(PyMethodDef *method, PyObject *self, PyObject **arg goto exit; } - if (kwargs != NULL && PyDict_GET_SIZE(kwargs) != 0) { - goto no_keyword_error; - } - result = (*meth) (self, args[0]); break; case METH_VARARGS: - if (!(flags & METH_KEYWORDS) - && kwargs != NULL && PyDict_GET_SIZE(kwargs) != 0) { + if (kwargs != NULL && PyDict_GET_SIZE(kwargs) != 0) { goto no_keyword_error; } /* fall through next case */ @@ -592,7 +591,7 @@ _PyMethodDef_RawFastCallKeywords(PyMethodDef *method, PyObject *self, PyObject * PyCFunction meth = method->ml_meth; int flags = method->ml_flags & ~(METH_CLASS | METH_STATIC | METH_COEXIST); - Py_ssize_t nkwargs = kwnames == NULL ? 0 : PyTuple_Size(kwnames); + Py_ssize_t nkwargs = kwnames == NULL ? 0 : PyTuple_GET_SIZE(kwnames); PyObject *result = NULL; if (Py_EnterRecursiveCall(" while calling a Python object")) { @@ -602,6 +601,10 @@ _PyMethodDef_RawFastCallKeywords(PyMethodDef *method, PyObject *self, PyObject * switch (flags) { case METH_NOARGS: + if (nkwargs) { + goto no_keyword_error; + } + if (nargs != 0) { PyErr_Format(PyExc_TypeError, "%.200s() takes no arguments (%zd given)", @@ -609,14 +612,14 @@ _PyMethodDef_RawFastCallKeywords(PyMethodDef *method, PyObject *self, PyObject * goto exit; } - if (nkwargs) { - goto no_keyword_error; - } - result = (*meth) (self, NULL); break; case METH_O: + if (nkwargs) { + goto no_keyword_error; + } + if (nargs != 1) { PyErr_Format(PyExc_TypeError, "%.200s() takes exactly one argument (%zd given)", @@ -624,10 +627,6 @@ _PyMethodDef_RawFastCallKeywords(PyMethodDef *method, PyObject *self, PyObject * goto exit; } - if (nkwargs) { - goto no_keyword_error; - } - result = (*meth) (self, args[0]); break; @@ -637,16 +636,17 @@ _PyMethodDef_RawFastCallKeywords(PyMethodDef *method, PyObject *self, PyObject * break; case METH_VARARGS: + if (nkwargs) { + goto no_keyword_error; + } + /* fall through next case */ + case METH_VARARGS | METH_KEYWORDS: { /* Slow-path: create a temporary tuple for positional arguments and a temporary dict for keyword arguments */ PyObject *argtuple; - if (!(flags & METH_KEYWORDS) && nkwargs) { - goto no_keyword_error; - } - argtuple = _PyStack_AsTuple(args, nargs); if (argtuple == NULL) { goto exit; @@ -717,6 +717,7 @@ static PyObject * cfunction_call_varargs(PyObject *func, PyObject *args, PyObject *kwargs) { assert(!PyErr_Occurred()); + assert(kwargs == NULL || PyDict_Check(kwargs)); PyCFunction meth = PyCFunction_GET_FUNCTION(func); PyObject *self = PyCFunction_GET_SELF(func); @@ -732,7 +733,7 @@ cfunction_call_varargs(PyObject *func, PyObject *args, PyObject *kwargs) Py_LeaveRecursiveCall(); } else { - if (kwargs != NULL && PyDict_Size(kwargs) != 0) { + if (kwargs != NULL && PyDict_GET_SIZE(kwargs) != 0) { PyErr_Format(PyExc_TypeError, "%.200s() takes no keyword arguments", ((PyCFunctionObject*)func)->m_ml->ml_name); return NULL; diff --git a/Objects/descrobject.c b/Objects/descrobject.c index 1d11605a9b8..c20ca9be4df 100644 --- a/Objects/descrobject.c +++ b/Objects/descrobject.c @@ -1176,7 +1176,7 @@ wrapper_call(wrapperobject *wp, PyObject *args, PyObject *kwds) if (kwds != NULL && (!PyDict_Check(kwds) || PyDict_GET_SIZE(kwds) != 0)) { PyErr_Format(PyExc_TypeError, - "wrapper %s doesn't take keyword arguments", + "wrapper %s() takes no keyword arguments", wp->descr->d_base->name); return NULL; } diff --git a/Python/getargs.c b/Python/getargs.c index 58c9a998ff8..af1f2a2a24d 100644 --- a/Python/getargs.c +++ b/Python/getargs.c @@ -1704,13 +1704,21 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format, break; } if (max < nargs) { - PyErr_Format(PyExc_TypeError, - "%.200s%s takes %s %d positional arguments" - " (%d given)", - (fname == NULL) ? "function" : fname, - (fname == NULL) ? "" : "()", - (min != INT_MAX) ? "at most" : "exactly", - max, nargs); + if (max == 0) { + PyErr_Format(PyExc_TypeError, + "%.200s%s takes no positional arguments", + (fname == NULL) ? "function" : fname, + (fname == NULL) ? "" : "()"); + } + else { + PyErr_Format(PyExc_TypeError, + "%.200s%s takes %s %d positional arguments" + " (%d given)", + (fname == NULL) ? "function" : fname, + (fname == NULL) ? "" : "()", + (min != INT_MAX) ? "at most" : "exactly", + max, nargs); + } return cleanreturn(0, &freelist); } } @@ -2079,12 +2087,20 @@ vgetargskeywordsfast_impl(PyObject **args, Py_ssize_t nargs, return cleanreturn(0, &freelist); } if (parser->max < nargs) { - PyErr_Format(PyExc_TypeError, - "%200s%s takes %s %d positional arguments (%d given)", - (parser->fname == NULL) ? "function" : parser->fname, - (parser->fname == NULL) ? "" : "()", - (parser->min != INT_MAX) ? "at most" : "exactly", - parser->max, nargs); + if (parser->max == 0) { + PyErr_Format(PyExc_TypeError, + "%200s%s takes no positional arguments", + (parser->fname == NULL) ? "function" : parser->fname, + (parser->fname == NULL) ? "" : "()"); + } + else { + PyErr_Format(PyExc_TypeError, + "%200s%s takes %s %d positional arguments (%d given)", + (parser->fname == NULL) ? "function" : parser->fname, + (parser->fname == NULL) ? "" : "()", + (parser->min != INT_MAX) ? "at most" : "exactly", + parser->max, nargs); + } return cleanreturn(0, &freelist); } @@ -2489,7 +2505,7 @@ _PyArg_NoKeywords(const char *funcname, PyObject *kwargs) return 1; } - PyErr_Format(PyExc_TypeError, "%.200s does not take keyword arguments", + PyErr_Format(PyExc_TypeError, "%.200s() takes no keyword arguments", funcname); return 0; } @@ -2506,7 +2522,7 @@ _PyArg_NoStackKeywords(const char *funcname, PyObject *kwnames) return 1; } - PyErr_Format(PyExc_TypeError, "%.200s does not take keyword arguments", + PyErr_Format(PyExc_TypeError, "%.200s() takes no keyword arguments", funcname); return 0; } @@ -2524,7 +2540,7 @@ _PyArg_NoPositional(const char *funcname, PyObject *args) if (PyTuple_GET_SIZE(args) == 0) return 1; - PyErr_Format(PyExc_TypeError, "%.200s does not take positional arguments", + PyErr_Format(PyExc_TypeError, "%.200s() takes no positional arguments", funcname); return 0; }