From a1814637d8be7132d33792f0fa2f517f4dbe3cce Mon Sep 17 00:00:00 2001 From: Matthias Bussonnier Date: Tue, 14 Feb 2017 16:54:59 -0800 Subject: [PATCH 1/3] bpo-29546: Improve from-import error message with location Add location information like canonical module name where identifier cannot be found and file location if available. First iteration of this was gh-91 --- Doc/whatsnew/3.7.rst | 3 +++ Lib/test/test_import/__init__.py | 14 +++++++++++++- Misc/NEWS | 2 ++ Python/ceval.c | 17 ++++++++++++++--- 4 files changed, 32 insertions(+), 4 deletions(-) diff --git a/Doc/whatsnew/3.7.rst b/Doc/whatsnew/3.7.rst index 21621c5ee4eac9..f6367fd1f4a672 100644 --- a/Doc/whatsnew/3.7.rst +++ b/Doc/whatsnew/3.7.rst @@ -83,6 +83,9 @@ Other Language Changes whitespace, not only spaces. (Contributed by Robert Xiao in :issue:`28927`.) +* :exc:`ImportError` now displays module name and module ``__file__`` path when + ``from ... import ...`` fails. :issue:`29546`. + New Modules =========== diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py index df678f17f18e4e..ace1a46f9b91a7 100644 --- a/Lib/test/test_import/__init__.py +++ b/Lib/test/test_import/__init__.py @@ -85,6 +85,15 @@ def test_from_import_missing_attr_has_name_and_path(self): from os import i_dont_exist self.assertEqual(cm.exception.name, 'os') self.assertEqual(cm.exception.path, os.__file__) + self.assertRegex(str(cm.exception), "cannot import name 'i_dont_exist' from 'os' \(.*/Lib/os.py\)") + + def test_from_import_missing_attr_has_name_and_so_path(self): + import _opcode + with self.assertRaises(ImportError) as cm: + from _opcode import i_dont_exist + self.assertEqual(cm.exception.name, '_opcode') + self.assertEqual(cm.exception.path, _opcode.__file__) + self.assertRegex(str(cm.exception), "cannot import name 'i_dont_exist' from '_opcode' \(.*\.(so|dll)\)") def test_from_import_missing_attr_has_name(self): with self.assertRaises(ImportError) as cm: @@ -365,9 +374,12 @@ def __getattr__(self, _): module_name = 'test_from_import_AttributeError' self.addCleanup(unload, module_name) sys.modules[module_name] = AlwaysAttributeError() - with self.assertRaises(ImportError): + with self.assertRaises(ImportError) as cm: from test_from_import_AttributeError import does_not_exist + self.assertEqual(str(cm.exception), + "cannot import name 'does_not_exist' from '' (unknown location)") + @skip_if_dont_write_bytecode class FilePermissionTests(unittest.TestCase): diff --git a/Misc/NEWS b/Misc/NEWS index 0c2122f8ca04d1..0d4451f05ca51b 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -21,6 +21,8 @@ Core and Builtins - bpo-29546: Set the 'path' and 'name' attribute on ImportError for ``from ... import ...``. +- bpo-29546: Improve from-import error message with location + - Issue #29319: Prevent RunMainFromImporter overwriting sys.path[0]. - Issue #29337: Fixed possible BytesWarning when compare the code objects. diff --git a/Python/ceval.c b/Python/ceval.c index 69c93838419609..bc22218d6ca6f9 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -4995,7 +4995,7 @@ import_from(PyObject *v, PyObject *name) { PyObject *x; _Py_IDENTIFIER(__name__); - PyObject *fullmodname, *pkgname, *pkgpath; + PyObject *fullmodname, *pkgname, *pkgpath, *pkgname_or_unknown; x = PyObject_GetAttr(v, name); if (x != NULL || !PyErr_ExceptionMatches(PyExc_AttributeError)) @@ -5022,12 +5022,23 @@ import_from(PyObject *v, PyObject *name) return x; error: pkgpath = PyModule_GetFilenameObject(v); + if (pkgname == NULL) { + pkgname_or_unknown = PyUnicode_FromString(""); + } else { + pkgname_or_unknown = pkgname; + } if (pkgpath == NULL || !PyUnicode_Check(pkgpath)) { PyErr_Clear(); - PyErr_SetImportError(PyUnicode_FromFormat("cannot import name %R", name), pkgname, NULL); + PyErr_SetImportError( + PyUnicode_FromFormat("cannot import name %R from %R (unknown location)", + name, pkgname_or_unknown), + pkgname, NULL); } else { - PyErr_SetImportError(PyUnicode_FromFormat("cannot import name %R", name), pkgname, pkgpath); + PyErr_SetImportError( + PyUnicode_FromFormat("cannot import name %R from %R (%S)", + name, pkgname_or_unknown, pkgpath), + pkgname, pkgpath); } return NULL; From 6ec81c713c8fb904a02b432e980db307c6f9bf56 Mon Sep 17 00:00:00 2001 From: Matthias Bussonnier Date: Fri, 17 Feb 2017 17:23:25 -0800 Subject: [PATCH 2/3] Update reference counting from Barry Warsaw: - It's possible the code jumps to error: after pkgname is decref'd. I think that decref should be deferred until just before the good-path return. - pkgname_or_unknown will be a new reference if pkgname is NULL, or it will steal the pkgname reference if it's not. In either case, the error stanza should decref pkgname_or_unknown, which will take care of ensuring the object gets decref for any goto error path. - pkgpath doesn't get decref'd. I think you should Py_XDECREF it before the return NULL just in case pkgpath is itself NULL. --- Python/ceval.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Python/ceval.c b/Python/ceval.c index bc22218d6ca6f9..d77871d6f30cfa 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -5009,7 +5009,6 @@ import_from(PyObject *v, PyObject *name) goto error; } fullmodname = PyUnicode_FromFormat("%U.%U", pkgname, name); - Py_DECREF(pkgname); if (fullmodname == NULL) { return NULL; } @@ -5018,6 +5017,7 @@ import_from(PyObject *v, PyObject *name) if (x == NULL) { goto error; } + Py_DECREF(pkgname); Py_INCREF(x); return x; error: @@ -5041,6 +5041,8 @@ import_from(PyObject *v, PyObject *name) pkgname, pkgpath); } + Py_XDECREF(pkgname_or_unknown); + Py_XDECREF(pkgpath); return NULL; } From 1b6e462f8d5cf4fc0076ba4bdbe71cabe01c2fb6 Mon Sep 17 00:00:00 2001 From: Matthias Bussonnier Date: Mon, 20 Feb 2017 15:08:27 -0800 Subject: [PATCH 3/3] Handle if PyUnicode_FromString fails. --- Python/ceval.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Python/ceval.c b/Python/ceval.c index d77871d6f30cfa..81c89dfadfaac0 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -5024,6 +5024,10 @@ import_from(PyObject *v, PyObject *name) pkgpath = PyModule_GetFilenameObject(v); if (pkgname == NULL) { pkgname_or_unknown = PyUnicode_FromString(""); + if (pkgname_or_unknown == NULL) { + Py_XDECREF(pkgpath); + return NULL; + } } else { pkgname_or_unknown = pkgname; }