gh-107944: Improve error message for getargs with bad keyword arguments (#114792)

This commit is contained in:
Shantanu 2024-02-08 01:04:41 -08:00 committed by GitHub
parent 9e90313320
commit 17689e3c41
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 113 additions and 29 deletions

View File

@ -101,6 +101,17 @@ Improved Error Messages
variables. See also :ref:`using-on-controlling-color`. variables. See also :ref:`using-on-controlling-color`.
(Contributed by Pablo Galindo Salgado in :gh:`112730`.) (Contributed by Pablo Galindo Salgado in :gh:`112730`.)
* When an incorrect keyword argument is passed to a function, the error message
now potentially suggests the correct keyword argument.
(Contributed by Pablo Galindo Salgado and Shantanu Jain in :gh:`107944`.)
>>> "better error messages!".split(max_split=1)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
"better error messages!".split(max_split=1)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^
TypeError: split() got an unexpected keyword argument 'max_split'. Did you mean 'maxsplit'?
Other Language Changes Other Language Changes
====================== ======================

View File

@ -155,7 +155,7 @@ class CFunctionCallsErrorMessages(unittest.TestCase):
min, 0, default=1, key=2, foo=3) min, 0, default=1, key=2, foo=3)
def test_varargs17_kw(self): def test_varargs17_kw(self):
msg = r"'foo' is an invalid keyword argument for print\(\)$" msg = r"print\(\) got an unexpected keyword argument 'foo'$"
self.assertRaisesRegex(TypeError, msg, self.assertRaisesRegex(TypeError, msg,
print, 0, sep=1, end=2, file=3, flush=4, foo=5) print, 0, sep=1, end=2, file=3, flush=4, foo=5)
@ -928,7 +928,7 @@ class TestErrorMessagesSuggestions(unittest.TestCase):
self.assertIn(f"Did you mean '{message}'?", str(cm.exception)) self.assertIn(f"Did you mean '{message}'?", str(cm.exception))
@contextlib.contextmanager @contextlib.contextmanager
def check_suggestion_not_pressent(self): def check_suggestion_not_present(self):
with self.assertRaises(TypeError) as cm: with self.assertRaises(TypeError) as cm:
yield yield
self.assertNotIn("Did you mean", str(cm.exception)) self.assertNotIn("Did you mean", str(cm.exception))
@ -946,7 +946,7 @@ class TestErrorMessagesSuggestions(unittest.TestCase):
for keyword, suggestion in cases: for keyword, suggestion in cases:
with self.subTest(keyword): with self.subTest(keyword):
ctx = self.check_suggestion_includes(suggestion) if suggestion else self.check_suggestion_not_pressent() ctx = self.check_suggestion_includes(suggestion) if suggestion else self.check_suggestion_not_present()
with ctx: with ctx:
foo(**{keyword:None}) foo(**{keyword:None})
@ -987,6 +987,32 @@ class TestErrorMessagesSuggestions(unittest.TestCase):
with self.check_suggestion_includes(suggestion): with self.check_suggestion_includes(suggestion):
func(bluch=None) func(bluch=None)
def test_unexpected_keyword_suggestion_via_getargs(self):
with self.check_suggestion_includes("maxsplit"):
"foo".split(maxsplt=1)
self.assertRaisesRegex(
TypeError, r"split\(\) got an unexpected keyword argument 'blech'$",
"foo".split, blech=1
)
with self.check_suggestion_not_present():
"foo".split(blech=1)
with self.check_suggestion_not_present():
"foo".split(more_noise=1, maxsplt=1)
# Also test the vgetargskeywords path
with self.check_suggestion_includes("name"):
ImportError(namez="oops")
self.assertRaisesRegex(
TypeError, r"ImportError\(\) got an unexpected keyword argument 'blech'$",
ImportError, blech=1
)
with self.check_suggestion_not_present():
ImportError(blech=1)
with self.check_suggestion_not_present():
ImportError(blech=1, namez="oops")
@cpython_only @cpython_only
class TestRecursion(unittest.TestCase): class TestRecursion(unittest.TestCase):

View File

@ -667,7 +667,7 @@ class Keywords_TestCase(unittest.TestCase):
try: try:
getargs_keywords((1,2),3,arg5=10,arg666=666) getargs_keywords((1,2),3,arg5=10,arg666=666)
except TypeError as err: except TypeError as err:
self.assertEqual(str(err), "'arg666' is an invalid keyword argument for this function") self.assertEqual(str(err), "this function got an unexpected keyword argument 'arg666'")
else: else:
self.fail('TypeError should have been raised') self.fail('TypeError should have been raised')
@ -675,7 +675,7 @@ class Keywords_TestCase(unittest.TestCase):
try: try:
getargs_keywords((1,2), 3, (4,(5,6)), (7,8,9), **{'\uDC80': 10}) getargs_keywords((1,2), 3, (4,(5,6)), (7,8,9), **{'\uDC80': 10})
except TypeError as err: except TypeError as err:
self.assertEqual(str(err), "'\udc80' is an invalid keyword argument for this function") self.assertEqual(str(err), "this function got an unexpected keyword argument '\udc80'")
else: else:
self.fail('TypeError should have been raised') self.fail('TypeError should have been raised')
@ -742,12 +742,12 @@ class KeywordOnly_TestCase(unittest.TestCase):
def test_invalid_keyword(self): def test_invalid_keyword(self):
# extraneous keyword arg # extraneous keyword arg
with self.assertRaisesRegex(TypeError, with self.assertRaisesRegex(TypeError,
"'monster' is an invalid keyword argument for this function"): "this function got an unexpected keyword argument 'monster'"):
getargs_keyword_only(1, 2, monster=666) getargs_keyword_only(1, 2, monster=666)
def test_surrogate_keyword(self): def test_surrogate_keyword(self):
with self.assertRaisesRegex(TypeError, with self.assertRaisesRegex(TypeError,
"'\udc80' is an invalid keyword argument for this function"): "this function got an unexpected keyword argument '\udc80'"):
getargs_keyword_only(1, 2, **{'\uDC80': 10}) getargs_keyword_only(1, 2, **{'\uDC80': 10})
def test_weird_str_subclass(self): def test_weird_str_subclass(self):
@ -761,7 +761,7 @@ class KeywordOnly_TestCase(unittest.TestCase):
"invalid keyword argument for this function"): "invalid keyword argument for this function"):
getargs_keyword_only(1, 2, **{BadStr("keyword_only"): 3}) getargs_keyword_only(1, 2, **{BadStr("keyword_only"): 3})
with self.assertRaisesRegex(TypeError, with self.assertRaisesRegex(TypeError,
"invalid keyword argument for this function"): "this function got an unexpected keyword argument"):
getargs_keyword_only(1, 2, **{BadStr("monster"): 666}) getargs_keyword_only(1, 2, **{BadStr("monster"): 666})
def test_weird_str_subclass2(self): def test_weird_str_subclass2(self):
@ -774,7 +774,7 @@ class KeywordOnly_TestCase(unittest.TestCase):
"invalid keyword argument for this function"): "invalid keyword argument for this function"):
getargs_keyword_only(1, 2, **{BadStr("keyword_only"): 3}) getargs_keyword_only(1, 2, **{BadStr("keyword_only"): 3})
with self.assertRaisesRegex(TypeError, with self.assertRaisesRegex(TypeError,
"invalid keyword argument for this function"): "this function got an unexpected keyword argument"):
getargs_keyword_only(1, 2, **{BadStr("monster"): 666}) getargs_keyword_only(1, 2, **{BadStr("monster"): 666})
@ -807,7 +807,7 @@ class PositionalOnlyAndKeywords_TestCase(unittest.TestCase):
def test_empty_keyword(self): def test_empty_keyword(self):
with self.assertRaisesRegex(TypeError, with self.assertRaisesRegex(TypeError,
"'' is an invalid keyword argument for this function"): "this function got an unexpected keyword argument ''"):
self.getargs(1, 2, **{'': 666}) self.getargs(1, 2, **{'': 666})
@ -1204,7 +1204,7 @@ class ParseTupleAndKeywords_Test(unittest.TestCase):
"function missing required argument 'a'"): "function missing required argument 'a'"):
parse((), {}, 'O', ['a']) parse((), {}, 'O', ['a'])
with self.assertRaisesRegex(TypeError, with self.assertRaisesRegex(TypeError,
"'b' is an invalid keyword argument"): "this function got an unexpected keyword argument 'b'"):
parse((), {'b': 1}, '|O', ['a']) parse((), {'b': 1}, '|O', ['a'])
with self.assertRaisesRegex(TypeError, with self.assertRaisesRegex(TypeError,
fr"argument for function given by name \('a'\) " fr"argument for function given by name \('a'\) "
@ -1278,10 +1278,10 @@ class ParseTupleAndKeywords_Test(unittest.TestCase):
fr"and position \(1\)"): fr"and position \(1\)"):
parse((1,), {name: 2}, 'O|O', [name, 'b']) parse((1,), {name: 2}, 'O|O', [name, 'b'])
with self.assertRaisesRegex(TypeError, with self.assertRaisesRegex(TypeError,
f"'{name}' is an invalid keyword argument"): f"this function got an unexpected keyword argument '{name}'"):
parse((), {name: 1}, '|O', ['b']) parse((), {name: 1}, '|O', ['b'])
with self.assertRaisesRegex(TypeError, with self.assertRaisesRegex(TypeError,
"'b' is an invalid keyword argument"): "this function got an unexpected keyword argument 'b'"):
parse((), {'b': 1}, '|O', [name]) parse((), {'b': 1}, '|O', [name])
invalid = name.encode() + (name.encode()[:-1] or b'\x80') invalid = name.encode() + (name.encode()[:-1] or b'\x80')
@ -1301,17 +1301,17 @@ class ParseTupleAndKeywords_Test(unittest.TestCase):
for name2 in ('b', 'ë', 'ĉ', '', '𐀁'): for name2 in ('b', 'ë', 'ĉ', '', '𐀁'):
with self.subTest(name2=name2): with self.subTest(name2=name2):
with self.assertRaisesRegex(TypeError, with self.assertRaisesRegex(TypeError,
f"'{name2}' is an invalid keyword argument"): f"this function got an unexpected keyword argument '{name2}'"):
parse((), {name2: 1}, '|O', [name]) parse((), {name2: 1}, '|O', [name])
name2 = name.encode().decode('latin1') name2 = name.encode().decode('latin1')
if name2 != name: if name2 != name:
with self.assertRaisesRegex(TypeError, with self.assertRaisesRegex(TypeError,
f"'{name2}' is an invalid keyword argument"): f"this function got an unexpected keyword argument '{name2}'"):
parse((), {name2: 1}, '|O', [name]) parse((), {name2: 1}, '|O', [name])
name3 = name + '3' name3 = name + '3'
with self.assertRaisesRegex(TypeError, with self.assertRaisesRegex(TypeError,
f"'{name2}' is an invalid keyword argument"): f"this function got an unexpected keyword argument '{name2}'"):
parse((), {name2: 1, name3: 2}, '|OO', [name, name3]) parse((), {name2: 1, name3: 2}, '|OO', [name, name3])
def test_nested_tuple(self): def test_nested_tuple(self):

View File

@ -1917,7 +1917,7 @@ class ImportErrorTests(unittest.TestCase):
self.assertEqual(exc.name, 'somename') self.assertEqual(exc.name, 'somename')
self.assertEqual(exc.path, 'somepath') self.assertEqual(exc.path, 'somepath')
msg = "'invalid' is an invalid keyword argument for ImportError" msg = r"ImportError\(\) got an unexpected keyword argument 'invalid'"
with self.assertRaisesRegex(TypeError, msg): with self.assertRaisesRegex(TypeError, msg):
ImportError('test', invalid='keyword') ImportError('test', invalid='keyword')

View File

@ -0,0 +1 @@
Improve error message for function calls with bad keyword arguments via getargs

View File

@ -8,6 +8,7 @@
#include "pycore_modsupport.h" // export _PyArg_NoKeywords() #include "pycore_modsupport.h" // export _PyArg_NoKeywords()
#include "pycore_pylifecycle.h" // _PyArg_Fini #include "pycore_pylifecycle.h" // _PyArg_Fini
#include "pycore_tuple.h" // _PyTuple_ITEMS() #include "pycore_tuple.h" // _PyTuple_ITEMS()
#include "pycore_pyerrors.h" // _Py_CalculateSuggestions()
/* Export Stable ABIs (abi only) */ /* Export Stable ABIs (abi only) */
PyAPI_FUNC(int) _PyArg_Parse_SizeT(PyObject *, const char *, ...); PyAPI_FUNC(int) _PyArg_Parse_SizeT(PyObject *, const char *, ...);
@ -1424,12 +1425,31 @@ error_unexpected_keyword_arg(PyObject *kwargs, PyObject *kwnames, PyObject *kwtu
int match = PySequence_Contains(kwtuple, keyword); int match = PySequence_Contains(kwtuple, keyword);
if (match <= 0) { if (match <= 0) {
if (!match) { if (!match) {
PyErr_Format(PyExc_TypeError, PyObject *kwlist = PySequence_List(kwtuple);
"'%S' is an invalid keyword " if (!kwlist) {
"argument for %.200s%s", return;
keyword, }
(fname == NULL) ? "this function" : fname, PyObject *suggestion_keyword = _Py_CalculateSuggestions(kwlist, keyword);
(fname == NULL) ? "" : "()"); Py_DECREF(kwlist);
if (suggestion_keyword) {
PyErr_Format(PyExc_TypeError,
"%.200s%s got an unexpected keyword argument '%S'."
" Did you mean '%S'?",
(fname == NULL) ? "this function" : fname,
(fname == NULL) ? "" : "()",
keyword,
suggestion_keyword);
Py_DECREF(suggestion_keyword);
}
else {
PyErr_Format(PyExc_TypeError,
"%.200s%s got an unexpected keyword argument '%S'",
(fname == NULL) ? "this function" : fname,
(fname == NULL) ? "" : "()",
keyword);
}
} }
return; return;
} }
@ -1457,6 +1477,9 @@ PyArg_ValidateKeywordArguments(PyObject *kwargs)
return 1; return 1;
} }
static PyObject *
new_kwtuple(const char * const *keywords, int total, int pos);
#define IS_END_OF_FORMAT(c) (c == '\0' || c == ';' || c == ':') #define IS_END_OF_FORMAT(c) (c == '\0' || c == ';' || c == ':')
static int static int
@ -1722,12 +1745,35 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format,
} }
} }
if (!match) { if (!match) {
PyErr_Format(PyExc_TypeError, PyObject *_pykwtuple = new_kwtuple(kwlist, len, pos);
"'%U' is an invalid keyword " if (!_pykwtuple) {
"argument for %.200s%s", return cleanreturn(0, &freelist);
key, }
(fname == NULL) ? "this function" : fname, PyObject *pykwlist = PySequence_List(_pykwtuple);
(fname == NULL) ? "" : "()"); Py_DECREF(_pykwtuple);
if (!pykwlist) {
return cleanreturn(0, &freelist);
}
PyObject *suggestion_keyword = _Py_CalculateSuggestions(pykwlist, key);
Py_DECREF(pykwlist);
if (suggestion_keyword) {
PyErr_Format(PyExc_TypeError,
"%.200s%s got an unexpected keyword argument '%S'."
" Did you mean '%S'?",
(fname == NULL) ? "this function" : fname,
(fname == NULL) ? "" : "()",
key,
suggestion_keyword);
Py_DECREF(suggestion_keyword);
}
else {
PyErr_Format(PyExc_TypeError,
"%.200s%s got an unexpected keyword argument '%S'",
(fname == NULL) ? "this function" : fname,
(fname == NULL) ? "" : "()",
key);
}
return cleanreturn(0, &freelist); return cleanreturn(0, &freelist);
} }
} }