Skip to content

Commit

Permalink
add error for shadowing
Browse files Browse the repository at this point in the history
  • Loading branch information
@hauntsaninja
hauntsaninja committed Sep 11, 2024
1 parent 6ce3a67 commit 601e702
Show file tree
Hide file tree
Showing 4 changed files with 217 additions and 119 deletions.
1 change: 1 addition & 0 deletions Include/internal/pycore_moduleobject.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ 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);

Expand Down
233 changes: 136 additions & 97 deletions Lib/test/test_import/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -786,104 +786,133 @@ def test_issue105979(self):
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 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\)"
)

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(os.path.join(tmp, "fractions.py"), 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)
popen = script_helper.spawn_python('-m', 'fractions', 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('-c', 'import 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'')
# 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'')

tmp_child = os.path.join(tmp, "child")
os.mkdir(tmp_child)
tmp_child = os.path.join(tmp, "child")
os.mkdir(tmp_child)

# 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)
# 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('-m', '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
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 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)

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\)"
)

popen = script_helper.spawn_python(os.path.join(tmp, "main.py"), 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)

popen = script_helper.spawn_python('-m', 'main', cwd=tmp)
stdout, stderr = popen.communicate()
self.assertRegex(stdout, expected_error)
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'")
# 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):
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'"
)
]
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")
for script, error in script_errors:
with open(os.path.join(tmp, "numpy.py"), "w", encoding='utf-8') as f:
f.write(script)

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"
)
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"
)

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(os.path.join(tmp, "numpy.py"))
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('-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)
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:
Expand All @@ -893,11 +922,17 @@ def test_script_maybe_not_shadowing_third_party(self):
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:
Expand Down Expand Up @@ -983,28 +1018,32 @@ class substr(str):
)

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\)"
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 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 the import system gives it precedence\)"
)

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
""")

popen = script_helper.spawn_python("main.py", cwd=tmp)
stdout, stderr = popen.communicate()
self.assertRegex(stdout, expected_error)
popen = script_helper.spawn_python("main.py", cwd=tmp)
stdout, stderr = popen.communicate()
self.assertRegex(stdout, expected_error)


@skip_if_dont_write_bytecode
Expand Down
14 changes: 9 additions & 5 deletions Objects/moduleobject.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -855,8 +855,8 @@ _PyModuleSpec_GetFileOrigin(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
Expand Down Expand Up @@ -985,7 +985,7 @@ _Py_module_getattro_impl(PyModuleObject *m, PyObject *name, int suppress)
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;
}
Expand All @@ -1011,7 +1011,10 @@ _Py_module_getattro_impl(PyModuleObject *m, PyObject *name, int suppress)
}
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
Expand All @@ -1037,7 +1040,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,
Expand Down
Loading

0 comments on commit 601e702

Please sign in to comment.