Issue 19944: Fix importlib.find_spec() so it imports parents as needed.

The function is also moved to importlib.util.

9 files changed, 272 insertions, 213 deletions

diff --git a/Lib/idlelib/EditorWindow.py b/Lib/idlelib/EditorWindow.py
index 390ffab..d183057 100644
--- a/Lib/idlelib/EditorWindow.py
+++ b/Lib/idlelib/EditorWindow.py
@@ -1,5 +1,6 @@
 import importlib
 import importlib.abc
+import importlib.util
 import os
 from platform import python_version
 import re
@@ -660,7 +661,7 @@ class EditorWindow(object):
             return
         # XXX Ought to insert current file's directory in front of path
         try:
-            spec = importlib.find_spec(name)
+            spec = importlib.util.find_spec(name)
         except (ValueError, ImportError) as msg:
             tkMessageBox.showerror("Import error", str(msg), parent=self.text)
             return
diff --git a/Lib/importlib/__init__.py b/Lib/importlib/__init__.py
index c66e46c..f6adc5c 100644
--- a/Lib/importlib/__init__.py
+++ b/Lib/importlib/__init__.py
@@ -11,8 +11,6 @@ __all__ = ['__import__', 'import_module', 'invalidate_caches', 'reload']
 # initialised below if the frozen one is not available).
 import _imp  # Just the builtin component, NOT the full Python module
 import sys
-import types
-import warnings
 
 try:
     import _frozen_importlib as _bootstrap
@@ -34,6 +32,10 @@ _r_long = _bootstrap._r_long
 # Fully bootstrapped at this point, import whatever you like, circular
 # dependencies and startup overhead minimisation permitting :)
 
+import types
+import warnings
+
+
 # Public API #########################################################
 
 from ._bootstrap import __import__
@@ -47,47 +49,16 @@ def invalidate_caches():
             finder.invalidate_caches()
 
 
-def find_spec(name, path=None):
-    """Return the spec for the specified module.
-
-    First, sys.modules is checked to see if the module was already imported. If
-    so, then sys.modules[name].__spec__ is returned. If that happens to be
-    set to None, then ValueError is raised. If the module is not in
-    sys.modules, then sys.meta_path is searched for a suitable spec with the
-    value of 'path' given to the finders. None is returned if no spec could
-    be found.
-
-    Dotted names do not have their parent packages implicitly imported. You will
-    most likely need to explicitly import all parent packages in the proper
-    order for a submodule to get the correct spec.
-
-    """
-    if name not in sys.modules:
-        return _bootstrap._find_spec(name, path)
-    else:
-        module = sys.modules[name]
-        if module is None:
-            return None
-        try:
-            spec = module.__spec__
-        except AttributeError:
-            raise ValueError('{}.__spec__ is not set'.format(name))
-        else:
-            if spec is None:
-                raise ValueError('{}.__spec__ is None'.format(name))
-            return spec
-
-
 def find_loader(name, path=None):
     """Return the loader for the specified module.
 
     This is a backward-compatible wrapper around find_spec().
 
-    This function is deprecated in favor of importlib.find_spec().
+    This function is deprecated in favor of importlib.util.find_spec().
 
     """
-    warnings.warn('Use importlib.find_spec() instead.', DeprecationWarning,
-                  stacklevel=2)
+    warnings.warn('Use importlib.util.find_spec() instead.',
+                  DeprecationWarning, stacklevel=2)
     try:
         loader = sys.modules[name].__loader__
         if loader is None:
@@ -167,7 +138,8 @@ def reload(module):
                 pkgpath = parent.__path__
         else:
             pkgpath = None
-        spec = module.__spec__ = _bootstrap._find_spec(name, pkgpath, module)
+        target = module
+        spec = module.__spec__ = _bootstrap._find_spec(name, pkgpath, target)
         methods = _bootstrap._SpecMethods(spec)
         methods.exec(module)
         # The module may have replaced itself in sys.modules!
diff --git a/Lib/importlib/util.py b/Lib/importlib/util.py
index 42fc9ea..6d73b1d 100644
--- a/Lib/importlib/util.py
+++ b/Lib/importlib/util.py
@@ -7,6 +7,7 @@ from ._bootstrap import source_from_cache
 from ._bootstrap import spec_from_loader
 from ._bootstrap import spec_from_file_location
 from ._bootstrap import _resolve_name
+from ._bootstrap import _find_spec
 
 from contextlib import contextmanager
 import functools
@@ -29,6 +30,77 @@ def resolve_name(name, package):
     return _resolve_name(name[level:], package, level)
 
 
+def _find_spec_from_path(name, path=None):
+    """Return the spec for the specified module.
+
+    First, sys.modules is checked to see if the module was already imported. If
+    so, then sys.modules[name].__spec__ is returned. If that happens to be
+    set to None, then ValueError is raised. If the module is not in
+    sys.modules, then sys.meta_path is searched for a suitable spec with the
+    value of 'path' given to the finders. None is returned if no spec could
+    be found.
+
+    Dotted names do not have their parent packages implicitly imported. You will
+    most likely need to explicitly import all parent packages in the proper
+    order for a submodule to get the correct spec.
+
+    """
+    if name not in sys.modules:
+        return _find_spec(name, path)
+    else:
+        module = sys.modules[name]
+        if module is None:
+            return None
+        try:
+            spec = module.__spec__
+        except AttributeError:
+            raise ValueError('{}.__spec__ is not set'.format(name))
+        else:
+            if spec is None:
+                raise ValueError('{}.__spec__ is None'.format(name))
+            return spec
+
+
+def find_spec(name, package=None):
+    """Return the spec for the specified module.
+
+    First, sys.modules is checked to see if the module was already imported. If
+    so, then sys.modules[name].__spec__ is returned. If that happens to be
+    set to None, then ValueError is raised. If the module is not in
+    sys.modules, then sys.meta_path is searched for a suitable spec with the
+    value of 'path' given to the finders. None is returned if no spec could
+    be found.
+
+    If the name is for submodule (contains a dot), the parent module is
+    automatically imported.
+
+    The name and package arguments work the same as importlib.import_module().
+    In other words, relative module names (with leading dots) work.
+
+    """
+    fullname = resolve_name(name, package) if name.startswith('.') else name
+    if fullname not in sys.modules:
+        parent_name = fullname.rpartition('.')[0]
+        if parent_name:
+            # Use builtins.__import__() in case someone replaced it.
+            parent = __import__(parent_name, fromlist=['__path__'])
+            return _find_spec(fullname, parent.__path__)
+        else:
+            return _find_spec(fullname, None)
+    else:
+        module = sys.modules[fullname]
+        if module is None:
+            return None
+        try:
+            spec = module.__spec__
+        except AttributeError:
+            raise ValueError('{}.__spec__ is not set'.format(name))
+        else:
+            if spec is None:
+                raise ValueError('{}.__spec__ is None'.format(name))
+            return spec
+
+
 @contextmanager
 def _module_to_load(name):
     is_reload = name in sys.modules
diff --git a/Lib/pkgutil.py b/Lib/pkgutil.py
index 7beae12..326657a 100644
--- a/Lib/pkgutil.py
+++ b/Lib/pkgutil.py
@@ -611,7 +611,7 @@ def get_data(package, resource):
     which does not support get_data(), then None is returned.
     """
 
-    spec = importlib.find_spec(package)
+    spec = importlib.util.find_spec(package)
     if spec is None:
         return None
     loader = spec.loader
diff --git a/Lib/pyclbr.py b/Lib/pyclbr.py
index a8d2b1f..dd58ada 100644
--- a/Lib/pyclbr.py
+++ b/Lib/pyclbr.py
@@ -42,7 +42,7 @@ Instances of this class have the following instance variables:
 import io
 import os
 import sys
-import importlib
+import importlib.util
 import tokenize
 from token import NAME, DEDENT, OP
 from operator import itemgetter
@@ -141,7 +141,7 @@ def _readmodule(module, path, inpackage=None):
     else:
         search_path = path + sys.path
     # XXX This will change once issue19944 lands.
-    spec = importlib.find_spec(fullmodule, search_path)
+    spec = importlib.util._find_spec_from_path(fullmodule, search_path)
     fname = spec.loader.get_filename(fullmodule)
     _modules[fullmodule] = dict
     if spec.loader.is_package(fullmodule):
diff --git a/Lib/runpy.py b/Lib/runpy.py
index dc08f4e..577deb2 100644
--- a/Lib/runpy.py
+++ b/Lib/runpy.py
@@ -13,9 +13,8 @@ importers when locating support scripts as well as when importing modules.
 import os
 import sys
 import importlib.machinery # importlib first so we can test #15386 via -m
+import importlib.util
 import types
-from importlib import find_spec
-from importlib.util import spec_from_loader
 from pkgutil import read_code, get_importer
 
 __all__ = [
@@ -100,33 +99,16 @@ def _run_module_code(code, init_globals=None,
     # may be cleared when the temporary module goes away
     return mod_globals.copy()
 
-
-def _fixed_find_spec(mod_name):
-    # find_spec has the same annoying behaviour as find_loader did (it
-    # fails to work properly for dotted names), so this is a fixed version
-    # ala pkgutil.get_loader
-    if mod_name.startswith('.'):
-        msg = "Relative module name {!r} not supported".format(mod_name)
-        raise ImportError(msg)
-    path = None
-    pkg_name = mod_name.rpartition(".")[0]
-    if pkg_name:
-        pkg = importlib.import_module(pkg_name)
-        path = getattr(pkg, "__path__", None)
-        if path is None:
-            return None
+# Helper to get the loader, code and filename for a module
+def _get_module_details(mod_name):
     try:
-        return importlib.find_spec(mod_name, path)
+        spec = importlib.util.find_spec(mod_name)
     except (ImportError, AttributeError, TypeError, ValueError) as ex:
         # 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} ({}: {})"
         raise ImportError(msg.format(mod_name, type(ex), ex)) from ex
-
-# Helper to get the loader, code and filename for a module
-def _get_module_details(mod_name):
-    spec = _fixed_find_spec(mod_name)
     if spec is None:
         raise ImportError("No module named %s" % mod_name)
     if spec.submodule_search_locations is not None:
diff --git a/Lib/test/test_importlib/test_api.py b/Lib/test/test_importlib/test_api.py
index c3c19f4..744001b 100644
--- a/Lib/test/test_importlib/test_api.py
+++ b/Lib/test/test_importlib/test_api.py
@@ -4,7 +4,6 @@ frozen_init, source_init = util.import_importlib('importlib')
 frozen_util, source_util = util.import_importlib('importlib.util')
 frozen_machinery, source_machinery = util.import_importlib('importlib.machinery')
 
-from contextlib import contextmanager
 import os.path
 import sys
 from test import support
@@ -13,37 +12,6 @@ import unittest
 import warnings
 
 
-@contextmanager
-def temp_module(name, content='', *, pkg=False):
-    conflicts = [n for n in sys.modules if n.partition('.')[0] == name]
-    with support.temp_cwd(None) as cwd:
-        with util.uncache(name, *conflicts):
-            with support.DirsOnSysPath(cwd):
-                frozen_init.invalidate_caches()
-
-                location = os.path.join(cwd, name)
-                if pkg:
-                    modpath = os.path.join(location, '__init__.py')
-                    os.mkdir(name)
-                else:
-                    modpath = location + '.py'
-                    if content is None:
-                        # Make sure the module file gets created.
-                        content = ''
-                if content is not None:
-                    # not a namespace package
-                    with open(modpath, 'w') as modfile:
-                        modfile.write(content)
-                yield location
-
-
-def submodule(parent, name, pkg_dir, content=''):
-    path = os.path.join(pkg_dir, name + '.py')
-    with open(path, 'w') as subfile:
-        subfile.write(content)
-    return '{}.{}'.format(parent, name), path
-
-
 class ImportModuleTests:
 
     """Test importlib.import_module."""
@@ -210,121 +178,6 @@ class Source_FindLoaderTests(FindLoaderTests, unittest.TestCase):
     init = source_init
 
 
-class FindSpecTests:
-
-    class FakeMetaFinder:
-        @staticmethod
-        def find_spec(name, path=None, target=None): return name, path, target
-
-    def test_sys_modules(self):
-        name = 'some_mod'
-        with util.uncache(name):
-            module = types.ModuleType(name)
-            loader = 'a loader!'
-            spec = self.machinery.ModuleSpec(name, loader)
-            module.__loader__ = loader
-            module.__spec__ = spec
-            sys.modules[name] = module
-            found = self.init.find_spec(name)
-            self.assertEqual(found, spec)
-
-    def test_sys_modules_without___loader__(self):
-        name = 'some_mod'
-        with util.uncache(name):
-            module = types.ModuleType(name)
-            del module.__loader__
-            loader = 'a loader!'
-            spec = self.machinery.ModuleSpec(name, loader)
-            module.__spec__ = spec
-            sys.modules[name] = module
-            found = self.init.find_spec(name)
-            self.assertEqual(found, spec)
-
-    def test_sys_modules_spec_is_None(self):
-        name = 'some_mod'
-        with util.uncache(name):
-            module = types.ModuleType(name)
-            module.__spec__ = None
-            sys.modules[name] = module
-            with self.assertRaises(ValueError):
-                self.init.find_spec(name)
-
-    def test_sys_modules_loader_is_None(self):
-        name = 'some_mod'
-        with util.uncache(name):
-            module = types.ModuleType(name)
-            spec = self.machinery.ModuleSpec(name, None)
-            module.__spec__ = spec
-            sys.modules[name] = module
-            found = self.init.find_spec(name)
-            self.assertEqual(found, spec)
-
-    def test_sys_modules_spec_is_not_set(self):
-        name = 'some_mod'
-        with util.uncache(name):
-            module = types.ModuleType(name)
-            try:
-                del module.__spec__
-            except AttributeError:
-                pass
-            sys.modules[name] = module
-            with self.assertRaises(ValueError):
-                self.init.find_spec(name)
-
-    def test_success(self):
-        name = 'some_mod'
-        with util.uncache(name):
-            with util.import_state(meta_path=[self.FakeMetaFinder]):
-                self.assertEqual((name, None, None),
-                                 self.init.find_spec(name))
-
-    def test_success_path(self):
-        # Searching on a path should work.
-        name = 'some_mod'
-        path = 'path to some place'
-        with util.uncache(name):
-            with util.import_state(meta_path=[self.FakeMetaFinder]):
-                self.assertEqual((name, path, None),
-                                 self.init.find_spec(name, path))
-
-    def test_nothing(self):
-        # None is returned upon failure to find a loader.
-        self.assertIsNone(self.init.find_spec('nevergoingtofindthismodule'))
-
-    def test_find_submodule(self):
-        name = 'spam'
-        subname = 'ham'
-        with temp_module(name, pkg=True) as pkg_dir:
-            fullname, _ = submodule(name, subname, pkg_dir)
-            spec = self.init.find_spec(fullname, [pkg_dir])
-            self.assertIsNot(spec, None)
-            self.assertNotIn(name, sorted(sys.modules))
-            # Ensure successive calls behave the same.
-            spec_again = self.init.find_spec(fullname, [pkg_dir])
-            self.assertEqual(spec_again, spec)
-
-    def test_find_submodule_missing_path(self):
-        name = 'spam'
-        subname = 'ham'
-        with temp_module(name, pkg=True) as pkg_dir:
-            fullname, _ = submodule(name, subname, pkg_dir)
-            spec = self.init.find_spec(fullname)
-            self.assertIs(spec, None)
-            self.assertNotIn(name, sorted(sys.modules))
-            # Ensure successive calls behave the same.
-            spec = self.init.find_spec(fullname)
-            self.assertIs(spec, None)
-
-
-class Frozen_FindSpecTests(FindSpecTests, unittest.TestCase):
-    init = frozen_init
-    machinery = frozen_machinery
-
-class Source_FindSpecTests(FindSpecTests, unittest.TestCase):
-    init = source_init
-    machinery = source_machinery
-
-
 class ReloadTests:
 
     """Test module reloading for builtin and extension modules."""
@@ -484,8 +337,8 @@ class ReloadTests:
         # See #19851.
         name = 'spam'
         subname = 'ham'
-        with temp_module(name, pkg=True) as pkg_dir:
-            fullname, _ = submodule(name, subname, pkg_dir)
+        with util.temp_module(name, pkg=True) as pkg_dir:
+            fullname, _ = util.submodule(name, subname, pkg_dir)
             ham = self.init.import_module(fullname)
             reloaded = self.init.reload(ham)
             self.assertIs(reloaded, ham)
diff --git a/Lib/test/test_importlib/test_util.py b/Lib/test/test_importlib/test_util.py
index b6fe864..b2823c6 100644
--- a/Lib/test/test_importlib/test_util.py
+++ b/Lib/test/test_importlib/test_util.py
@@ -1,5 +1,7 @@
 from importlib import util
 from . import util as test_util
+frozen_init, source_init = test_util.import_importlib('importlib')
+frozen_machinery, source_machinery = test_util.import_importlib('importlib.machinery')
 frozen_util, source_util = test_util.import_importlib('importlib.util')
 
 import os
@@ -310,6 +312,151 @@ Frozen_ResolveNameTests, Source_ResolveNameTests = test_util.test_both(
         util=[frozen_util, source_util])
 
 
+class FindSpecTests:
+
+    class FakeMetaFinder:
+        @staticmethod
+        def find_spec(name, path=None, target=None): return name, path, target
+
+    def test_sys_modules(self):
+        name = 'some_mod'
+        with test_util.uncache(name):
+            module = types.ModuleType(name)
+            loader = 'a loader!'
+            spec = self.machinery.ModuleSpec(name, loader)
+            module.__loader__ = loader
+            module.__spec__ = spec
+            sys.modules[name] = module
+            found = self.util.find_spec(name)
+            self.assertEqual(found, spec)
+
+    def test_sys_modules_without___loader__(self):
+        name = 'some_mod'
+        with test_util.uncache(name):
+            module = types.ModuleType(name)
+            del module.__loader__
+            loader = 'a loader!'
+            spec = self.machinery.ModuleSpec(name, loader)
+            module.__spec__ = spec
+            sys.modules[name] = module
+            found = self.util.find_spec(name)
+            self.assertEqual(found, spec)
+
+    def test_sys_modules_spec_is_None(self):
+        name = 'some_mod'
+        with test_util.uncache(name):
+            module = types.ModuleType(name)
+            module.__spec__ = None
+            sys.modules[name] = module
+            with self.assertRaises(ValueError):
+                self.util.find_spec(name)
+
+    def test_sys_modules_loader_is_None(self):
+        name = 'some_mod'
+        with test_util.uncache(name):
+            module = types.ModuleType(name)
+            spec = self.machinery.ModuleSpec(name, None)
+            module.__spec__ = spec
+            sys.modules[name] = module
+            found = self.util.find_spec(name)
+            self.assertEqual(found, spec)
+
+    def test_sys_modules_spec_is_not_set(self):
+        name = 'some_mod'
+        with test_util.uncache(name):
+            module = types.ModuleType(name)
+            try:
+                del module.__spec__
+            except AttributeError:
+                pass
+            sys.modules[name] = module
+            with self.assertRaises(ValueError):
+                self.util.find_spec(name)
+
+    def test_success(self):
+        name = 'some_mod'
+        with test_util.uncache(name):
+            with test_util.import_state(meta_path=[self.FakeMetaFinder]):
+                self.assertEqual((name, None, None),
+                                 self.util.find_spec(name))
+
+#    def test_success_path(self):
+#        # Searching on a path should work.
+#        name = 'some_mod'
+#        path = 'path to some place'
+#        with test_util.uncache(name):
+#            with test_util.import_state(meta_path=[self.FakeMetaFinder]):
+#                self.assertEqual((name, path, None),
+#                                 self.util.find_spec(name, path))
+
+    def test_nothing(self):
+        # None is returned upon failure to find a loader.
+        self.assertIsNone(self.util.find_spec('nevergoingtofindthismodule'))
+
+    def test_find_submodule(self):
+        name = 'spam'
+        subname = 'ham'
+        with test_util.temp_module(name, pkg=True) as pkg_dir:
+            fullname, _ = test_util.submodule(name, subname, pkg_dir)
+            spec = self.util.find_spec(fullname)
+            self.assertIsNot(spec, None)
+            self.assertIn(name, sorted(sys.modules))
+            self.assertNotIn(fullname, sorted(sys.modules))
+            # Ensure successive calls behave the same.
+            spec_again = self.util.find_spec(fullname)
+            self.assertEqual(spec_again, spec)
+
+    def test_find_submodule_parent_already_imported(self):
+        name = 'spam'
+        subname = 'ham'
+        with test_util.temp_module(name, pkg=True) as pkg_dir:
+            self.init.import_module(name)
+            fullname, _ = test_util.submodule(name, subname, pkg_dir)
+            spec = self.util.find_spec(fullname)
+            self.assertIsNot(spec, None)
+            self.assertIn(name, sorted(sys.modules))
+            self.assertNotIn(fullname, sorted(sys.modules))
+            # Ensure successive calls behave the same.
+            spec_again = self.util.find_spec(fullname)
+            self.assertEqual(spec_again, spec)
+
+    def test_find_relative_module(self):
+        name = 'spam'
+        subname = 'ham'
+        with test_util.temp_module(name, pkg=True) as pkg_dir:
+            fullname, _ = test_util.submodule(name, subname, pkg_dir)
+            relname = '.' + subname
+            spec = self.util.find_spec(relname, name)
+            self.assertIsNot(spec, None)
+            self.assertIn(name, sorted(sys.modules))
+            self.assertNotIn(fullname, sorted(sys.modules))
+            # Ensure successive calls behave the same.
+            spec_again = self.util.find_spec(fullname)
+            self.assertEqual(spec_again, spec)
+
+    def test_find_relative_module_missing_package(self):
+        name = 'spam'
+        subname = 'ham'
+        with test_util.temp_module(name, pkg=True) as pkg_dir:
+            fullname, _ = test_util.submodule(name, subname, pkg_dir)
+            relname = '.' + subname
+            with self.assertRaises(ValueError):
+                self.util.find_spec(relname)
+            self.assertNotIn(name, sorted(sys.modules))
+            self.assertNotIn(fullname, sorted(sys.modules))
+
+
+class Frozen_FindSpecTests(FindSpecTests, unittest.TestCase):
+    init = frozen_init
+    machinery = frozen_machinery
+    util = frozen_util
+
+class Source_FindSpecTests(FindSpecTests, unittest.TestCase):
+    init = source_init
+    machinery = source_machinery
+    util = source_util
+
+
 class MagicNumberTests:
 
     def test_length(self):
diff --git a/Lib/test/test_importlib/util.py b/Lib/test/test_importlib/util.py
index a0dee6e..885cec3 100644
--- a/Lib/test/test_importlib/util.py
+++ b/Lib/test/test_importlib/util.py
@@ -1,5 +1,5 @@
 from contextlib import contextmanager
-from importlib import util
+from importlib import util, invalidate_caches
 import os.path
 from test import support
 import unittest
@@ -46,6 +46,13 @@ def case_insensitive_tests(test):
                             "requires a case-insensitive filesystem")(test)
 
 
+def submodule(parent, name, pkg_dir, content=''):
+    path = os.path.join(pkg_dir, name + '.py')
+    with open(path, 'w') as subfile:
+        subfile.write(content)
+    return '{}.{}'.format(parent, name), path
+
+
 @contextmanager
 def uncache(*names):
     """Uncache a module from sys.modules.
@@ -71,6 +78,31 @@ def uncache(*names):
             except KeyError:
                 pass
 
+
+@contextmanager
+def temp_module(name, content='', *, pkg=False):
+    conflicts = [n for n in sys.modules if n.partition('.')[0] == name]
+    with support.temp_cwd(None) as cwd:
+        with uncache(name, *conflicts):
+            with support.DirsOnSysPath(cwd):
+                invalidate_caches()
+
+                location = os.path.join(cwd, name)
+                if pkg:
+                    modpath = os.path.join(location, '__init__.py')
+                    os.mkdir(name)
+                else:
+                    modpath = location + '.py'
+                    if content is None:
+                        # Make sure the module file gets created.
+                        content = ''
+                if content is not None:
+                    # not a namespace package
+                    with open(modpath, 'w') as modfile:
+                        modfile.write(content)
+                yield location
+
+
 @contextmanager
 def import_state(**kwargs):
     """Context manager to manage the various importers and stored state in the