[Python-checkins] cpython: greatly improve argument parsing error messages (closes #12265)
benjamin.peterson
python-checkins at python.org
Mon Jun 6 05:05:09 CEST 2011
http://hg.python.org/cpython/rev/44d46d74ef4f
changeset: 70652:44d46d74ef4f
parent: 70647:e2a2811ec9e8
user: Benjamin Peterson <benjamin at python.org>
date: Sun Jun 05 22:04:07 2011 -0500
summary:
greatly improve argument parsing error messages (closes #12265)
files:
Lib/inspect.py | 109 +++---
Lib/test/test_extcall.py | 82 ++++-
Lib/test/test_keywordonlyarg.py | 2 +-
Misc/NEWS | 3 +
Python/ceval.c | 309 ++++++++++---------
5 files changed, 304 insertions(+), 201 deletions(-)
diff --git a/Lib/inspect.py b/Lib/inspect.py
--- a/Lib/inspect.py
+++ b/Lib/inspect.py
@@ -914,6 +914,29 @@
specs.append(formatvarkw(varkw) + formatvalue(locals[varkw]))
return '(' + ', '.join(specs) + ')'
+def _positional_error(f_name, args, kwonly, varargs, defcount, given, values):
+ atleast = len(args) - defcount
+ if given is None:
+ given = len([arg for arg in args if arg in values])
+ kwonly_given = len([arg for arg in kwonly if arg in values])
+ if varargs:
+ plural = atleast != 1
+ sig = "at least %d" % (atleast,)
+ elif defcount:
+ plural = True
+ sig = "from %d to %d" % (atleast, len(args))
+ else:
+ plural = len(args) != 1
+ sig = str(len(args))
+ kwonly_sig = ""
+ if kwonly_given:
+ msg = " positional argument%s (and %d keyword-only argument%s)"
+ kwonly_sig = (msg % ("s" if given != 1 else "", kwonly_given,
+ "s" if kwonly_given != 1 else ""))
+ raise TypeError("%s() takes %s positional argument%s but %d%s %s given" %
+ (f_name, sig, "s" if plural else "", given, kwonly_sig,
+ "was" if given == 1 and not kwonly_given else "were"))
+
def getcallargs(func, *positional, **named):
"""Get the mapping of arguments to values.
@@ -925,64 +948,50 @@
f_name = func.__name__
arg2value = {}
+
if ismethod(func) and func.__self__ is not None:
# implicit 'self' (or 'cls' for classmethods) argument
positional = (func.__self__,) + positional
num_pos = len(positional)
- num_total = num_pos + len(named)
num_args = len(args)
num_defaults = len(defaults) if defaults else 0
- for arg, value in zip(args, positional):
- arg2value[arg] = value
+
+ n = min(num_pos, num_args)
+ for i in range(n):
+ arg2value[args[i]] = positional[i]
if varargs:
- if num_pos > num_args:
- arg2value[varargs] = positional[-(num_pos-num_args):]
- else:
- arg2value[varargs] = ()
- elif 0 < num_args < num_pos:
- raise TypeError('%s() takes %s %d positional %s (%d given)' % (
- f_name, 'at most' if defaults else 'exactly', num_args,
- 'arguments' if num_args > 1 else 'argument', num_total))
- elif num_args == 0 and num_total:
- if varkw or kwonlyargs:
- if num_pos:
- # XXX: We should use num_pos, but Python also uses num_total:
- raise TypeError('%s() takes exactly 0 positional arguments '
- '(%d given)' % (f_name, num_total))
- else:
- raise TypeError('%s() takes no arguments (%d given)' %
- (f_name, num_total))
-
- for arg in itertools.chain(args, kwonlyargs):
- if arg in named:
- if arg in arg2value:
- raise TypeError("%s() got multiple values for keyword "
- "argument '%s'" % (f_name, arg))
- else:
- arg2value[arg] = named.pop(arg)
- for kwonlyarg in kwonlyargs:
- if kwonlyarg not in arg2value:
- try:
- arg2value[kwonlyarg] = kwonlydefaults[kwonlyarg]
- except KeyError:
- raise TypeError("%s() needs keyword-only argument %s" %
- (f_name, kwonlyarg))
- if defaults: # fill in any missing values with the defaults
- for arg, value in zip(args[-num_defaults:], defaults):
+ arg2value[varargs] = tuple(positional[n:])
+ possible_kwargs = set(args + kwonlyargs)
+ if varkw:
+ arg2value[varkw] = {}
+ for kw, value in named.items():
+ if kw not in possible_kwargs:
+ if not varkw:
+ raise TypeError("%s() got an unexpected keyword argument %r" %
+ (f_name, kw))
+ arg2value[varkw][kw] = value
+ continue
+ if kw in arg2value:
+ raise TypeError("%s() got multiple values for argument %r" %
+ (f_name, kw))
+ arg2value[kw] = value
+ if num_pos > num_args and not varargs:
+ _positional_error(f_name, args, kwonlyargs, varargs, num_defaults,
+ num_pos, arg2value)
+ if num_pos < num_args:
+ for arg in args[:num_args - num_defaults]:
if arg not in arg2value:
- arg2value[arg] = value
- if varkw:
- arg2value[varkw] = named
- elif named:
- unexpected = next(iter(named))
- raise TypeError("%s() got an unexpected keyword argument '%s'" %
- (f_name, unexpected))
- unassigned = num_args - len([arg for arg in args if arg in arg2value])
- if unassigned:
- num_required = num_args - num_defaults
- raise TypeError('%s() takes %s %d %s (%d given)' % (
- f_name, 'at least' if defaults else 'exactly', num_required,
- 'arguments' if num_required > 1 else 'argument', num_total))
+ _positional_error(f_name, args, kwonlyargs, varargs,
+ num_defaults, None, arg2value)
+ for i, arg in enumerate(args[num_args - num_defaults:]):
+ if arg not in arg2value:
+ arg2value[arg] = defaults[i]
+ for kwarg in kwonlyargs:
+ if kwarg not in arg2value:
+ if kwarg not in kwonlydefaults:
+ raise TypeError("%s() requires keyword-only argument %r" %
+ (f_name, kwarg))
+ arg2value[kwarg] = kwonlydefaults[kwarg]
return arg2value
# -------------------------------------------------- stack frame extraction
diff --git a/Lib/test/test_extcall.py b/Lib/test/test_extcall.py
--- a/Lib/test/test_extcall.py
+++ b/Lib/test/test_extcall.py
@@ -66,17 +66,17 @@
>>> g()
Traceback (most recent call last):
...
- TypeError: g() takes at least 1 argument (0 given)
+ TypeError: g() takes at least 1 positional argument but 0 were given
>>> g(*())
Traceback (most recent call last):
...
- TypeError: g() takes at least 1 argument (0 given)
+ TypeError: g() takes at least 1 positional argument but 0 were given
>>> g(*(), **{})
Traceback (most recent call last):
...
- TypeError: g() takes at least 1 argument (0 given)
+ TypeError: g() takes at least 1 positional argument but 0 were given
>>> g(1)
1 () {}
@@ -151,7 +151,7 @@
>>> g(1, 2, 3, **{'x': 4, 'y': 5})
Traceback (most recent call last):
...
- TypeError: g() got multiple values for keyword argument 'x'
+ TypeError: g() got multiple values for argument 'x'
>>> f(**{1:2})
Traceback (most recent call last):
@@ -263,29 +263,91 @@
>>> f(**x)
1 2
-A obscure message:
+Some additional tests about positional argument errors:
>>> def f(a, b):
... pass
>>> f(b=1)
Traceback (most recent call last):
...
- TypeError: f() takes exactly 2 arguments (1 given)
-
-The number of arguments passed in includes keywords:
+ TypeError: f() takes 2 positional arguments but 1 was given
>>> def f(a):
... pass
>>> f(6, a=4, *(1, 2, 3))
Traceback (most recent call last):
...
- TypeError: f() takes exactly 1 positional argument (5 given)
+ TypeError: f() got multiple values for argument 'a'
>>> def f(a, *, kw):
... pass
>>> f(6, 4, kw=4)
Traceback (most recent call last):
...
- TypeError: f() takes exactly 1 positional argument (3 given)
+ TypeError: f() takes 1 positional argument but 2 positional arguments (and 1 keyword-only argument) were given
+
+ >>> def f(a):
+ ... pass
+ >>> f()
+ Traceback (most recent call last):
+ ...
+ TypeError: f() takes 1 positional argument but 0 were given
+
+ >>> def f(a, b):
+ ... pass
+ >>> f(1)
+ Traceback (most recent call last):
+ ...
+ TypeError: f() takes 2 positional arguments but 1 was given
+
+ >>> def f(a, *b):
+ ... pass
+ >>> f()
+ Traceback (most recent call last):
+ ...
+ TypeError: f() takes at least 1 positional argument but 0 were given
+
+ >>> def f(a, *, kw=4):
+ ... pass
+ >>> f(kw=4)
+ Traceback (most recent call last):
+ ...
+ TypeError: f() takes 1 positional argument but 0 positional arguments (and 1 keyword-only argument) were given
+
+ >>> def f(a, b=2):
+ ... pass
+ >>> f()
+ Traceback (most recent call last):
+ ...
+ TypeError: f() takes from 1 to 2 positional arguments but 0 were given
+
+ >>> def f(a, *b):
+ ... pass
+ >>> f()
+ Traceback (most recent call last):
+ ...
+ TypeError: f() takes at least 1 positional argument but 0 were given
+
+ >>> def f(*, kw):
+ ... pass
+ >>> f(3, kw=4)
+ Traceback (most recent call last):
+ ...
+ TypeError: f() takes 0 positional arguments but 1 positional argument (and 1 keyword-only argument) were given
+
+ >>> def f(a, c=3, *b, kw):
+ ... pass
+ >>> f()
+ Traceback (most recent call last):
+ ...
+ TypeError: f() takes at least 1 positional argument but 0 were given
+ >>> f(kw=3)
+ Traceback (most recent call last):
+ ...
+ TypeError: f() takes at least 1 positional argument but 0 positional arguments (and 1 keyword-only argument) were given
+ >>> f(kw=3, c=4)
+ Traceback (most recent call last):
+ ...
+ TypeError: f() takes at least 1 positional argument but 1 positional argument (and 1 keyword-only argument) were given
"""
import sys
diff --git a/Lib/test/test_keywordonlyarg.py b/Lib/test/test_keywordonlyarg.py
--- a/Lib/test/test_keywordonlyarg.py
+++ b/Lib/test/test_keywordonlyarg.py
@@ -78,7 +78,7 @@
pass
with self.assertRaises(TypeError) as exc:
f(1, 2, 3)
- expected = "f() takes at most 2 positional arguments (3 given)"
+ expected = "f() takes from 1 to 2 positional arguments but 3 were given"
self.assertEqual(str(exc.exception), expected)
def testSyntaxErrorForFunctionCall(self):
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -10,6 +10,9 @@
Core and Builtins
-----------------
+- Issue #12265: Make error messages produced by passing an invalid set of
+ arguments to a function more informative.
+
- Issue #12225: Still allow Python to build if Python is not in its hg repo or
mercurial is not installed.
diff --git a/Python/ceval.c b/Python/ceval.c
--- a/Python/ceval.c
+++ b/Python/ceval.c
@@ -3045,6 +3045,63 @@
return retval;
}
+static void
+positional_argument_error(PyCodeObject *co, int given, int defcount, PyObject **fastlocals)
+{
+ int plural;
+ int kwonly_given = 0;
+ int atleast = co->co_argcount - defcount;
+ int i;
+ PyObject *sig, *kwonly_sig;
+
+ if (given == -1) {
+ given = 0;
+ for (i = 0; i < co->co_argcount; i++)
+ if (GETLOCAL(i))
+ given++;
+ }
+ for (i = co->co_argcount; i < co->co_argcount + co->co_kwonlyargcount; i++)
+ if (GETLOCAL(i))
+ kwonly_given++;
+ if (co->co_flags & CO_VARARGS) {
+ plural = atleast != 1;
+ sig = PyUnicode_FromFormat("at least %d", atleast);
+ }
+ else if (defcount) {
+ plural = 1;
+ sig = PyUnicode_FromFormat("from %d to %d", atleast, co->co_argcount);
+ }
+ else {
+ plural = co->co_argcount != 1;
+ sig = PyUnicode_FromFormat("%d", co->co_argcount);
+ }
+ if (sig == NULL)
+ return;
+ if (kwonly_given) {
+ const char *format = " positional argument%s (and %d keyword-only argument%s)";
+ kwonly_sig = PyUnicode_FromFormat(format, given != 1 ? "s" : "", kwonly_given,
+ kwonly_given != 1 ? "s" : "");
+ if (kwonly_sig == NULL) {
+ Py_DECREF(sig);
+ return;
+ }
+ }
+ else {
+ /* This will not fail. */
+ kwonly_sig = PyUnicode_FromString("");
+ }
+ PyErr_Format(PyExc_TypeError,
+ "%U() takes %U positional argument%s but %d%U %s given",
+ co->co_name,
+ sig,
+ plural ? "s" : "",
+ given,
+ kwonly_sig,
+ given == 1 && !kwonly_given ? "was" : "were");
+ Py_DECREF(sig);
+ Py_DECREF(kwonly_sig);
+}
+
/* This is gonna seem *real weird*, but if you put some other code between
PyEval_EvalFrame() and PyEval_EvalCodeEx() you will need to adjust
the test in the if statements in Misc/gdbinit (pystack and pystackv). */
@@ -3061,6 +3118,9 @@
PyThreadState *tstate = PyThreadState_GET();
PyObject *x, *u;
int total_args = co->co_argcount + co->co_kwonlyargcount;
+ int i;
+ int n = argcount;
+ PyObject *kwdict = NULL;
if (globals == NULL) {
PyErr_SetString(PyExc_SystemError,
@@ -3077,161 +3137,130 @@
fastlocals = f->f_localsplus;
freevars = f->f_localsplus + co->co_nlocals;
- if (total_args || co->co_flags & (CO_VARARGS | CO_VARKEYWORDS)) {
- int i;
- int n = argcount;
- PyObject *kwdict = NULL;
- if (co->co_flags & CO_VARKEYWORDS) {
- kwdict = PyDict_New();
- if (kwdict == NULL)
- goto fail;
- i = total_args;
- if (co->co_flags & CO_VARARGS)
- i++;
- SETLOCAL(i, kwdict);
- }
- if (argcount > co->co_argcount) {
- if (!(co->co_flags & CO_VARARGS)) {
- PyErr_Format(PyExc_TypeError,
- "%U() takes %s %d "
- "positional argument%s (%d given)",
- co->co_name,
- defcount ? "at most" : "exactly",
- co->co_argcount,
- co->co_argcount == 1 ? "" : "s",
- argcount + kwcount);
- goto fail;
- }
- n = co->co_argcount;
- }
- for (i = 0; i < n; i++) {
+ /* Parse arguments. */
+ if (co->co_flags & CO_VARKEYWORDS) {
+ kwdict = PyDict_New();
+ if (kwdict == NULL)
+ goto fail;
+ i = total_args;
+ if (co->co_flags & CO_VARARGS)
+ i++;
+ SETLOCAL(i, kwdict);
+ }
+ if (argcount > co->co_argcount)
+ n = co->co_argcount;
+ for (i = 0; i < n; i++) {
+ x = args[i];
+ Py_INCREF(x);
+ SETLOCAL(i, x);
+ }
+ if (co->co_flags & CO_VARARGS) {
+ u = PyTuple_New(argcount - n);
+ if (u == NULL)
+ goto fail;
+ SETLOCAL(total_args, u);
+ for (i = n; i < argcount; i++) {
x = args[i];
Py_INCREF(x);
- SETLOCAL(i, x);
+ PyTuple_SET_ITEM(u, i-n, x);
}
- if (co->co_flags & CO_VARARGS) {
- u = PyTuple_New(argcount - n);
- if (u == NULL)
+ }
+ for (i = 0; i < kwcount; i++) {
+ PyObject **co_varnames;
+ PyObject *keyword = kws[2*i];
+ PyObject *value = kws[2*i + 1];
+ int j;
+ if (keyword == NULL || !PyUnicode_Check(keyword)) {
+ PyErr_Format(PyExc_TypeError,
+ "%U() keywords must be strings",
+ co->co_name);
+ goto fail;
+ }
+ /* Speed hack: do raw pointer compares. As names are
+ normally interned this should almost always hit. */
+ co_varnames = ((PyTupleObject *)(co->co_varnames))->ob_item;
+ for (j = 0; j < total_args; j++) {
+ PyObject *nm = co_varnames[j];
+ if (nm == keyword)
+ goto kw_found;
+ }
+ /* Slow fallback, just in case */
+ for (j = 0; j < total_args; j++) {
+ PyObject *nm = co_varnames[j];
+ int cmp = PyObject_RichCompareBool(
+ keyword, nm, Py_EQ);
+ if (cmp > 0)
+ goto kw_found;
+ else if (cmp < 0)
goto fail;
- SETLOCAL(total_args, u);
- for (i = n; i < argcount; i++) {
- x = args[i];
- Py_INCREF(x);
- PyTuple_SET_ITEM(u, i-n, x);
- }
}
- for (i = 0; i < kwcount; i++) {
- PyObject **co_varnames;
- PyObject *keyword = kws[2*i];
- PyObject *value = kws[2*i + 1];
- int j;
- if (keyword == NULL || !PyUnicode_Check(keyword)) {
- PyErr_Format(PyExc_TypeError,
- "%U() keywords must be strings",
- co->co_name);
- goto fail;
- }
- /* Speed hack: do raw pointer compares. As names are
- normally interned this should almost always hit. */
- co_varnames = ((PyTupleObject *)(co->co_varnames))->ob_item;
- for (j = 0; j < total_args; j++) {
- PyObject *nm = co_varnames[j];
- if (nm == keyword)
- goto kw_found;
- }
- /* Slow fallback, just in case */
- for (j = 0; j < total_args; j++) {
- PyObject *nm = co_varnames[j];
- int cmp = PyObject_RichCompareBool(
- keyword, nm, Py_EQ);
- if (cmp > 0)
- goto kw_found;
- else if (cmp < 0)
- goto fail;
- }
- if (j >= total_args && kwdict == NULL) {
- PyErr_Format(PyExc_TypeError,
- "%U() got an unexpected "
- "keyword argument '%S'",
- co->co_name,
- keyword);
- goto fail;
- }
- PyDict_SetItem(kwdict, keyword, value);
- continue;
- kw_found:
- if (GETLOCAL(j) != NULL) {
- PyErr_Format(PyExc_TypeError,
- "%U() got multiple "
- "values for keyword "
- "argument '%S'",
+ if (j >= total_args && kwdict == NULL) {
+ PyErr_Format(PyExc_TypeError,
+ "%U() got an unexpected "
+ "keyword argument '%S'",
co->co_name,
keyword);
- goto fail;
- }
- Py_INCREF(value);
- SETLOCAL(j, value);
+ goto fail;
}
- if (co->co_kwonlyargcount > 0) {
- for (i = co->co_argcount; i < total_args; i++) {
- PyObject *name;
- if (GETLOCAL(i) != NULL)
- continue;
- name = PyTuple_GET_ITEM(co->co_varnames, i);
- if (kwdefs != NULL) {
- PyObject *def = PyDict_GetItem(kwdefs, name);
- if (def) {
- Py_INCREF(def);
- SETLOCAL(i, def);
- continue;
- }
- }
- PyErr_Format(PyExc_TypeError,
- "%U() needs keyword-only argument %S",
- co->co_name, name);
+ PyDict_SetItem(kwdict, keyword, value);
+ continue;
+ kw_found:
+ if (GETLOCAL(j) != NULL) {
+ PyErr_Format(PyExc_TypeError,
+ "%U() got multiple "
+ "values for argument '%S'",
+ co->co_name,
+ keyword);
+ goto fail;
+ }
+ Py_INCREF(value);
+ SETLOCAL(j, value);
+ }
+ if (argcount > co->co_argcount && !(co->co_flags & CO_VARARGS)) {
+ positional_argument_error(co, argcount, defcount, fastlocals);
+ goto fail;
+ }
+ if (argcount < co->co_argcount) {
+ int m = co->co_argcount - defcount;
+ for (i = argcount; i < m; i++) {
+ if (GETLOCAL(i) == NULL) {
+ positional_argument_error(co, -1, defcount, fastlocals);
goto fail;
}
}
- if (argcount < co->co_argcount) {
- int m = co->co_argcount - defcount;
- for (i = argcount; i < m; i++) {
- if (GETLOCAL(i) == NULL) {
- int j, given = 0;
- for (j = 0; j < co->co_argcount; j++)
- if (GETLOCAL(j))
- given++;
- PyErr_Format(PyExc_TypeError,
- "%U() takes %s %d "
- "argument%s "
- "(%d given)",
- co->co_name,
- ((co->co_flags & CO_VARARGS) ||
- defcount) ? "at least"
- : "exactly",
- m, m == 1 ? "" : "s", given);
- goto fail;
- }
- }
- if (n > m)
- i = n - m;
- else
- i = 0;
- for (; i < defcount; i++) {
- if (GETLOCAL(m+i) == NULL) {
- PyObject *def = defs[i];
- Py_INCREF(def);
- SETLOCAL(m+i, def);
- }
+ if (n > m)
+ i = n - m;
+ else
+ i = 0;
+ for (; i < defcount; i++) {
+ if (GETLOCAL(m+i) == NULL) {
+ PyObject *def = defs[i];
+ Py_INCREF(def);
+ SETLOCAL(m+i, def);
}
}
}
- else if (argcount > 0 || kwcount > 0) {
- PyErr_Format(PyExc_TypeError,
- "%U() takes no arguments (%d given)",
- co->co_name,
- argcount + kwcount);
- goto fail;
+ if (co->co_kwonlyargcount > 0) {
+ for (i = co->co_argcount; i < total_args; i++) {
+ PyObject *name;
+ if (GETLOCAL(i) != NULL)
+ continue;
+ name = PyTuple_GET_ITEM(co->co_varnames, i);
+ if (kwdefs != NULL) {
+ PyObject *def = PyDict_GetItem(kwdefs, name);
+ if (def) {
+ Py_INCREF(def);
+ SETLOCAL(i, def);
+ continue;
+ }
+ }
+ PyErr_Format(PyExc_TypeError,
+ "%U() requires keyword-only argument '%S'",
+ co->co_name, name);
+ goto fail;
+ }
}
+
/* Allocate and initialize storage for cell vars, and copy free
vars into frame. This isn't too efficient right now. */
if (PyTuple_GET_SIZE(co->co_cellvars)) {
--
Repository URL: http://hg.python.org/cpython
More information about the Python-checkins
mailing list