refactor: replace imp with importlib

invoke/config.py

@@ -2,8 +2,10 @@
 import json
 import os
 import types
+from importlib.util import spec_from_loader
 from os import PathLike
 from os.path import join, splitext, expanduser
+from types import ModuleType
 from typing import Any, Dict, Iterator, Optional, Tuple, Type, Union
 
 from .env import Environment
@@ -24,7 +26,11 @@
 def load_source(name: str, path: str) -> Dict[str, Any]:
     if not os.path.exists(path):
         return {}
-    return vars(SourceFileLoader("mod", path).load_module())
+    loader = SourceFileLoader("mod", path)
+    mod = ModuleType("mod")
+    mod.__spec__ = spec_from_loader("mod", loader)
+    loader.exec_module(mod)
+    return vars(mod)
 
 
 class DataProxy:
@@ -908,8 +914,7 @@ def _load_yaml(self, path: PathLike) -> Any:
         with open(path) as fd:
             return yaml.safe_load(fd)
 
-    def _load_yml(self, path: PathLike) -> Any:
-        return self._load_yaml(path)
+    _load_yml = _load_yaml
 
     def _load_json(self, path: PathLike) -> Any:
         with open(path) as fd:

invoke/loader.py

@@ -1,8 +1,9 @@
 import os
 import sys
-import imp
+from importlib.machinery import ModuleSpec
+from importlib.util import module_from_spec, spec_from_file_location
 from types import ModuleType
-from typing import Any, IO, Optional, Tuple
+from typing import Any, Optional, Tuple
 
 from . import Config
 from .exceptions import CollectionNotFound
@@ -29,14 +30,14 @@ def __init__(self, config: Optional["Config"] = None) -> None:
             config = Config()
         self.config = config
 
-    def find(self, name: str) -> Tuple[IO, str, Tuple[str, str, int]]:
+    def find(self, name: str) -> Optional[ModuleSpec]:
         """
         Implementation-specific finder method seeking collection ``name``.
 
-        Must return a 4-tuple valid for use by `imp.load_module`, which is
+        Must return a ModuleSpec valid for use by `importlib`, which is
         typically a name string followed by the contents of the 3-tuple
-        returned by `imp.find_module` (``file``, ``pathname``,
-        ``description``.)
+        returned by `importlib.module_from_spec` (``name``, ``loader``,
+        ``origin``.)
 
         For a sample implementation, see `.FilesystemLoader`.
 
@@ -65,28 +66,22 @@ def load(self, name: Optional[str] = None) -> Tuple[ModuleType, str]:
         """
         if name is None:
             name = self.config.tasks.collection_name
-        # Find the named tasks module, depending on implementation.
-        # Will raise an exception if not found.
-        fd, path, desc = self.find(name)
-        try:
+        spec = self.find(name)
+        if spec and spec.loader and spec.origin:
+            path = spec.origin
             # Ensure containing directory is on sys.path in case the module
             # being imported is trying to load local-to-it names.
-            parent = os.path.dirname(path)
-            if parent not in sys.path:
-                sys.path.insert(0, parent)
-            # FIXME: deprecated capability that needs replacement
+            if os.path.isfile(spec.origin):
+                path = os.path.dirname(spec.origin)
+            if path not in sys.path:
+                sys.path.insert(0, path)
             # Actual import
-            module = imp.load_module(name, fd, path, desc)  # type: ignore
-            # Return module + path.
-            # TODO: is there a reason we're not simply having clients refer to
-            # os.path.dirname(module.__file__)?
-            return module, parent
-        finally:
-            # Ensure we clean up the opened file object returned by find(), if
-            # there was one (eg found packages, vs modules, don't open any
-            # file.)
-            if fd:
-                fd.close()
+            module = module_from_spec(spec)
+            spec.loader.exec_module(module)
+            return module, os.path.dirname(spec.origin)
+        msg = "ImportError loading {!r}, raising ImportError"
+        debug(msg.format(name))
+        raise ImportError
 
 
 class FilesystemLoader(Loader):
@@ -103,6 +98,8 @@ class FilesystemLoader(Loader):
     # as auto-dashes, and has to grow one of those for every bit Collection
     # ever needs to know
     def __init__(self, start: Optional[str] = None, **kwargs: Any) -> None:
+        # finder = kwargs.pop("finder_class", CollectionFinder)
+        # sys.meta_path.append(finder)
         super().__init__(**kwargs)
         if start is None:
             start = self.config.tasks.search_root
@@ -113,25 +110,35 @@ def start(self) -> str:
         # Lazily determine default CWD if configured value is falsey
         return self._start or os.getcwd()
 
-    def find(self, name: str) -> Tuple[IO, str, Tuple[str, str, int]]:
-        # Accumulate all parent directories
-        start = self.start
-        debug("FilesystemLoader find starting at {!r}".format(start))
-        parents = [os.path.abspath(start)]
-        parents.append(os.path.dirname(parents[-1]))
-        while parents[-1] != parents[-2]:
-            parents.append(os.path.dirname(parents[-1]))
-        # Make sure we haven't got duplicates on the end
-        if parents[-1] == parents[-2]:
-            parents = parents[:-1]
-        # Use find_module with our list of parents. ImportError from
-        # find_module means "couldn't find" not "found and couldn't import" so
-        # we turn it into a more obvious exception class.
+    def find(self, name: str) -> Optional[ModuleSpec]:
+        debug("FilesystemLoader find starting at {!r}".format(self.start))
+        spec = None
+        module = "{}.py".format(name)
+        paths = self.start.split(os.sep)
         try:
-            tup = imp.find_module(name, parents)
-            debug("Found module: {!r}".format(tup[1]))
-            return tup
-        except ImportError:
+            # walk the path upwards to check for dynamic import
+            for x in reversed(range(len(paths) + 1)):
+                path = os.sep.join(paths[0:x])
+                if module in os.listdir(path):
+                    spec = spec_from_file_location(
+                        name, os.path.join(path, module)
+                    )
+                    break
+                elif name in os.listdir(path) and os.path.exists(
+                    os.path.join(path, name, "__init__.py")
+                ):
+                    basepath = os.path.join(path, name)
+                    spec = spec_from_file_location(
+                        name,
+                        os.path.join(basepath, "__init__.py"),
+                        submodule_search_locations=[basepath],
+                    )
+                    break
+            if spec:
+                debug("Found module: {!r}".format(spec))
+                return spec
+        except (FileNotFoundError, ModuleNotFoundError):
             msg = "ImportError loading {!r}, raising CollectionNotFound"
             debug(msg.format(name))
-            raise CollectionNotFound(name=name, start=start)
+            raise CollectionNotFound(name=name, start=self.start)
+        return None

sites/www/changelog.rst

@@ -2,6 +2,8 @@
 Changelog
 =========
 
+- :support:`675` Implement `importlib` and deprecate `imp` module. Patches
+  provided by Jesse P. Johnson
 - :bug:`376` Resolve equality comparison bug for non-collections. Patch via
   Jesse P. Johnson
 - :support:`906` Implement type hints and type checking tests with mypy to

tests/loader.py

@@ -1,7 +1,7 @@
-import imp
 import os
 import sys
-import types
+from importlib.util import spec_from_file_location
+from types import ModuleType
 
 from pytest import raises
 
@@ -21,8 +21,13 @@ class _BasicLoader(Loader):
     """
 
     def find(self, name):
-        self.fd, self.path, self.desc = t = imp.find_module(name, [support])
-        return t
+        path = os.path.join(support, name)
+        if os.path.exists(f"{path}.py"):
+            path = f"{path}.py"
+        elif os.path.exists(path):
+            path = os.path.join(path, "__init__.py")
+        spec = spec_from_file_location(name, path)
+        return spec
 
 
 class Loader_:
@@ -33,7 +38,7 @@ def exhibits_default_config_object(self):
 
     def returns_module_and_location(self):
         mod, path = _BasicLoader().load("namespacing")
-        assert isinstance(mod, types.ModuleType)
+        assert isinstance(mod, ModuleType)
         assert path == support
 
     def may_configure_config_via_constructor(self):
@@ -52,11 +57,6 @@ def doesnt_dupliate_parent_dir_addition(self):
         # other tests will pollute it (!).
         assert sys.path.count(support) == 1
 
-    def closes_opened_file_object(self):
-        loader = _BasicLoader()
-        loader.load("foo")
-        assert loader.fd.closed
-
     def can_load_package(self):
         loader = _BasicLoader()
         # make sure it doesn't explode
@@ -89,7 +89,7 @@ def exposes_start_point_as_attribute(self):
         assert FSLoader().start == os.getcwd()
 
     def start_point_is_configurable_via_kwarg(self):
-        start = "/tmp/"
+        start = "/tmp"
         assert FSLoader(start=start).start == start
 
     def start_point_is_configurable_via_config(self):
@@ -102,13 +102,17 @@ def raises_CollectionNotFound_if_not_found(self):
 
     def raises_ImportError_if_found_collection_cannot_be_imported(self):
         # Instead of masking with a CollectionNotFound
-        with raises(ImportError):
+        with raises(ModuleNotFoundError):
             self.loader.load("oops")
 
+    # TODO: Need CollectionImportError here
+
     def searches_towards_root_of_filesystem(self):
         # Loaded while root is in same dir as .py
         directly = self.loader.load("foo")
         # Loaded while root is multiple dirs deeper than the .py
         deep = os.path.join(support, "ignoreme", "ignoremetoo")
         indirectly = FSLoader(start=deep).load("foo")
-        assert directly == indirectly
+        assert directly[0].__file__ == indirectly[0].__file__
+        assert directly[0].__spec__ == indirectly[0].__spec__
+        assert directly[1] == indirectly[1]