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.
diff --git a/Lib/test/test_call.py b/Lib/test/test_call.py
index e2b8e0f..0992a0e 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 @@
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 @@
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 c431f0d..f525255 100644
--- a/Lib/test/test_itertools.py
+++ b/Lib/test/test_itertools.py
@@ -1996,7 +1996,7 @@
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 4c74eab..6c8a640 100644
--- a/Objects/call.c
+++ b/Objects/call.c
@@ -466,6 +466,10 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
goto exit;
}
- if (nkwargs) {
- goto no_keyword_error;
- }
-
result = (*meth) (self, args[0]);
break;
@@ -637,16 +636,17 @@
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 @@
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 @@
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 1d11605..c20ca9b 100644
--- a/Objects/descrobject.c
+++ b/Objects/descrobject.c
@@ -1176,7 +1176,7 @@
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 58c9a99..af1f2a2 100644
--- a/Python/getargs.c
+++ b/Python/getargs.c
@@ -1704,13 +1704,21 @@
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 @@
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 @@
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 @@
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 @@
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;
}