From 64c8f705c0121a4b45ca2c3bc7b47b282e9efcd8 Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Sun, 9 Apr 2017 09:47:12 +0200 Subject: bpo-29951: Include function name for some error messages in `PyArg_ParseTuple*` (#916) Also changed format specifier for function name from "%s" to "%.200s" and exception messages should start with lowercase letter. --- Lib/test/test_capi.py | 8 +++--- Lib/test/test_exceptions.py | 2 +- Lib/test/test_getargs2.py | 13 ++++----- Python/getargs.c | 64 +++++++++++++++++++++++++++++---------------- 4 files changed, 54 insertions(+), 33 deletions(-) diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py index 2a53f3d..6e14248 100644 --- a/Lib/test/test_capi.py +++ b/Lib/test/test_capi.py @@ -533,19 +533,19 @@ class SkipitemTest(unittest.TestCase): parse((1, 2, 3), {}, b'OOO', ['', '', 'a']) parse((1, 2), {'a': 3}, b'OOO', ['', '', 'a']) with self.assertRaisesRegex(TypeError, - r'Function takes at least 2 positional arguments \(1 given\)'): + r'function takes at least 2 positional arguments \(1 given\)'): parse((1,), {'a': 3}, b'OOO', ['', '', 'a']) parse((1,), {}, b'O|OO', ['', '', 'a']) with self.assertRaisesRegex(TypeError, - r'Function takes at least 1 positional arguments \(0 given\)'): + r'function takes at least 1 positional arguments \(0 given\)'): parse((), {}, b'O|OO', ['', '', 'a']) parse((1, 2), {'a': 3}, b'OO$O', ['', '', 'a']) with self.assertRaisesRegex(TypeError, - r'Function takes exactly 2 positional arguments \(1 given\)'): + r'function takes exactly 2 positional arguments \(1 given\)'): parse((1,), {'a': 3}, b'OO$O', ['', '', 'a']) parse((1,), {}, b'O|O$O', ['', '', 'a']) with self.assertRaisesRegex(TypeError, - r'Function takes at least 1 positional arguments \(0 given\)'): + r'function takes at least 1 positional arguments \(0 given\)'): parse((), {}, b'O|O$O', ['', '', 'a']) with self.assertRaisesRegex(SystemError, r'Empty parameter name after \$'): parse((1,), {}, b'O|$OO', ['', '', 'a']) diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py index e6fa346..3378ceb 100644 --- a/Lib/test/test_exceptions.py +++ b/Lib/test/test_exceptions.py @@ -1090,7 +1090,7 @@ class ImportErrorTests(unittest.TestCase): self.assertEqual(exc.name, 'somename') self.assertEqual(exc.path, 'somepath') - msg = "'invalid' is an invalid keyword argument for this function" + msg = "'invalid' is an invalid keyword argument for ImportError" with self.assertRaisesRegex(TypeError, msg): ImportError('test', invalid='keyword') diff --git a/Lib/test/test_getargs2.py b/Lib/test/test_getargs2.py index e5d9aa6..86df3a4 100644 --- a/Lib/test/test_getargs2.py +++ b/Lib/test/test_getargs2.py @@ -553,7 +553,8 @@ class Keywords_TestCase(unittest.TestCase): try: getargs_keywords(arg1=(1,2)) except TypeError as err: - self.assertEqual(str(err), "Required argument 'arg2' (pos 2) not found") + self.assertEqual( + str(err), "function missing required argument 'arg2' (pos 2)") else: self.fail('TypeError should have been raised') @@ -626,16 +627,16 @@ class KeywordOnly_TestCase(unittest.TestCase): ) # required arg missing with self.assertRaisesRegex(TypeError, - r"Required argument 'required' \(pos 1\) not found"): + r"function missing required argument 'required' \(pos 1\)"): getargs_keyword_only(optional=2) with self.assertRaisesRegex(TypeError, - r"Required argument 'required' \(pos 1\) not found"): + r"function missing required argument 'required' \(pos 1\)"): getargs_keyword_only(keyword_only=3) def test_too_many_args(self): with self.assertRaisesRegex(TypeError, - r"Function takes at most 2 positional arguments \(3 given\)"): + r"function takes at most 2 positional arguments \(3 given\)"): getargs_keyword_only(1, 2, 3) with self.assertRaisesRegex(TypeError, @@ -674,11 +675,11 @@ class PositionalOnlyAndKeywords_TestCase(unittest.TestCase): self.assertEqual(self.getargs(1), (1, -1, -1)) # required positional arg missing with self.assertRaisesRegex(TypeError, - r"Function takes at least 1 positional arguments \(0 given\)"): + r"function takes at least 1 positional arguments \(0 given\)"): self.getargs() with self.assertRaisesRegex(TypeError, - r"Function takes at least 1 positional arguments \(0 given\)"): + r"function takes at least 1 positional arguments \(0 given\)"): self.getargs(keyword=3) def test_empty_keyword(self): diff --git a/Python/getargs.c b/Python/getargs.c index 8cb672d..58c9a99 100644 --- a/Python/getargs.c +++ b/Python/getargs.c @@ -1584,7 +1584,7 @@ PyArg_ValidateKeywordArguments(PyObject *kwargs) } if (!_PyDict_HasOnlyStringKeys(kwargs)) { PyErr_SetString(PyExc_TypeError, - "keyword arguments must be strings"); + "keywords must be strings"); return 0; } return 1; @@ -1655,7 +1655,7 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format, nkwargs = (kwargs == NULL) ? 0 : PyDict_GET_SIZE(kwargs); if (nargs + nkwargs > len) { PyErr_Format(PyExc_TypeError, - "%s%s takes at most %d argument%s (%zd given)", + "%.200s%s takes at most %d argument%s (%zd given)", (fname == NULL) ? "function" : fname, (fname == NULL) ? "" : "()", len, @@ -1705,8 +1705,10 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format, } if (max < nargs) { PyErr_Format(PyExc_TypeError, - "Function takes %s %d positional arguments" + "%.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); @@ -1752,8 +1754,10 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format, * or the end of the format. */ } else { - PyErr_Format(PyExc_TypeError, "Required argument " - "'%s' (pos %d) not found", + PyErr_Format(PyExc_TypeError, "%.200s%s missing required " + "argument '%s' (pos %d)", + (fname == NULL) ? "function" : fname, + (fname == NULL) ? "" : "()", kwlist[i], i+1); return cleanreturn(0, &freelist); } @@ -1779,8 +1783,10 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format, if (skip) { PyErr_Format(PyExc_TypeError, - "Function takes %s %d positional arguments" + "%.200s%s takes %s %d positional arguments" " (%d given)", + (fname == NULL) ? "function" : fname, + (fname == NULL) ? "" : "()", (Py_MIN(pos, min) < i) ? "at least" : "exactly", Py_MIN(pos, min), nargs); return cleanreturn(0, &freelist); @@ -1802,8 +1808,10 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format, if (current_arg) { /* arg present in tuple and in dict */ PyErr_Format(PyExc_TypeError, - "Argument given by name ('%s') " + "argument for %.200s%s given by name ('%s') " "and position (%d)", + (fname == NULL) ? "function" : fname, + (fname == NULL) ? "" : "()", kwlist[i], i+1); return cleanreturn(0, &freelist); } @@ -1826,8 +1834,10 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format, if (!match) { PyErr_Format(PyExc_TypeError, "'%U' is an invalid keyword " - "argument for this function", - key); + "argument for %.200s%s", + key, + (fname == NULL) ? "this function" : fname, + (fname == NULL) ? "" : "()"); return cleanreturn(0, &freelist); } } @@ -2060,7 +2070,7 @@ vgetargskeywordsfast_impl(PyObject **args, Py_ssize_t nargs, } if (nargs + nkwargs > len) { PyErr_Format(PyExc_TypeError, - "%s%s takes at most %d argument%s (%zd given)", + "%.200s%s takes at most %d argument%s (%zd given)", (parser->fname == NULL) ? "function" : parser->fname, (parser->fname == NULL) ? "" : "()", len, @@ -2070,7 +2080,9 @@ vgetargskeywordsfast_impl(PyObject **args, Py_ssize_t nargs, } if (parser->max < nargs) { PyErr_Format(PyExc_TypeError, - "Function takes %s %d positional arguments (%d given)", + "%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); @@ -2115,15 +2127,19 @@ vgetargskeywordsfast_impl(PyObject **args, Py_ssize_t nargs, if (i < pos) { Py_ssize_t min = Py_MIN(pos, parser->min); PyErr_Format(PyExc_TypeError, - "Function takes %s %d positional arguments" + "%.200s%s takes %s %d positional arguments" " (%d given)", + (parser->fname == NULL) ? "function" : parser->fname, + (parser->fname == NULL) ? "" : "()", min < parser->max ? "at least" : "exactly", min, nargs); } else { keyword = PyTuple_GET_ITEM(kwtuple, i - pos); - PyErr_Format(PyExc_TypeError, "Required argument " - "'%U' (pos %d) not found", + PyErr_Format(PyExc_TypeError, "%.200s%s missing required " + "argument '%U' (pos %d)", + (parser->fname == NULL) ? "function" : parser->fname, + (parser->fname == NULL) ? "" : "()", keyword, i+1); } return cleanreturn(0, &freelist); @@ -2153,8 +2169,10 @@ vgetargskeywordsfast_impl(PyObject **args, Py_ssize_t nargs, if (current_arg) { /* arg present in tuple and in dict */ PyErr_Format(PyExc_TypeError, - "Argument given by name ('%U') " + "argument for %.200s%s given by name ('%U') " "and position (%d)", + (parser->fname == NULL) ? "function" : parser->fname, + (parser->fname == NULL) ? "" : "()", keyword, i+1); return cleanreturn(0, &freelist); } @@ -2184,8 +2202,10 @@ vgetargskeywordsfast_impl(PyObject **args, Py_ssize_t nargs, if (!match) { PyErr_Format(PyExc_TypeError, "'%U' is an invalid keyword " - "argument for this function", - keyword); + "argument for %.200s%s", + keyword, + (parser->fname == NULL) ? "this function" : parser->fname, + (parser->fname == NULL) ? "" : "()"); } return cleanreturn(0, &freelist); } @@ -2365,7 +2385,7 @@ unpack_stack(PyObject **args, Py_ssize_t nargs, const char *name, if (name != NULL) PyErr_Format( PyExc_TypeError, - "%s expected %s%zd arguments, got %zd", + "%.200s expected %s%zd arguments, got %zd", name, (min == max ? "" : "at least "), min, nargs); else PyErr_Format( @@ -2384,7 +2404,7 @@ unpack_stack(PyObject **args, Py_ssize_t nargs, const char *name, if (name != NULL) PyErr_Format( PyExc_TypeError, - "%s expected %s%zd arguments, got %zd", + "%.200s expected %s%zd arguments, got %zd", name, (min == max ? "" : "at most "), max, nargs); else PyErr_Format( @@ -2469,7 +2489,7 @@ _PyArg_NoKeywords(const char *funcname, PyObject *kwargs) return 1; } - PyErr_Format(PyExc_TypeError, "%s does not take keyword arguments", + PyErr_Format(PyExc_TypeError, "%.200s does not take keyword arguments", funcname); return 0; } @@ -2486,7 +2506,7 @@ _PyArg_NoStackKeywords(const char *funcname, PyObject *kwnames) return 1; } - PyErr_Format(PyExc_TypeError, "%s does not take keyword arguments", + PyErr_Format(PyExc_TypeError, "%.200s does not take keyword arguments", funcname); return 0; } @@ -2504,7 +2524,7 @@ _PyArg_NoPositional(const char *funcname, PyObject *args) if (PyTuple_GET_SIZE(args) == 0) return 1; - PyErr_Format(PyExc_TypeError, "%s does not take positional arguments", + PyErr_Format(PyExc_TypeError, "%.200s does not take positional arguments", funcname); return 0; } -- cgit v0.12