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.

5 files changed, 116 insertions, 45 deletions

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 @@ class CFunctionCalls(unittest.TestCase):
         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 @@ class CFunctionCalls(unittest.TestCase):
         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 @@ class SubclassWithKwargsTest(unittest.TestCase):
                 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 @@ _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 1d11605..c20ca9b 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 58c9a99..af1f2a2 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;
 }