From 47dee11ba76a12d22277562b9ccea51259a5ecc0 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 27 Sep 2016 20:45:35 +0300 Subject: [PATCH] Issue #21578: Fixed misleading error message when ImportError called with invalid keyword args. --- Lib/test/test_exceptions.py | 17 +++++++++++++ Misc/NEWS | 3 +++ Objects/exceptions.c | 50 +++++++++++++++++++------------------ 3 files changed, 46 insertions(+), 24 deletions(-) diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py index 458ddc1ed82..96c3a48c317 100644 --- a/Lib/test/test_exceptions.py +++ b/Lib/test/test_exceptions.py @@ -1096,6 +1096,23 @@ class ImportErrorTests(unittest.TestCase): self.assertEqual(exc.name, 'somename') self.assertEqual(exc.path, 'somepath') + msg = "'invalid' is an invalid keyword argument for this function" + with self.assertRaisesRegex(TypeError, msg): + ImportError('test', invalid='keyword') + + with self.assertRaisesRegex(TypeError, msg): + ImportError('test', name='name', invalid='keyword') + + with self.assertRaisesRegex(TypeError, msg): + ImportError('test', path='path', invalid='keyword') + + with self.assertRaisesRegex(TypeError, msg): + ImportError(invalid='keyword') + + msg = "'invalid|another' is an invalid keyword argument for this function" + with self.assertRaisesRegex(TypeError, msg): + ImportError('test', invalid='keyword', another=True) + def test_non_str_argument(self): # Issue #15778 with check_warnings(('', BytesWarning), quiet=True): diff --git a/Misc/NEWS b/Misc/NEWS index 661402f8407..b6363b65a41 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -10,6 +10,9 @@ Release date: TBA Core and Builtins ----------------- +- Issue #21578: Fixed misleading error message when ImportError called with + invalid keyword args. + - Issue #28203: Fix incorrect type in error message from ``complex(1.0, {2:3})``. Patch by Soumya Sharma. diff --git a/Objects/exceptions.c b/Objects/exceptions.c index 0749e908496..981ead2172a 100644 --- a/Objects/exceptions.c +++ b/Objects/exceptions.c @@ -618,36 +618,38 @@ SimpleExtendsException(PyExc_BaseException, KeyboardInterrupt, static int ImportError_init(PyImportErrorObject *self, PyObject *args, PyObject *kwds) { + static char *kwlist[] = {"name", "path", 0}; + PyObject *empty_tuple; PyObject *msg = NULL; PyObject *name = NULL; PyObject *path = NULL; -/* Macro replacement doesn't allow ## to start the first line of a macro, - so we move the assignment and NULL check into the if-statement. */ -#define GET_KWD(kwd) { \ - kwd = PyDict_GetItemString(kwds, #kwd); \ - if (kwd) { \ - Py_INCREF(kwd); \ - Py_XSETREF(self->kwd, kwd); \ - if (PyDict_DelItemString(kwds, #kwd)) \ - return -1; \ - } \ - } - - if (kwds) { - GET_KWD(name); - GET_KWD(path); - } - - if (BaseException_init((PyBaseExceptionObject *)self, args, kwds) == -1) - return -1; - if (PyTuple_GET_SIZE(args) != 1) - return 0; - if (!PyArg_UnpackTuple(args, "ImportError", 1, 1, &msg)) + if (BaseException_init((PyBaseExceptionObject *)self, args, NULL) == -1) return -1; - Py_INCREF(msg); - Py_XSETREF(self->msg, msg); + empty_tuple = PyTuple_New(0); + if (!empty_tuple) + return -1; + if (!PyArg_ParseTupleAndKeywords(empty_tuple, kwds, "|$OO:ImportError", kwlist, + &name, &path)) { + Py_DECREF(empty_tuple); + return -1; + } + Py_DECREF(empty_tuple); + + if (name) { + Py_INCREF(name); + Py_XSETREF(self->name, name); + } + if (path) { + Py_INCREF(path); + Py_XSETREF(self->path, path); + } + if (PyTuple_GET_SIZE(args) == 1) { + msg = PyTuple_GET_ITEM(args, 0); + Py_INCREF(msg); + Py_XSETREF(self->msg, msg); + } return 0; }