From 500f5338a8fe13719478589333fcd296e8e8eb02 Mon Sep 17 00:00:00 2001 From: Shantanu <12621235+hauntsaninja@users.noreply.github.com> Date: Thu, 24 Oct 2024 12:11:12 -0700 Subject: [PATCH] gh-123930: Better error for "from imports" when script shadows module (#123929) --- Doc/whatsnew/3.13.rst | 4 +- Include/internal/pycore_moduleobject.h | 2 + Lib/test/test_import/__init__.py | 327 ++++++++++++------ ...-09-11-01-32-07.gh-issue-123930.BkPfB6.rst | 4 + Objects/moduleobject.c | 28 +- Python/ceval.c | 152 +++++--- 6 files changed, 344 insertions(+), 173 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2024-09-11-01-32-07.gh-issue-123930.BkPfB6.rst diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst index f9e74a9b8ff..de4c7fd4c04 100644 --- a/Doc/whatsnew/3.13.rst +++ b/Doc/whatsnew/3.13.rst @@ -274,7 +274,7 @@ Improved error messages File "/home/me/random.py", line 3, in print(random.randint(5)) ^^^^^^^^^^^^^^ - AttributeError: module 'random' has no attribute 'randint' (consider renaming '/home/me/random.py' since it has the same name as the standard library module named 'random' and the import system gives it precedence) + AttributeError: module 'random' has no attribute 'randint' (consider renaming '/home/me/random.py' since it has the same name as the standard library module named 'random' and prevents importing that standard library module) Similarly, if a script has the same name as a third-party module that it attempts to import and this results in errors, @@ -289,7 +289,7 @@ Improved error messages File "/home/me/numpy.py", line 3, in np.array([1, 2, 3]) ^^^^^^^^ - AttributeError: module 'numpy' has no attribute 'array' (consider renaming '/home/me/numpy.py' if it has the same name as a third-party module you intended to import) + AttributeError: module 'numpy' has no attribute 'array' (consider renaming '/home/me/numpy.py' if it has the same name as a library you intended to import) (Contributed by Shantanu Jain in :gh:`95754`.) diff --git a/Include/internal/pycore_moduleobject.h b/Include/internal/pycore_moduleobject.h index cc2dda48ed9..9bb282a13a9 100644 --- a/Include/internal/pycore_moduleobject.h +++ b/Include/internal/pycore_moduleobject.h @@ -11,6 +11,8 @@ extern "C" { extern void _PyModule_Clear(PyObject *); extern void _PyModule_ClearDict(PyObject *); extern int _PyModuleSpec_IsInitializing(PyObject *); +extern int _PyModuleSpec_GetFileOrigin(PyObject *, PyObject **); +extern int _PyModule_IsPossiblyShadowing(PyObject *); extern int _PyModule_IsExtension(PyObject *obj); diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py index 5d0d02480b3..5b7ba90b2cc 100644 --- a/Lib/test/test_import/__init__.py +++ b/Lib/test/test_import/__init__.py @@ -804,104 +804,133 @@ class ImportTests(unittest.TestCase): str(cm.exception)) def test_script_shadowing_stdlib(self): - with os_helper.temp_dir() as tmp: - with open(os.path.join(tmp, "fractions.py"), "w", encoding='utf-8') as f: - f.write("import fractions\nfractions.Fraction") - - expected_error = ( - rb"AttributeError: module 'fractions' has no attribute 'Fraction' " - rb"\(consider renaming '.*fractions.py' since it has the " - rb"same name as the standard library module named 'fractions' " - rb"and the import system gives it precedence\)" + script_errors = [ + ( + "import fractions\nfractions.Fraction", + rb"AttributeError: module 'fractions' has no attribute 'Fraction'" + ), + ( + "from fractions import Fraction", + rb"ImportError: cannot import name 'Fraction' from 'fractions'" ) + ] + for script, error in script_errors: + with self.subTest(script=script), os_helper.temp_dir() as tmp: + with open(os.path.join(tmp, "fractions.py"), "w", encoding='utf-8') as f: + f.write(script) - popen = script_helper.spawn_python(os.path.join(tmp, "fractions.py"), cwd=tmp) - stdout, stderr = popen.communicate() - self.assertRegex(stdout, expected_error) + expected_error = error + ( + rb" \(consider renaming '.*fractions.py' since it has the " + rb"same name as the standard library module named 'fractions' " + rb"and prevents importing that standard library module\)" + ) - popen = script_helper.spawn_python('-m', 'fractions', cwd=tmp) - stdout, stderr = popen.communicate() - self.assertRegex(stdout, expected_error) + popen = script_helper.spawn_python(os.path.join(tmp, "fractions.py"), cwd=tmp) + stdout, stderr = popen.communicate() + self.assertRegex(stdout, expected_error) - popen = script_helper.spawn_python('-c', 'import fractions', cwd=tmp) - stdout, stderr = popen.communicate() - self.assertRegex(stdout, expected_error) + popen = script_helper.spawn_python('-m', 'fractions', cwd=tmp) + stdout, stderr = popen.communicate() + self.assertRegex(stdout, expected_error) - # and there's no error at all when using -P - popen = script_helper.spawn_python('-P', 'fractions.py', cwd=tmp) - stdout, stderr = popen.communicate() - self.assertEqual(stdout, b'') + popen = script_helper.spawn_python('-c', 'import fractions', cwd=tmp) + stdout, stderr = popen.communicate() + self.assertRegex(stdout, expected_error) - tmp_child = os.path.join(tmp, "child") - os.mkdir(tmp_child) + # and there's no error at all when using -P + popen = script_helper.spawn_python('-P', 'fractions.py', cwd=tmp) + stdout, stderr = popen.communicate() + self.assertEqual(stdout, b'') - # test the logic with different cwd - popen = script_helper.spawn_python(os.path.join(tmp, "fractions.py"), cwd=tmp_child) - stdout, stderr = popen.communicate() - self.assertRegex(stdout, expected_error) + tmp_child = os.path.join(tmp, "child") + os.mkdir(tmp_child) - popen = script_helper.spawn_python('-m', 'fractions', cwd=tmp_child) - stdout, stderr = popen.communicate() - self.assertEqual(stdout, b'') # no error + # test the logic with different cwd + popen = script_helper.spawn_python(os.path.join(tmp, "fractions.py"), cwd=tmp_child) + stdout, stderr = popen.communicate() + self.assertRegex(stdout, expected_error) - popen = script_helper.spawn_python('-c', 'import fractions', cwd=tmp_child) - stdout, stderr = popen.communicate() - self.assertEqual(stdout, b'') # no error + popen = script_helper.spawn_python('-m', 'fractions', cwd=tmp_child) + stdout, stderr = popen.communicate() + self.assertEqual(stdout, b'') # no error + + popen = script_helper.spawn_python('-c', 'import fractions', cwd=tmp_child) + stdout, stderr = popen.communicate() + self.assertEqual(stdout, b'') # no error def test_package_shadowing_stdlib_module(self): - with os_helper.temp_dir() as tmp: - os.mkdir(os.path.join(tmp, "fractions")) - with open(os.path.join(tmp, "fractions", "__init__.py"), "w", encoding='utf-8') as f: - f.write("shadowing_module = True") - with open(os.path.join(tmp, "main.py"), "w", encoding='utf-8') as f: - f.write(""" -import fractions -fractions.shadowing_module -fractions.Fraction -""") - - expected_error = ( - rb"AttributeError: module 'fractions' has no attribute 'Fraction' " - rb"\(consider renaming '.*fractions.__init__.py' since it has the " - rb"same name as the standard library module named 'fractions' " - rb"and the import system gives it precedence\)" + script_errors = [ + ( + "fractions.Fraction", + rb"AttributeError: module 'fractions' has no attribute 'Fraction'" + ), + ( + "from fractions import Fraction", + rb"ImportError: cannot import name 'Fraction' from 'fractions'" ) + ] + for script, error in script_errors: + with self.subTest(script=script), os_helper.temp_dir() as tmp: + os.mkdir(os.path.join(tmp, "fractions")) + with open( + os.path.join(tmp, "fractions", "__init__.py"), "w", encoding='utf-8' + ) as f: + f.write("shadowing_module = True") + with open(os.path.join(tmp, "main.py"), "w", encoding='utf-8') as f: + f.write("import fractions; fractions.shadowing_module\n") + f.write(script) - popen = script_helper.spawn_python(os.path.join(tmp, "main.py"), cwd=tmp) - stdout, stderr = popen.communicate() - self.assertRegex(stdout, expected_error) + expected_error = error + ( + rb" \(consider renaming '.*[\\/]fractions[\\/]+__init__.py' since it has the " + rb"same name as the standard library module named 'fractions' " + rb"and prevents importing that standard library module\)" + ) - popen = script_helper.spawn_python('-m', 'main', cwd=tmp) - stdout, stderr = popen.communicate() - self.assertRegex(stdout, expected_error) + popen = script_helper.spawn_python(os.path.join(tmp, "main.py"), cwd=tmp) + stdout, stderr = popen.communicate() + self.assertRegex(stdout, expected_error) - # and there's no shadowing at all when using -P - popen = script_helper.spawn_python('-P', 'main.py', cwd=tmp) - stdout, stderr = popen.communicate() - self.assertRegex(stdout, b"module 'fractions' has no attribute 'shadowing_module'") + popen = script_helper.spawn_python('-m', 'main', cwd=tmp) + stdout, stderr = popen.communicate() + self.assertRegex(stdout, expected_error) + + # and there's no shadowing at all when using -P + popen = script_helper.spawn_python('-P', 'main.py', cwd=tmp) + stdout, stderr = popen.communicate() + self.assertRegex(stdout, b"module 'fractions' has no attribute 'shadowing_module'") def test_script_shadowing_third_party(self): - with os_helper.temp_dir() as tmp: - with open(os.path.join(tmp, "numpy.py"), "w", encoding='utf-8') as f: - f.write("import numpy\nnumpy.array") - - expected_error = ( - rb"AttributeError: module 'numpy' has no attribute 'array' " - rb"\(consider renaming '.*numpy.py' if it has the " - rb"same name as a third-party module you intended to import\)\s+\Z" + script_errors = [ + ( + "import numpy\nnumpy.array", + rb"AttributeError: module 'numpy' has no attribute 'array'" + ), + ( + "from numpy import array", + rb"ImportError: cannot import name 'array' from 'numpy'" ) + ] + for script, error in script_errors: + with self.subTest(script=script), os_helper.temp_dir() as tmp: + with open(os.path.join(tmp, "numpy.py"), "w", encoding='utf-8') as f: + f.write(script) - popen = script_helper.spawn_python(os.path.join(tmp, "numpy.py")) - stdout, stderr = popen.communicate() - self.assertRegex(stdout, expected_error) + expected_error = error + ( + rb" \(consider renaming '.*numpy.py' if it has the " + rb"same name as a library you intended to import\)\s+\Z" + ) - popen = script_helper.spawn_python('-m', 'numpy', cwd=tmp) - stdout, stderr = popen.communicate() - self.assertRegex(stdout, expected_error) + popen = script_helper.spawn_python(os.path.join(tmp, "numpy.py")) + stdout, stderr = popen.communicate() + self.assertRegex(stdout, expected_error) - popen = script_helper.spawn_python('-c', 'import numpy', cwd=tmp) - stdout, stderr = popen.communicate() - self.assertRegex(stdout, expected_error) + popen = script_helper.spawn_python('-m', 'numpy', cwd=tmp) + stdout, stderr = popen.communicate() + self.assertRegex(stdout, expected_error) + + popen = script_helper.spawn_python('-c', 'import numpy', cwd=tmp) + stdout, stderr = popen.communicate() + self.assertRegex(stdout, expected_error) def test_script_maybe_not_shadowing_third_party(self): with os_helper.temp_dir() as tmp: @@ -911,15 +940,23 @@ fractions.Fraction expected_error = ( rb"AttributeError: module 'numpy' has no attribute 'attr'\s+\Z" ) - popen = script_helper.spawn_python('-c', 'import numpy; numpy.attr', cwd=tmp) stdout, stderr = popen.communicate() self.assertRegex(stdout, expected_error) + expected_error = ( + rb"ImportError: cannot import name 'attr' from 'numpy' \(.*\)\s+\Z" + ) + popen = script_helper.spawn_python('-c', 'from numpy import attr', cwd=tmp) + stdout, stderr = popen.communicate() + self.assertRegex(stdout, expected_error) + def test_script_shadowing_stdlib_edge_cases(self): with os_helper.temp_dir() as tmp: with open(os.path.join(tmp, "fractions.py"), "w", encoding='utf-8') as f: f.write("shadowing_module = True") + + # Unhashable str subclass with open(os.path.join(tmp, "main.py"), "w", encoding='utf-8') as f: f.write(""" import fractions @@ -932,11 +969,28 @@ try: except TypeError as e: print(str(e)) """) + popen = script_helper.spawn_python("main.py", cwd=tmp) + stdout, stderr = popen.communicate() + self.assertEqual(stdout.rstrip(), b"unhashable type: 'substr'") + + with open(os.path.join(tmp, "main.py"), "w", encoding='utf-8') as f: + f.write(""" +import fractions +fractions.shadowing_module +class substr(str): + __hash__ = None +fractions.__name__ = substr('fractions') +try: + from fractions import Fraction +except TypeError as e: + print(str(e)) +""") popen = script_helper.spawn_python("main.py", cwd=tmp) stdout, stderr = popen.communicate() self.assertEqual(stdout.rstrip(), b"unhashable type: 'substr'") + # Various issues with sys module with open(os.path.join(tmp, "main.py"), "w", encoding='utf-8') as f: f.write(""" import fractions @@ -961,22 +1015,49 @@ try: except AttributeError as e: print(str(e)) """) - popen = script_helper.spawn_python("main.py", cwd=tmp) stdout, stderr = popen.communicate() - self.assertEqual( - stdout.splitlines(), - [ - b"module 'fractions' has no attribute 'Fraction'", - b"module 'fractions' has no attribute 'Fraction'", - b"module 'fractions' has no attribute 'Fraction'", - ], - ) + lines = stdout.splitlines() + self.assertEqual(len(lines), 3) + for line in lines: + self.assertEqual(line, b"module 'fractions' has no attribute 'Fraction'") with open(os.path.join(tmp, "main.py"), "w", encoding='utf-8') as f: f.write(""" import fractions fractions.shadowing_module + +import sys +sys.stdlib_module_names = None +try: + from fractions import Fraction +except ImportError as e: + print(str(e)) + +del sys.stdlib_module_names +try: + from fractions import Fraction +except ImportError as e: + print(str(e)) + +sys.path = [0] +try: + from fractions import Fraction +except ImportError as e: + print(str(e)) +""") + popen = script_helper.spawn_python("main.py", cwd=tmp) + stdout, stderr = popen.communicate() + lines = stdout.splitlines() + self.assertEqual(len(lines), 3) + for line in lines: + self.assertRegex(line, rb"cannot import name 'Fraction' from 'fractions' \(.*\)") + + # Various issues with origin + with open(os.path.join(tmp, "main.py"), "w", encoding='utf-8') as f: + f.write(""" +import fractions +fractions.shadowing_module del fractions.__spec__.origin try: fractions.Fraction @@ -992,37 +1073,61 @@ except AttributeError as e: popen = script_helper.spawn_python("main.py", cwd=tmp) stdout, stderr = popen.communicate() - self.assertEqual( - stdout.splitlines(), - [ - b"module 'fractions' has no attribute 'Fraction'", - b"module 'fractions' has no attribute 'Fraction'" - ], - ) - - def test_script_shadowing_stdlib_sys_path_modification(self): - with os_helper.temp_dir() as tmp: - with open(os.path.join(tmp, "fractions.py"), "w", encoding='utf-8') as f: - f.write("shadowing_module = True") - - expected_error = ( - rb"AttributeError: module 'fractions' has no attribute 'Fraction' " - rb"\(consider renaming '.*fractions.py' since it has the " - rb"same name as the standard library module named 'fractions' " - rb"and the import system gives it precedence\)" - ) + lines = stdout.splitlines() + self.assertEqual(len(lines), 2) + for line in lines: + self.assertEqual(line, b"module 'fractions' has no attribute 'Fraction'") with open(os.path.join(tmp, "main.py"), "w", encoding='utf-8') as f: f.write(""" -import sys -sys.path.insert(0, "this_folder_does_not_exist") import fractions -fractions.Fraction -""") +fractions.shadowing_module +del fractions.__spec__.origin +try: + from fractions import Fraction +except ImportError as e: + print(str(e)) +fractions.__spec__.origin = 0 +try: + from fractions import Fraction +except ImportError as e: + print(str(e)) +""") popen = script_helper.spawn_python("main.py", cwd=tmp) stdout, stderr = popen.communicate() - self.assertRegex(stdout, expected_error) + lines = stdout.splitlines() + self.assertEqual(len(lines), 2) + for line in lines: + self.assertRegex(line, rb"cannot import name 'Fraction' from 'fractions' \(.*\)") + + def test_script_shadowing_stdlib_sys_path_modification(self): + script_errors = [ + ( + "import fractions\nfractions.Fraction", + rb"AttributeError: module 'fractions' has no attribute 'Fraction'" + ), + ( + "from fractions import Fraction", + rb"ImportError: cannot import name 'Fraction' from 'fractions'" + ) + ] + for script, error in script_errors: + with self.subTest(script=script), os_helper.temp_dir() as tmp: + with open(os.path.join(tmp, "fractions.py"), "w", encoding='utf-8') as f: + f.write("shadowing_module = True") + with open(os.path.join(tmp, "main.py"), "w", encoding='utf-8') as f: + f.write('import sys; sys.path.insert(0, "this_folder_does_not_exist")\n') + f.write(script) + expected_error = error + ( + rb" \(consider renaming '.*fractions.py' since it has the " + rb"same name as the standard library module named 'fractions' " + rb"and prevents importing that standard library module\)" + ) + + popen = script_helper.spawn_python("main.py", cwd=tmp) + stdout, stderr = popen.communicate() + self.assertRegex(stdout, expected_error) @skip_if_dont_write_bytecode diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2024-09-11-01-32-07.gh-issue-123930.BkPfB6.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-09-11-01-32-07.gh-issue-123930.BkPfB6.rst new file mode 100644 index 00000000000..3c8eb02b2dc --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2024-09-11-01-32-07.gh-issue-123930.BkPfB6.rst @@ -0,0 +1,4 @@ +Improve the error message when a script shadowing a module from the standard +library causes :exc:`ImportError` to be raised during a "from" import. +Similarly, improve the error message when a script shadowing a third party module +attempts to "from" import an attribute from that third party module while still initialising. diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index c06badd5f3e..535b0d068f0 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -836,15 +836,15 @@ _PyModuleSpec_IsUninitializedSubmodule(PyObject *spec, PyObject *name) return rc; } -static int -_get_file_origin_from_spec(PyObject *spec, PyObject **p_origin) +int +_PyModuleSpec_GetFileOrigin(PyObject *spec, PyObject **p_origin) { PyObject *has_location = NULL; int rc = PyObject_GetOptionalAttr(spec, &_Py_ID(has_location), &has_location); if (rc <= 0) { return rc; } - // If origin is not a location, or doesn't exist, or is not a str), we could consider falling + // If origin is not a location, or doesn't exist, or is not a str, we could consider falling // back to module.__file__. But the cases in which module.__file__ is not __spec__.origin // are cases in which we probably shouldn't be guessing. rc = PyObject_IsTrue(has_location); @@ -867,8 +867,8 @@ _get_file_origin_from_spec(PyObject *spec, PyObject **p_origin) return 1; } -static int -_is_module_possibly_shadowing(PyObject *origin) +int +_PyModule_IsPossiblyShadowing(PyObject *origin) { // origin must be a unicode subtype // Returns 1 if the module at origin could be shadowing a module of the @@ -993,11 +993,11 @@ _Py_module_getattro_impl(PyModuleObject *m, PyObject *name, int suppress) } PyObject *origin = NULL; - if (_get_file_origin_from_spec(spec, &origin) < 0) { + if (_PyModuleSpec_GetFileOrigin(spec, &origin) < 0) { goto done; } - int is_possibly_shadowing = _is_module_possibly_shadowing(origin); + int is_possibly_shadowing = _PyModule_IsPossiblyShadowing(origin); if (is_possibly_shadowing < 0) { goto done; } @@ -1018,20 +1018,23 @@ _Py_module_getattro_impl(PyModuleObject *m, PyObject *name, int suppress) "module '%U' has no attribute '%U' " "(consider renaming '%U' since it has the same " "name as the standard library module named '%U' " - "and the import system gives it precedence)", + "and prevents importing that standard library module)", mod_name, name, origin, mod_name); } else { int rc = _PyModuleSpec_IsInitializing(spec); - if (rc > 0) { + if (rc < 0) { + goto done; + } + else if (rc > 0) { if (is_possibly_shadowing) { assert(origin); - // For third-party modules, only mention the possibility of + // For non-stdlib modules, only mention the possibility of // shadowing if the module is being initialized. PyErr_Format(PyExc_AttributeError, "module '%U' has no attribute '%U' " "(consider renaming '%U' if it has the same name " - "as a third-party module you intended to import)", + "as a library you intended to import)", mod_name, name, origin); } else if (origin) { @@ -1049,7 +1052,8 @@ _Py_module_getattro_impl(PyModuleObject *m, PyObject *name, int suppress) mod_name, name); } } - else if (rc == 0) { + else { + assert(rc == 0); rc = _PyModuleSpec_IsUninitializedSubmodule(spec, name); if (rc > 0) { PyErr_Format(PyExc_AttributeError, diff --git a/Python/ceval.c b/Python/ceval.c index ece7ef1d320..beee5325cd6 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -2802,7 +2802,7 @@ PyObject * _PyEval_ImportFrom(PyThreadState *tstate, PyObject *v, PyObject *name) { PyObject *x; - PyObject *fullmodname, *pkgname, *pkgpath, *pkgname_or_unknown, *errmsg; + PyObject *fullmodname, *mod_name, *origin, *mod_name_or_unknown, *errmsg, *spec; if (PyObject_GetOptionalAttr(v, name, &x) != 0) { return x; @@ -2810,16 +2810,16 @@ _PyEval_ImportFrom(PyThreadState *tstate, PyObject *v, PyObject *name) /* Issue #17636: in case this failed because of a circular relative import, try to fallback on reading the module directly from sys.modules. */ - if (PyObject_GetOptionalAttr(v, &_Py_ID(__name__), &pkgname) < 0) { + if (PyObject_GetOptionalAttr(v, &_Py_ID(__name__), &mod_name) < 0) { return NULL; } - if (pkgname == NULL || !PyUnicode_Check(pkgname)) { - Py_CLEAR(pkgname); + if (mod_name == NULL || !PyUnicode_Check(mod_name)) { + Py_CLEAR(mod_name); goto error; } - fullmodname = PyUnicode_FromFormat("%U.%U", pkgname, name); + fullmodname = PyUnicode_FromFormat("%U.%U", mod_name, name); if (fullmodname == NULL) { - Py_DECREF(pkgname); + Py_DECREF(mod_name); return NULL; } x = PyImport_GetModule(fullmodname); @@ -2827,63 +2827,121 @@ _PyEval_ImportFrom(PyThreadState *tstate, PyObject *v, PyObject *name) if (x == NULL && !_PyErr_Occurred(tstate)) { goto error; } - Py_DECREF(pkgname); + Py_DECREF(mod_name); return x; + error: - if (pkgname == NULL) { - pkgname_or_unknown = PyUnicode_FromString(""); - if (pkgname_or_unknown == NULL) { + if (mod_name == NULL) { + mod_name_or_unknown = PyUnicode_FromString(""); + if (mod_name_or_unknown == NULL) { return NULL; } } else { - pkgname_or_unknown = pkgname; + mod_name_or_unknown = mod_name; } + // mod_name is no longer an owned reference + assert(mod_name_or_unknown); + assert(mod_name == NULL || mod_name == mod_name_or_unknown); - pkgpath = NULL; - if (PyModule_Check(v)) { - pkgpath = PyModule_GetFilenameObject(v); - if (pkgpath == NULL) { - if (!PyErr_ExceptionMatches(PyExc_SystemError)) { - Py_DECREF(pkgname_or_unknown); - return NULL; - } - // module filename missing - _PyErr_Clear(tstate); - } + origin = NULL; + if (PyObject_GetOptionalAttr(v, &_Py_ID(__spec__), &spec) < 0) { + Py_DECREF(mod_name_or_unknown); + return NULL; } - if (pkgpath == NULL || !PyUnicode_Check(pkgpath)) { - Py_CLEAR(pkgpath); + if (spec == NULL) { errmsg = PyUnicode_FromFormat( "cannot import name %R from %R (unknown location)", - name, pkgname_or_unknown + name, mod_name_or_unknown + ); + goto done_with_errmsg; + } + if (_PyModuleSpec_GetFileOrigin(spec, &origin) < 0) { + goto done; + } + + int is_possibly_shadowing = _PyModule_IsPossiblyShadowing(origin); + if (is_possibly_shadowing < 0) { + goto done; + } + int is_possibly_shadowing_stdlib = 0; + if (is_possibly_shadowing) { + PyObject *stdlib_modules = PySys_GetObject("stdlib_module_names"); + if (stdlib_modules && PyAnySet_Check(stdlib_modules)) { + is_possibly_shadowing_stdlib = PySet_Contains(stdlib_modules, mod_name_or_unknown); + if (is_possibly_shadowing_stdlib < 0) { + goto done; + } + } + } + + if (is_possibly_shadowing_stdlib) { + assert(origin); + errmsg = PyUnicode_FromFormat( + "cannot import name %R from %R " + "(consider renaming %R since it has the same " + "name as the standard library module named %R " + "and prevents importing that standard library module)", + name, mod_name_or_unknown, origin, mod_name_or_unknown ); } else { - PyObject *spec; - int rc = PyObject_GetOptionalAttr(v, &_Py_ID(__spec__), &spec); - if (rc > 0) { - rc = _PyModuleSpec_IsInitializing(spec); - Py_DECREF(spec); - } + int rc = _PyModuleSpec_IsInitializing(spec); if (rc < 0) { - Py_DECREF(pkgname_or_unknown); - Py_DECREF(pkgpath); - return NULL; + goto done; + } + else if (rc > 0) { + if (is_possibly_shadowing) { + assert(origin); + // For non-stdlib modules, only mention the possibility of + // shadowing if the module is being initialized. + errmsg = PyUnicode_FromFormat( + "cannot import name %R from %R " + "(consider renaming %R if it has the same name " + "as a library you intended to import)", + name, mod_name_or_unknown, origin + ); + } + else if (origin) { + errmsg = PyUnicode_FromFormat( + "cannot import name %R from partially initialized module %R " + "(most likely due to a circular import) (%S)", + name, mod_name_or_unknown, origin + ); + } + else { + errmsg = PyUnicode_FromFormat( + "cannot import name %R from partially initialized module %R " + "(most likely due to a circular import)", + name, mod_name_or_unknown + ); + } + } + else { + assert(rc == 0); + if (origin) { + errmsg = PyUnicode_FromFormat( + "cannot import name %R from %R (%S)", + name, mod_name_or_unknown, origin + ); + } + else { + errmsg = PyUnicode_FromFormat( + "cannot import name %R from %R (unknown location)", + name, mod_name_or_unknown + ); + } } - const char *fmt = - rc ? - "cannot import name %R from partially initialized module %R " - "(most likely due to a circular import) (%S)" : - "cannot import name %R from %R (%S)"; - - errmsg = PyUnicode_FromFormat(fmt, name, pkgname_or_unknown, pkgpath); } - /* NULL checks for errmsg and pkgname done by PyErr_SetImportError. */ - _PyErr_SetImportErrorWithNameFrom(errmsg, pkgname, pkgpath, name); - Py_XDECREF(errmsg); - Py_DECREF(pkgname_or_unknown); - Py_XDECREF(pkgpath); +done_with_errmsg: + /* NULL checks for errmsg, mod_name, origin done by PyErr_SetImportError. */ + _PyErr_SetImportErrorWithNameFrom(errmsg, mod_name, origin, name); + Py_DECREF(errmsg); + +done: + Py_XDECREF(origin); + Py_XDECREF(spec); + Py_DECREF(mod_name_or_unknown); return NULL; } @@ -3243,5 +3301,3 @@ _PyEval_LoadName(PyThreadState *tstate, _PyInterpreterFrame *frame, PyObject *na } return value; } - -