From c28d061a0aedee9e2b5bb4e94467ab2b791d48e6 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Tue, 1 Sep 2026 16:50:19 -0400 Subject: [PATCH 1/7] feat(core): add a PackageSource for a lockfile-supplied version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Inherited` had grown to mean four different things, one of which — a version read out of a lockfile with no manifest declaration behind it anywhere — is not inheritance at all. `Locked` names that case. It shares `Inherited`'s mechanics exactly: checkable, no position, never rewritable. Both `is_checkable` and `has_position` name it explicitly, because `PackageSource` is `#[non_exhaustive]` and every match on it carries a wildcard arm — a variant left out of `has_position` silently gains line 1 of a file that never declared it and becomes rewritable. The new test pins both halves. --- crates/dependable-core/src/item.rs | 86 +++++++++++++++++++++++++++--- 1 file changed, 80 insertions(+), 6 deletions(-) diff --git a/crates/dependable-core/src/item.rs b/crates/dependable-core/src/item.rs index aef39c4..fa9fb60 100644 --- a/crates/dependable-core/src/item.rs +++ b/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 `` value. + /// POM `` 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,32 @@ 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. + Locked, } #[cfg(test)] @@ -250,6 +292,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. From 63e1027372ff2c4f2452e2bcaa2dee8c106bab31 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Tue, 1 Sep 2026 16:55:14 -0400 Subject: [PATCH 2/7] refactor(swift): report a resolved pin as Locked, not Inherited MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `Package.resolved` pin is not inherited from anything: `Package.swift` is a program this crate declines to read, so no manifest declares the package at all. Calling it `Inherited` sent a consumer looking for a central declaration to bump that does not exist. Behaviour is unchanged — `Locked` is checkable, has no position, and is never rewritable, exactly as before. `unfetchable` gains the matching arm, since `Locked` would otherwise fall to the wildcard and report `Local` — "there is no registry for this" — of a package that is published and merely has no version recorded. The three inheritance-warning predicates and `resolve_workspace_inheritance` keep reading `Inherited` by name and so no longer speak for Swift, which is right: none of them has a root to name for a locked entry. --- .../src/lockfiles/swift_package_resolved.rs | 20 ++++++++++++------- .../src/parsers/cargo_workspace.rs | 4 +++- crates/dependable-fetch/src/check.rs | 8 ++++++++ crates/dependable-fetch/src/tree.rs | 5 ++++- 4 files changed, 28 insertions(+), 9 deletions(-) diff --git a/crates/dependable-core/src/lockfiles/swift_package_resolved.rs b/crates/dependable-core/src/lockfiles/swift_package_resolved.rs index ff179aa..183be17 100644 --- a/crates/dependable-core/src/lockfiles/swift_package_resolved.rs +++ b/crates/dependable-core/src/lockfiles/swift_package_resolved.rs @@ -293,15 +293,17 @@ fn pin_item(pin: &Pin) -> Option { .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)) + (PackageSource::Locked, version.clone(), Some(version)) } else { (PackageSource::Git, state, None) }; @@ -387,7 +389,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 diff --git a/crates/dependable-core/src/parsers/cargo_workspace.rs b/crates/dependable-core/src/parsers/cargo_workspace.rs index 7d258bd..37805ed 100644 --- a/crates/dependable-core/src/parsers/cargo_workspace.rs +++ b/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; } diff --git a/crates/dependable-fetch/src/check.rs b/crates/dependable-fetch/src/check.rs index 0535cb1..0e41507 100644 --- a/crates/dependable-fetch/src/check.rs +++ b/crates/dependable-fetch/src/check.rs @@ -833,6 +833,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 @@ -996,6 +1000,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, _ => DependencyStatus::Local, }; CheckResult::new(item.clone(), status) diff --git a/crates/dependable-fetch/src/tree.rs b/crates/dependable-fetch/src/tree.rs index df04dac..fd7a529 100644 --- a/crates/dependable-fetch/src/tree.rs +++ b/crates/dependable-fetch/src/tree.rs @@ -244,7 +244,10 @@ 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; were it ever to, `registry+` is the right + // answer for it anyway — a lockfile pin is a registry package. + let source = match item.source { PackageSource::Git => Some("git+".to_owned()), PackageSource::Local => None, From 2b2f94f7e725f19ae990f4e7312eba67c6402461 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Tue, 1 Sep 2026 16:55:21 -0400 Subject: [PATCH 3/7] feat(cli): emit "locked" as a list source token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `list --format json` gains a sixth `source` token. `inherited` stays `false` beside it: the flag exists to send a consumer to the file where the version can be bumped, and a lockfile pin has no such file. Stays within `dependable.list/v1`. The document's *shape* is unchanged, and the token sets inside it were already open — `source` has always fallen back to `"unknown"` for a variant the CLI does not name, so no consumer could safely match them exhaustively. The schema comment now says so. `check --format json` keeps `inherited_from` off a locked entry for the same reason, and the workspace note on stderr keeps reading `Inherited` by name rather than "skipped by fix", because it has no root to point a Swift reader at. `annotation` deliberately gains no arm: a locked entry states a version, so it is not unresolved, and its `kind` already says the useful thing. --- README.md | 13 +++++++++---- crates/dependable/src/output/json.rs | 4 ++++ crates/dependable/src/output/list.rs | 20 ++++++++++++++++++-- crates/dependable/src/runner.rs | 5 +++++ 4 files changed, 36 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index d45c914..f74513b 100644 --- a/README.md +++ b/README.md @@ -416,10 +416,15 @@ 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 `registry`, `jsr`, `git`, `local` (a `path` entry), +`inherited`, or `locked`. `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 `` 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 diff --git a/crates/dependable/src/output/json.rs b/crates/dependable/src/output/json.rs index 7a4d60f..ed05dc5 100644 --- a/crates/dependable/src/output/json.rs +++ b/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()) diff --git a/crates/dependable/src/output/list.rs b/crates/dependable/src/output/list.rs index 49d8878..db9170e 100644 --- a/crates/dependable/src/output/list.rs +++ b/crates/dependable/src/output/list.rs @@ -16,6 +16,12 @@ use crate::output::posix; /// The identifier of the JSON document's shape. Consumers can pin on it; any /// incompatible change to the shape takes a new version. +/// +/// The *shape* — which fields exist and what type each holds. The token sets inside +/// those fields are open and always have been: `source` already falls back to +/// `"unknown"` for a variant this function does not name, so a consumer that matched +/// exhaustively on them was never safe. Adding a token (`"locked"`) therefore stays +/// within `v1`; removing a field, renaming one, or changing one's type would not. const SCHEMA: &str = "dependable.list/v1"; /// One discovered manifest: its identity and the dependencies it declares. @@ -237,13 +243,18 @@ struct DependencyDto<'a> { locked: Option<&'a str>, registry: Option<&'a str>, /// Whether this dependency's version is declared somewhere other than its own - /// entry: a Cargo `workspace = true` resolved against the root, a Gradle - /// `[versions]` alias, a shared Maven `` value. + /// entry *but still in a manifest*: a Cargo `workspace = true` resolved against + /// the root, a Gradle `[versions]` alias, a shared Maven `` value. /// /// True for every entry whose `source` is `inherited`, so the two fields can no /// longer contradict each other on the same object. Also true where the root's /// declaration supplied a `path` or `git` source, which replaces `source` /// outright and would otherwise lose the fact that it was inherited at all. + /// + /// False for `"source": "locked"`, and deliberately: a lockfile pin was not + /// inherited from anything, because nothing declared it. The point of the field + /// is to send a consumer to the file where the version can be bumped, and for a + /// locked entry there is no such file. inherited: bool, #[serde(skip_serializing_if = "Option::is_none")] features: Option<&'a [String]>, @@ -278,6 +289,7 @@ fn source_token(source: PackageSource) -> &'static str { PackageSource::Local => "local", PackageSource::Git => "git", PackageSource::Inherited => "inherited", + PackageSource::Locked => "locked", _ => "unknown", } } @@ -305,6 +317,10 @@ fn annotation(item: &Item) -> &'static str { // would otherwise render as a bare `—` that reads like a parse failure. A // resolved one falls through to its section, so a `dev` dep still says so. PackageSource::Inherited if item.version_constraint.is_empty() => " (unresolved)", + // `Locked` deliberately has no arm. A lockfile pin states a version, so it is + // not unresolved, and it is not a source a reader of a table needs told about + // — what a reader wants to know is that it is not a declared direct + // dependency, which is exactly what its `kind` says below. _ => match item.kind { DependencyKind::Dev => " (dev)", DependencyKind::Build => " (build)", diff --git a/crates/dependable/src/runner.rs b/crates/dependable/src/runner.rs index 385b029..6798532 100644 --- a/crates/dependable/src/runner.rs +++ b/crates/dependable/src/runner.rs @@ -991,6 +991,11 @@ fn resolve_report_settings(args: &crate::cli::ReportArgs, cfg: &Config) -> Setti /// silently left alone by `fix`, because the version string is in the workspace root and /// there is no line here to rewrite. Without this the two commands appear to contradict /// each other, and nothing points at the file that can actually be changed. +/// +/// `PackageSource::Inherited` and not merely "has no position": a +/// [`PackageSource::Locked`] entry is also skipped by `fix`, but there is no root +/// holding its version, so this note has no file to send the reader to. Swift is told +/// so once per project instead, by the check itself. fn report_inherited_skips(manifest: &Path, report: &ManifestReport) { let Some(root) = &report.workspace_root else { return; From 6a9b420f3ed8f54e539af3a03e92a96bd6233e61 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Tue, 1 Sep 2026 16:55:27 -0400 Subject: [PATCH 4/7] test: hold the locked/inherited split at the surface a consumer reads `a_swift_pin_is_reported_as_locked_and_never_as_inherited` fails on the previous behaviour: it asserts `"source": "locked"` with `"inherited": false` on a `Package.resolved` pin, and that no Swift entry claims to inherit. The two halves are computed by different functions from the same field, so either can drift back alone. The Maven fixture keeps asserting `"source": "inherited"` with `"inherited": true`, because POM entries are unchanged; its comment now states the implication runs one way only, and that a locked entry is the deliberate non-inheritor. --- crates/dependable/tests/fixture_maven.rs | 14 ++++-- crates/dependable/tests/fixture_swift.rs | 59 ++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/crates/dependable/tests/fixture_maven.rs b/crates/dependable/tests/fixture_maven.rs index 20525e5..870ca83 100644 --- a/crates/dependable/tests/fixture_maven.rs +++ b/crates/dependable/tests/fixture_maven.rs @@ -184,11 +184,15 @@ fn the_cli_lists_an_unresolvable_dependency_instead_of_omitting_it() { assert_eq!(guava["source"], "registry"); assert_eq!(guava["inherited"], false, "its version is its own: {guava}"); - // `source` and `inherited` describe the same fact, so they can never disagree - // on one object. They used to: the boolean was filled in only by Cargo - // workspace resolution, so every non-Cargo `"source": "inherited"` arrived - // beside `"inherited": false`, and a consumer reading both got a - // contradiction. + // `source: "inherited"` and `inherited: true` describe the same fact, so they can + // never disagree on one object. They used to: the boolean was filled in only by + // Cargo workspace resolution, so every non-Cargo `"source": "inherited"` arrived + // beside `"inherited": false`, and a consumer reading both got a contradiction. + // + // The implication runs one way only. `inherited: true` with some other `source` + // is legal — a root declaration supplying a `path` or `git` source replaces + // `source` outright — and `"source": "locked"` is deliberately *not* inherited, + // because a lockfile pin has no declaration anywhere to have been inherited from. for name in [ "com.fasterxml.jackson.core:jackson-core", "com.fasterxml.jackson.core:jackson-databind", diff --git a/crates/dependable/tests/fixture_swift.rs b/crates/dependable/tests/fixture_swift.rs index c1f0298..c8ebe02 100644 --- a/crates/dependable/tests/fixture_swift.rs +++ b/crates/dependable/tests/fixture_swift.rs @@ -150,6 +150,15 @@ fn a_pin_is_named_by_the_url_osv_keys_advisories_by() { #[test] fn a_swift_dependency_has_no_position_and_is_never_rewritable() { for item in pins("sample-swift/Package.resolved") { + if !item.is_checkable() { + continue; + } + assert_eq!( + item.source, + PackageSource::Locked, + "{}: a checkable Swift pin got its version from the lockfile", + item.name + ); assert!( !item.has_position(), "{}: no span in Package.swift means nothing may point at one", @@ -241,6 +250,56 @@ fn list_surfaces_the_pins_a_package_swift_never_declared() { assert!(stdout.contains("2.65.0"), "stdout: {stdout}"); } +/// The consumer-visible half of the split, and the thing that falsifies a regression: +/// `list --format json` must call a Swift pin `"source": "locked"`, and must say +/// `"inherited": false` beside it. +/// +/// It used to say `"inherited"`, which sends a consumer looking for the central +/// declaration to bump — a `[workspace.dependencies]` table, a `[versions]` alias, a +/// `` value. A Swift project has none: `Package.resolved` is the only +/// place the version is written, and `Package.swift` is a program nothing parses. The +/// two assertions are one fact stated twice on purpose, because they are computed +/// from the same field in two different functions (`source_token` and the `inherited` +/// flag) and either can drift back on its own. +#[test] +fn a_swift_pin_is_reported_as_locked_and_never_as_inherited() { + let manifest = fixture("sample-swift/Package.swift"); + let output = run(&[ + "list", + "--manifest", + manifest.to_str().unwrap(), + "--format", + "json", + ]); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(output.status.success(), "list failed: {stderr}"); + + let doc: serde_json::Value = serde_json::from_slice(&output.stdout).expect("valid JSON"); + let dependencies = doc["projects"][0]["dependencies"] + .as_array() + .unwrap_or_else(|| panic!("no dependencies in {doc}")) + .clone(); + assert!(!dependencies.is_empty(), "{doc}"); + + let nio = dependencies + .iter() + .find(|d| d["name"] == "github.com/apple/swift-nio") + .unwrap_or_else(|| panic!("no swift-nio in {doc}")); + assert_eq!(nio["constraint"], "2.65.0", "{nio}"); + assert_eq!(nio["source"], "locked", "{nio}"); + assert_eq!( + nio["inherited"], false, + "nothing declares it, so there is nowhere to go and bump it: {nio}" + ); + + for entry in &dependencies { + assert_ne!( + entry["source"], "inherited", + "no Swift entry inherits from anything: {entry}" + ); + } +} + /// `--no-lock-file` is documented as "do not report locked versions" — it suppresses /// an *annotation*. A `Package.resolved` is not an annotation: it is the only /// dependency list a Swift project has. Honouring the flag there turned From c59d66cb6ece0a8ec9f699b58fc0e144c7fe28a7 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Tue, 1 Sep 2026 17:22:45 -0400 Subject: [PATCH 5/7] fix(swift): an empty version string is not a locked pin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `pin_item` chose `Locked` on any `Some(version)`, with no emptiness filter — unlike the name path two blocks above, which filters one. A `Package.resolved` carrying `"state": { "version": "" }` therefore produced `Locked` with an empty `version_constraint`, and `dependable list` printed github.com/apple/swift-nio — (indirect) a bare `—` with nothing to explain it. `Locked` has no annotation arm precisely because a lockfile pin always states a version, and `locked_note` stays silent when there is no locked version to name, so the line said nothing at all. That is the exact output the `Inherited` `(unresolved)` arm exists to prevent, for the reason its own comment gives: a bare dash reads like a parse failure. Filter the emptiness at the producer rather than adding a renderer arm, so `Locked` with an empty constraint is unrepresentable instead of merely unrendered. The degenerate pin now falls through to the branch/revision state and the `Git` arm it already had, printing `635b25 (git)`. SwiftPM writes `null`, never `""`, for a versionless pin, so this needs a hand-edited or third-party-generated file to reach. --- .../src/lockfiles/swift_package_resolved.rs | 97 ++++++++++++++++++- 1 file changed, 94 insertions(+), 3 deletions(-) diff --git a/crates/dependable-core/src/lockfiles/swift_package_resolved.rs b/crates/dependable-core/src/lockfiles/swift_package_resolved.rs index de24348..8850aa0 100644 --- a/crates/dependable-core/src/lockfiles/swift_package_resolved.rs +++ b/crates/dependable-core/src/lockfiles/swift_package_resolved.rs @@ -312,9 +312,16 @@ fn pin_item(pin: &Pin) -> Option { .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()) @@ -329,7 +336,7 @@ fn pin_item(pin: &Pin) -> Option { // the git dependency it looks like. let (source, constraint, locked) = if local { (PackageSource::Local, state, None) - } else if let Some(version) = pin.version.clone() { + } else if let Some(version) = version { (PackageSource::Locked, version.clone(), Some(version)) } else { (PackageSource::Git, state, None) @@ -504,6 +511,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 = 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#"{ From 321777639201e5c47d0e1fa43f7ccc41992d6659 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Tue, 1 Sep 2026 17:23:37 -0400 Subject: [PATCH 6/7] test(swift): assert no pin has a position, not just the checkable ones MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `a_swift_dependency_has_no_position_and_is_never_rewritable` had gained a `continue` for non-checkable pins, added only so the `source == Locked` assertion inside the loop would hold. It also dropped the `!has_position()`, `!is_rewritable()`, `version_line == 0` and zero-width-span assertions for the branch pin and the local package — which the doc comment still claimed to cover. That coverage is the point: those two pins carry `version_line: 0` like every other, so a `has_position` that stopped consulting `is_checkable` would hand them line 1 of `Package.swift` in SARIF and in GitHub annotations. Dropping the `is_checkable() &&` conjunct leaves the narrowed test green and the surviving `a_branch_pin_and_a_local_package_report_what_they_always_did`, which asserts only `!is_checkable()`, green too. Make the `Locked` assertion conditional inside the loop body instead, so every pin is asserted on. With the conjunct removed the test now fails on `sample-helpers`. --- crates/dependable/tests/fixture_swift.rs | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/crates/dependable/tests/fixture_swift.rs b/crates/dependable/tests/fixture_swift.rs index 6399ec2..cbe1371 100644 --- a/crates/dependable/tests/fixture_swift.rs +++ b/crates/dependable/tests/fixture_swift.rs @@ -147,18 +147,23 @@ fn a_pin_is_named_by_the_url_osv_keys_advisories_by() { /// and asserts it round-trips. There is nothing to slice here, and that is the /// assertion: a Swift version is written in `Package.resolved`, not in any file /// this tool parsed, so nothing may point at it and `--fix` can never rewrite it. +/// +/// This holds for *every* pin, not only the checkable ones. The branch pin and the +/// local package carry `version_line: 0` like the rest, and a `has_position` that +/// stopped consulting `is_checkable` would hand them line 1 of `Package.swift` in +/// SARIF and in GitHub annotations. Skipping them here would leave that regression +/// to be caught by nothing. #[test] fn a_swift_dependency_has_no_position_and_is_never_rewritable() { for item in pins("sample-swift/Package.resolved") { - if !item.is_checkable() { - continue; + if item.is_checkable() { + assert_eq!( + item.source, + PackageSource::Locked, + "{}: a checkable Swift pin got its version from the lockfile", + item.name + ); } - assert_eq!( - item.source, - PackageSource::Locked, - "{}: a checkable Swift pin got its version from the lockfile", - item.name - ); assert!( !item.has_position(), "{}: no span in Package.swift means nothing may point at one", From 3055745b805b7f3d7b74681dc5721f12460ca32d Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Tue, 1 Sep 2026 17:24:56 -0400 Subject: [PATCH 7/7] docs: state the open source-token set and the Registry/Locked boundary The `dependable.list/v1` doc comment now says the schema pins the document's shape and that the token sets inside those fields are open. `README.md` still gave `source` as a closed enumeration and omitted the `unknown` fallback that argument rests on, so the two contradicted each other. State the open-set policy in the README and name `unknown`. Record the narrower reading of the `Registry`/`Locked` boundary on the variant itself: the test is whether a manifest declares the dependency, not where the version string came from. A `Cargo.toml` entry whose exact version came out of `Cargo.lock` stays `Registry`, because it was declared and `--fix` rewrites the declaration. Issue #98 was ambiguous between the two readings; a lockfile-first reader added later needs the settled one. In `build_workspace_graph`, give `Locked` its own arm. The correctness argument for the wildcard was carried entirely by a comment, and a comment is the one thing a `#[non_exhaustive]` enum will not check. Also close the blank line that had separated that comment from the `match` it explains. --- README.md | 9 +++++++-- crates/dependable-core/src/item.rs | 9 +++++++++ crates/dependable-fetch/src/tree.rs | 8 +++++--- crates/dependable/src/output/list.rs | 1 + 4 files changed, 22 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index f74513b..1ecc9ad 100644 --- a/README.md +++ b/README.md @@ -416,8 +416,13 @@ 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), -`inherited`, or `locked`. `inherited` means the version is declared elsewhere in a +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 `` value — and such a dependency is checked wherever it is used and rewritten only where it is declared; see diff --git a/crates/dependable-core/src/item.rs b/crates/dependable-core/src/item.rs index fa9fb60..7c6430b 100644 --- a/crates/dependable-core/src/item.rs +++ b/crates/dependable-core/src/item.rs @@ -231,6 +231,15 @@ pub enum PackageSource { /// 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, } diff --git a/crates/dependable-fetch/src/tree.rs b/crates/dependable-fetch/src/tree.rs index fd7a529..e5a984e 100644 --- a/crates/dependable-fetch/src/tree.rs +++ b/crates/dependable-fetch/src/tree.rs @@ -245,12 +245,14 @@ fn shallow_graph( // 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. Only Cargo manifests reach - // here, so `Locked` cannot; were it ever to, `registry+` is the right - // answer for it anyway — a lockfile pin is a registry package. - + // 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( diff --git a/crates/dependable/src/output/list.rs b/crates/dependable/src/output/list.rs index db9170e..cfd8c85 100644 --- a/crates/dependable/src/output/list.rs +++ b/crates/dependable/src/output/list.rs @@ -22,6 +22,7 @@ use crate::output::posix; /// `"unknown"` for a variant this function does not name, so a consumer that matched /// exhaustively on them was never safe. Adding a token (`"locked"`) therefore stays /// within `v1`; removing a field, renaming one, or changing one's type would not. +/// The README states the same policy for readers who never open this file. const SCHEMA: &str = "dependable.list/v1"; /// One discovered manifest: its identity and the dependencies it declares.