From c90df005ee3e9991927d78fa0b7f074fc20e7a82 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Tue, 1 Sep 2026 20:24:56 -0400 Subject: [PATCH 1/4] fix(cli): head an unread dependency list as unread in list too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `list` counted the items it held, so a SwiftPM project with no readable `Package.resolved` was headed "(0 dependencies)" — an inventory reporting an empty inventory for a list it never opened. `check` and the HTML report already say "dependency list unread" here; `list` did not, because `run_list` discarded the bool `report_lockfile_notices` returns. Carry that bool onto `ProjectReport` and use it in both surfaces `list` owns: the table heading gets the same words `check` and the HTML report use, and `--format json` gains a per-project `dependencies_unread`. The JSON half matters on its own — nothing on a `list` project object separated "declares nothing" from "nothing was read", and `lockfile: null` does not answer it because `--no-lock-file` produces that too. The `items.is_empty()` conjunct is load-bearing, and matches the two existing consumers: a `Package.resolved` that parses to zero pins really does declare nothing and is still counted, as is a partially read list. An added key stays within `dependable.list/v1`: a consumer that reads the fields it knows is unaffected by one it never looks at, and bumping the version would break every consumer pinning `v1` in order to protect none of them. README worded the pin as "which fields exist", which an added field contradicts, so it now states the policy it actually follows. `--format text` is unchanged: its contract is one record per dependency at fixed arity, and a manifest-level record has no place in it. Refs #109 --- README.md | 15 ++- crates/dependable/src/features.rs | 1 + crates/dependable/src/output/list.rs | 47 ++++++++-- crates/dependable/src/runner.rs | 3 +- crates/dependable/tests/fixture_swift.rs | 114 +++++++++++++++++++++++ 5 files changed, 170 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 1ecc9ad..043636a 100644 --- a/README.md +++ b/README.md @@ -411,6 +411,14 @@ that declares only central versions — a virtual Cargo workspace root, `pnpm-workspace.yaml`, `Directory.Packages.props` — has `"role": "workspace"` and no name of its own. +`dependencies_unread` says whether the file that *is* that project's dependency list +went unread, so an empty `dependencies` array says nothing about it — today only a +`Package.swift` with no readable `Package.resolved` beside it. `lockfile: null` is not +the same question, since `--no-lock-file` produces that too, and a `Package.resolved` +that records no pins leaves `dependencies_unread` false because the project really +does declare nothing. The table output heads such a project `(dependency list unread)` +in place of a count, in the same words `check` uses. + Values a single manifest cannot supply are resolved from the repository and marked as such: `version_inherited` for a Cargo `version.workspace = true`, `inherited` for a constraint taken from `[workspace.dependencies]`, and `lockfile` for the lockfile that @@ -419,8 +427,11 @@ supplied the locked versions — a workspace keeps one at its root, above its me 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 +`dependable.list/v1`. What that version pins is what a consumer may rely on remaining +true — no field is removed, renamed, or retyped without a new version — and not the +token sets inside those fields, nor a promise that no field is ever *added*. A key you +never read cannot break you, and bumping the version for one would break every +consumer pinning `v1` in order to protect none of them. Match the tokens 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 diff --git a/crates/dependable/src/features.rs b/crates/dependable/src/features.rs index f5bd663..a1acb81 100644 --- a/crates/dependable/src/features.rs +++ b/crates/dependable/src/features.rs @@ -109,6 +109,7 @@ mod tests { version_inherited: false, role: ProjectRole::Package, lockfile: None, + dependencies_unread: false, inherited: Vec::new(), items: parse(ManifestKind::CargoToml, manifest) .expect("fixture should parse") diff --git a/crates/dependable/src/output/list.rs b/crates/dependable/src/output/list.rs index cfd8c85..5136be9 100644 --- a/crates/dependable/src/output/list.rs +++ b/crates/dependable/src/output/list.rs @@ -17,11 +17,15 @@ 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. +/// The *shape*, and specifically what a consumer may rely on remaining true: +/// removing a field, renaming one, or changing one's type takes a new version. +/// *Adding* a field does not — a consumer that reads the fields it knows is +/// unaffected by a key it never looks at, and a version bump would break every +/// consumer pinning `v1` in order to protect none of them. The token sets inside +/// those fields are likewise 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"`) or a +/// field (`dependencies_unread`) therefore stays within `v1`. /// The README states the same policy for readers who never open this file. const SCHEMA: &str = "dependable.list/v1"; @@ -42,6 +46,16 @@ pub struct ProjectReport { pub role: ProjectRole, /// The lockfile that supplied locked versions, relative to the scanned root. pub lockfile: Option, + /// Whether the file that *is* this project's dependency list went unread, so + /// [`Self::items`] being empty says nothing about the project. + /// + /// The same fact [`crate::output::ManifestReport::dependencies_unread`] carries + /// for `check`, and set from the same notices: only a SwiftPM project can set + /// it, because a `Package.swift` is a program this tool declines to read, so + /// with no readable `Package.resolved` beside it there is no dependency list at + /// all. A `Package.resolved` that parses to zero pins leaves this `false` — that + /// project really does declare nothing. + pub dependencies_unread: bool, /// Dependencies whose constraint was inherited from a workspace root. pub inherited: Vec, /// The declared dependencies, in manifest order. @@ -98,11 +112,21 @@ fn table(reports: &[ProjectReport]) { ProjectRole::Workspace => " [workspace]", _ => "", }; + // A count is a claim about the project. Where the file that *is* the + // dependency list went unread there was nothing to count, so the heading + // says that instead — in the same words `check` and the HTML report use, so + // a reader comparing the three sees one phrase and not three. The + // `is_empty` conjunct is load-bearing: a partially-read list still gets + // counted rather than disclaimed. + let scope = if report.items.is_empty() && report.dependencies_unread { + "dependency list unread".to_owned() + } else { + format!("{} dependencies", report.items.len()) + }; println!( - "{} — {identity}{}{role} ({} dependencies)", + "{} — {identity}{}{role} ({scope})", report.relative.display(), report.ecosystem.display_name(), - report.items.len() ); for item in &report.items { let constraint = if item.version_constraint.is_empty() { @@ -170,6 +194,7 @@ fn json(reports: &[ProjectReport], root: &Path) -> anyhow::Result<()> { role: role_token(report.role), manifest: posix(&report.relative), lockfile: report.lockfile.as_deref().map(posix), + dependencies_unread: report.dependencies_unread, dependencies: report .items .iter() @@ -229,6 +254,14 @@ struct ProjectDto<'a> { role: &'static str, manifest: String, lockfile: Option, + /// Whether the file that *is* this project's dependency list went unread, so an + /// empty `dependencies` says nothing about the project. Always emitted — the + /// answer is always known — and `false` for every ecosystem but SwiftPM. + /// + /// `lockfile: null` is not a substitute: `--no-lock-file` produces that too, and + /// a `Package.resolved` that parses to zero pins leaves this `false` because the + /// project really does declare nothing. Additive, so the schema is unchanged. + dependencies_unread: bool, dependencies: Vec>, } diff --git a/crates/dependable/src/runner.rs b/crates/dependable/src/runner.rs index 91a3cb8..c013ca2 100644 --- a/crates/dependable/src/runner.rs +++ b/crates/dependable/src/runner.rs @@ -624,7 +624,7 @@ pub async fn run_list(args: ListArgs) -> anyhow::Result { let Some(kind) = ManifestKind::detect(manifest) else { continue; }; - let _ = report_lockfile_notices(manifest); + let dependencies_unread = report_lockfile_notices(manifest); let content = std::fs::read_to_string(manifest) .with_context(|| format!("reading {}", manifest.display()))?; let mut parsed = match parse(kind, &content) { @@ -679,6 +679,7 @@ pub async fn run_list(args: ListArgs) -> anyhow::Result { version_inherited, role: meta.role, lockfile, + dependencies_unread, inherited, items: parsed.items, features: BTreeMap::new(), diff --git a/crates/dependable/tests/fixture_swift.rs b/crates/dependable/tests/fixture_swift.rs index 690f34e..e601fc2 100644 --- a/crates/dependable/tests/fixture_swift.rs +++ b/crates/dependable/tests/fixture_swift.rs @@ -1006,3 +1006,117 @@ fn the_check_heading_says_the_list_went_unread_rather_than_counting_zero() { "a resolved project with no pins declares none, and says so: {empty}" ); } + +/// The same claim, in the same words, in the command whose entire job is to say +/// what a project declares. `list` counted the items it held, so a Swift project +/// with no readable `Package.resolved` was headed "(0 dependencies)" — an inventory +/// reporting an empty inventory for a list it never opened. +#[test] +fn the_list_heading_says_the_list_went_unread_rather_than_counting_zero() { + let unread_dir = scratch("swift_list_heading_unread"); + std::fs::copy( + fixture("sample-swift/Package.swift"), + unread_dir.join("Package.swift"), + ) + .unwrap(); + + let empty_dir = scratch("swift_list_heading_empty"); + std::fs::copy( + fixture("sample-swift/Package.swift"), + empty_dir.join("Package.swift"), + ) + .unwrap(); + std::fs::write( + empty_dir.join("Package.resolved"), + r#"{"pins":[],"version":2}"#, + ) + .unwrap(); + + let unread = run(&["list", unread_dir.to_str().unwrap()]); + let empty = run(&["list", empty_dir.to_str().unwrap()]); + assert!(unread.status.success(), "an unread list is not a failure"); + assert!(empty.status.success()); + + let unread_out = String::from_utf8_lossy(&unread.stdout); + assert!( + !unread_out.contains("(0 dependencies)"), + "nothing was counted because nothing was read: {unread_out}" + ); + assert!( + unread_out.contains("Swift (dependency list unread)"), + "the heading has to say so, in check's words: {unread_out}" + ); + // The heading *joins* the warning, it does not replace it: the reason the list + // went unread is still named, and still on stderr where every `--format` sees it. + let unread_err = String::from_utf8_lossy(&unread.stderr); + assert!( + unread_err.contains("warning:"), + "the warning naming the cause must survive: {unread_err}" + ); + + // And a project that really is resolved and really has no pins still counts. + let empty_out = String::from_utf8_lossy(&empty.stdout); + assert!( + empty_out.contains("Swift (0 dependencies)"), + "a resolved project with no pins declares none, and says so: {empty_out}" + ); +} + +/// The machine-readable half. Before this the two documents below were identical +/// apart from the root path, so no consumer of `list --format json` could tell a +/// project that declares nothing from one whose dependency list was never opened. +/// `lockfile: null` does not answer it — `--no-lock-file` produces that too. +#[test] +fn list_json_distinguishes_an_unread_dependency_list_from_an_empty_one() { + let unread_dir = scratch("swift_list_json_unread"); + std::fs::copy( + fixture("sample-swift/Package.swift"), + unread_dir.join("Package.swift"), + ) + .unwrap(); + + let empty_dir = scratch("swift_list_json_empty"); + std::fs::copy( + fixture("sample-swift/Package.swift"), + empty_dir.join("Package.swift"), + ) + .unwrap(); + std::fs::write( + empty_dir.join("Package.resolved"), + r#"{"pins":[],"version":2}"#, + ) + .unwrap(); + + let document = |dir: &Path| { + let output = run(&["list", dir.to_str().unwrap(), "--format", "json"]); + assert!( + output.status.success(), + "stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + let mut doc: serde_json::Value = + serde_json::from_slice(&output.stdout).expect("list emits JSON"); + // The one field that legitimately differs between the two runs, removed so + // the comparison below is about the projects and not about scratch paths. + doc.as_object_mut().expect("an object").remove("root"); + doc + }; + + let unread = document(&unread_dir); + let empty = document(&empty_dir); + assert_ne!( + unread, empty, + "an unread list and an empty one were indistinguishable, which is the defect" + ); + + assert_eq!(unread["projects"][0]["dependencies_unread"], true); + assert_eq!(empty["projects"][0]["dependencies_unread"], false); + + // Both counts are still zero — which is exactly why the field has to exist. + assert_eq!(unread["summary"]["dependencies"], 0); + assert_eq!(empty["summary"]["dependencies"], 0); + + // An added field stays within v1; a consumer pinning the version keeps working. + assert_eq!(unread["schema"], "dependable.list/v1"); + assert_eq!(empty["schema"], "dependable.list/v1"); +} From 71b5ff01936cb9ad0f70ad3e6bb6ef434c6cfb7c Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 13:45:30 -0400 Subject: [PATCH 2/4] test(cli): compare the two list documents on the field under test alone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `assert_ne!` over the two `list --format json` documents discriminated nothing. It stripped `root`, but the two runs also differ in `lockfile`: the unread project has no `Package.resolved` to name, so its `lockfile` is null, while the empty one names the file. The assertion fired on that difference alone — it passed unchanged with `dependencies_unread` absent from `ProjectDto` entirely — so its message stated something untrue. The shape came from the `check --format json` twin, where it is load-bearing: that document carries no per-manifest `lockfile` key, so those two really were byte-identical before `manifests_unread`. The premise does not transfer to a document that carries one. Strip `lockfile` too, for the same reason `root` was already stripped. With the field removed from `ProjectDto` the two documents are now byte-identical and the assertion fails, which is what it always claimed to be testing. --- crates/dependable/tests/fixture_swift.rs | 26 ++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/crates/dependable/tests/fixture_swift.rs b/crates/dependable/tests/fixture_swift.rs index a9e980d..60cc6fa 100644 --- a/crates/dependable/tests/fixture_swift.rs +++ b/crates/dependable/tests/fixture_swift.rs @@ -1106,10 +1106,12 @@ fn the_list_heading_says_the_list_went_unread_rather_than_counting_zero() { ); } -/// The machine-readable half. Before this the two documents below were identical -/// apart from the root path, so no consumer of `list --format json` could tell a +/// The machine-readable half. Before this the two documents below differed in one +/// field only, `lockfile` — `null` against `"Package.resolved"` — and that field +/// answers a different question, since `--no-lock-file` produces the same `null` +/// for a project whose list was read perfectly well. Set it aside, as the +/// comparison below does, and no consumer of `list --format json` could tell a /// project that declares nothing from one whose dependency list was never opened. -/// `lockfile: null` does not answer it — `--no-lock-file` produces that too. #[test] fn list_json_distinguishes_an_unread_dependency_list_from_an_empty_one() { let unread_dir = scratch("swift_list_json_unread"); @@ -1140,9 +1142,21 @@ fn list_json_distinguishes_an_unread_dependency_list_from_an_empty_one() { ); let mut doc: serde_json::Value = serde_json::from_slice(&output.stdout).expect("list emits JSON"); - // The one field that legitimately differs between the two runs, removed so - // the comparison below is about the projects and not about scratch paths. - doc.as_object_mut().expect("an object").remove("root"); + // Every field that differs between the two runs for a reason other than + // the one under test, removed so the comparison below discriminates on + // `dependencies_unread` alone. `root` is a scratch path. `lockfile` is the + // subtler one: it is `null` for the unread project only because there is no + // `Package.resolved` there to name, so leaving it in would satisfy the + // comparison whether or not `dependencies_unread` exists at all — which is + // precisely the confusion this test exists to refute. + let object = doc.as_object_mut().expect("an object"); + object.remove("root"); + for project in object["projects"].as_array_mut().expect("an array") { + project + .as_object_mut() + .expect("an object") + .remove("lockfile"); + } doc }; From 50ba69221e79128dc01fad1877354133651c8b81 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 13:45:35 -0400 Subject: [PATCH 3/4] docs(cli): show dependencies_unread in the README list JSON example MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `dependencies_unread` has no `skip_serializing_if`, so `list --format json` emits it on every project, not only a SwiftPM one. The README example omitted it, which made it the single field a decoder written from that example — the artefact people copy — would reject on a document the tool actually produces. The example is otherwise a faithful superset of the schema: every field without `skip_serializing_if`, plus the two optional ones. Restore that property. --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 043636a..ca3a92f 100644 --- a/README.md +++ b/README.md @@ -384,6 +384,7 @@ dependable list --licenses # add each dependency's declared license "role": "package", "manifest": "crates/dependable-core/Cargo.toml", "lockfile": "Cargo.lock", + "dependencies_unread": false, "dependencies": [ { "name": "serde", From f0b400d95175143c387b0ede49fb2bcfedf0a9f5 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 13:45:40 -0400 Subject: [PATCH 4/4] refactor(cli): mark report_lockfile_notices must_use MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Its return value is load-bearing in three places — the `list` heading, `list --format json`, and the `--fail-on any` exit code — its own doc comment says the caller has to carry it, and every function it delegates to (`lockfile_notices`, `locate_lockfile`, `lockfile_items`, `swift_package_resolved_items`) already carries the attribute. This wrapper was the one unmarked link in that chain, and CLAUDE.md asks for `#[must_use]` on important return types. Be honest about what it buys: `#[must_use]` would not have caught the bug this branch fixes, because `let _ = f()` is exactly the spelling that silences the lint and exactly what the base code had. It guards the bare-statement spelling only. Add it anyway — the convention is stated and the attribute is free. --- crates/dependable/src/runner.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/dependable/src/runner.rs b/crates/dependable/src/runner.rs index 8efb5f2..f3a7dff 100644 --- a/crates/dependable/src/runner.rs +++ b/crates/dependable/src/runner.rs @@ -303,6 +303,7 @@ impl Engine { /// Returns whether any notice means the project's dependency list itself went /// unread, which the caller has to carry into the exit code: a run that knows /// nothing about a project must not report it clean. +#[must_use] fn report_lockfile_notices(manifest: &Path) -> bool { let Some(kind) = ManifestKind::detect(manifest) else { return false;