Issue #14582: Import returns the module returned by a loader instead

of sys.modules when possible.

This is being done for two reasons. One is to gain a little bit of
performance by skipping an unnecessary dict lookup in sys.modules. But
the other (and main) reason is to be a little bit more clear in how
things should work from the perspective of import's interactions with
loaders. Otherwise loaders can easily forget to return the module even
though PEP 302 explicitly states they are expected to return the module
they loaded.

2 files changed, 14 insertions, 34 deletions

diff --git a/Lib/importlib/_bootstrap.py b/Lib/importlib/_bootstrap.py
index ee9936f..0135217 100644
--- a/Lib/importlib/_bootstrap.py
+++ b/Lib/importlib/_bootstrap.py
@@ -974,12 +974,12 @@ def _find_and_load(name, import_):
     loader = _find_module(name, path)
     if loader is None:
         raise ImportError(_ERR_MSG.format(name), name=name)
-    elif name not in sys.modules:
-        # The parent import may have already imported this module.
-        loader.load_module(name)
+    elif name in sys.modules:
+        # The parent module already imported this module.
+        module = sys.modules[name]
+    else:
+        module = loader.load_module(name)
         verbose_message('import {!r} # {!r}', name, loader)
-    # Backwards-compatibility; be nicer to skip the dict lookup.
-    module = sys.modules[name]
     if parent:
         # Set the module as an attribute on its parent.
         parent_module = sys.modules[parent]
@@ -1078,7 +1078,11 @@ def __import__(name, globals={}, locals={}, fromlist=[], level=0):
         # Return up to the first dot in 'name'. This is complicated by the fact
         # that 'name' may be relative.
         if level == 0:
-            return sys.modules[name.partition('.')[0]]
+            index = name.find('.')
+            if index == -1:
+                return module
+            else:
+                return sys.modules[name[:index]]
         elif not name:
             return module
         else:
diff --git a/Lib/importlib/test/import_/test_caching.py b/Lib/importlib/test/import_/test_caching.py
index 3baff55..467eee1 100644
--- a/Lib/importlib/test/import_/test_caching.py
+++ b/Lib/importlib/test/import_/test_caching.py
@@ -47,36 +47,12 @@ class UseCache(unittest.TestCase):
         mock.load_module = MethodType(load_module, mock)
         return mock
 
-    # __import__ inconsistent between loaders and built-in import when it comes
-    #   to when to use the module in sys.modules and when not to.
-    @import_util.importlib_only
-    def test_using_cache_after_loader(self):
-        # [from cache on return]
-        with self.create_mock('module') as mock:
+    def test_using_loader_return(self):
+        loader_return = 'hi there!'
+        with self.create_mock('module', return_=loader_return) as mock:
             with util.import_state(meta_path=[mock]):
                 module = import_util.import_('module')
-                self.assertEqual(id(module), id(sys.modules['module']))
-
-    # See test_using_cache_after_loader() for reasoning.
-    @import_util.importlib_only
-    def test_using_cache_for_assigning_to_attribute(self):
-        # [from cache to attribute]
-        with self.create_mock('pkg.__init__', 'pkg.module') as importer:
-            with util.import_state(meta_path=[importer]):
-                module = import_util.import_('pkg.module')
-                self.assertTrue(hasattr(module, 'module'))
-                self.assertTrue(id(module.module), id(sys.modules['pkg.module']))
-
-    # See test_using_cache_after_loader() for reasoning.
-    @import_util.importlib_only
-    def test_using_cache_for_fromlist(self):
-        # [from cache for fromlist]
-        with self.create_mock('pkg.__init__', 'pkg.module') as importer:
-            with util.import_state(meta_path=[importer]):
-                module = import_util.import_('pkg', fromlist=['module'])
-                self.assertTrue(hasattr(module, 'module'))
-                self.assertEqual(id(module.module),
-                                 id(sys.modules['pkg.module']))
+                self.assertEqual(module, loader_return)
 
 
 def test_main():