packages: Replace DependenciesMapping with string descriptions

The only purpose of the DependenciesMapping enum is to differentiate the
various types of mappings stored inside a Package object, e.g. allowing
a Package object like this to be created:

  Package(
    "foo",
    {
      DependenciesMapping.LOCAL_ENV: {"bar", "baz"},
      DependenciesMapping.IDENTITY: {"foo"},
    }
  )

However, the way we are currently structuring our resolver stack, we
will never mix mappings from _different_ resolvers inside a single
Package object.

What we _do_ see, however, are Package objects coming from a single
resolver that nonetheless contain mappings that ultimately come from
different sources. This could be:

 - User-defined mappings spread across multiple files, where a single
   package gets imports contributed from different files.
 - Soon, we will be accepting multiple --pyenv options which could also
   allow a single package to appear in multiple environments, with
   different imports being contributed from different environments.

It would be very useful for debugging if the Package.mappings dict
allowed for more specific information about the source of a given
mapping.

By converting the keys in this dict from DependenciesMapping enum values
into free-form text strings, we can allow for this extra information,
consider for example the following Package objects in the future:

  Package(
    "foo",
    {
      "User-defined mapping from mappings.toml": {"foo"},
      "User-defined mapping from pyproject.toml": {"bar", "baz"},
    }
  )

or

  Package(
    "foo",
    {
      "Python env at .venv/lib/python3.7/site-packages": {"foo", "bar"},
      "Python env at __pypackages__/3.10/lib": {"foo", "baz"},
    }
  )

This commit removes the DependenciesMapping enum, and uses text strings
for the various types of mappings instead:

 - UserDefinedMapping: USER_DEFINED -> "User-defined mapping"
   (to be made more precise about the mapping source in a future commit)
 - LocalPackageResolver: LOCAL_ENV -> "Python env at {site_packages}"
   ("{site_packages}" is replaced by the dir containing the package)
 - TemporaryPipInstallResolver: LOCAL_ENV -> "Temporarily pip-installed"
 - IdentityMapping: IDENTITY -> "Identity mapping"

Author: jherland

fawltydeps/packages.py

@@ -8,7 +8,6 @@
 from abc import ABC, abstractmethod
 from contextlib import contextmanager
 from dataclasses import dataclass, field
-from enum import Enum
 from itertools import chain
 from pathlib import Path
 from typing import Dict, Iterable, Iterator, List, Optional, Set
@@ -37,14 +36,6 @@
 logger = logging.getLogger(__name__)
 
 
-class DependenciesMapping(str, Enum):
-    """Types of dependency and imports mapping"""
-
-    IDENTITY = "identity"
-    LOCAL_ENV = "local_env"
-    USER_DEFINED = "user_defined"
-
-
 @dataclass
 class Package:
     """Encapsulate an installable Python package.
@@ -55,7 +46,7 @@ class Package:
     """
 
     package_name: str
-    mappings: Dict[DependenciesMapping, Set[str]] = field(default_factory=dict)
+    mappings: Dict[str, Set[str]] = field(default_factory=dict)
     import_names: Set[str] = field(default_factory=set)
 
     def __post_init__(self) -> None:
@@ -79,15 +70,14 @@ def normalize_name(package_name: str) -> str:
         """
         return package_name.lower().replace("-", "_")
 
-    def add_import_names(
-        self, *import_names: str, mapping: DependenciesMapping
-    ) -> None:
+    def add_import_names(self, *import_names: str, description: str) -> None:
         """Add import names provided by this package.
 
-        Import names must be associated with a DependenciesMapping enum value,
-        as keeping track of this is extremely helpful when debugging.
+        Import names must be associated with a description that provides some
+        details about its source (keeping track of this is extremely helpful
+        when debugging).
         """
-        self.mappings.setdefault(mapping, set()).update(import_names)
+        self.mappings.setdefault(description, set()).update(import_names)
         self.import_names.update(import_names)
 
     def is_used(self, imported_names: Iterable[str]) -> bool:
@@ -177,7 +167,7 @@ def _custom_mappings() -> Iterator[CustomMapping]:
         return {
             name: Package(
                 name,
-                {DependenciesMapping.USER_DEFINED: set(imports)},
+                {"User-defined mapping": set(imports)},
             )
             for name, imports in custom_mapping.items()
         }
@@ -194,7 +184,11 @@ def lookup_packages(self, package_names: Set[str]) -> Dict[str, Package]:
 class LocalPackageResolver(BasePackageResolver):
     """Lookup imports exposed by packages installed in a Python environment."""
 
-    def __init__(self, pyenv_path: Optional[Path] = None) -> None:
+    def __init__(
+        self,
+        pyenv_path: Optional[Path] = None,
+        description_template: str = "Python env at {path}",
+    ) -> None:
         """Lookup packages installed in the given virtualenv.
 
         Default to the current python environment if `pyenv_path` is not given
@@ -203,6 +197,7 @@ def __init__(self, pyenv_path: Optional[Path] = None) -> None:
         Use importlib_metadata to look up the mapping between packages and their
         provided import names.
         """
+        self.description_template = description_template
         if pyenv_path is not None:
             self.pyenv_path = self.determine_package_dir(pyenv_path)
             if self.pyenv_path is None:
@@ -272,7 +267,8 @@ def packages(self) -> Dict[str, Package]:
                 _top_level_declared(dist)  # type: ignore
                 or _top_level_inferred(dist)  # type: ignore
             )
-            package = Package(dist.name, {DependenciesMapping.LOCAL_ENV: imports})
+            description = self.description_template.format(path=parent_dir)
+            package = Package(dist.name, {description: imports})
             ret[Package.normalize_name(dist.name)] = package
 
         return ret
@@ -338,7 +334,8 @@ def lookup_packages(self, package_names: Set[str]) -> Dict[str, Package]:
         """
         logger.info("Installing dependencies into a new temporary Python environment.")
         with self.temp_installed_requirements(sorted(package_names)) as venv_dir:
-            local_resolver = LocalPackageResolver(venv_dir)
+            description_template = "Temporarily pip-installed"
+            local_resolver = LocalPackageResolver(venv_dir, description_template)
             return local_resolver.lookup_packages(package_names)
 
 
@@ -354,7 +351,7 @@ def lookup_package(package_name: str) -> Package:
         """Convert a package name into a Package with the same import name."""
         ret = Package(package_name)
         import_name = Package.normalize_name(package_name)
-        ret.add_import_names(import_name, mapping=DependenciesMapping.IDENTITY)
+        ret.add_import_names(import_name, description="Identity mapping")
         logger.info(
             f"{package_name!r} was not resolved. "
             f"Assuming it can be imported as {import_name!r}."

tests/test_cmdline.py

@@ -546,7 +546,7 @@ def test_check_json__simple_project__can_report_both_undeclared_and_unused(
         "resolved_deps": {
             "pandas": {
                 "package_name": "pandas",
-                "mappings": {"identity": ["pandas"]},
+                "mappings": {"Identity mapping": ["pandas"]},
             }
         },
         "undeclared_deps": [

tests/test_local_env.py

@@ -5,7 +5,6 @@
 import pytest
 
 from fawltydeps.packages import (
-    DependenciesMapping,
     IdentityMapping,
     LocalPackageResolver,
     Package,
@@ -96,13 +95,14 @@ def test_local_env__default_venv__contains_pip_and_setuptools(tmp_path):
         "pip": {"pip"},
         "setuptools": {"setuptools", "pkg_resources"},
     }
+    package_dir = tmp_path / f"lib/python{major}.{minor}/site-packages"
     for package_name, import_names in expect.items():
         assert package_name in lpl.packages
         p = lpl.packages[package_name]
         assert package_name == p.package_name
         assert len(p.mappings) == 1
-        assert DependenciesMapping.LOCAL_ENV in p.mappings
-        assert import_names.issubset(p.mappings[DependenciesMapping.LOCAL_ENV])
+        assert f"Python env at {package_dir}" in p.mappings
+        assert import_names.issubset(p.mappings[f"Python env at {package_dir}"])
 
 
 def test_local_env__current_venv__contains_prepared_packages(isolate_default_resolver):
@@ -129,7 +129,7 @@ def test_resolve_dependencies__in_empty_venv__reverts_to_id_mapping(tmp_path):
 
 
 def test_resolve_dependencies__in_fake_venv__returns_local_and_id_deps(fake_venv):
-    venv_dir, _ = fake_venv(
+    venv_dir, site_packages = fake_venv(
         {
             "pip": {"pip"},
             "setuptools": {"setuptools", "pkg_resources"},
@@ -138,9 +138,9 @@ def test_resolve_dependencies__in_fake_venv__returns_local_and_id_deps(fake_venv
     )
     actual = resolve_dependencies(["PIP", "pandas", "empty-pkg"], pyenv_path=venv_dir)
     assert actual == {
-        "PIP": Package("pip", {DependenciesMapping.LOCAL_ENV: {"pip"}}),
-        "pandas": Package("pandas", {DependenciesMapping.IDENTITY: {"pandas"}}),
-        "empty-pkg": Package("empty_pkg", {DependenciesMapping.LOCAL_ENV: set()}),
+        "PIP": Package("pip", {f"Python env at {site_packages}": {"pip"}}),
+        "pandas": Package("pandas", {"Identity mapping": {"pandas"}}),
+        "empty-pkg": Package("empty_pkg", {f"Python env at {site_packages}": set()}),
     }
 
 
@@ -155,6 +155,6 @@ def test_on_installed_venv__returns_local_deps(request, monkeypatch):
         ["leftpadx", "click"], pyenv_path=None, install_deps=True
     )
     assert actual == {
-        "leftpadx": Package("leftpadx", {DependenciesMapping.LOCAL_ENV: {"leftpad"}}),
-        "click": Package("click", {DependenciesMapping.LOCAL_ENV: {"click"}}),
+        "leftpadx": Package("leftpadx", {"Temporarily pip-installed": {"leftpad"}}),
+        "click": Package("click", {"Temporarily pip-installed": {"click"}}),
     }

tests/test_packages.py

@@ -6,7 +6,6 @@
 import pytest
 
 from fawltydeps.packages import (
-    DependenciesMapping,
     IdentityMapping,
     LocalPackageResolver,
     Package,
@@ -15,7 +14,12 @@
 )
 from fawltydeps.types import UnparseablePathException
 
-from .utils import SAMPLE_PROJECTS_DIR, default_sys_path_env_for_tests, test_vectors
+from .utils import (
+    SAMPLE_PROJECTS_DIR,
+    default_sys_path_env_for_tests,
+    expand_package_mappings_placeholders,
+    test_vectors,
+)
 
 
 def test_package__empty_package__matches_nothing():
@@ -121,7 +125,7 @@ def test_package__local_env_mapping(
     package_name, import_names, matching_imports, non_matching_imports
 ):
     p = Package(package_name)
-    p.add_import_names(*import_names, mapping=DependenciesMapping.LOCAL_ENV)
+    p.add_import_names(*import_names, description="Python env at /some/path")
     assert p.package_name == package_name  # package name is not normalized
     assert p.is_used(matching_imports)
     assert not p.is_used(non_matching_imports)
@@ -131,7 +135,7 @@ def test_package__both_mappings():
     id_mapping = IdentityMapping()
     p = id_mapping.lookup_package("FooBar")
     import_names = ["foo", "bar", "baz"]
-    p.add_import_names(*import_names, mapping=DependenciesMapping.LOCAL_ENV)
+    p.add_import_names(*import_names, description="Python env at /some/path")
     assert p.package_name == "FooBar"  # package name is not normalized
     assert p.is_used(["foobar"])  # but identity-mapped import name _is_.
     assert p.is_used(["foo"])
@@ -140,8 +144,8 @@ def test_package__both_mappings():
     assert not p.is_used(["fooba"])
     assert not p.is_used(["foobarbaz"])
     assert p.mappings == {
-        DependenciesMapping.IDENTITY: {"foobar"},
-        DependenciesMapping.LOCAL_ENV: {"foo", "bar", "baz"},
+        "Identity mapping": {"foobar"},
+        "Python env at /some/path": {"foo", "bar", "baz"},
     }
     assert p.import_names == {"foobar", "foo", "bar", "baz"}
 
@@ -280,19 +284,26 @@ def test_LocalPackageResolver_lookup_packages(
 @pytest.mark.parametrize("vector", [pytest.param(v, id=v.id) for v in test_vectors])
 def test_resolve_dependencies(vector, isolate_default_resolver):
     dep_names = [dd.name for dd in vector.declared_deps]
-    isolate_default_resolver(default_sys_path_env_for_tests)
-    assert resolve_dependencies(dep_names) == vector.expect_resolved_deps
+    site_packages = isolate_default_resolver(default_sys_path_env_for_tests)
+    expected = expand_package_mappings_placeholders(
+        vector.expect_resolved_deps,
+        site_packages=site_packages,
+    )
+    assert resolve_dependencies(dep_names) == expected
 
 
 def test_resolve_dependencies__informs_once_when_id_mapping_is_used(
     caplog, isolate_default_resolver
 ):
     dep_names = ["some-foo", "pip", "some-foo"]
-    isolate_default_resolver(default_sys_path_env_for_tests)
-    expect = {
-        "pip": Package("pip", {DependenciesMapping.LOCAL_ENV: {"pip"}}),
-        "some-foo": Package("some-foo", {DependenciesMapping.IDENTITY: {"some_foo"}}),
-    }
+    site_packages = isolate_default_resolver(default_sys_path_env_for_tests)
+    expect = expand_package_mappings_placeholders(
+        {
+            "pip": Package("pip", {"Python env at {site_packages}": {"pip"}}),
+            "some-foo": Package("some-foo", {"Identity mapping": {"some_foo"}}),
+        },
+        site_packages=site_packages,
+    )
     expect_log = [
         (
             "fawltydeps.packages",

tests/test_resolver.py

@@ -6,14 +6,9 @@
 from hypothesis import HealthCheck, given, settings
 from hypothesis import strategies as st
 
-from fawltydeps.packages import (
-    DependenciesMapping,
-    Package,
-    UserDefinedMapping,
-    resolve_dependencies,
-)
+from fawltydeps.packages import Package, UserDefinedMapping, resolve_dependencies
 
-from .utils import default_sys_path_env_for_tests
+from .utils import default_sys_path_env_for_tests, expand_package_mappings_placeholders
 
 # The deps in each category should be disjoint
 other_deps = ["pandas", "numpy", "other"]
@@ -69,7 +64,7 @@ def generate_expected_resolved_deps(
     user_defined_deps: Optional[List[str]] = None,
     user_mapping_file: Optional[Path] = None,
     user_mapping_from_config: Optional[Dict[str, List[str]]] = None,
-):
+) -> Dict[str, Package]:
     """
     Returns a dict of resolved packages.
 
@@ -81,10 +76,7 @@ def generate_expected_resolved_deps(
     if locally_installed_deps:
         ret.update(
             {
-                dep: Package(
-                    Package.normalize_name(dep),
-                    {DependenciesMapping.LOCAL_ENV: set(imports)},
-                )
+                dep: Package(dep, {"Python env at {site_packages}": set(imports)})
                 for dep, imports in locally_installed_deps.items()
             }
         )
@@ -97,10 +89,7 @@ def generate_expected_resolved_deps(
         ret.update(resolved_packages)
     if other_deps:
         ret.update(
-            {
-                dep: Package(dep, {DependenciesMapping.IDENTITY: {dep}})
-                for dep in other_deps
-            }
+            {dep: Package(dep, {"Identity mapping": {dep}}) for dep in other_deps}
         )
     return ret
 
@@ -157,7 +146,11 @@ def test_resolve_dependencies__generates_expected_mappings(
         other_deps=other_deps,
     )
 
-    isolate_default_resolver(installed_deps)
+    site_packages = isolate_default_resolver(installed_deps)
+    expected = expand_package_mappings_placeholders(
+        expected,
+        site_packages=site_packages,
+    )
     obtained = resolve_dependencies(
         dep_names,
         custom_mapping_files=set([custom_mapping_file])

tests/utils.py

@@ -3,12 +3,12 @@
 import io
 import logging
 import subprocess
-from dataclasses import dataclass, field
+from dataclasses import dataclass, field, replace
 from pathlib import Path
 from typing import Any, Dict, Iterable, List, Optional, Tuple
 
 from fawltydeps.main import main
-from fawltydeps.packages import DependenciesMapping, Package
+from fawltydeps.packages import Package
 from fawltydeps.types import (
     DeclaredDependency,
     Location,
@@ -54,13 +54,26 @@ 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, {DependenciesMapping.LOCAL_ENV: imports})
+            return Package(dep, {"Python env at {site_packages}": imports})
         # fall back to identity mapping
-        return Package(dep, {DependenciesMapping.IDENTITY: {dep}})
+        return Package(dep, {"Identity mapping": {dep}})
 
     return {dep: make_package(dep) for dep in deps}
 
 
+def expand_package_mappings_placeholders(
+    resolved_deps: Dict[str, Package], **kwargs
+) -> Dict[str, Package]:
+    def expand_one(package: Package) -> Package:
+        mappings = {
+            description.format(**kwargs): imports
+            for description, imports in package.mappings.items()
+        }
+        return replace(package, mappings=mappings)
+
+    return {name: expand_one(package) for name, package in resolved_deps.items()}
+
+
 def undeclared_factory(*deps: str) -> List[UndeclaredDependency]:
     return [UndeclaredDependency(dep, [Location("<stdin>")]) for dep in deps]
 
@@ -251,8 +264,8 @@ class FDTestVector:  # pylint: disable=too-many-instance-attributes
             DeclaredDependency(name="pip", source=Location(Path("requirements2.txt"))),
         ],
         expect_resolved_deps={
-            "Pip": Package("pip", {DependenciesMapping.LOCAL_ENV: {"pip"}}),
-            "pip": Package("pip", {DependenciesMapping.LOCAL_ENV: {"pip"}}),
+            "Pip": Package("pip", {"Python env at {site_packages}": {"pip"}}),
+            "pip": Package("pip", {"Python env at {site_packages}": {"pip"}}),
         },
         expect_unused_deps=[
             UnusedDependency("Pip", [Location(Path("requirements1.txt"))]),