Repository: tox-dev/pipdeptree (python)

Issue:
Success code returned on missing packages
I was totally confused by output of pipdeptree when I tried to call it with `pipdeptree -p ansiblelint`, as it listed not deps and returned as success.

It was my mistake to mention module name instead of correct packaga name `ansible-lint` but clearly returning 0 is a bug, as a missing package should clearly return an error.

Pull request:
Exit with error when `-p` given patterns that fail to match
Should resolve #127.

Note that the `include` parameter in `filter_nodes()` was turned into a list so that when printing an error they will appear in order (at the cost of allowing duplicate patterns to be added). I don't see a performance cost by doing this since previously we seem to just iterate `include` rather than do any lookups.

When the -w fail option and the --packages option is given, patterns that fail to match with any package names will printed to stderr and return with exit code 1.

-w suppress and -w silence work as attended.

Patch:
diff --git a/src/pipdeptree/__main__.py b/src/pipdeptree/__main__.py
index c581f3d..4268f8e 100644
--- a/src/pipdeptree/__main__.py
+++ b/src/pipdeptree/__main__.py
@@ -27,11 +27,17 @@ def main(args: Sequence[str] | None = None) -> None | int:
     if options.reverse:
         tree = tree.reverse()
 
-    show_only = set(options.packages.split(",")) if options.packages else None
+    show_only = options.packages.split(",") if options.packages else None
     exclude = set(options.exclude.split(",")) if options.exclude else None
 
     if show_only is not None or exclude is not None:
-        tree = tree.filter_nodes(show_only, exclude)
+        try:
+            tree = tree.filter_nodes(show_only, exclude)
+        except ValueError as e:
+            if options.warn in ("suppress", "fail"):
+                print(e, file=sys.stderr)  # noqa: T201
+                return_code |= 1 if options.warn == "fail" else 0
+            return return_code
 
     render(options, tree)
 
diff --git a/src/pipdeptree/_models/dag.py b/src/pipdeptree/_models/dag.py
index 48fbbdb..dd38600 100644
--- a/src/pipdeptree/_models/dag.py
+++ b/src/pipdeptree/_models/dag.py
@@ -91,14 +91,15 @@ class PackageDAG(Mapping[DistPackage, List[ReqPackage]]):
         node = self.get_node_as_parent(node_key)
         return self._obj[node] if node else []
 
-    def filter_nodes(self, include: set[str] | None, exclude: set[str] | None) -> PackageDAG:  # noqa: C901
+    def filter_nodes(self, include: list[str] | None, exclude: set[str] | None) -> PackageDAG:  # noqa: C901, PLR0912
         """
         Filters nodes in a graph by given parameters.
 
         If a node is included, then all it's children are also included.
 
-        :param include: set of node keys to include (or None)
+        :param include: list of node keys to include (or None)
         :param exclude: set of node keys to exclude (or None)
+        :raises ValueError: If include has node keys that do not exist in the graph
         :returns: filtered version of the graph
         """
         # If neither of the filters are specified, short circuit
@@ -110,25 +111,40 @@ class PackageDAG(Mapping[DistPackage, List[ReqPackage]]):
         # documentation, `key` is simply
         # `project_name.lower()`. Refer:
         # https://setuptools.readthedocs.io/en/latest/pkg_resources.html#distribution-objects
+        include_with_casing_preserved: list[str] = []
         if include:
-            include = {s.lower() for s in include}
+            include_with_casing_preserved = include
+            include = [s.lower() for s in include]
         exclude = {s.lower() for s in exclude} if exclude else set()
 
         # Check for mutual exclusion of show_only and exclude sets
         # after normalizing the values to lowercase
         if include and exclude:
-            assert not (include & exclude)
+            assert not (set(include) & exclude)
 
         # Traverse the graph in a depth first manner and filter the
         # nodes according to `show_only` and `exclude` sets
         stack: deque[DistPackage] = deque()
         m: dict[DistPackage, list[ReqPackage]] = {}
         seen = set()
+        matched_includes: set[str] = set()
         for node in self._obj:
             if any(fnmatch(node.key, e) for e in exclude):
                 continue
-            if include is None or any(fnmatch(node.key, i) for i in include):
+            if include is None:
                 stack.append(node)
+            else:
+                should_append = False
+                for i in include:
+                    if fnmatch(node.key, i):
+                        # Add all patterns that match with the node key. Otherwise if we break, patterns like py* or
+                        # pytest* (which both should match "pytest") may cause one pattern to be missed and will
+                        # raise an error
+                        matched_includes.add(i)
+                        should_append = True
+                if should_append:
+                    stack.append(node)
+
             while stack:
                 n = stack.pop()
                 cldn = [c for c in self._obj[n] if not any(fnmatch(c.key, e) for e in exclude)]
@@ -144,6 +160,10 @@ class PackageDAG(Mapping[DistPackage, List[ReqPackage]]):
                             # a dependency is missing
                             continue
 
+        non_existent_includes = [i for i in include_with_casing_preserved if i.lower() not in matched_includes]
+        if non_existent_includes:
+            raise ValueError("No packages matched using the following patterns: " + ", ".join(non_existent_includes))
+
         return self.__class__(m)
 
     def reverse(self) -> ReversedPackageDAG:


Test patch:
diff --git a/tests/_models/test_dag.py b/tests/_models/test_dag.py
index 5e16698..d7e1c8a 100644
--- a/tests/_models/test_dag.py
+++ b/tests/_models/test_dag.py
@@ -39,14 +39,14 @@ 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))
+    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))
+    graph = dag_to_dict(t_fnmatch.filter_nodes(["b.*"], None))
     assert graph == {"b.a": ["b.b"], "b.b": ["a.b"], "a.b": ["a.c"]}
 
 
@@ -62,6 +62,16 @@ def test_package_dag_filter_fnmatch_exclude_a(t_fnmatch: PackageDAG) -> None:
     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"})
+
+
+def test_package_dag_filter_nonexistent_packages(t_fnmatch: PackageDAG) -> None:
+    with pytest.raises(ValueError, match="No packages matched using the following patterns: x, y, z"):
+        t_fnmatch.filter_nodes(["x", "y", "z"], None)
+
+
 def test_package_dag_reverse(example_dag: PackageDAG) -> None:
     def sort_map_values(m: dict[str, Any]) -> dict[str, Any]:
         return {k: sorted(v) for k, v in m.items()}
diff --git a/tests/render/test_text.py b/tests/render/test_text.py
index e136bbe..767c8a5 100644
--- a/tests/render/test_text.py
+++ b/tests/render/test_text.py
@@ -525,7 +525,7 @@ def test_render_text_list_all_and_packages_options_used(
     package_dag = PackageDAG.from_pkgs(list(mock_pkgs(graph)))
 
     # NOTE: Mimicking the --packages option being used here.
-    package_dag = package_dag.filter_nodes({"examplePy"}, None)
+    package_dag = package_dag.filter_nodes(["examplePy"], None)
 
     render_text(package_dag, max_depth=float("inf"), encoding="utf-8", list_all=True, frozen=False)
     captured = capsys.readouterr()