Mirror
/
cpython
mirror of https://github.com/python/cpython
4
1
Fork 0
Code Issues Releases Wiki Activity

gh-123930: Better error for "from imports" when script shadows module (#123929)

This commit is contained in:
Shantanu 2024-10-24 12:11:12 -07:00 committed by GitHub
parent 3f24bde0b6
commit 500f5338a8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 344 additions and 173 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
Doc/whatsnew/3.13.rst
View File

@ -274,7 +274,7 @@ Improved error messages
File "/home/me/random.py", line 3, in <module>
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 <module>
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`.)

2
Include/internal/pycore_moduleobject.h
View File

@ -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);

215
Lib/test/test_import/__init__.py
View File

@ -804,15 +804,25 @@ 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 = (
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)
expected_error = error + (
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\)"
rb"and prevents importing that standard library module\)"
)
popen = script_helper.spawn_python(os.path.join(tmp, "fractions.py"), cwd=tmp)
@ -849,22 +859,31 @@ class ImportTests(unittest.TestCase):
self.assertEqual(stdout, b'') # no error
def test_package_shadowing_stdlib_module(self):
with os_helper.temp_dir() as tmp:
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:
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
""")
f.write("import fractions; fractions.shadowing_module\n")
f.write(script)
expected_error = (
rb"AttributeError: module 'fractions' has no attribute 'Fraction' "
rb"\(consider renaming '.*fractions.__init__.py' since it has the "
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 the import system gives it precedence\)"
rb"and prevents importing that standard library module\)"
)
popen = script_helper.spawn_python(os.path.join(tmp, "main.py"), cwd=tmp)
@ -881,14 +900,24 @@ fractions.Fraction
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 = (
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)
expected_error = error + (
rb" \(consider renaming '.*numpy.py' if it has the "
rb"same name as a third-party module you intended to import\)\s+\Z"
rb"same name as a library you intended to import\)\s+\Z"
)
popen = script_helper.spawn_python(os.path.join(tmp, "numpy.py"))
@ -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,33 +1073,57 @@ 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()
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()

4
Misc/NEWS.d/next/Core_and_Builtins/2024-09-11-01-32-07.gh-issue-123930.BkPfB6.rst Normal file
View File

@ -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.

28
Objects/moduleobject.c
View File

@ -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,

148
Python/ceval.c
View File

@ -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("<unknown module name>");
if (pkgname_or_unknown == NULL) {
if (mod_name == NULL) {
mod_name_or_unknown = PyUnicode_FromString("<unknown module name>");
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);
origin = NULL;
if (PyObject_GetOptionalAttr(v, &_Py_ID(__spec__), &spec) < 0) {
Py_DECREF(mod_name_or_unknown);
return NULL;
}
// module filename missing
_PyErr_Clear(tstate);
}
}
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;
}
const char *fmt =
rc ?
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)" :
"cannot import name %R from %R (%S)";
errmsg = PyUnicode_FromFormat(fmt, name, pkgname_or_unknown, pkgpath);
"(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
);
}
}
}
/* 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;
}