Change importlib.machinery.PathFinder to not have implicit semantics (that's

not handled by importlib._bootstrap._DefaultPathFinder).

3 files changed, 72 insertions, 57 deletions

diff --git a/Lib/importlib/_bootstrap.py b/Lib/importlib/_bootstrap.py
index 54e2f9d..1b4053d 100644
--- a/Lib/importlib/_bootstrap.py
+++ b/Lib/importlib/_bootstrap.py
@@ -610,36 +610,25 @@ class PathFinder:
 
     """Meta path finder for sys.(path|path_hooks|path_importer_cache)."""
 
-    _default_hook = staticmethod(chaining_fs_path_hook(ExtensionFileImporter,
-                                                        PyFileImporter))
-
-    # The list of implicit hooks cannot be a class attribute because of
-    # bootstrapping issues for accessing imp.
     @classmethod
-    def _implicit_hooks(cls):
-        """Return a list of the implicit path hooks."""
-        return [cls._default_hook, imp.NullImporter]
-
-    @classmethod
-    def _path_hooks(cls, path):
-        """Search sys.path_hooks for a finder for 'path'.
+    def _path_hooks(cls, path, hooks=None):
+        """Search sequence of hooks for a finder for 'path'.
 
-        Guaranteed to return a finder for the path as NullImporter is the
-        default importer for any path that does not have an explicit finder.
+        If 'hooks' is false then use sys.path_hooks.
 
         """
-        for hook in sys.path_hooks + cls._implicit_hooks():
+        if not hooks:
+            hooks = sys.path_hooks
+        for hook in hooks:
             try:
                 return hook(path)
             except ImportError:
                 continue
         else:
-            # This point should never be reached thanks to NullImporter.
-            raise SystemError("no hook could find an importer for "
-                              "{0}".format(path))
+            raise ImportError("no path hook found for {0}".format(path))
 
     @classmethod
-    def _path_importer_cache(cls, path):
+    def _path_importer_cache(cls, path, default=None):
         """Get the finder for the path from sys.path_importer_cache.
 
         If the path is not in the cache, find the appropriate finder and cache
@@ -657,9 +646,9 @@ class PathFinder:
             finder = cls._path_hooks(path)
             sys.path_importer_cache[path] = finder
         else:
-            if finder is None:
+            if finder is None and default:
                 # Raises ImportError on failure.
-                finder = cls._default_hook(path)
+                finder = default(path)
                 sys.path_importer_cache[path] = finder
         return finder
 
@@ -680,6 +669,30 @@ class PathFinder:
             return None
 
 
+class _DefaultPathFinder(PathFinder):
+
+    """Subclass of PathFinder that implements implicit semantics for
+    __import__."""
+
+    _default_hook = staticmethod(chaining_fs_path_hook(ExtensionFileImporter,
+                                                        PyFileImporter))
+
+    @classmethod
+    def _path_hooks(cls, path):
+        """Search sys.path_hooks as well as implicit path hooks."""
+        try:
+            return super()._path_hooks(path)
+        except ImportError:
+            implicit_hooks = [cls._default_hook, imp.NullImporter]
+            return super()._path_hooks(path, implicit_hooks)
+
+    @classmethod
+    def _path_importer_cache(cls, path):
+        """Use the default path hook when None is stored in
+        sys.path_importer_cache."""
+        return super()._path_importer_cache(path, cls._default_hook)
+
+
 class ImportLockContext(object):
 
     """Context manager for the import lock."""
@@ -693,6 +706,8 @@ class ImportLockContext(object):
         imp.release_lock()
 
 
+_IMPLICIT_META_PATH = [BuiltinImporter, FrozenImporter, _DefaultPathFinder]
+
 def _gcd_import(name, package=None, level=0):
     """Import and return the module based on its name, the package the call is
     being made from, and the level adjustment.
@@ -736,8 +751,7 @@ def _gcd_import(name, package=None, level=0):
             # Backwards-compatibility; be nicer to skip the dict lookup.
             parent_module = sys.modules[parent]
             path = parent_module.__path__
-        meta_path = (sys.meta_path +
-                     [BuiltinImporter, FrozenImporter, PathFinder])
+        meta_path = sys.meta_path + _IMPLICIT_META_PATH
         for finder in meta_path:
             loader = finder.find_module(name, path)
             if loader is not None:
diff --git a/Lib/importlib/test/import_/test_path.py b/Lib/importlib/test/import_/test_path.py
index 249b95b..8cf5699 100644
--- a/Lib/importlib/test/import_/test_path.py
+++ b/Lib/importlib/test/import_/test_path.py
@@ -1,3 +1,4 @@
+from importlib import _bootstrap
 from importlib import machinery
 from .. import util
 from . import util as import_util
@@ -12,20 +13,13 @@ import unittest
 
 class FinderTests(unittest.TestCase):
 
-    """Tests for SysPathImporter."""
+    """Tests for PathFinder."""
 
     def test_failure(self):
         # Test None returned upon not finding a suitable finder.
-        def mock_implicit_hooks():
-            return []
-        # XXX Not blackbox.
-        original_hooks = machinery.PathFinder._implicit_hooks
-        machinery.PathFinder._implicit_hooks = staticmethod(mock_implicit_hooks)
-        try:
-            with util.import_state():
-                self.assert_(machinery.PathFinder.find_module('XXX') is None)
-        finally:
-            machinery.PathFinder._implicit_hooks = original_hooks
+        module = '<test module>'
+        with util.import_state():
+            self.assert_(machinery.PathFinder.find_module(module) is None)
 
     def test_sys_path(self):
         # Test that sys.path is used when 'path' is None.
@@ -48,23 +42,6 @@ class FinderTests(unittest.TestCase):
             loader = machinery.PathFinder.find_module(module, [path])
             self.assert_(loader is importer)
 
-    def test_path_importer_cache_has_None(self):
-        # Test that the default hook is used when sys.path_importer_cache
-        # contains None for a path.
-        module = '<test module>'
-        importer = util.mock_modules(module)
-        path = '<test path>'
-        # XXX Not blackbox.
-        original_hook = machinery.PathFinder._default_hook
-        mock_hook = import_util.mock_path_hook(path, importer=importer)
-        machinery.PathFinder._default_hook = staticmethod(mock_hook)
-        try:
-            with util.import_state(path_importer_cache={path: None}):
-                loader = machinery.PathFinder.find_module(module, path=[path])
-                self.assert_(loader is importer)
-        finally:
-            machinery.PathFinder._default_hook = original_hook
-
     def test_path_hooks(self):
         # Test that sys.path_hooks is used.
         # Test that sys.path_importer_cache is set.
@@ -78,6 +55,11 @@ class FinderTests(unittest.TestCase):
             self.assert_(path in sys.path_importer_cache)
             self.assert_(sys.path_importer_cache[path] is importer)
 
+
+class DefaultPathFinderTests(unittest.TestCase):
+
+    """Test importlib._bootstrap._DefaultPathFinder."""
+
     def test_implicit_hooks(self):
         # Test that the implicit path hooks are used.
         existing_path = os.path.dirname(support.TESTFN)
@@ -85,22 +67,41 @@ class FinderTests(unittest.TestCase):
         module = '<module>'
         assert not os.path.exists(bad_path)
         with util.import_state():
-            nothing = machinery.PathFinder.find_module(module,
-                                                       path=[existing_path])
+            nothing = _bootstrap._DefaultPathFinder.find_module(module,
+                                                           path=[existing_path])
             self.assert_(nothing is None)
             self.assert_(existing_path in sys.path_importer_cache)
             self.assert_(not isinstance(sys.path_importer_cache[existing_path],
                                         imp.NullImporter))
-            nothing = machinery.PathFinder.find_module(module, path=[bad_path])
+            nothing = _bootstrap._DefaultPathFinder.find_module(module,
+                                                                path=[bad_path])
             self.assert_(nothing is None)
             self.assert_(bad_path in sys.path_importer_cache)
             self.assert_(isinstance(sys.path_importer_cache[bad_path],
                                     imp.NullImporter))
 
+    def test_path_importer_cache_has_None(self):
+        # Test that the default hook is used when sys.path_importer_cache
+        # contains None for a path.
+        module = '<test module>'
+        importer = util.mock_modules(module)
+        path = '<test path>'
+        # XXX Not blackbox.
+        original_hook = _bootstrap._DefaultPathFinder._default_hook
+        mock_hook = import_util.mock_path_hook(path, importer=importer)
+        _bootstrap._DefaultPathFinder._default_hook = staticmethod(mock_hook)
+        try:
+            with util.import_state(path_importer_cache={path: None}):
+                loader = _bootstrap._DefaultPathFinder.find_module(module,
+                                                                    path=[path])
+                self.assert_(loader is importer)
+        finally:
+            _bootstrap._DefaultPathFinder._default_hook = original_hook
+
 
 def test_main():
     from test.support import run_unittest
-    run_unittest(PathTests, __path__Tests, FinderTests)
+    run_unittest(FinderTests, DefaultPathFinderTests)
 
 if __name__ == '__main__':
     test_main()
diff --git a/Lib/importlib/test/import_/util.py b/Lib/importlib/test/import_/util.py
index 6ab0651..1a74c38 100644
--- a/Lib/importlib/test/import_/util.py
+++ b/Lib/importlib/test/import_/util.py
@@ -9,8 +9,8 @@ def import_(*args, **kwargs):
     """Delegate to allow for injecting different implementations of import."""
     if using___import__:
         return __import__(*args, **kwargs)
-    #return importlib.Import()(*args, **kwargs)
-    return importlib._bootstrap._import(*args, **kwargs)
+    else:
+        return importlib._bootstrap._import(*args, **kwargs)
 
 
 def importlib_only(fxn):