Fix key mismatch between requirements and dists (#325)

CHANGES.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## 2.7.0
+
+- Normalize keys with pep503 regex and recover package names from metadata instead of relying on DistInfoDistribution
+
 ## 2.6.0
 
 - Handle mermaid output for a reversed tree

src/pipdeptree/_models/dag.py

@@ -9,7 +9,7 @@
     from pip._vendor.pkg_resources import DistInfoDistribution
 
 
-from .package import DistPackage, ReqPackage
+from .package import DistPackage, ReqPackage, pep503_normalize
 
 
 class PackageDAG(Mapping[DistPackage, List[ReqPackage]]):
@@ -43,7 +43,8 @@ def from_pkgs(cls, pkgs: list[DistInfoDistribution]) -> PackageDAG:
         for p in dist_pkgs:
             reqs = []
             for r in p.requires():
-                d = idx.get(r.key)
+                # Requirement key is not sufficiently normalized in pkg_resources - apply additional normalization
+                d = idx.get(pep503_normalize(r.key))
                 # pip's _vendor.packaging.requirements.Requirement uses the exact casing of a dependency's name found in
                 # a project's build config, which is not ideal when rendering.
                 # See https://github.com/tox-dev/pipdeptree/issues/242

src/pipdeptree/_models/package.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import re
 from abc import ABC, abstractmethod
 from importlib import import_module
 from importlib.metadata import PackageNotFoundError, metadata, version
@@ -13,17 +14,18 @@
     from pip._vendor.pkg_resources import DistInfoDistribution
 
 
+def pep503_normalize(name: str) -> str:
+    return re.sub("[-_.]+", "-", name)
+
+
 class Package(ABC):
     """Abstract class for wrappers around objects that pip returns."""
 
     UNKNOWN_LICENSE_STR = "(Unknown license)"
 
     def __init__(self, obj: DistInfoDistribution) -> None:
         self._obj: DistInfoDistribution = obj
-
-    @property
-    def key(self) -> str:
-        return self._obj.key  # type: ignore[no-any-return]
+        self.key = pep503_normalize(obj.key)
 
     @property
     def project_name(self) -> str:
@@ -119,10 +121,20 @@ class DistPackage(Package):
     def __init__(self, obj: DistInfoDistribution, req: ReqPackage | None = None) -> None:
         super().__init__(obj)
         self.req = req
+        self._project_name = ""
 
     def requires(self) -> list[Requirement]:
         return self._obj.requires()  # type: ignore[no-untyped-call,no-any-return]
 
+    @property
+    def project_name(self) -> str:
+        if not self._project_name:
+            try:
+                self._project_name = metadata(self.key)["name"]
+            except (PackageNotFoundError, KeyError):
+                self._project_name = self._obj.project_name
+        return self._project_name
+
     @property
     def version(self) -> str:
         return self._obj.version  # type: ignore[no-any-return]

tests/_models/test_dag.py

@@ -25,10 +25,10 @@ def test_package_dag_get_node_as_parent(example_dag: PackageDAG) -> None:
 @pytest.fixture(scope="session")
 def t_fnmatch(mock_pkgs: Callable[[MockGraph], Iterator[Mock]]) -> PackageDAG:
     graph: MockGraph = {
-        ("a.a", "1"): [("a.b", []), ("a.c", [])],
-        ("a.b", "1"): [("a.c", [])],
-        ("b.a", "1"): [("b.b", [])],
-        ("b.b", "1"): [("a.b", [])],
+        ("a-a", "1"): [("a-b", []), ("a-c", [])],
+        ("a-b", "1"): [("a-c", [])],
+        ("b-a", "1"): [("b-b", [])],
+        ("b-b", "1"): [("a-b", [])],
     }
     return PackageDAG.from_pkgs(list(mock_pkgs(graph)))
 
@@ -38,33 +38,33 @@ def dag_to_dict(g: PackageDAG) -> dict[str, list[str]]:
 
 
 def test_package_dag_filter_fnmatch_include_a(t_fnmatch: PackageDAG) -> None:
-    # test include for a.*in the result we got only a.* nodes
-    graph = dag_to_dict(t_fnmatch.filter_nodes(["a.*"], None))
-    assert graph == {"a.a": ["a.b", "a.c"], "a.b": ["a.c"]}
+    # test include for a-*in the result we got only a-* nodes
+    graph = dag_to_dict(t_fnmatch.filter_nodes(["a-*"], None))
+    assert graph == {"a-a": ["a-b", "a-c"], "a-b": ["a-c"]}
 
 
 def test_package_dag_filter_fnmatch_include_b(t_fnmatch: PackageDAG) -> None:
-    # test include for b.*, which has a.b and a.c in tree, but not a.a
-    # in the result we got the b.* nodes plus the a.b node as child in the tree
-    graph = dag_to_dict(t_fnmatch.filter_nodes(["b.*"], None))
-    assert graph == {"b.a": ["b.b"], "b.b": ["a.b"], "a.b": ["a.c"]}
+    # test include for b-*, which has a-b and a-c in tree, but not a-a
+    # in the result we got the b-* nodes plus the a-b node as child in the tree
+    graph = dag_to_dict(t_fnmatch.filter_nodes(["b-*"], None))
+    assert graph == {"b-a": ["b-b"], "b-b": ["a-b"], "a-b": ["a-c"]}
 
 
 def test_package_dag_filter_fnmatch_exclude_c(t_fnmatch: PackageDAG) -> None:
-    # test exclude for b.* in the result we got only a.* nodes
-    graph = dag_to_dict(t_fnmatch.filter_nodes(None, {"b.*"}))
-    assert graph == {"a.a": ["a.b", "a.c"], "a.b": ["a.c"]}
+    # test exclude for b-* in the result we got only a-* nodes
+    graph = dag_to_dict(t_fnmatch.filter_nodes(None, {"b-*"}))
+    assert graph == {"a-a": ["a-b", "a-c"], "a-b": ["a-c"]}
 
 
 def test_package_dag_filter_fnmatch_exclude_a(t_fnmatch: PackageDAG) -> None:
-    # test exclude for a.* in the result we got only b.* nodes
-    graph = dag_to_dict(t_fnmatch.filter_nodes(None, {"a.*"}))
-    assert graph == {"b.a": ["b.b"], "b.b": []}
+    # test exclude for a-* in the result we got only b-* nodes
+    graph = dag_to_dict(t_fnmatch.filter_nodes(None, {"a-*"}))
+    assert graph == {"b-a": ["b-b"], "b-b": []}
 
 
 def test_package_dag_filter_include_exclude_both_used(t_fnmatch: PackageDAG) -> None:
     with pytest.raises(AssertionError):
-        t_fnmatch.filter_nodes(["a.a", "a.b"], {"a.b"})
+        t_fnmatch.filter_nodes(["a-a", "a-b"], {"a-b"})
 
 
 def test_package_dag_filter_nonexistent_packages(t_fnmatch: PackageDAG) -> None:
@@ -104,3 +104,16 @@ def test_package_dag_from_pkgs(mock_pkgs: Callable[[MockGraph], Iterator[Mock]])
     c = package_dag.get_children(parent_key)
     assert len(c) == 1
     assert c[0].project_name == "HelloPy"
+
+
+def test_package_dag_from_pkgs_uses_pep503normalize(mock_pkgs: Callable[[MockGraph], Iterator[Mock]]) -> None:
+    # ensure that requirement gets matched with a dists even when it's key needs pep503 normalization to match
+    graph: dict[tuple[str, str], list[tuple[str, list[tuple[str, str]]]]] = {
+        ("parent-package", "1.2.3"): [("flufl.lock", [(">=", "2.0.0")])],
+        ("flufl-lock", "2.2.0"): [],
+    }
+    package_dag = PackageDAG.from_pkgs(list(mock_pkgs(graph)))
+    parent_key = "parent-package"
+    c = package_dag.get_children(parent_key)
+    assert c[0].dist
+    assert c[0].project_name == "flufl-lock"

tests/_models/test_package.py

@@ -96,20 +96,39 @@ def test_dist_package_as_dict() -> None:
 )
 def test_dist_package_licenses(mocked_metadata: Mock, expected_output: str, monkeypatch: pytest.MonkeyPatch) -> None:
     monkeypatch.setattr("pipdeptree._models.package.metadata", lambda _: mocked_metadata)
-    dist = DistPackage(Mock(project_name="a"))
+    dist = DistPackage(Mock(project_name="a", key="a"))
     licenses_str = dist.licenses()
 
     assert licenses_str == expected_output
 
 
 def test_dist_package_licenses_importlib_cant_find_package(monkeypatch: pytest.MonkeyPatch) -> None:
     monkeypatch.setattr("pipdeptree._models.package.metadata", Mock(side_effect=PackageNotFoundError()))
-    dist = DistPackage(Mock(project_name="a"))
+    dist = DistPackage(Mock(project_name="a", key="a"))
     licenses_str = dist.licenses()
 
     assert licenses_str == Package.UNKNOWN_LICENSE_STR
 
 
+def test_dist_package_project_name_recovered(monkeypatch: pytest.MonkeyPatch) -> None:
+    monkeypatch.setattr("pipdeptree._models.package.metadata", lambda _: {"name": "good"})
+    foo = Mock(key="foo", project_name="bad", version="20.4.1")
+    dp = DistPackage(foo)
+    assert dp.project_name == "good"
+
+
+def test_dist_package_key_pep503_normalized() -> None:
+    foobar = Mock(key="foo.bar", project_name="foo.bar", version="20.4.1")
+    dp = DistPackage(foobar)
+    assert dp.key == "foo-bar"
+
+
+def test_req_package_key_pep503_normalized() -> None:
+    bar_req = Mock(key="bar.bar-bar-bar", project_name="Bar.Bar-Bar_Bar", version="4.1.0", specs=[(">=", "4.0")])
+    rp = ReqPackage(bar_req)
+    assert rp.key == "bar-bar-bar-bar"
+
+
 def test_req_package_render_as_root() -> None:
     bar = Mock(key="bar", project_name="bar", version="4.1.0")
     bar_req = Mock(key="bar", project_name="bar", version="4.1.0", specs=[(">=", "4.0")])