requirement: Fix --skip-editable flag (#499)

* requirement: Fix `--skip-editable` flag * CHANGELOG: record changes Signed-off-by: William Woodruff <[email protected]> Signed-off-by: William Woodruff <[email protected]> Co-authored-by: William Woodruff <[email protected]>
pypa · Jan 25, 2023 · a64cb8b · a64cb8b
1 parent 41dab8c
commit a64cb8b
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 37 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,10 @@ All versions prior to 0.0.9 are untracked.
 * Fixed an issue where hash checking would fail when using third-party indices
   ([#462](https://github.com/pypa/pip-audit/pull/462))
 
+* Fixed the behavior of the `--skip-editable` flag, which had regressed
+  with an internal API change
+  ([#499](https://github.com/pypa/pip-audit/pull/499))
+
 ## [2.4.14]
 
 ### Fixed

diff --git a/pip_audit/_cli.py b/pip_audit/_cli.py
@@ -416,11 +416,10 @@ def audit() -> None:  # pragma: no cover
             # within the RequirementSource instead of in-line here.
             source = RequirementSource(
                 req_files,
-                ResolveLibResolver(
-                    index_urls, args.timeout, args.cache_dir, args.skip_editable, state
-                ),
+                ResolveLibResolver(index_urls, args.timeout, args.cache_dir, state),
                 require_hashes=args.require_hashes,
                 no_deps=args.no_deps,
+                skip_editable=args.skip_editable,
                 state=state,
             )
         elif args.project_path is not None:
@@ -430,9 +429,7 @@ def audit() -> None:  # pragma: no cover
             # Determine which kind of project file exists in the project path
             source = _dep_source_from_project_path(
                 args.project_path,
-                ResolveLibResolver(
-                    index_urls, args.timeout, args.cache_dir, args.skip_editable, state
-                ),
+                ResolveLibResolver(index_urls, args.timeout, args.cache_dir, state),
                 state,
             )
         else:

diff --git a/pip_audit/_dependency_source/requirement.py b/pip_audit/_dependency_source/requirement.py
@@ -48,6 +48,7 @@ def __init__(
         *,
         require_hashes: bool = False,
         no_deps: bool = False,
+        skip_editable: bool = False,
         state: AuditState = AuditState(),
     ) -> None:
         """
@@ -64,12 +65,16 @@ def __init__(
         dependency resolution is not performed and the inputs are checked
         and treated as "frozen".
 
+        `skip_editable` controls whether requirements marked as "editable" are skipped.
+        By default, editable requirements are not skipped.
+
         `state` is an `AuditState` to use for state callbacks.
         """
         self._filenames = filenames
         self._resolver = resolver
         self._require_hashes = require_hashes
         self._no_deps = no_deps
+        self._skip_editable = skip_editable
         self.state = state
         self._dep_cache: dict[Path, dict[Requirement, set[Dependency]]] = {}
 
@@ -105,6 +110,10 @@ def collect(self) -> Iterator[Dependency]:
                             "please specify them with #egg=your_package_name==your_package_version",
                         )
                         continue
+                    if self._skip_editable and req.is_editable:
+                        yield SkippedDependency(
+                            name=req.name, skip_reason="requirement marked as editable"
+                        )
                     if req.marker is None or req.marker.evaluate():
                         # This means we have a duplicate requirement for the same package
                         if req.name in req_names:

diff --git a/pip_audit/_dependency_source/resolvelib/resolvelib.py b/pip_audit/_dependency_source/resolvelib/resolvelib.py
@@ -6,10 +6,8 @@
 
 import logging
 from pathlib import Path
-from typing import Union
 
-from packaging.requirements import Requirement as _Requirement
-from pip_api import Requirement as ParsedRequirement
+from packaging.requirements import Requirement
 from requests.exceptions import HTTPError
 from resolvelib import BaseReporter, Resolver
 from resolvelib.resolvers import ResolutionImpossible
@@ -32,10 +30,6 @@
 PYPI_URL = "https://pypi.org/simple/"
 
 
-# TODO: Replace with _Requirement | ParsedRequirement once our minimum is 3.10.
-Requirement = Union[_Requirement, ParsedRequirement]
-
-
 class ResolveLibResolver(DependencyResolver):
     """
     An implementation of `DependencyResolver` that uses `resolvelib` as its
@@ -47,7 +41,6 @@ def __init__(
         index_urls: list[str] = [PYPI_URL],
         timeout: int | None = None,
         cache_dir: Path | None = None,
-        skip_editable: bool = False,
         state: AuditState = AuditState(),
     ) -> None:
         """
@@ -56,9 +49,6 @@ def __init__(
         `timeout` and `cache_dir` are optional arguments for HTTP timeouts
         and caching, respectively.
 
-        `skip_editable` controls whether requirements marked as "editable" are skipped.
-        By default, editable requirements are not skipped.
-
         `state` is an `AuditState` to use for state callbacks.
         """
         self.index_urls = index_urls
@@ -68,7 +58,6 @@ def __init__(
         self.session = caching_session(cache_dir, use_pip=True)
         self.state = state
         self.reporter = BaseReporter()
-        self._skip_editable = skip_editable
 
     def resolve(
         self,
@@ -79,15 +68,6 @@ def resolve(
         Resolve the given `Requirement` into a `Dependency` list.
         """
 
-        # HACK: `resolve` takes both `packaging.Requirement` and `pip_api.Requirement`,
-        # since the latter is a subclass. But only the latter knows whether the
-        # requirement is editable, so we need to check for it here.
-        if isinstance(req, ParsedRequirement):
-            if req.editable and self._skip_editable:
-                return [
-                    SkippedDependency(name=req.name, skip_reason="requirement marked as editable")
-                ]
-
         provider = PyPIProvider(self.index_urls, req_hashes, self.session, self.timeout, self.state)
         resolver: Resolver = Resolver(provider, self.reporter)
 

diff --git a/test/dependency_source/resolvelib/test_resolvelib.py b/test/dependency_source/resolvelib/test_resolvelib.py
@@ -7,7 +7,6 @@
 import requests
 from packaging.requirements import Requirement
 from packaging.version import Version
-from pip_api import Requirement as ParsedRequirement
 from requests.exceptions import HTTPError
 
 from pip_audit._dependency_source import RequirementHashes, resolvelib
@@ -499,15 +498,6 @@ def get_multiple_index_package_mock(url):
     assert resolved_deps[req] == [ResolvedDependency("flask", Version("0.5"))]
 
 
-def test_resolvelib_skip_editable():
-    resolver = resolvelib.ResolveLibResolver(skip_editable=True)
-    req = ParsedRequirement("foo==1.0.0", editable=True, filename="stub", lineno=1)
-
-    deps = resolver.resolve(req, RequirementHashes())  # type: ignore
-    assert len(deps) == 1
-    assert deps[0] == SkippedDependency(name="foo", skip_reason="requirement marked as editable")
-
-
 def test_resolvelib_no_links(monkeypatch):
     # Simulate the project page containing no versions
     data = ""

diff --git a/test/dependency_source/test_requirement.py b/test/dependency_source/test_requirement.py
@@ -169,6 +169,20 @@ def test_requirement_source_non_editable_without_egg_fragment(monkeypatch):
     )
 
 
+@pytest.mark.online
+def test_requirement_source_editable_skip(monkeypatch):
+    source = requirement.RequirementSource(
+        [Path("requirements1.txt")], ResolveLibResolver(), skip_editable=True
+    )
+
+    monkeypatch.setattr(
+        pip_requirements_parser, "get_file_content", lambda _: "-e file:flask.py#egg=flask==2.0.1"
+    )
+
+    specs = list(source.collect())
+    assert SkippedDependency(name="flask", skip_reason="requirement marked as editable") in specs
+
+
 def _check_fixes(
     input_reqs: list[str],
     expected_reqs: list[str],