packages: Introduce a list of package resolvers

This does not change current behavior, but does a preliminary
refactoring on the way towards having multiple package-to-import
resolvers as discussed in #256 and associated issues.

Introduce a BasePackageResolver abstract base class to define the
interface for package resolvers, and provide, for now, two subclasses:

- LocalPackageResolver (renamed from LocalPackageLookup)
- IdentityMapping (refactored from resolve_dependencies())

Also refactor resolved_dependencies() to operate on a list of
BasePackageResolver objects: For each dependency name we go through the
list of resolvers until a resolver returns a Package object for this
name. We then add this Package to the returned mapping.

For now this list of resolvers is hardcoded to exactly two entries, the
LocalPackageResolver we were already using, and the IdentityMapping that
we were effectively falling back on for packages not found in the former.

Ideas for future work that build on this:
- Introduce more/different BasePackageResolver subclasses to do
  different kinds of package resolving. E.g. user-defined mapping from a
  config file. Resolving by looking up packages remotely on PyPI, etc.
- Make the list of resolvers user-configurable instead of hardcoded.
- Properly handle the case where none of the resolver find a package
  mapping, i.e. _unmapped_ depenedencies.

Author: jherland

fawltydeps/packages.py

@@ -5,6 +5,7 @@
 import sys
 import tempfile
 import venv
+from abc import ABC, abstractmethod
 from contextlib import contextmanager, nullcontext
 from dataclasses import dataclass, field
 from enum import Enum
@@ -105,7 +106,22 @@ def is_used(self, imported_names: Iterable[str]) -> bool:
         return bool(self.import_names.intersection(imported_names))
 
 
-class LocalPackageLookup:
+class BasePackageResolver(ABC):
+    """Define the interface for doing package -> import names lookup."""
+
+    @abstractmethod
+    def lookup_package(self, package_name: str) -> Optional[Package]:
+        """Convert a package name into a Package object with available imports.
+
+        Return a Package object that encapsulates the package-name-to-import-
+        names mapping for the given package name.
+
+        Return None if this PackageResolver is unable to resolve the package.
+        """
+        raise NotImplementedError
+
+
+class LocalPackageResolver(BasePackageResolver):
     """Lookup imports exposed by packages installed in a Python environment."""
 
     def __init__(self, pyenv_path: Optional[Path] = None) -> None:
@@ -201,10 +217,10 @@ def lookup_package(self, package_name: str) -> Optional[Package]:
 @contextmanager
 def temp_installed_requirements(
     requirements: List[str],
-) -> Iterator[LocalPackageLookup]:
+) -> Iterator[LocalPackageResolver]:
     """Install packages into a temporary venv and provide lookup.
 
-    Provide a `LocalPackageLookup` object into the caller's context which
+    Provide a `LocalPackageResolver` object into the caller's context which
     provide dependency-to-imports mapping for the given requirements.
     The requirements are downloaded and installed into a temporary virtualenv,
     which is automatically deleted at the end of this context.
@@ -217,7 +233,24 @@ def temp_installed_requirements(
             check=True,  # fail if any of the commands fail
         )
         (venv_dir / ".installed").touch()
-        yield LocalPackageLookup(venv_dir)
+        yield LocalPackageResolver(venv_dir)
+
+
+class IdentityMapping(BasePackageResolver):
+    """An imperfect package resolver that assumes package name == import name.
+
+    This will resolve _any_ package name into a corresponding identical import
+    name (modulo normalization, see Package.normalize_name() for details).
+    """
+
+    def lookup_package(self, package_name: str) -> Optional[Package]:
+        """Convert a package name into a Package with the same import name."""
+        ret = Package.identity_mapping(package_name)
+        logger.info(
+            f"Could not find {package_name!r} in the current environment. "
+            f"Assuming it can be imported as {', '.join(sorted(ret.import_names))}"
+        )
+        return ret
 
 
 def resolve_dependencies(
@@ -227,19 +260,13 @@ def resolve_dependencies(
 ) -> Dict[str, Package]:
     """Associate dependencies with corresponding Package objects.
 
-    Use LocalPackageLookup to find Package objects for each of the given
+    Use LocalPackageResolver to find Package objects for each of the given
     dependencies inside the virtualenv given by 'pyenv_path'. When 'pyenv_path'
     is None (the default), look for packages in the current Python environment
     (i.e. equivalent to sys.path).
 
-    For dependencies that cannot be found with LocalPackageLookup,
-    fabricate an identity mapping (a pseudo-package making available an import
-    of the same name as the package, modulo normalization).
-
     Return a dict mapping dependency names to the resolved Package objects.
     """
-    ret = {}
-
     # consume the iterable once
     deps = list(dep_names)
 
@@ -251,17 +278,26 @@ def resolve_dependencies(
             )
         local_packages_context = temp_installed_requirements(deps)
     else:
-        local_packages_context = nullcontext(LocalPackageLookup(pyenv_path))  # type: ignore
+        local_packages_context = nullcontext(LocalPackageResolver(pyenv_path))  # type: ignore
 
     with local_packages_context as local_packages:
+        # This defines the "stack" of resolvers that we will use to convert
+        # dependencies into provided import names. We call .lookup_package() on
+        # each resolver in order until one of them returns a Package object. At
+        # that point we are happy, and don't consult any of the later resolvers.
+        resolvers = [
+            local_packages,
+            IdentityMapping(),
+        ]
+        ret = {}
+
         for name in deps:
             if name not in ret:
-                package = local_packages.lookup_package(name)
-                if package is None:  # fall back to identity mapping
-                    package = Package.identity_mapping(name)
-                    logger.info(
-                        f"Could not find {name!r} in the current environment. Assuming "
-                        f"it can be imported as {', '.join(sorted(package.import_names))}"
-                    )
-                ret[name] = package
-    return ret
+                for resolver in resolvers:
+                    package = resolver.lookup_package(name)
+                    if package is None:  # skip to next resolver
+                        continue
+                    ret[name] = package
+                    break  # skip to next dependency name
+
+        return ret

tests/test_local_env.py

@@ -6,7 +6,7 @@
 
 from fawltydeps.packages import (
     DependenciesMapping,
-    LocalPackageLookup,
+    LocalPackageResolver,
     Package,
     resolve_dependencies,
 )
@@ -31,7 +31,7 @@ def test_determine_package_dir__various_paths_in_venv(tmp_path, subdir):
     venv.create(tmp_path, with_pip=False)
     path = tmp_path / subdir
     expect = tmp_path / f"lib/python{major}.{minor}/site-packages"
-    assert LocalPackageLookup.determine_package_dir(path) == expect
+    assert LocalPackageResolver.determine_package_dir(path) == expect
 
 
 @pytest.mark.parametrize(
@@ -49,18 +49,18 @@ def test_determine_package_dir__various_paths_in_poetry2nix_env(
     )
     path = tmp_path / subdir
     expect = tmp_path / f"lib/python{major}.{minor}/site-packages"
-    assert LocalPackageLookup.determine_package_dir(path) == expect
+    assert LocalPackageResolver.determine_package_dir(path) == expect
 
 
 def test_local_env__empty_venv__has_no_packages(tmp_path):
     venv.create(tmp_path, with_pip=False)
-    lpl = LocalPackageLookup(tmp_path)
+    lpl = LocalPackageResolver(tmp_path)
     assert lpl.packages == {}
 
 
 def test_local_env__default_venv__contains_pip_and_setuptools(tmp_path):
     venv.create(tmp_path, with_pip=True)
-    lpl = LocalPackageLookup(tmp_path)
+    lpl = LocalPackageResolver(tmp_path)
     # We cannot do a direct comparison, as different Python/pip/setuptools
     # versions differ in exactly which packages are provided. The following
     # is a subset that we can expect across all of our supported versions.
@@ -78,7 +78,7 @@ def test_local_env__default_venv__contains_pip_and_setuptools(tmp_path):
 
 
 def test_local_env__current_venv__contains_our_test_dependencies():
-    lpl = LocalPackageLookup()
+    lpl = LocalPackageResolver()
     expect_package_names = [
         # Present in ~all venvs:
         "pip",

tests/test_packages.py

@@ -6,7 +6,7 @@
 
 from fawltydeps.packages import (
     DependenciesMapping,
-    LocalPackageLookup,
+    LocalPackageResolver,
     Package,
     resolve_dependencies,
 )
@@ -184,7 +184,7 @@ def test_package__both_mappings():
     ],
 )
 def test_LocalPackageLookup_lookup_package(dep_name, expect_import_names):
-    lpl = LocalPackageLookup()
+    lpl = LocalPackageResolver()
     actual = lpl.lookup_package(dep_name)
     if expect_import_names is None:
         assert actual is None

tests/test_real_projects.py

@@ -19,7 +19,7 @@
 import pytest
 from pkg_resources import Requirement
 
-from fawltydeps.packages import LocalPackageLookup
+from fawltydeps.packages import LocalPackageResolver
 from fawltydeps.types import TomlData
 
 from .project_helpers import BaseExperiment, BaseProject, JsonData, parse_toml
@@ -36,7 +36,7 @@
 
 
 def verify_requirements(venv_path: Path, requirements: List[str]) -> None:
-    lpl = LocalPackageLookup(venv_path)
+    lpl = LocalPackageResolver(venv_path)
     for req in requirements:
         if "python_version" in req:  # we don't know how to parse these (yet)
             continue  # skip checking this requirement

tests/utils.py

@@ -6,7 +6,7 @@
 from pathlib import Path
 from typing import Any, Dict, Iterable, List, Optional, Tuple
 
-from fawltydeps.packages import DependenciesMapping, LocalPackageLookup, Package
+from fawltydeps.packages import DependenciesMapping, LocalPackageResolver, Package
 from fawltydeps.types import (
     DeclaredDependency,
     Location,
@@ -35,7 +35,7 @@ def collect_dep_names(deps: Iterable[DeclaredDependency]) -> Iterable[str]:
 # - pip (exposes a single import name: pip)
 # - isort (exposes no top_level.txt, but 'isort' import name can be inferred)
 
-local_env = LocalPackageLookup()
+local_env = LocalPackageResolver()
 
 
 def imports_factory(*imports: str) -> List[ParsedImport]: