Skip to content

Commit 5eb788b

Browse files
serhiy-storchakaserhiy-storchaka
authored
bpo-30534: Fixed error messages when pass keyword arguments (python#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.
1 parent 5cefb6c commit 5eb788b

File tree

5 files changed

+116
-45
lines changed

5 files changed

+116
-45
lines changed

‎Lib/test/test_call.py‎

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import unittest
2+
from test.support import cpython_only
23

34
# The test cases here cover several paths through the function calling
45
# code. They depend on the METH_XXX flag that is used to define a C
@@ -33,9 +34,6 @@ def test_varargs2_ext(self):
3334
else:
3435
raise RuntimeError
3536

36-
def test_varargs0_kw(self):
37-
self.assertRaises(TypeError, {}.__contains__, x=2)
38-
3937
def test_varargs1_kw(self):
4038
self.assertRaises(TypeError, {}.__contains__, x=2)
4139

@@ -122,5 +120,61 @@ def test_oldargs1_2_kw(self):
122120
self.assertRaises(TypeError, [].count, x=2, y=2)
123121

124122

123+
@cpython_only
124+
class CFunctionCallsErrorMessages(unittest.TestCase):
125+
126+
def test_varargs0(self):
127+
msg = r"__contains__\(\) takes exactly one argument \(0 given\)"
128+
self.assertRaisesRegex(TypeError, msg, {}.__contains__)
129+
130+
def test_varargs2(self):
131+
msg = r"__contains__\(\) takes exactly one argument \(2 given\)"
132+
self.assertRaisesRegex(TypeError, msg, {}.__contains__, 0, 1)
133+
134+
def test_varargs1_kw(self):
135+
msg = r"__contains__\(\) takes no keyword arguments"
136+
self.assertRaisesRegex(TypeError, msg, {}.__contains__, x=2)
137+
138+
def test_varargs2_kw(self):
139+
msg = r"__contains__\(\) takes no keyword arguments"
140+
self.assertRaisesRegex(TypeError, msg, {}.__contains__, x=2, y=2)
141+
142+
def test_oldargs0_1(self):
143+
msg = r"keys\(\) takes no arguments \(1 given\)"
144+
self.assertRaisesRegex(TypeError, msg, {}.keys, 0)
145+
146+
def test_oldargs0_2(self):
147+
msg = r"keys\(\) takes no arguments \(2 given\)"
148+
self.assertRaisesRegex(TypeError, msg, {}.keys, 0, 1)
149+
150+
def test_oldargs0_1_kw(self):
151+
msg = r"keys\(\) takes no keyword arguments"
152+
self.assertRaisesRegex(TypeError, msg, {}.keys, x=2)
153+
154+
def test_oldargs0_2_kw(self):
155+
msg = r"keys\(\) takes no keyword arguments"
156+
self.assertRaisesRegex(TypeError, msg, {}.keys, x=2, y=2)
157+
158+
def test_oldargs1_0(self):
159+
msg = r"count\(\) takes exactly one argument \(0 given\)"
160+
self.assertRaisesRegex(TypeError, msg, [].count)
161+
162+
def test_oldargs1_2(self):
163+
msg = r"count\(\) takes exactly one argument \(2 given\)"
164+
self.assertRaisesRegex(TypeError, msg, [].count, 1, 2)
165+
166+
def test_oldargs1_0_kw(self):
167+
msg = r"count\(\) takes no keyword arguments"
168+
self.assertRaisesRegex(TypeError, msg, [].count, x=2)
169+
170+
def test_oldargs1_1_kw(self):
171+
msg = r"count\(\) takes no keyword arguments"
172+
self.assertRaisesRegex(TypeError, msg, [].count, {}, x=2)
173+
174+
def test_oldargs1_2_kw(self):
175+
msg = r"count\(\) takes no keyword arguments"
176+
self.assertRaisesRegex(TypeError, msg, [].count, x=2, y=2)
177+
178+
125179
if __name__ == "__main__":
126180
unittest.main()

‎Lib/test/test_itertools.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1996,7 +1996,7 @@ def __init__(self, newarg=None, *args):
19961996
Subclass(newarg=1)
19971997
except TypeError as err:
19981998
# we expect type errors because of wrong argument count
1999-
self.assertNotIn("does not take keyword arguments", err.args[0])
1999+
self.assertNotIn("keyword arguments", err.args[0])
20002000

20012001
@support.cpython_only
20022002
class SizeofTest(unittest.TestCase):

‎Objects/call.c‎

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -466,38 +466,37 @@ _PyMethodDef_RawFastCallDict(PyMethodDef *method, PyObject *self, PyObject **arg
466466
switch (flags)
467467
{
468468
case METH_NOARGS:
469+
if (kwargs != NULL && PyDict_GET_SIZE(kwargs) != 0) {
470+
goto no_keyword_error;
471+
}
472+
469473
if (nargs != 0) {
470474
PyErr_Format(PyExc_TypeError,
471475
"%.200s() takes no arguments (%zd given)",
472476
method->ml_name, nargs);
473477
goto exit;
474478
}
475479

476-
if (kwargs != NULL && PyDict_GET_SIZE(kwargs) != 0) {
477-
goto no_keyword_error;
478-
}
479-
480480
result = (*meth) (self, NULL);
481481
break;
482482

483483
case METH_O:
484+
if (kwargs != NULL && PyDict_GET_SIZE(kwargs) != 0) {
485+
goto no_keyword_error;
486+
}
487+
484488
if (nargs != 1) {
485489
PyErr_Format(PyExc_TypeError,
486490
"%.200s() takes exactly one argument (%zd given)",
487491
method->ml_name, nargs);
488492
goto exit;
489493
}
490494

491-
if (kwargs != NULL && PyDict_GET_SIZE(kwargs) != 0) {
492-
goto no_keyword_error;
493-
}
494-
495495
result = (*meth) (self, args[0]);
496496
break;
497497

498498
case METH_VARARGS:
499-
if (!(flags & METH_KEYWORDS)
500-
&& kwargs != NULL && PyDict_GET_SIZE(kwargs) != 0) {
499+
if (kwargs != NULL && PyDict_GET_SIZE(kwargs) != 0) {
501500
goto no_keyword_error;
502501
}
503502
/* fall through next case */
@@ -592,7 +591,7 @@ _PyMethodDef_RawFastCallKeywords(PyMethodDef *method, PyObject *self, PyObject *
592591

593592
PyCFunction meth = method->ml_meth;
594593
int flags = method->ml_flags & ~(METH_CLASS | METH_STATIC | METH_COEXIST);
595-
Py_ssize_t nkwargs = kwnames == NULL ? 0 : PyTuple_Size(kwnames);
594+
Py_ssize_t nkwargs = kwnames == NULL ? 0 : PyTuple_GET_SIZE(kwnames);
596595
PyObject *result = NULL;
597596

598597
if (Py_EnterRecursiveCall(" while calling a Python object")) {
@@ -602,32 +601,32 @@ _PyMethodDef_RawFastCallKeywords(PyMethodDef *method, PyObject *self, PyObject *
602601
switch (flags)
603602
{
604603
case METH_NOARGS:
604+
if (nkwargs) {
605+
goto no_keyword_error;
606+
}
607+
605608
if (nargs != 0) {
606609
PyErr_Format(PyExc_TypeError,
607610
"%.200s() takes no arguments (%zd given)",
608611
method->ml_name, nargs);
609612
goto exit;
610613
}
611614

612-
if (nkwargs) {
613-
goto no_keyword_error;
614-
}
615-
616615
result = (*meth) (self, NULL);
617616
break;
618617

619618
case METH_O:
619+
if (nkwargs) {
620+
goto no_keyword_error;
621+
}
622+
620623
if (nargs != 1) {
621624
PyErr_Format(PyExc_TypeError,
622625
"%.200s() takes exactly one argument (%zd given)",
623626
method->ml_name, nargs);
624627
goto exit;
625628
}
626629

627-
if (nkwargs) {
628-
goto no_keyword_error;
629-
}
630-
631630
result = (*meth) (self, args[0]);
632631
break;
633632

@@ -637,16 +636,17 @@ _PyMethodDef_RawFastCallKeywords(PyMethodDef *method, PyObject *self, PyObject *
637636
break;
638637

639638
case METH_VARARGS:
639+
if (nkwargs) {
640+
goto no_keyword_error;
641+
}
642+
/* fall through next case */
643+
640644
case METH_VARARGS | METH_KEYWORDS:
641645
{
642646
/* Slow-path: create a temporary tuple for positional arguments
643647
and a temporary dict for keyword arguments */
644648
PyObject *argtuple;
645649

646-
if (!(flags & METH_KEYWORDS) && nkwargs) {
647-
goto no_keyword_error;
648-
}
649-
650650
argtuple = _PyStack_AsTuple(args, nargs);
651651
if (argtuple == NULL) {
652652
goto exit;
@@ -717,6 +717,7 @@ static PyObject *
717717
cfunction_call_varargs(PyObject *func, PyObject *args, PyObject *kwargs)
718718
{
719719
assert(!PyErr_Occurred());
720+
assert(kwargs == NULL || PyDict_Check(kwargs));
720721

721722
PyCFunction meth = PyCFunction_GET_FUNCTION(func);
722723
PyObject *self = PyCFunction_GET_SELF(func);
@@ -732,7 +733,7 @@ cfunction_call_varargs(PyObject *func, PyObject *args, PyObject *kwargs)
732733
Py_LeaveRecursiveCall();
733734
}
734735
else {
735-
if (kwargs != NULL && PyDict_Size(kwargs) != 0) {
736+
if (kwargs != NULL && PyDict_GET_SIZE(kwargs) != 0) {
736737
PyErr_Format(PyExc_TypeError, "%.200s() takes no keyword arguments",
737738
((PyCFunctionObject*)func)->m_ml->ml_name);
738739
return NULL;

‎Objects/descrobject.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1176,7 +1176,7 @@ wrapper_call(wrapperobject *wp, PyObject *args, PyObject *kwds)
11761176

11771177
if (kwds != NULL && (!PyDict_Check(kwds) || PyDict_GET_SIZE(kwds) != 0)) {
11781178
PyErr_Format(PyExc_TypeError,
1179-
"wrapper %s doesn't take keyword arguments",
1179+
"wrapper %s() takes no keyword arguments",
11801180
wp->descr->d_base->name);
11811181
return NULL;
11821182
}

‎Python/getargs.c‎

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1704,13 +1704,21 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format,
17041704
break;
17051705
}
17061706
if (max < nargs) {
1707-
PyErr_Format(PyExc_TypeError,
1708-
"%.200s%s takes %s %d positional arguments"
1709-
" (%d given)",
1710-
(fname == NULL) ? "function" : fname,
1711-
(fname == NULL) ? "" : "()",
1712-
(min != INT_MAX) ? "at most" : "exactly",
1713-
max, nargs);
1707+
if (max == 0) {
1708+
PyErr_Format(PyExc_TypeError,
1709+
"%.200s%s takes no positional arguments",
1710+
(fname == NULL) ? "function" : fname,
1711+
(fname == NULL) ? "" : "()");
1712+
}
1713+
else {
1714+
PyErr_Format(PyExc_TypeError,
1715+
"%.200s%s takes %s %d positional arguments"
1716+
" (%d given)",
1717+
(fname == NULL) ? "function" : fname,
1718+
(fname == NULL) ? "" : "()",
1719+
(min != INT_MAX) ? "at most" : "exactly",
1720+
max, nargs);
1721+
}
17141722
return cleanreturn(0, &freelist);
17151723
}
17161724
}
@@ -2079,12 +2087,20 @@ vgetargskeywordsfast_impl(PyObject **args, Py_ssize_t nargs,
20792087
return cleanreturn(0, &freelist);
20802088
}
20812089
if (parser->max < nargs) {
2082-
PyErr_Format(PyExc_TypeError,
2083-
"%200s%s takes %s %d positional arguments (%d given)",
2084-
(parser->fname == NULL) ? "function" : parser->fname,
2085-
(parser->fname == NULL) ? "" : "()",
2086-
(parser->min != INT_MAX) ? "at most" : "exactly",
2087-
parser->max, nargs);
2090+
if (parser->max == 0) {
2091+
PyErr_Format(PyExc_TypeError,
2092+
"%200s%s takes no positional arguments",
2093+
(parser->fname == NULL) ? "function" : parser->fname,
2094+
(parser->fname == NULL) ? "" : "()");
2095+
}
2096+
else {
2097+
PyErr_Format(PyExc_TypeError,
2098+
"%200s%s takes %s %d positional arguments (%d given)",
2099+
(parser->fname == NULL) ? "function" : parser->fname,
2100+
(parser->fname == NULL) ? "" : "()",
2101+
(parser->min != INT_MAX) ? "at most" : "exactly",
2102+
parser->max, nargs);
2103+
}
20882104
return cleanreturn(0, &freelist);
20892105
}
20902106

@@ -2489,7 +2505,7 @@ _PyArg_NoKeywords(const char *funcname, PyObject *kwargs)
24892505
return 1;
24902506
}
24912507

2492-
PyErr_Format(PyExc_TypeError, "%.200s does not take keyword arguments",
2508+
PyErr_Format(PyExc_TypeError, "%.200s() takes no keyword arguments",
24932509
funcname);
24942510
return 0;
24952511
}
@@ -2506,7 +2522,7 @@ _PyArg_NoStackKeywords(const char *funcname, PyObject *kwnames)
25062522
return 1;
25072523
}
25082524

2509-
PyErr_Format(PyExc_TypeError, "%.200s does not take keyword arguments",
2525+
PyErr_Format(PyExc_TypeError, "%.200s() takes no keyword arguments",
25102526
funcname);
25112527
return 0;
25122528
}
@@ -2524,7 +2540,7 @@ _PyArg_NoPositional(const char *funcname, PyObject *args)
25242540
if (PyTuple_GET_SIZE(args) == 0)
25252541
return 1;
25262542

2527-
PyErr_Format(PyExc_TypeError, "%.200s does not take positional arguments",
2543+
PyErr_Format(PyExc_TypeError, "%.200s() takes no positional arguments",
25282544
funcname);
25292545
return 0;
25302546
}

0 commit comments

Comments
 (0)