Issue #27487: Warn if submodule already imported before runpy execution

Also try to clarify the find_spec() error message.

diff --git a/Lib/runpy.py b/Lib/runpy.py
index af6205db49d494bd226b048144d59adab433c7b8..e290c218b9d6f80045c88fe4e3a38c4bb946c4a2 100644 (file)
--- a/Lib/runpy.py
+++ b/Lib/runpy.py
@@ -114,6 +114,15 @@ def _get_module_details(mod_name, error=ImportError):
             if e.name is None or (e.name != pkg_name and
                     not pkg_name.startswith(e.name + ".")):
                 raise
+        # Warn if the module has already been imported under its normal name
+        existing = sys.modules.get(mod_name)
+        if existing is not None and not hasattr(existing, "__path__"):
+            from warnings import warn
+            msg = "{mod_name!r} found in sys.modules after import of " \
+                "package {pkg_name!r}, but prior to execution of " \
+                "{mod_name!r}; this may result in unpredictable " \
+                "behaviour".format(mod_name=mod_name, pkg_name=pkg_name)
+            warn(RuntimeWarning(msg))
 
     try:
         spec = importlib.util.find_spec(mod_name)
@@ -121,7 +130,7 @@ def _get_module_details(mod_name, error=ImportError):
         # This hack fixes an impedance mismatch between pkgutil and
         # importlib, where the latter raises other errors for cases where
         # pkgutil previously raised ImportError
-        msg = "Error while finding spec for {!r} ({}: {})"
+        msg = "Error while finding module specification for {!r} ({}: {})"
         raise error(msg.format(mod_name, type(ex).__name__, ex)) from ex
     if spec is None:
         raise error("No module named %s" % mod_name)

diff --git a/Lib/test/test_cmd_line_script.py b/Lib/test/test_cmd_line_script.py
index b2be9b1b709258e1fd3b1920dc89e77bd003620a..befe0e45d1452a1f884d672e596a02d3733679b1 100644 (file)
--- a/Lib/test/test_cmd_line_script.py
+++ b/Lib/test/test_cmd_line_script.py
@@ -426,8 +426,9 @@ class CmdLineTest(unittest.TestCase):
         # Exercise error reporting for various invalid package executions
         tests = (
             ('builtins', br'No code object available'),
-            ('builtins.x', br'Error while finding spec.*AttributeError'),
-            ('builtins.x.y', br'Error while finding spec.*'
+            ('builtins.x', br'Error while finding module specification.*'
+                br'AttributeError'),
+            ('builtins.x.y', br'Error while finding module specification.*'
                 br'ImportError.*No module named.*not a package'),
             ('os.path', br'loader.*cannot handle'),
             ('importlib', br'No module named.*'
@@ -450,7 +451,8 @@ class CmdLineTest(unittest.TestCase):
             with open('test_pkg/__init__.pyc', 'wb'):
                 pass
             err = self.check_dash_m_failure('test_pkg')
-            self.assertRegex(err, br'Error while finding spec.*'
+            self.assertRegex(err,
+                br'Error while finding module specification.*'
                 br'ImportError.*bad magic number')
             self.assertNotIn(b'is a package', err)
             self.assertNotIn(b'Traceback', err)

diff --git a/Lib/test/test_runpy.py b/Lib/test/test_runpy.py
index 87c83ecfaed81cafb3630ca0ef23893cdd68e2a2..db55db702bffbba758e70784f24b5155a3dfeb9c 100644 (file)
--- a/Lib/test/test_runpy.py
+++ b/Lib/test/test_runpy.py
@@ -7,6 +7,7 @@ import re
 import tempfile
 import importlib, importlib.machinery, importlib.util
 import py_compile
+import warnings
 from test.support import (
     forget, make_legacy_pyc, unload, verbose, no_tracing,
     create_empty_file, temp_dir)
@@ -246,7 +247,7 @@ class RunModuleTestCase(unittest.TestCase, CodeExecutionMixin):
                                                           mod_fname)
         return pkg_dir, mod_fname, mod_name, mod_spec
 
-    def _del_pkg(self, top, depth, mod_name):
+    def _del_pkg(self, top):
         for entry in list(sys.modules):
             if entry.startswith("__runpy_pkg__"):
                 del sys.modules[entry]
@@ -320,7 +321,7 @@ class RunModuleTestCase(unittest.TestCase, CodeExecutionMixin):
                 self._fix_ns_for_legacy_pyc(expected_ns, alter_sys)
                 self.check_code_execution(create_ns, expected_ns)
         finally:
-            self._del_pkg(pkg_dir, depth, mod_name)
+            self._del_pkg(pkg_dir)
         if verbose > 1: print("Module executed successfully")
 
     def _check_package(self, depth, alter_sys=False,
@@ -361,7 +362,7 @@ class RunModuleTestCase(unittest.TestCase, CodeExecutionMixin):
                 self._fix_ns_for_legacy_pyc(expected_ns, alter_sys)
                 self.check_code_execution(create_ns, expected_ns)
         finally:
-            self._del_pkg(pkg_dir, depth, pkg_name)
+            self._del_pkg(pkg_dir)
         if verbose > 1: print("Package executed successfully")
 
     def _add_relative_modules(self, base_dir, source, depth):
@@ -424,7 +425,7 @@ from ..uncle.cousin import nephew
                 self.assertIn("nephew", d2)
                 del d2 # Ensure __loader__ entry doesn't keep file open
         finally:
-            self._del_pkg(pkg_dir, depth, mod_name)
+            self._del_pkg(pkg_dir)
         if verbose > 1: print("Module executed successfully")
 
     def test_run_module(self):
@@ -447,7 +448,7 @@ from ..uncle.cousin import nephew
         result = self._make_pkg("", 1, "__main__")
         pkg_dir, _, mod_name, _ = result
         mod_name = mod_name.replace(".__main__", "")
-        self.addCleanup(self._del_pkg, pkg_dir, 1, mod_name)
+        self.addCleanup(self._del_pkg, pkg_dir)
         init = os.path.join(pkg_dir, "__runpy_pkg__", "__init__.py")
 
         exceptions = (ImportError, AttributeError, TypeError, ValueError)
@@ -470,6 +471,31 @@ from ..uncle.cousin import nephew
                 else:
                     self.fail("Nothing raised; expected {}".format(name))
 
+    def test_submodule_imported_warning(self):
+        pkg_dir, _, mod_name, _ = self._make_pkg("", 1)
+        try:
+            __import__(mod_name)
+            with self.assertWarnsRegex(RuntimeWarning,
+                    r"found in sys\.modules"):
+                run_module(mod_name)
+        finally:
+            self._del_pkg(pkg_dir)
+
+    def test_package_imported_no_warning(self):
+        pkg_dir, _, mod_name, _ = self._make_pkg("", 1, "__main__")
+        self.addCleanup(self._del_pkg, pkg_dir)
+        package = mod_name.replace(".__main__", "")
+        # No warning should occur if we only imported the parent package
+        __import__(package)
+        self.assertIn(package, sys.modules)
+        with warnings.catch_warnings():
+            warnings.simplefilter("error", RuntimeWarning)
+            run_module(package)
+        # But the warning should occur if we imported the __main__ submodule
+        __import__(mod_name)
+        with self.assertWarnsRegex(RuntimeWarning, r"found in sys\.modules"):
+            run_module(package)
+
     def test_run_package_in_namespace_package(self):
         for depth in range(1, 4):
             if verbose > 1: print("Testing package depth:", depth)
@@ -524,7 +550,7 @@ from ..uncle.cousin import nephew
         try:
             self.check_code_execution(create_ns, expected_ns)
         finally:
-            self._del_pkg(pkg_dir, depth, mod_name)
+            self._del_pkg(pkg_dir)
 
     def test_pkgutil_walk_packages(self):
         # This is a dodgy hack to use the test_runpy infrastructure to test
@@ -548,7 +574,7 @@ from ..uncle.cousin import nephew
         expected_modules.add(pkg_name + ".runpy_test")
         pkg_dir, mod_fname, mod_name, mod_spec = (
                self._make_pkg("", max_depth))
-        self.addCleanup(self._del_pkg, pkg_dir, max_depth, mod_name)
+        self.addCleanup(self._del_pkg, pkg_dir)
         for depth in range(2, max_depth+1):
             self._add_relative_modules(pkg_dir, "", depth)
         for finder, mod_name, ispkg in pkgutil.walk_packages([pkg_dir]):

diff --git a/Misc/NEWS b/Misc/NEWS
index 3d2cc1939c6e4c79c8f7fd5900d06ec8153b1561..1ecb6f1f94fff49efb501353f459c0bdb5ae3275 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -10,6 +10,10 @@ Release date: TBA
 Core and Builtins
 -----------------
 
+- Issue #27487: Warn if a submodule argument to "python -m" or
+  runpy.run_module() is found in sys.modules after parent packages are
+  imported, but before the submodule is executed.
+
 - Issue #27558: Fix a SystemError in the implementation of "raise" statement.
   In a brand new thread, raise a RuntimeError since there is no active
   exception to reraise. Patch written by Xiang Zhang.