Simplify DeclaredDependency and fix typing{-,_}extensions

fawltydeps/check.py

@@ -3,12 +3,13 @@
 import logging
 import sys
 from itertools import groupby
-from typing import List, Optional, Tuple
+from typing import Dict, Iterable, List, Optional, Tuple
 
 from fawltydeps.settings import Settings
 from fawltydeps.types import (
     DeclaredDependency,
     DependenciesMapping,
+    Package,
     ParsedImport,
     UndeclaredDependency,
     UnusedDependency,
@@ -26,116 +27,106 @@
 
 
 class LocalPackageLookup:
-    """Lookup of import names exposed by local packages."""
+    """Lookup import names exposed by packages installed in the current venv."""
 
     def __init__(self) -> None:
-        """Collect packages distribution mapping
-
-        Packages names are changed to lower case for
-        coherent comparison with declared dependencies."""
-
-        self.import_name_to_package_mapping = {
-            k: [vv.lower() for vv in v] for k, v in packages_distributions().items()
-        }  # Called only _once_
-
-    def lookup_package(self, package: str) -> Optional[Tuple[str, ...]]:
-        """Convert a package name to installed import names.
-
-        (Although this function generally works with _all_ packages, we will apply
-        it only to the subset that is the dependencies of the current project.)
+        """Collect packages installed in the current python environment.
 
         Use importlib.metadata to look up the mapping between packages and their
-        provided import names, and return the import names associated with the given
-        package/distribution name in the current Python environment. This obviously
-        depends on which Python environment (e.g. virtualenv) we're calling from.
-
-        Return None if we're unable to find any import names for the given package.
-        This is typically because the package is missing from the current
-        environment, or because it fails to declare its importable modules.
+        provided import names. This obviously depends on the Python environment
+        (e.g. virtualenv) that we're calling from.
+        """
+        # We call packages_distributions() only _once here, and build a cache of
+        # Package objects from the information extracted.
+        self.packages: Dict[str, Package] = {}
+        for import_name, package_names in packages_distributions().items():
+            for package_name in package_names:
+                package = self.packages.setdefault(
+                    Package.normalize_name(package_name),
+                    Package(package_name),
+                )
+                package.add_import_names(
+                    import_name, mapping=DependenciesMapping.LOCAL_ENV
+                )
+
+    def lookup_package(self, package_name: str) -> Optional[Package]:
+        """Convert a package name to a locally available Package object.
+
+        (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 the Package object that encapsulates the package-name-to-import-
+        names mapping for the given package name.
+
+        Return None if we're unable to find any import names for the given
+        package name. This is typically because the package is missing from the
+        current environment, or because we fail to determine its provided import
+        names.
         """
-        ret = [
-            import_name
-            for import_name, packages in self.import_name_to_package_mapping.items()
-            if package.lower() in packages
-        ]
+        return self.packages.get(Package.normalize_name(package_name))
 
-        return tuple(ret) or None
 
+def resolve_dependencies(dep_names: Iterable[str]) -> Dict[str, Package]:
+    """Associate dependencies with corresponding Package objects.
 
-def dependency_to_imports_mapping(
-    dependency: DeclaredDependency, local_package_lookup: LocalPackageLookup
-) -> DeclaredDependency:
-    """For a single `DeclaredDependency` map the dependency name
+    Use LocalPackageLookup to find Package objects for each of the given
+    dependencies. 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).
 
-    to imports names exposed by a dependency.
-    Create a new `DeclaredDependency` object and with updated
-    names of imports and mapping type used.
+    Return a dict mapping dependency names to the resolved Package objects.
     """
-    import_names = local_package_lookup.lookup_package(dependency.name)
-    return (
-        dependency.replace_mapping(
-            import_names, DependenciesMapping.DEPENDENCY_TO_IMPORT
-        )
-        if import_names
-        # Fallback to IDENTITY mapping
-        else dependency
-    )
-
-
-def map_dependencies_to_imports(
-    dependencies: List[DeclaredDependency],
-) -> List[DeclaredDependency]:
-    """Map dependencies names to list of imports names exposed by a package"""
-
-    local_package_lookup = LocalPackageLookup()
-
-    return [
-        dependency_to_imports_mapping(d, local_package_lookup) for d in dependencies
-    ]
+    ret = {}
+    local_packages = LocalPackageLookup()
+    for name in dep_names:
+        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)
+            ret[name] = package
+    return ret
 
 
 def compare_imports_to_dependencies(
     imports: List[ParsedImport],
     dependencies: List[DeclaredDependency],
     settings: Settings,
-) -> Tuple[List[UndeclaredDependency], List[UnusedDependency]]:
-    """
-    Compares imports to dependencies
+) -> Tuple[Dict[str, Package], List[UndeclaredDependency], List[UnusedDependency]]:
+    """Compares imports to dependencies.
 
     Returns set of undeclared imports and set of unused dependencies.
     For undeclared dependencies returns files and line numbers
     where they were imported in the code.
     """
 
     # TODO consider empty list of dependency to import
-    mapped_dependencies = map_dependencies_to_imports(dependencies)
+    packages = resolve_dependencies(dep.name for dep in dependencies)
 
-    names_from_imports = {i.name for i in imports}
-    names_from_dependencies = {
-        d for dep in mapped_dependencies for d in dep.import_names
-    }
+    imported_names = {i.name for i in imports}
+    declared_names = {name for p in packages.values() for name in p.import_names}
 
     undeclared = [
         i
         for i in imports
-        if i.name not in names_from_dependencies.union(settings.ignore_undeclared)
+        if i.name not in declared_names.union(settings.ignore_undeclared)
     ]
     undeclared.sort(key=lambda i: i.name)  # groupby requires pre-sorting
     undeclared_grouped = [
-        UndeclaredDependency(name, list(imports))
+        UndeclaredDependency(name, [i.source for i in imports])
         for name, imports in groupby(undeclared, key=lambda i: i.name)
     ]
 
     unused = [
         dep
-        for dep in mapped_dependencies
+        for dep in dependencies
         if (dep.name not in settings.ignore_unused)
-        and len(set(dep.import_names) & names_from_imports) == 0
+        and not packages[dep.name].is_used(imported_names)
     ]
-    unused.sort(key=lambda d: d.name)  # groupby requires pre-sorting
+    unused.sort(key=lambda dep: dep.name)  # groupby requires pre-sorting
     unused_grouped = [
-        UnusedDependency(name, list(deps))
+        UnusedDependency(name, [dep.source for dep in deps])
         for name, deps in groupby(unused, key=lambda d: d.name)
     ]
 
-    return undeclared_grouped, unused_grouped
+    return packages, undeclared_grouped, unused_grouped

fawltydeps/main.py

@@ -17,7 +17,7 @@
 from functools import partial
 from operator import attrgetter
 from pathlib import Path
-from typing import List, Optional, TextIO, no_type_check
+from typing import Dict, List, Optional, TextIO, no_type_check
 
 from pydantic.json import custom_pydantic_encoder  # pylint: disable=no-name-in-module
 
@@ -33,6 +33,7 @@
 )
 from fawltydeps.types import (
     DeclaredDependency,
+    Package,
     ParsedImport,
     UndeclaredDependency,
     UnparseablePathException,
@@ -69,6 +70,7 @@ class Analysis:
     settings: Settings
     imports: Optional[List[ParsedImport]] = None
     declared_deps: Optional[List[DeclaredDependency]] = None
+    resolved_deps: Optional[Dict[str, Package]] = None
     undeclared_deps: Optional[List[UndeclaredDependency]] = None
     unused_deps: Optional[List[UnusedDependency]] = None
     version: str = version()
@@ -108,7 +110,11 @@ def create(cls, settings: Settings) -> "Analysis":
         if ret.is_enabled(Action.REPORT_UNDECLARED, Action.REPORT_UNUSED):
             assert ret.imports is not None  # convince Mypy that these cannot
             assert ret.declared_deps is not None  # be None at this time.
-            ret.undeclared_deps, ret.unused_deps = compare_imports_to_dependencies(
+            (
+                ret.resolved_deps,
+                ret.undeclared_deps,
+                ret.unused_deps,
+            ) = compare_imports_to_dependencies(
                 imports=ret.imports,
                 dependencies=ret.declared_deps,
                 settings=settings,

fawltydeps/types.py

@@ -5,7 +5,7 @@
 from enum import Enum
 from functools import total_ordering
 from pathlib import Path
-from typing import Any, Dict, List, Optional, Tuple, Union
+from typing import Any, Dict, Iterable, List, Optional, Set, Tuple, Union
 
 from fawltydeps.utils import hide_dataclass_fields
 
@@ -26,13 +26,6 @@ def __init__(self, ctx: str, path: Path):
         self.msg = f"{ctx}: {path}"
 
 
-class DependenciesMapping(Enum):
-    """Types of dependency and imports mapping"""
-
-    IDENTITY = "IDENTITY"
-    DEPENDENCY_TO_IMPORT = "DEPENDENCY_TO_IMPORT"
-
-
 @total_ordering
 @dataclass(frozen=True)
 class Location:
@@ -129,31 +122,91 @@ class DeclaredDependency:
 
     name: str
     source: Location
-    import_names: Tuple[str, ...] = ()
-    mapping: DependenciesMapping = DependenciesMapping.IDENTITY
+
+
+class DependenciesMapping(str, Enum):
+    """Types of dependency and imports mapping"""
+
+    IDENTITY = "identity"
+    LOCAL_ENV = "local_env"
+
+
+@dataclass
+class Package:
+    """Encapsulate an installable Python package.
+
+    This encapsulates the mapping between a package name (i.e. something you can
+    pass to `pip install`) and the import names that it provides once it is
+    installed.
+    """
+
+    package_name: str
+    mappings: Dict[DependenciesMapping, Set[str]] = field(default_factory=dict)
+    import_names: Set[str] = field(default_factory=set)
 
     def __post_init__(self) -> None:
-        """Set an identity mapping by default"""
-        if self.mapping == DependenciesMapping.IDENTITY:
-            if self.import_names not in {(), (self.name,)}:
-                raise ValueError(
-                    "Don't pass custom import_names with IDENTITY mapping!"
-                )
-            object.__setattr__(self, "import_names", (self.name,))
+        # The .import_names member is entirely redundant, as it can always be
+        # calculated from a union of self.mappings.values(). However, it is
+        # still used often enough (.is_used() is called once per declared
+        # dependency) that it makes sense to pre-calculate it, and rather hide
+        # the redundancy from our JSON output
+        self.import_names = {name for names in self.mappings.values() for name in names}
+        hide_dataclass_fields(self, "import_names")
+
+    @staticmethod
+    def normalize_name(package_name: str) -> str:
+        """Perform standard normalization of package names.
+
+        Verbatim package names are not always appropriate to use in various
+        contexts: For example, a package can be installed using one spelling
+        (e.g. typing-extensions), but once installed, it is presented in the
+        context of the local environment with a slightly different spelling
+        (e.g. typing_extension).
+        """
+        return package_name.lower().replace("-", "_")
+
+    def add_import_names(
+        self, *import_names: str, mapping: DependenciesMapping
+    ) -> 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.
+        """
+        self.mappings.setdefault(mapping, set()).update(import_names)
+        self.import_names.update(import_names)
+
+    def add_identity_import(self) -> None:
+        """Add identity mapping to this package.
+
+        This builds on an assumption that a package 'foo' installed with e.g.
+        `pip install foo`, will also provide an import name 'foo'. This
+        assumption does not always hold, but sometimes we don't have much else
+        to go on...
+        """
+        self.add_import_names(
+            self.normalize_name(self.package_name),
+            mapping=DependenciesMapping.IDENTITY,
+        )
+
+    @classmethod
+    def identity_mapping(cls, package_name: str) -> "Package":
+        """Factory for conveniently creating identity-mapped package object."""
+        ret = cls(package_name)
+        ret.add_identity_import()
+        return ret
 
-    def replace_mapping(
-        self, import_names: Tuple[str, ...], mapping: DependenciesMapping
-    ) -> "DeclaredDependency":
-        """Supply a custom mapping of dependency to imports"""
-        return replace(self, import_names=import_names, mapping=mapping)
+    def is_used(self, imported_names: Iterable[str]) -> bool:
+        """Return True iff this package is among the given import names."""
+        return bool(self.import_names.intersection(imported_names))
 
 
 @dataclass
 class UndeclaredDependency:
     """Undeclared dependency found by analysis in the 'check' module."""
 
     name: str
-    references: List[ParsedImport]
+    references: List[Location]
 
     def render(self, include_references: bool) -> str:
         """Return a human-readable string representation.
@@ -170,7 +223,7 @@ class UnusedDependency:
     """Unused dependency found by analysis in the 'check' module."""
 
     name: str
-    references: List[DeclaredDependency]
+    references: List[Location]
 
     def render(self, include_references: bool) -> str:
         """Return a human-readable string representation.
@@ -189,7 +242,7 @@ def render_problematic_dependency(
     """Create text representation of the given unused or undeclared dependency."""
     ret = f"{dep.name!r}"
     if context is not None:
-        unique_locations = {ref.source for ref in dep.references}
+        unique_locations = set(dep.references)
         ret += f" {context}:" + "".join(
             f"\n    {loc}" for loc in sorted(unique_locations)
         )