refactor(core): split a lockfile-supplied version out of PackageSource::Inherited

README.md

@@ -416,10 +416,20 @@ such: `version_inherited` for a Cargo `version.workspace = true`, `inherited` fo
 constraint taken from `[workspace.dependencies]`, and `lockfile` for the lockfile that
 supplied the locked versions — a workspace keeps one at its root, above its members.
 
-A dependency's `source` is `registry`, `jsr`, `git`, `local` (a `path` entry), or
-`inherited` — a Cargo `dep.workspace = true`, whose version is declared once at the
-workspace root. An inherited dependency is checked wherever it is used and rewritten
-only where it is declared; see [Monorepos and workspaces](#monorepos-and-workspaces).
+A dependency's `source` is today `registry`, `jsr`, `git`, `local` (a `path` entry),
+`inherited`, or `locked`, plus `unknown` for anything this list does not name. That
+list is open, not closed: a new ecosystem may add a token to it within
+`dependable.list/v1`, which pins the document's *shape* — which fields exist and what
+type each holds — and not the token sets inside those fields. Match the ones you care
+about and let the rest fall through to a default; `unknown` is why exhaustive matching
+on `source` was never safe. `inherited` means the version is declared elsewhere in a
+manifest — a Cargo `dep.workspace = true` resolved against the workspace root, a
+Gradle `[versions]` alias, a shared Maven `<properties>` value — and such a dependency
+is checked wherever it is used and rewritten only where it is declared; see
+[Monorepos and workspaces](#monorepos-and-workspaces). `locked` means the version came
+from a lockfile and no manifest declares it at all, which today is a SwiftPM
+`Package.resolved` pin: it is checked and scanned like any other, but there is no
+declaration anywhere to point at or to rewrite, so `inherited` stays `false` for it.
 
 `license` appears only with `--licenses`, which — together with `--features` — is
 the one thing in `list` that touches the network: a license is published by the

crates/dependable-core/src/item.rs

@@ -47,11 +47,16 @@ impl Item {
     /// nothing to ask a registry for; a check reports such an item as
     /// [`Undetermined`](crate::result::DependencyStatus::Undetermined) rather than
     /// claiming it has no registry.
+    ///
+    /// A [`Locked`](PackageSource::Locked) item is read the same way and for the same
+    /// reason: the version is real and worth asking a registry about, it simply was
+    /// not written here. A lockfile entry that recorded no version at all — a branch
+    /// pin — states nothing to check either.
     #[must_use]
     pub fn is_checkable(&self) -> bool {
         match self.source {
             PackageSource::Registry | PackageSource::Jsr => true,
-            PackageSource::Inherited => !self.version_constraint.is_empty(),
+            PackageSource::Inherited | PackageSource::Locked => !self.version_constraint.is_empty(),
             _ => false,
         }
     }
@@ -62,14 +67,25 @@ impl Item {
     /// Since `0` is a legal line and column index, an unrecorded span is indistinguishable
     /// from a real one by value; it has to be inferred from the source instead. Every
     /// parser that declines to record a span also gives the item a source nothing would
-    /// fetch, so [`is_checkable`](Self::is_checkable) covers all of them but one: a
+    /// fetch, so [`is_checkable`](Self::is_checkable) covers all of them but two: a
     /// resolved [`Inherited`](PackageSource::Inherited) item is worth checking and still
     /// has no home here, because the version string it was resolved from belongs to
     /// another entry — a workspace root's table, a catalog `[versions]` alias, a shared
-    /// POM `<properties>` value.
+    /// POM `<properties>` value — and a [`Locked`](PackageSource::Locked) item is worth
+    /// checking with no manifest entry anywhere to belong to.
+    ///
+    /// Both are named here explicitly rather than inferred from anything about the
+    /// item, so a source added later starts out *without* a position and has to be
+    /// added to this list deliberately. Getting that wrong is not a compile error: it
+    /// silently hands the new source line `0` of this file, points reporters at it,
+    /// and lets [`is_rewritable`](Self::is_rewritable) write over it.
     #[must_use]
     pub fn has_position(&self) -> bool {
-        self.is_checkable() && self.source != PackageSource::Inherited
+        self.is_checkable()
+            && !matches!(
+                self.source,
+                PackageSource::Inherited | PackageSource::Locked
+            )
     }
 
     /// Whether the recorded span may be rewritten in place — it points here, and there is
@@ -162,8 +178,9 @@ pub enum PackageSource {
     Local,
     /// A git dependency — skipped for version checks.
     Git,
-    /// The dependency's version is declared somewhere other than this entry, so
-    /// there is no version string here to check against or to rewrite.
+    /// The dependency's version is declared **elsewhere in a manifest** — another
+    /// entry, another table, another file — so there is no version string on this
+    /// entry to check against or to rewrite.
     ///
     /// Three parsers emit it, for the same reason and with the same consequences:
     ///
@@ -189,7 +206,41 @@ pub enum PackageSource {
     /// would rewrite is not this dependency's own. Empty, no version was found at
     /// all, and a check reports
     /// [`DependencyStatus::Undetermined`](crate::result::DependencyStatus::Undetermined).
+    ///
+    /// A version that came from a *lockfile* is [`Locked`](Self::Locked), not this. It
+    /// was never declared, so there is no central declaration for a consumer reading
+    /// `inherited` to go and bump.
     Inherited,
+    /// The version came from a **lockfile**, and no manifest declares it anywhere.
+    ///
+    /// SwiftPM is the case that needs it. A `Package.swift` is a Swift *program*, so
+    /// this crate declines to read dependencies out of it; the entries come from
+    /// `Package.resolved` instead, which records what the resolver picked and nothing
+    /// about what was asked for. The package is real and published, so it is worth
+    /// checking and worth scanning for advisories — but there is no manifest span
+    /// anywhere to point at or to rewrite, which is what
+    /// [`has_position`](Item::has_position) reads the source to decide.
+    ///
+    /// Mechanically identical to [`Inherited`](Self::Inherited)
+    /// ([`is_checkable`](Item::is_checkable) without
+    /// [`has_position`](Item::has_position)), and kept apart from it because the two
+    /// tell a consumer different things. *Inherited* invites going to the central
+    /// declaration and bumping it; a locked entry has no such declaration, and a
+    /// consumer filtering on it would be chasing a file that does not exist.
+    ///
+    /// Distinct from a [`Registry`](Self::Registry) item that merely carries a
+    /// [`locked_version`](Item::locked_version): that one was declared, was read from
+    /// a manifest, and has a span. This one has no declaration behind it at all.
+    ///
+    /// The test is the *declaration*, never the provenance of the version string:
+    /// `Locked` means no manifest anywhere in the repository declares this dependency,
+    /// so there is nothing to point at. A `Cargo.toml` entry whose exact version came
+    /// out of `Cargo.lock` is still `Registry` — it was declared, and `--fix` rewrites
+    /// the declaration. A lockfile-first reader added later — a `Gemfile.lock`, a
+    /// `poetry.lock` — inherits that boundary: pins the project's own manifest also
+    /// declares stay with the source of that declaration and keep their span, and only
+    /// the pins no manifest mentions are `Locked`.
+    Locked,
     /// The entry names a package this manifest cannot identify, whatever version it
     /// states beside it.
     ///
@@ -272,6 +323,38 @@ mod tests {
         }
     }
 
+    /// The invariant `Locked` exists to hold. It has to behave exactly as `Inherited`
+    /// does — checked, never pointed at, never rewritten — and nothing in the compiler
+    /// enforces that: `PackageSource` is `#[non_exhaustive]` and every match on it
+    /// carries a wildcard arm, so a `Locked` that fell through `is_checkable` would
+    /// stop being checked, and one omitted from `has_position` would silently claim
+    /// line 1 of a file that never declared it and become rewritable by `--fix`.
+    #[test]
+    fn a_locked_item_is_checkable_and_has_no_position() {
+        let mut item = find(&items("[dependencies]\nserde = \"1.0.200\"\n"), "serde");
+        item.source = PackageSource::Locked;
+        item.locked_version = Some("1.0.200".to_owned());
+
+        assert!(
+            item.is_checkable(),
+            "a lockfile pin is a real version to check"
+        );
+        assert!(
+            !item.has_position(),
+            "no manifest declared it, so no line may be pointed at"
+        );
+        assert!(
+            !item.is_rewritable(),
+            "`--fix` has nothing here to write to"
+        );
+
+        item.version_constraint.clear();
+        assert!(
+            !item.is_checkable(),
+            "a pin with no version recorded states nothing to check"
+        );
+    }
+
     /// An inherited dependency is checkable once — and only once — the workspace root
     /// has supplied a constraint. It is never rewritable, because the string it would
     /// rewrite is in the root, not here.

crates/dependable-core/src/lockfiles/swift_package_resolved.rs

@@ -325,23 +325,32 @@ fn pin_item(pin: &Pin) -> Option<Item> {
             .or_else(|| pin.identity.clone())
     }?;
 
+    // An empty string is not a version. SwiftPM writes `null` for a pin with no
+    // version, but a hand-edited or third-party-generated file can write `""`, and
+    // that must not be mistaken for a resolved one — the name path above filters
+    // emptiness for the same reason. Filtering here is what makes `Locked` with an
+    // empty constraint unrepresentable: the degenerate pin falls through to the
+    // branch/revision state and the `Git` arm, exactly as a versionless pin does.
+    let version = pin.version.clone().filter(|version| !version.is_empty());
+
     // What the pin resolved to, in descending order of usefulness to a reader.
-    let state = pin
-        .version
+    let state = version
         .clone()
         .or_else(|| pin.branch.clone())
         .or_else(|| pin.revision.clone())
         .unwrap_or_default();
 
-    // `Inherited`, not `Registry`: the version was written somewhere other than
-    // this entry — in `Package.resolved`, never in the manifest — so there is no
-    // span in `Package.swift` to report or to rewrite, which is exactly what
-    // `Item::has_position` reads the source to decide. A branch pin has no
-    // version at all and is the git dependency it looks like.
+    // `Locked`, not `Registry`: the version was written in `Package.resolved` and
+    // never in a manifest, so there is no span in `Package.swift` to report or to
+    // rewrite, which is exactly what `Item::has_position` reads the source to
+    // decide. Not `Inherited` either — nothing was inherited, because nothing
+    // declared it; a consumer told "inherited" would go looking for a central
+    // declaration that does not exist. A branch pin has no version at all and is
+    // the git dependency it looks like.
     let (source, constraint, locked) = if local {
         (PackageSource::Local, state, None)
-    } else if let Some(version) = pin.version.clone() {
-        (PackageSource::Inherited, version.clone(), Some(version))
+    } else if let Some(version) = version {
+        (PackageSource::Locked, version.clone(), Some(version))
     } else {
         (PackageSource::Git, state, None)
     };
@@ -427,7 +436,11 @@ mod tests {
         let nio = find(&items, "github.com/apple/swift-nio");
         assert_eq!(nio.locked_version.as_deref(), Some("2.65.0"));
         assert_eq!(nio.version_constraint, "2.65.0");
-        assert_eq!(nio.source, PackageSource::Inherited);
+        assert_eq!(
+            nio.source,
+            PackageSource::Locked,
+            "the version came from this file, not from a declaration anywhere"
+        );
     }
 
     /// The pin set is the only record of what the project depends on, so a pin has
@@ -511,6 +524,90 @@ mod tests {
         assert!(!items[0].is_checkable());
     }
 
+    /// An empty version string states nothing, so it may not produce a `Locked`
+    /// item: `Locked` is the claim that a lockfile supplied the resolved version,
+    /// and the renderers act on that claim. `dependable list` gives a `Locked` pin
+    /// no annotation precisely because it always states a version, so an empty
+    /// constraint would print a bare `—` — the very output the `Inherited`
+    /// `(unresolved)` arm exists to prevent. Filtering it at the producer makes
+    /// `Locked` with an empty constraint unrepresentable rather than merely
+    /// unrendered.
+    #[test]
+    fn an_empty_version_is_not_a_locked_pin() {
+        let lock = r#"{
+  "pins": [
+    {
+      "identity": "swift-nio",
+      "kind": "remoteSourceControl",
+      "location": "https://github.com/apple/swift-nio.git",
+      "state": { "revision": "635b25", "version": "" }
+    }
+  ],
+  "version": 2
+}"#;
+        let items = items(lock);
+        assert_eq!(items[0].name, "github.com/apple/swift-nio");
+        assert_ne!(
+            items[0].source,
+            PackageSource::Locked,
+            "an empty string is not a resolved version"
+        );
+        assert_eq!(items[0].source, PackageSource::Git);
+        assert_eq!(items[0].locked_version, None);
+        assert_eq!(
+            items[0].version_constraint, "635b25",
+            "with no version the pin states what it does have: the revision"
+        );
+        assert!(!items[0].is_checkable());
+    }
+
+    /// The invariant the filter buys, asserted over every pin shape that reaches
+    /// `pin_item`: nothing that calls itself `Locked` may state an empty version.
+    #[test]
+    fn a_locked_pin_always_states_a_version() {
+        let lock = r#"{
+  "pins": [
+    { "identity": "a", "kind": "remoteSourceControl", "location": "https://github.com/acme/a.git",
+      "state": { "revision": "1111", "version": "1.0.0" } },
+    { "identity": "b", "kind": "remoteSourceControl", "location": "https://github.com/acme/b.git",
+      "state": { "revision": "2222", "version": "" } },
+    { "identity": "c", "kind": "remoteSourceControl", "location": "https://github.com/acme/c.git",
+      "state": { "branch": "main", "revision": "3333", "version": "" } },
+    { "identity": "d", "kind": "fileSystem", "location": "/Users/me/d", "state": { "version": "" } }
+  ],
+  "version": 2
+}"#;
+        let items = items(lock);
+        assert_eq!(items.len(), 4);
+        for item in &items {
+            if item.source == PackageSource::Locked {
+                assert!(
+                    !item.version_constraint.is_empty(),
+                    "{} is Locked with no version",
+                    item.name
+                );
+                assert!(
+                    item.locked_version
+                        .as_deref()
+                        .is_some_and(|v| !v.is_empty()),
+                    "{} is Locked with no locked version",
+                    item.name
+                );
+            }
+        }
+        let sources: Vec<PackageSource> = items.iter().map(|item| item.source).collect();
+        assert_eq!(
+            sources,
+            [
+                PackageSource::Locked,
+                PackageSource::Git,
+                PackageSource::Git,
+                PackageSource::Local,
+            ]
+        );
+        assert_eq!(find(&items, "github.com/acme/c").version_constraint, "main");
+    }
+
     #[test]
     fn a_local_package_is_named_by_its_identity_and_never_fetched() {
         let lock = r#"{

crates/dependable-core/src/parsers/cargo_workspace.rs

@@ -97,7 +97,9 @@ pub fn resolve_workspace_inheritance(items: &mut [Item], declarations: &[Item])
     for item in items {
         // Only an entry that says it inherits, and has nothing of its own to say, can be
         // resolved. A `path` dependency sharing a name with a root declaration is not
-        // inheriting — Cargo uses the path — and must not be rewritten here.
+        // inheriting — Cargo uses the path — and must not be rewritten here. Nor is a
+        // `Locked` entry, whose version a lockfile already supplied and which claims no
+        // root above it; the `!=` covers it, and should keep covering it.
         if item.source != PackageSource::Inherited || !item.version_constraint.is_empty() {
             continue;
         }

crates/dependable-fetch/src/check.rs

@@ -889,6 +889,10 @@ impl Checker {
 
 /// Name every entry that says it inherits but that the governing root never declared.
 ///
+/// Reads `PackageSource::Inherited` specifically, not "checkable without a position":
+/// a `Locked` entry also lacks a position, but nothing above it ever promised to
+/// declare it, so there is no root to accuse.
+///
 /// Cargo refuses to build such a manifest, so it is a real error and not a shrug — but it
 /// is not this tool's error, and a version check that aborted on it would be less useful
 /// than one that reports everything else and says what it could not resolve. The item
@@ -1052,6 +1056,10 @@ fn unfetchable(item: &Item) -> CheckResult {
         // which of `spring-boot-starter-web` is simply false, and is the wrong
         // token for a CI consumer to read.
         PackageSource::Inherited => DependencyStatus::Undetermined,
+        // Same reasoning, different reason for the version to be missing: a lockfile
+        // pin with no version recorded is still a real package on a real host, and
+        // `Local` would say there is no host for it.
+        PackageSource::Locked => DependencyStatus::Undetermined,
         // A coordinate this manifest could not state is the same shape of
         // ignorance reached through the name instead of the version: nothing was
         // asked, so nothing is known. `Local` would again say the wrong thing —

crates/dependable-fetch/src/tree.rs

@@ -244,10 +244,15 @@ fn shallow_graph(
                 // Synthesize a source so classification matches the item's kind. An
                 // inherited entry has already taken its root declaration's source above,
                 // so a centrally-declared `path` crate lands on the `Local` arm and a
-                // centrally-declared registry crate does not.
+                // centrally-declared registry crate does not. Only Cargo manifests reach
+                // here, so `Locked` cannot; `registry+` is the right answer for it
+                // anyway — a lockfile pin is a registry package — and it is written as
+                // its own arm rather than left to the wildcard so the claim is in the
+                // code, not only in this comment.
                 let source = match item.source {
                     PackageSource::Git => Some("git+".to_owned()),
                     PackageSource::Local => None,
+                    PackageSource::Locked => Some("registry+".to_owned()),
                     _ => Some("registry+".to_owned()),
                 };
                 external_pkgs.push(LockedPackage::new(

crates/dependable/src/output/json.rs

@@ -89,6 +89,10 @@ pub fn render(reports: &[ManifestReport]) -> anyhow::Result<()> {
                 kind: result.item.kind.token(),
                 vulnerabilities: &result.current_vulnerabilities,
                 locked_at: result.item.locked_version.as_deref(),
+                // `Inherited` only. A `PackageSource::Locked` entry has no position
+                // either, but its version came from a lockfile and no manifest
+                // declares it, so naming the workspace root as the file to edit would
+                // point a consumer at a declaration that is not there.
                 inherited_from: (result.item.source == PackageSource::Inherited
                     && !result.item.version_constraint.is_empty())
                 .then(|| workspace_root.clone())