packages: Split SysPathPackageResolver out of LocalPackageResolver

Also introduce an InstalledPackageResolver base class to collect the
common parts shared between LocalPackageResolver and the new
SysPathPackageResolver.

fawltydeps/packages.py

@@ -205,26 +205,109 @@ def lookup_packages(self, package_names: Set[str]) -> Dict[str, Package]:
         }
 
 
-class LocalPackageResolver(BasePackageResolver):
+class InstalledPackageResolver(BasePackageResolver):
     """Lookup imports exposed by packages installed in a Python environment."""
 
-    def __init__(
-        self,
-        srcs: AbstractSet[PyEnvSource] = frozenset(),
-        use_current_env: bool = False,
-    ) -> None:
-        """Lookup packages installed in the given Python environments.
+    def __init__(self) -> None:
+        """Lookup packages installed in some Python environments.
+
+        Uses importlib_metadata to look up the mapping between packages and
+        their provided import names.
+        """
+        # We enumerate packages _once_ and cache the result here:
+        self._packages: Optional[Dict[str, Package]] = None
+
+    def _from_one_env(
+        self, env_paths: List[str]
+    ) -> Iterator[Tuple[CustomMapping, str]]:
+        """Return package-name-to-import-names mapping from one Python env.
+
+        This is roughly equivalent to calling importlib_metadata's
+        packages_distributions(), except that instead of implicitly querying
+        sys.path, we query the given env_paths instead.
+
+        Also, we are able to return packages that map to zero import names,
+        whereas packages_distributions() cannot.
+        """
+        seen = set()  # Package names (normalized) seen earlier in env_paths
+
+        # We're reaching into the internals of importlib_metadata here, which
+        # Mypy is not overly fond of, hence lots of "type: ignore"...
+        context = DistributionFinder.Context(path=env_paths)  # type: ignore
+        for dist in MetadataPathFinder().find_distributions(context):
+            normalized_name = Package.normalize_name(dist.name)
+            parent_dir = dist.locate_file("")
+            if normalized_name in seen:
+                # We already found another instance of this package earlier in
+                # env_paths. Assume that the earlier package is what Python's
+                # import machinery will choose, and that this later package is
+                # not interesting.
+                logger.debug(f"Skip {dist.name} {dist.version} under {parent_dir}")
+                continue
+
+            logger.debug(f"Found {dist.name} {dist.version} under {parent_dir}")
+            seen.add(normalized_name)
+            imports = list(
+                _top_level_declared(dist)  # type: ignore
+                or _top_level_inferred(dist)  # type: ignore
+            )
+            yield {dist.name: imports}, str(parent_dir)
+
+    @property
+    @abstractmethod
+    def packages(self) -> Dict[str, Package]:
+        """Return mapping of package names to Package objects."""
+        raise NotImplementedError
+
+    def lookup_packages(self, package_names: Set[str]) -> Dict[str, Package]:
+        """Convert package names to locally available Package objects.
+
+        (Although this function generally works with _all_ locally available
+        packages, we apply it only to the subset that is the dependencies of
+        the current project.)
+
+        Return a dict mapping package names to the Package objects that
+        encapsulate the package-name-to-import-names mappings.
+
+        Only return dict entries for the packages that we manage to find in the
+        local environment. Omit any packages for which we're unable to determine
+        what imports names they provide. This applies to packages that are
+        missing from the local environment, or packages where we fail to
+        determine its provided import names.
+        """
+        return {
+            name: self.packages[Package.normalize_name(name)]
+            for name in package_names
+            if Package.normalize_name(name) in self.packages
+        }
+
 
-        If 'use_current_env' is enabled, then the current python environment
-        (aka. sys.path) will also be included in the lookup.
+class SysPathPackageResolver(InstalledPackageResolver):
+    """Lookup imports exposed by packages installed in sys.path."""
+
+    @property
+    @calculated_once
+    def packages(self) -> Dict[str, Package]:
+        """Return mapping of package names to Package objects.
+
+        This enumerates the available packages in the current Python environment
+        (aka. sys.path) _once_, and caches the result for the remainder of this
+        object's life.
+        """
+        return accumulate_mappings(self.__class__, self._from_one_env(sys.path))
+
+
+class LocalPackageResolver(InstalledPackageResolver):
+    """Lookup imports packages installed in the given Python environments."""
+
+    def __init__(self, srcs: AbstractSet[PyEnvSource] = frozenset()) -> None:
+        """Lookup packages installed in the given Python environments.
 
         Use importlib_metadata to look up the mapping between packages and their
         provided import names.
         """
+        super().__init__()
         self.package_dirs: Set[Path] = set(src.path for src in srcs)
-        self.use_current_env: bool = use_current_env
-        # We enumerate packages for pyenv_path _once_ and cache the result here:
-        self._packages: Optional[Dict[str, Package]] = None
 
     @classmethod
     def find_package_dirs(cls, path: Path) -> Iterator[Path]:
@@ -270,42 +353,6 @@ def find_package_dirs(cls, path: Path) -> Iterator[Path]:
                     package_dir.relative_to(path)  # ValueError if not relative
                     yield package_dir
 
-    def _from_one_env(
-        self, env_paths: List[str]
-    ) -> Iterator[Tuple[CustomMapping, str]]:
-        """Return package-name-to-import-names mapping from one Python env.
-
-        This is roughly equivalent to calling importlib_metadata's
-        packages_distributions(), except that instead of implicitly querying
-        sys.path, we query env_paths instead.
-
-        Also, we are able to return packages that map to zero import names,
-        whereas packages_distributions() cannot.
-        """
-        seen = set()  # Package names (normalized) seen earlier in env_paths
-
-        # We're reaching into the internals of importlib_metadata here, which
-        # Mypy is not overly fond of, hence lots of "type: ignore"...
-        context = DistributionFinder.Context(path=env_paths)  # type: ignore
-        for dist in MetadataPathFinder().find_distributions(context):
-            normalized_name = Package.normalize_name(dist.name)
-            parent_dir = dist.locate_file("")
-            if normalized_name in seen:
-                # We already found another instance of this package earlier in
-                # env_paths. Assume that the earlier package is what Python's
-                # import machinery will choose, and that this later package is
-                # not interesting.
-                logger.debug(f"Skip {dist.name} {dist.version} under {parent_dir}")
-                continue
-
-            logger.debug(f"Found {dist.name} {dist.version} under {parent_dir}")
-            seen.add(normalized_name)
-            imports = list(
-                _top_level_declared(dist)  # type: ignore
-                or _top_level_inferred(dist)  # type: ignore
-            )
-            yield {dist.name: imports}, str(parent_dir)
-
     @property
     @calculated_once
     def packages(self) -> Dict[str, Package]:
@@ -319,33 +366,9 @@ def packages(self) -> Dict[str, Package]:
         def _pyenvs() -> Iterator[Tuple[CustomMapping, str]]:
             for package_dir in self.package_dirs:
                 yield from self._from_one_env([str(package_dir)])
-            if self.use_current_env:
-                yield from self._from_one_env(sys.path)
 
         return accumulate_mappings(self.__class__, _pyenvs())
 
-    def lookup_packages(self, package_names: Set[str]) -> Dict[str, Package]:
-        """Convert package names to locally available Package objects.
-
-        (Although this function generally works with _all_ locally available
-        packages, we apply it only to the subset that is the dependencies of
-        the current project.)
-
-        Return a dict mapping package names to the Package objects that
-        encapsulate the package-name-to-import-names mappings.
-
-        Only return dict entries for the packages that we manage to find in the
-        local environment. Omit any packages for which we're unable to determine
-        what imports names they provide. This applies to packages that are
-        missing from the local environment, or packages where we fail to
-        determine its provided import names.
-        """
-        return {
-            name: self.packages[Package.normalize_name(name)]
-            for name in package_names
-            if Package.normalize_name(name) in self.packages
-        }
-
 
 def pyenv_sources(*pyenv_paths: Path) -> Set[PyEnvSource]:
     """Helper for converting Python environment paths into PyEnvSources.
@@ -506,10 +529,10 @@ def setup_resolvers(
     )
 
     if pyenv_srcs:
-        yield LocalPackageResolver(pyenv_srcs, False)
+        yield LocalPackageResolver(pyenv_srcs)
 
     if use_current_env:
-        yield LocalPackageResolver(frozenset(), True)
+        yield SysPathPackageResolver()
 
     if install_deps:
         yield TemporaryPipInstallResolver()

tests/test_cmdline.py

@@ -949,7 +949,7 @@ def test_check_json__no_pyenvs_found__falls_back_to_current_env(fake_project):
             "pip-requirements-parser": {
                 "package_name": "pip_requirements_parser",
                 "import_names": ["packaging_legacy_version", "pip_requirements_parser"],
-                "resolved_with": "LocalPackageResolver",
+                "resolved_with": "SysPathPackageResolver",
                 "debug_info": {
                     f"{site_packages}": [
                         "packaging_legacy_version",

tests/test_local_env.py

@@ -10,6 +10,7 @@
     IdentityMapping,
     LocalPackageResolver,
     Package,
+    SysPathPackageResolver,
     pyenv_sources,
     resolve_dependencies,
     setup_resolvers,
@@ -135,7 +136,7 @@ def test_local_env__default_venv__contains_pip(tmp_path):
     assert str(expect_location) in pip.debug_info
 
 
-def test_local_env__current_venv__contains_prepared_packages(isolate_default_resolver):
+def test_sys_path_env__contains_prepared_packages(isolate_default_resolver):
     isolate_default_resolver(
         {
             "pip": {"pip"},
@@ -145,13 +146,13 @@ def test_local_env__current_venv__contains_prepared_packages(isolate_default_res
             "pytest": {"pytest"},
         }
     )
-    lpl = LocalPackageResolver(use_current_env=True)
+    sys_path = SysPathPackageResolver()
     expect_package_names = ["pip", "setuptools", "isort", "pydantic", "pytest"]
     for package_name in expect_package_names:
-        assert package_name in lpl.packages
+        assert package_name in sys_path.packages
 
 
-def test_local_env__prefers_first_package_found_in_sys_path(isolate_default_resolver):
+def test_sys_path_env__prefers_first_package_found(isolate_default_resolver):
     # Add the same package twice, The one that ends up _first_ in sys.path is
     # the one that Python would end up importing, and it is therefore also the
     # one that we should resolve to.
@@ -160,10 +161,10 @@ def test_local_env__prefers_first_package_found_in_sys_path(isolate_default_reso
     site_dir2 = isolate_default_resolver({"other": {"actual"}})
     assert site_dir1 != site_dir2
     assert sys.path[0] == str(site_dir2)
-    actual = LocalPackageResolver(use_current_env=True).lookup_packages({"other"})
+    actual = SysPathPackageResolver().lookup_packages({"other"})
     assert actual == {
         "other": Package(
-            "other", {"actual"}, LocalPackageResolver, {str(site_dir2): {"actual"}}
+            "other", {"actual"}, SysPathPackageResolver, {str(site_dir2): {"actual"}}
         ),
     }
 
@@ -272,19 +273,16 @@ def test_resolve_dependencies__when_no_env_found__fallback_to_current():
     # enables the use_current_env flag to setup_resolvers() in this case.
     resolvers = list(setup_resolvers(use_current_env=True))
 
-    # The resulting resolvers should include a single LocalPackageResolver whose
-    # .package_dirs is empty and .use_current_env is True.
-    local_resolvers = [r for r in resolvers if isinstance(r, LocalPackageResolver)]
-    assert len(local_resolvers) == 1
-    lpr = local_resolvers[0]
-    assert lpr.package_dirs == set()
-    assert lpr.use_current_env is True
+    # The resulting resolvers should include a single SysPathPackageResolver.
+    syspath_resolvers = [r for r in resolvers if isinstance(r, SysPathPackageResolver)]
+    assert len(syspath_resolvers) == 1
+    spr = syspath_resolvers[0]
 
     # The only thing we can assume about the _current_ env (in which FD runs)
-    # is that "fawltydeps" is installed (hence resolved via our 'lpr'), and that
-    # "other_module" is not installed (and thus resolved with id mapping).
+    # is that "fawltydeps" is installed (hence resolved via our 'spr'), and that
+    # "other_module" is not installed (and thus resolved with IdentityMapping).
     actual = resolve_dependencies(["fawltydeps", "other_module"], resolvers)
     assert actual == {
-        "fawltydeps": lpr.lookup_packages({"fawltydeps"})["fawltydeps"],
+        "fawltydeps": spr.lookup_packages({"fawltydeps"})["fawltydeps"],
         "other_module": Package("other_module", {"other_module"}, IdentityMapping),
     }

tests/test_packages.py

@@ -9,6 +9,7 @@
     IdentityMapping,
     LocalPackageResolver,
     Package,
+    SysPathPackageResolver,
     UserDefinedMapping,
     resolve_dependencies,
     setup_resolvers,
@@ -278,12 +279,12 @@ def test_user_defined_mapping__no_input__returns_empty_mapping():
         ),
     ],
 )
-def test_LocalPackageResolver_lookup_packages(
+def test_SysPathPackageResolver_lookup_packages(
     isolate_default_resolver, dep_name, expect_import_names
 ):
     isolate_default_resolver(default_sys_path_env_for_tests)
-    lpl = LocalPackageResolver(use_current_env=True)
-    actual = lpl.lookup_packages({dep_name})
+    sys_path = SysPathPackageResolver()
+    actual = sys_path.lookup_packages({dep_name})
     if expect_import_names is None:
         assert actual == {}
     else:
@@ -307,7 +308,7 @@ def test_resolve_dependencies__informs_once_when_id_mapping_is_used(
     dep_names = ["some-foo", "pip", "some-foo"]
     isolate_default_resolver(default_sys_path_env_for_tests)
     expect = {
-        "pip": Package("pip", {"pip"}, LocalPackageResolver),
+        "pip": Package("pip", {"pip"}, SysPathPackageResolver),
         "some-foo": Package("some-foo", {"some_foo"}, IdentityMapping),
     }
     expect_log = [

tests/test_resolver.py

@@ -10,8 +10,8 @@
 
 from fawltydeps.packages import (
     IdentityMapping,
-    LocalPackageResolver,
     Package,
+    SysPathPackageResolver,
     TemporaryPipInstallResolver,
     UserDefinedMapping,
     resolve_dependencies,
@@ -86,7 +86,7 @@ def generate_expected_resolved_deps(
     ret = {}
     ret.update(
         {
-            dep: Package(dep, imports, LocalPackageResolver)
+            dep: Package(dep, imports, SysPathPackageResolver)
             for dep, imports in locally_installed_deps.items()
         }
     )

tests/utils.py

@@ -10,7 +10,7 @@
 from typing import Any, Dict, Iterable, Iterator, List, Optional, Tuple, Union
 
 from fawltydeps.main import main
-from fawltydeps.packages import IdentityMapping, LocalPackageResolver, Package
+from fawltydeps.packages import IdentityMapping, Package, SysPathPackageResolver
 from fawltydeps.types import (
     DeclaredDependency,
     Location,
@@ -79,7 +79,7 @@ def resolved_factory(*deps: str) -> Dict[str, Package]:
     def make_package(dep: str) -> Package:
         imports = default_sys_path_env_for_tests.get(dep, None)
         if imports is not None:  # exists in local env
-            return Package(dep, imports, LocalPackageResolver)
+            return Package(dep, imports, SysPathPackageResolver)
         # fall back to identity mapping
         return Package(dep, {dep}, IdentityMapping)
 
@@ -285,8 +285,8 @@ class FDTestVector:  # pylint: disable=too-many-instance-attributes
             DeclaredDependency(name="pip", source=Location(Path("requirements2.txt"))),
         ],
         expect_resolved_deps={
-            "Pip": Package("pip", {"pip"}, LocalPackageResolver),
-            "pip": Package("pip", {"pip"}, LocalPackageResolver),
+            "Pip": Package("pip", {"pip"}, SysPathPackageResolver),
+            "pip": Package("pip", {"pip"}, SysPathPackageResolver),
         },
         expect_unused_deps=[
             UnusedDependency("Pip", [Location(Path("requirements1.txt"))]),