Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-95754: Better error when script shadows a standard library or third party module #113769

Merged
merged 51 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from 48 commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
afe03da
Refactor to reduce nesting
hauntsaninja Jan 6, 2024
ae438a4
Check for script shadowing stdlib
hauntsaninja Jan 6, 2024
0933591
Add What's New
hauntsaninja Jan 6, 2024
7f24b99
Avoid calling into Python
hauntsaninja Jan 7, 2024
f073672
📜🤖 Added by blurb_it.
blurb-it[bot] Jan 7, 2024
5e22968
just use wchar everywhere
hauntsaninja Jan 7, 2024
4db6e6c
fix warning
hauntsaninja Jan 7, 2024
c8864e4
Merge remote-tracking branch 'upstream/main' into stdlib-error
hauntsaninja Jan 21, 2024
2150dc5
do stdlib module name check via PySys
hauntsaninja Jan 21, 2024
0e1e3d8
add back test case
hauntsaninja Jan 22, 2024
2486d00
CleanImport sys is not enough
hauntsaninja Jan 22, 2024
9f0955d
handle set error
hauntsaninja Jan 22, 2024
24ccd8a
add substr test
hauntsaninja Jan 22, 2024
47d7f6c
remove dedent, fix whitespace, use bool
hauntsaninja Jan 28, 2024
5c0991d
Rework to handle third party shadowing
hauntsaninja Jan 29, 2024
dec51f3
Update test cases
hauntsaninja Jan 29, 2024
a32411b
Update What's New
hauntsaninja Jan 29, 2024
c1b79fa
Tweak test
hauntsaninja Jan 29, 2024
1feabb0
Update news entry
hauntsaninja Jan 29, 2024
eaf08ac
Tweak test names
hauntsaninja Jan 29, 2024
d9eb424
Tweak What's New language
hauntsaninja Jan 29, 2024
69df558
Tweak docstring, fix accidental commit
hauntsaninja Jan 29, 2024
ef30d33
Add hyphen in third-party
hauntsaninja Jan 29, 2024
f1c9ee9
Tweak test attribute
hauntsaninja Jan 29, 2024
a588c15
Fix nit
hauntsaninja Jan 29, 2024
ea0d2ad
Add missing static
hauntsaninja Jan 29, 2024
61b5517
Fix test failures on Windows
hauntsaninja Jan 29, 2024
a6b3d83
Use Py_ssize_t
hauntsaninja Jan 29, 2024
fe25068
Add test case for bad sys.path
hauntsaninja Jan 29, 2024
0981302
Further tweaks
hauntsaninja Jan 29, 2024
5069b98
Tweak assert for +1 logic
hauntsaninja Jan 29, 2024
9977cea
Use config->sys_path_0 / config->module_search_path + add test for sy…
hauntsaninja Feb 2, 2024
f742261
stack allocate
hauntsaninja Feb 4, 2024
3d205f3
more cwd tests
hauntsaninja Feb 4, 2024
8654c5c
Merge remote-tracking branch 'upstream/main' into stdlib-error
hauntsaninja Feb 8, 2024
146d394
further tweak error message
hauntsaninja Mar 6, 2024
a38bad5
control flow feedback
hauntsaninja Mar 8, 2024
822d8eb
handle has_location correctly
hauntsaninja Mar 8, 2024
3ec37d1
remove explicit safe path check
hauntsaninja Mar 8, 2024
a87a91d
Merge remote-tracking branch 'upstream/main' into stdlib-error
hauntsaninja Mar 8, 2024
64bb4fd
make regen-global-objects
hauntsaninja Mar 8, 2024
786b74e
fix spacing
hauntsaninja Mar 8, 2024
d7f1bc0
Revert "remove explicit safe path check"
hauntsaninja Mar 8, 2024
e55576d
test the no origin case
hauntsaninja Mar 8, 2024
a388887
output param feedback
hauntsaninja Mar 8, 2024
3d36032
test non-str origin
hauntsaninja Mar 8, 2024
616ced2
add comment about module.__file__
hauntsaninja Mar 8, 2024
afbcd40
test package shadowing module
hauntsaninja Mar 8, 2024
2890b6b
feedback
hauntsaninja Mar 8, 2024
1d65725
try another error message variation
hauntsaninja Apr 1, 2024
300c557
style nits
hauntsaninja Apr 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions Doc/whatsnew/3.13.rst
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,40 @@ Improved Error Messages
variables. See also :ref:`using-on-controlling-color`.
(Contributed by Pablo Galindo Salgado in :gh:`112730`.)

* A common mistake is to write a script with the same name as a
standard library module. When this results in errors, we now
display a more helpful error message:

.. code-block:: shell-session

$ python random.py
Traceback (most recent call last):
File "/home/random.py", line 1, in <module>
import random; print(random.randint(5))
^^^^^^^^^^^^^
File "/home/random.py", line 1, in <module>
import random; print(random.randint(5))
^^^^^^^^^^^^^^
AttributeError: module 'random' has no attribute 'randint' (consider renaming '/home/random.py' since it has the same name as the standard library module named 'random'and takes precedence over it on sys.path)
hauntsaninja marked this conversation as resolved.
Show resolved Hide resolved

Similarly, if a script has the same name as a third-party
module it attempts to import, and this results in errors,
we also display a more helpful error message:

.. code-block:: shell-session

$ python numpy.py
Traceback (most recent call last):
File "/home/numpy.py", line 1, in <module>
import numpy as np; np.array([1,2,3])
^^^^^^^^^^^^^^^^^^
File "/home/numpy.py", line 1, in <module>
import numpy as np; np.array([1,2,3])
^^^^^^^^
AttributeError: module 'numpy' has no attribute 'array' (consider renaming '/home/numpy.py' if it has the same name as a third-party module you intended to import)

(Contributed by Shantanu Jain in :gh:`95754`.)

* When an incorrect keyword argument is passed to a function, the error message
now potentially suggests the correct keyword argument.
(Contributed by Pablo Galindo Salgado and Shantanu Jain in :gh:`107944`.)
Expand Down
1 change: 1 addition & 0 deletions Include/internal/pycore_global_objects_fini_generated.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Include/internal/pycore_global_strings.h
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,7 @@ struct _Py_global_strings {
STRUCT_FOR_ID(h)
STRUCT_FOR_ID(handle)
STRUCT_FOR_ID(handle_seq)
STRUCT_FOR_ID(has_location)
STRUCT_FOR_ID(hash_name)
STRUCT_FOR_ID(header)
STRUCT_FOR_ID(headers)
Expand Down
1 change: 1 addition & 0 deletions Include/internal/pycore_runtime_init_generated.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions Include/internal/pycore_unicodeobject_generated.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

221 changes: 221 additions & 0 deletions Lib/test/test_import/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -790,6 +790,227 @@ def test_issue105979(self):
self.assertIn("Frozen object named 'x' is invalid",
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 takes precedence over it on sys.path\)"
)

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('-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'')

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

# test the logic in with different cwd
hauntsaninja marked this conversation as resolved.
Show resolved Hide resolved
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('-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 takes precedence over it on sys.path\)"
)

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)

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

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('-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:
with open(os.path.join(tmp, "numpy.py"), "w", encoding='utf-8') as f:
f.write("this_script_does_not_attempt_to_import_numpy = True")

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)

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")
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:
fractions.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'")

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:
fractions.Fraction
except AttributeError as e:
print(str(e))

del sys.stdlib_module_names
try:
fractions.Fraction
except AttributeError as e:
print(str(e))

sys.path = [0]
try:
fractions.Fraction
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'",
],
)

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
except AttributeError as e:
print(str(e))

fractions.__spec__.origin = 0
try:
fractions.Fraction
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'"
],
)

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 takes precedence over it on sys.path\)"
)

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")
ericsnowcurrently marked this conversation as resolved.
Show resolved Hide resolved
import fractions
fractions.Fraction
""")

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


@skip_if_dont_write_bytecode
class FilePermissionTests(unittest.TestCase):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Improve the error message when a script shadowing a module from the standard
library causes :exc:`AttributeError` to be raised. Similarly, improve the error
message when a script shadowing a third party module attempts to access an
attribute from that third party module while still initialising.
Loading
Loading