Skip to content

bpo-30534: Fix error messages when pass keyword arguments #1901

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
serhiy-storchaka merged 5 commits into from
Jun 6, 2017
Merged

bpo-30534: Fix error messages when pass keyword arguments #1901

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
838d2ab
bpo-30534: Fix error messages when pass keyword arguments
serhiy-storchaka Jun 1, 2017
7b4c077
Unify error messages for functions that don't take positional or keyword
serhiy-storchaka Jun 3, 2017
e78eee9
Merge branch 'master' into call-no-keyword-args
serhiy-storchaka Jun 3, 2017
39556df
Add tests.
serhiy-storchaka Jun 3, 2017
e470e26
Addressed review comments.
serhiy-storchaka Jun 6, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 57 additions & 3 deletions Lib/test/test_call.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -33,9 +34,6 @@ def test_varargs2_ext(self):
else:
raise RuntimeError

def test_varargs0_kw(self):
self.assertRaises(TypeError, {}.__contains__, x=2)

Copy link
Member

@vstinner vstinner Jun 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may keep this test for other Python implementations. (This test doesn't check the error message.)

Copy link
Member Author

@serhiy-storchaka serhiy-storchaka Jun 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a duplicate of the following test.

Copy link
Member

@vstinner vstinner Jun 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh ok, you are right.

def test_varargs1_kw(self):
self.assertRaises(TypeError, {}.__contains__, x=2)

Expand Down Expand Up @@ -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()
2 changes: 1 addition & 1 deletion Lib/test/test_itertools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
49 changes: 25 additions & 24 deletions Objects/call.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,38 +466,37 @@ _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)",
method->ml_name, nargs);
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)",
method->ml_name, nargs);
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 */
Expand Down Expand Up @@ -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")) {
Expand All @@ -602,32 +601,32 @@ _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)",
method->ml_name, nargs);
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)",
method->ml_name, nargs);
goto exit;
}

if (nkwargs) {
goto no_keyword_error;
}

result = (*meth) (self, args[0]);
break;

Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion Objects/descrobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
48 changes: 32 additions & 16 deletions Python/getargs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand Down