feat: add cargo-ensure-no-unused-workspace-deps by martin-kolinek · Pull Request #102 · microsoft/ox-tools

martin-kolinek · 2026-08-25T16:13:06Z

What

Adds cargo-ensure-no-unused-workspace-deps: a cargo subcommand that fails when a [workspace.dependencies] entry is inherited by no workspace member, plus its design doc.

docs/design/README.md — the design.
src/ — detection, --fix, allow-list. 18 integration tests, 100% line and function coverage.

Work item: 7790070.

Why

cargo udeps resolves the crate graph and asks which declared dependencies go unused. A [workspace.dependencies] entry that no member inherits never enters that graph, so it is invisible to udeps — and to cargo machete, for the same structural reason.

That blind spot accumulated 48 stale entries in this repo before #99 swept them out by hand, with udeps green throughout. Per review feedback on #99, the check belongs in cargo-anvil so every repo using it benefits, rather than living as a one-off script.

The rule

An entry is unused when no workspace member declares it with workspace = true — across dependencies, dev-dependencies, build-dependencies and their [target.'cfg(…)'] forms, in both the inline and dotted spellings.

Manifest-only, so the gate has no false positives and needs no compilation, toolchain pin, or network. It slots into the modified tier next to ensure-no-cyclic-deps and ensure-no-default-features, and composes with udeps without overlap:

Question	Answered by
Is this catalog entry inherited by any member?	this tool
Is an inherited dependency actually referenced in code?	`udeps`

Prior art evaluated

cargo-shear does implement a shear/unused_workspace_dependency diagnostic, and it does catch the case — verified by injecting an unused entry into this repo's root manifest. But it derives the verdict from static source-usage analysis, so it inherits that analysis's macro-expansion blind spots: it reports 8 entries here (rustdoc-types-v50..v57) that are inherited and genuinely used, via generate_version_support!("50", rustdoc_types_v50). It also skips the check entirely for single-member workspaces. Its other diagnostics remain independently interesting — separate decision.
cargo-unused-workspace-deps on crates.io does exactly this, but is one release from Sept 2025 with no commits since — not something to pin as an anvil dependency.

Points to review

--fix is in scope, unlike the sibling ensure-no-default-features ("the tool reports; the human edits"). Rationale: removing an entry nobody inherits is mechanical and lossless — Cargo.lock is unaffected by construction — unlike deciding which features to keep. The comment-carrying rules are specified normatively, including the empty-table case.
Members come from cargo metadata --no-deps rather than re-deriving members/globs/exclude textually. Costs a subprocess, but any disagreement with Cargo that drops a member is a false positive.
Allowlist lives in [workspace.metadata.…] allowed = […], not a CLI flag, because the generated recipe invokes the tool with a fixed argument list.

Not in this PR

The anvil wiring (versions.just pin, tools.just install/validate, checks/ recipe, pr-fast group, checks.md catalog row). Anvil installs pinned tools from crates.io, so the wiring has nothing to pin until the crate's first release; design §7 says so explicitly.

_{🤖 Authored by Clawpilot (an AI agent), not by a human.}

`cargo udeps` resolves the crate graph and asks which declared dependencies go unreferenced, so a `[workspace.dependencies]` entry that no member inherits is invisible to it -- it never enters the graph at all. That blind spot accumulated 48 stale entries before PR #99 swept them out by hand, with udeps green throughout. Adds the design doc for a new sibling gate that closes it, named to match the existing `cargo-ensure-no-cyclic-deps` and `cargo-ensure-no-default-features` check tools. The rule is manifest-only: an entry is unused when no workspace member declares it with `workspace = true`, across dependencies, dev- and build-dependencies and their `[target.'cfg(...)']` forms, in both the inline and dotted spellings. That keeps the gate free of false positives and cheap enough for the text/metadata tier -- no compilation, no toolchain pin, no network. cargo-shear was evaluated and rejected as the vehicle: it does implement an unused-workspace-dependency diagnostic, but derives it from static source-usage analysis, so its verdict inherits that analysis's macro-expansion blind spots -- it reports eight entries here that are inherited and genuinely used through macro arguments. Design only; no crate skeleton and no anvil wiring yet. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

`members` globs `crates/*`, so the new crate directory -- which holds a design doc and no manifest yet -- was read as a workspace member with an unreadable `Cargo.toml`. Every cargo invocation failed at metadata time, which is why the whole check suite went red on a docs-only change. The design-docs-first workflow lands the design before the code, so the gap between doc and manifest is expected rather than accidental. Excludes the directory until the crate lands, at which point the entry goes away. `Cargo.lock` is unaffected: the excluded directory contributes no package. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Codecov Comments Bot (codecov-commenter) · 2026-08-25T16:23:54Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.9%. Comparing base (48c5311) to head (b573896).

❌ Your project status has failed because the head coverage (99.9%) is below the target coverage (100.0%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@          Coverage Diff           @@
##            main    #102    +/-   ##
======================================
  Coverage   99.9%   99.9%            
======================================
  Files        135     139     +4     
  Lines      17470   17797   +327     
======================================
+ Hits       17469   17796   +327     
  Misses         1       1

Flag	Coverage Δ
linux	`99.9% <100.0%> (?)`
linux-arm	`99.9% <100.0%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

The design doc alone cannot live under `crates/`: `members` globs `crates/*`, so a directory without a manifest breaks `cargo metadata`, and `cargo sort --workspace` walks the same glob itself -- it ignores the workspace `exclude` that would otherwise paper over it, and 2.1.4 (pinned, and the latest release) has no ignore flag. So the crate lands as a skeleton and the design doc keeps its final path. Following `automation`, the crate is `publish = false` -- there is nothing worth releasing until the implementation exists -- and carries the documented `min-lines-percent = 0.0` coverage opt-out, since a crate with no executable code produces no instrumented regions and would otherwise be graded NO DATA. The implementation change removes both. Replaces the workspace `exclude` added in the previous commit. Verified locally: cargo metadata, cargo sort --check --check-format, clippy -D warnings, rustdoc -D warnings, fmt --check, cargo heather, ensure-no-default-features, ensure-no-cyclic-deps, and cargo-spellcheck all pass, and `cargo anvil --dry-run` reports nothing to write. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Fills in the skeleton with the check itself: read the workspace root's `[workspace.dependencies]`, enumerate members with `cargo metadata --no-deps`, and report every catalog entry that no member declares with `workspace = true`. `--fix` removes them through `toml_edit`, carrying a removed entry's comments to the next survivor so a group header keeps labeling the group it introduces. Addresses review feedback on the design: a manifest with no `[workspace]` table is no longer an error. cargo-anvil manages single-crate repositories too, and a generated recipe runs the same command everywhere, so a hard error would make the check unusable in exactly the repositories that never had the problem. It now reports the situation and succeeds -- the property holds vacuously -- with `--require-workspace` restoring the strict reading for callers that know they are pointing at a workspace root. Two unreachable paths were removed rather than left uncovered: `remove` takes the catalog table for granted (callers only fix a catalog they already read entries from) and the write-failure context is formatted eagerly instead of in a closure no test can portably reach. The package is at 100% line and function coverage. Verified end to end against this repository: it reports all 70 catalog entries as inherited, flags an injected entry, and `--fix` removes it and restores the manifest byte for byte. The crate is now publishable, so the skeleton's `publish = false` and its coverage opt-out are gone. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

martin-kolinek · 2026-08-26T10:21:54Z

🤖 Clawpilot here! Posted automatically by Clawpilot (an AI agent), not by a human. Please verify before acting.

Agreed, and changed — the missing-[workspace] case now succeeds instead of erroring.

Your reasoning is the deciding one: the generated anvil recipe runs the same command in every repo it manages, including single-crate ones. A hard error there would fail exactly the repositories that never had the problem, and each would need a local opt-out to get back to green. Succeeding is also the honest answer — "no catalog entry goes uninherited" holds vacuously when there is no catalog.

Implemented as you suggested, with the switch inverted so the lenient reading is the default:

$ cargo ensure-no-unused-workspace-deps                        # single-crate repo
ℹ️ Cargo.toml has no [workspace] table; there is no dependency catalog to check.   # exit 0

$ cargo ensure-no-unused-workspace-deps --require-workspace
❌ Cargo.toml has no [workspace] table.                                            # exit 1

--require-workspace keeps the original strict behaviour for callers that know they are pointing at a workspace root and want a misdirected --manifest-path to fail rather than pass quietly. Both paths are covered by tests (a_manifest_without_a_workspace_table_passes_with_a_note, require_workspace_rejects_a_manifest_without_a_workspace_table), and the design doc's exit-code section and a new "Manifests that are not workspace roots" section record the rationale.

The rest of the crate is implemented in the same push (4d7b097) — detection, --fix, allow-list, 18 integration tests, 100% line and function coverage. Verified end to end against this repo: all 70 catalog entries report as inherited, an injected entry is flagged, and --fix removes it and restores the manifest byte for byte.

Leaving this thread unresolved for you to close.

github-actions · 2026-08-26T10:26:37Z

⚠️ Breaking Changes Detected

error: failed to retrieve local crate data from git revision

Caused by:
    0: failed to retrieve manifest file from git revision source
    1: possibly due to errors: [
         failed to parse /home/runner/work/ox-tools/ox-tools/target/semver-checks/git-origin_main/f3db67486d159c2faf7ea3788a63c4b0eb93822d/crates/cargo-aprz-lib/tests/fixtures/tiny-virtual-workspace/Cargo.toml: no `package` table,
         failed to parse /home/runner/work/ox-tools/ox-tools/target/semver-checks/git-origin_main/f3db67486d159c2faf7ea3788a63c4b0eb93822d/Cargo.toml: no `package` table,
         failed to parse /home/runner/work/ox-tools/ox-tools/target/semver-checks/git-origin_main/f3db67486d159c2faf7ea3788a63c4b0eb93822d/crates/cargo-anvil/tests/fixtures/customized/Cargo.toml: no `package` table,
         failed to parse /home/runner/work/ox-tools/ox-tools/target/semver-checks/git-origin_main/f3db67486d159c2faf7ea3788a63c4b0eb93822d/crates/cargo_ensure_no_cyclic_deps/tests/fixtures/with_dev_cycle/Cargo.toml: no `package` table,
         failed to parse /home/runner/work/ox-tools/ox-tools/target/semver-checks/git-origin_main/f3db67486d159c2faf7ea3788a63c4b0eb93822d/crates/cargo_ensure_no_cyclic_deps/tests/fixtures/without_cycle/Cargo.toml: no `package` table,
         failed to parse /home/runner/work/ox-tools/ox-tools/target/semver-checks/git-origin_main/f3db67486d159c2faf7ea3788a63c4b0eb93822d/crates/cargo_ensure_no_cyclic_deps/tests/fixtures/with_self_dev_dep/Cargo.toml: no `package` table,
         failed to parse /home/runner/work/ox-tools/ox-tools/target/semver-checks/git-origin_main/f3db67486d159c2faf7ea3788a63c4b0eb93822d/crates/cargo-anvil/tests/fixtures/opt-outs/Cargo.toml: no `package` table,
         failed to parse /home/runner/work/ox-tools/ox-tools/target/semver-checks/git-origin_main/f3db67486d159c2faf7ea3788a63c4b0eb93822d/crates/cargo-anvil/tests/fixtures/migration/Cargo.toml: no `package` table,
         failed to parse /home/runner/work/ox-tools/ox-tools/target/semver-checks/git-origin_main/f3db67486d159c2faf7ea3788a63c4b0eb93822d/crates/cargo_ensure_no_cyclic_deps/tests/fixtures/with_cycle/Cargo.toml: no `package` table,
       ]
    2: package `cargo-ensure-no-unused-workspace-deps` not found in /home/runner/work/ox-tools/ox-tools/target/semver-checks/git-origin_main/f3db67486d159c2faf7ea3788a63c4b0eb93822d

Stack backtrace:
   0: <anyhow::Error>::msg::<alloc::string::String>
   1: <cargo_semver_checks::rustdoc_gen::RustdocFromProjectRoot>::get_crate_source
   2: <cargo_semver_checks::rustdoc_gen::StatefulRustdocGenerator<cargo_semver_checks::rustdoc_gen::CoupledState>>::prepare_generator
   3: <cargo_semver_checks::Check>::check_release::{closure#5}
   4: <cargo_semver_checks::Check>::check_release
   5: cargo_semver_checks::exit_on_error::<cargo_semver_checks::Report, cargo_semver_checks::main::{closure#5}>
   6: cargo_semver_checks::main
   7: std::sys::backtrace::__rust_begin_short_backtrace::<fn(), ()>
   8: main
   9: <unknown>
  10: __libc_start_main
  11: _start

If the breaking changes are intentional then everything is fine - this message is merely informative.

Remember to apply a version number bump with the correct severity when publishing a version with breaking changes (1.x.x -> 2.x.x or 0.1.x -> 0.2.x).

`cargo mutants` caught a real gap: deleting the `!` in `remove`'s `else if !carried.is_empty()` survived the suite. With that mutation the carry-forward never fires, but the pending comments are not lost -- the trailing-block path still appends them after the last surviving entry, so the comment remains somewhere in the file and a `contains` assertion stays green while the comment has silently left its group. The test now pins the position: the carried comment must precede the next surviving entry's own decor. That is the property the code exists to provide, and it fails under the mutation. 43 mutants, 41 caught, 2 unviable, 0 missed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Addresses seven review comments. `--fix` no longer truncates the workspace root in place. It writes a temporary file in the manifest's own directory and renames it over the original -- the house pattern from cargo-anvil -- so an interrupted run cannot leave the one file that breaks every other tool in the repo truncated. Before the rename the manifest is re-read and compared against the bytes that were parsed: `cargo metadata` runs in between as a child process, and an editor save landing in that window now aborts the fix instead of being silently overwritten. An empty catalog no longer skips the stale allow-list report. That is the boundary where *every* allowed name suppresses nothing, so it is exactly where the documented contract mattered most. Carried comments are now reported. The carry-forward cannot tell a group header from a note about one specific dependency, so a note about a removed entry lands on the next survivor and reads as if it were about that one -- worse than dropping it, because a dropped comment is visible in the diff and a wrong attribution is not. The relocation is printed on stderr, naming the source entries, the target, and the number of comment lines, and the hazard is documented in the design doc and crate docs. Tests for the two behaviours whose absence was noted: `--fix` keeps an allowed entry while removing the others (the one failure mode that destroys user data rather than printing something wrong), and member globs plus `exclude` follow Cargo, which is the reason the tool shells out to `cargo metadata` at all. The design doc no longer describes the anvil wiring as done; it is a follow-up, because anvil installs pinned tools from crates.io and the crate is unreleased. Adds the three artifacts `scripts/add-crate.ps1` would have produced: the crate's `CHANGELOG.md` scaffold, the root README crates entry, and the root CHANGELOG index entry. Only the scaffold is written -- release tooling owns changelog content. 100% line and function coverage; 53 mutants, 50 caught, 3 unviable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

The new `write_back` unit tests do real filesystem work in a temp directory, and the anvil miri leg runs lib unit tests under filesystem isolation, so `mkdir` came back unsupported and `anvil-miri` failed. Guards the module with `#[cfg(not(miri))]`, the same way every other filesystem-touching test in this repo is guarded. Coverage is unaffected: the coverage run does not use miri. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Addresses two review comments; both reproduced against the built binary before changing anything. **Replacing by rename brought the temp file's identity with it.** `NamedTempFile` creates its file at mode 0600 (confirmed in tempfile-3.20.0 `src/file/imp/unix.rs:24`), and a rename carries the source mode rather than inheriting the target's, so on Unix a 0644 workspace root came back owner-only after every successful --fix -- a change git does not track. `persist` also replaced a symlinked manifest with a regular file, where the previous in-place write followed the link. Now the manifest's permissions are read up front and applied to the replacement before the rename, and the path is canonicalized first so the rename lands on the real file. Both choices are stated in the doc comment. **The carry report could claim moves that never happened.** With a dotted last survivor (`serde.version = "1"`) the append is skipped -- only a plain value has a suffix -- but the `Carry` was pushed regardless, so stderr claimed the comment had been carried onto `serde` while it was absent from the output. Reproduced exactly as described. `onto` is now derived from the operation that actually placed the text, so the unplaceable cases report a drop, and the message no longer asserts a reason ("every entry was removed") that is false for the dotted case. **Comments on removed sub-table entries vanished unreported.** For `[workspace.dependencies.name]` the decor lives on the table, not the key, so `comments_of` saw an empty prefix and no `Carry` was recorded. `decor_prefix` now reads both. Tests: dotted-last-survivor drop, sub-table carry, and the stderr assertion the trailing-removal path was missing. Adds symlink terms to `.spelling`. 100% line and function coverage; 54 mutants, 52 caught, 2 unviable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Both reported cases reproduced against the built binary first. `toml_edit` keeps an entry's leading comments in one of three places: on the key for a plain value, on the table for a `[workspace.dependencies.name]` sub-table, and -- measured, not assumed -- on the *first inner key* for a dotted `name.version = "1"`. The previous push read the table slot but always wrote the key slot, so a sub-table survivor had its own comment rendered twice: `--fix` put a line into the manifest that the user never wrote. A dotted survivor was worse in the other direction -- setting its outer key decor renders nothing, so the carried comment was lost while stderr still claimed it had been carried. `leading_comments` and `prepend_comments` now resolve the same slot, so what is read is what is written. That removes the duplication, and it also lets a dotted survivor carry the text properly rather than dropping it, which is better than the reported failure mode required. The same lookup fixes a third case neither comment covered: a removed *dotted* entry's comment lived on the inner key, so it used to vanish with no `Carry` recorded at all -- the exact contract violation the reporting exists to prevent. It is now carried and reported. Trailing removals are unchanged: carried text has to land after the final survivor and only a plain value has a suffix, so a sub-table survivor there still drops the comments -- reported, not silent. Also corrects the `Carry::onto` doc, which still described `None` as meaning the table was emptied. 100% line and function coverage; 60 mutants, 58 caught, 2 unviable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Adds a new Ox Tools crate, cargo-ensure-no-unused-workspace-deps, implementing a manifest-only check (with optional --fix) that fails when a [workspace.dependencies] entry is inherited by no workspace member, alongside a design doc and comprehensive integration tests.

Changes:

Introduces the new cargo-ensure-no-unused-workspace-deps crate (CLI, detection logic, and --fix manifest rewrite).
Adds a full design document describing the rule, UX, and --fix comment-carry semantics.
Adds integration tests that exercise inheritance detection, allow-list behavior, and --fix rewriting/reporting.

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
README.md	Adds the new crate to the repository’s crate list.
CHANGELOG.md	Adds the new crate to the top-level changelog index.
Cargo.lock	Adds the new crate package entry to the lockfile.
.spelling	Adds “symlink*” words used in the new docs/messages.
crates/cargo-ensure-no-unused-workspace-deps/Cargo.toml	New crate manifest and dependencies.
crates/cargo-ensure-no-unused-workspace-deps/src/main.rs	Binary entry point wiring into the library `run()`.
crates/cargo-ensure-no-unused-workspace-deps/src/lib.rs	CLI surface, member enumeration via `cargo metadata`, reporting, and atomic rewrite logic.
crates/cargo-ensure-no-unused-workspace-deps/src/detect.rs	Manifest scanning to detect which catalog entries are inherited.
crates/cargo-ensure-no-unused-workspace-deps/src/fix.rs	`--fix` removal + comment carry/drop behavior using `toml_edit`.
crates/cargo-ensure-no-unused-workspace-deps/tests/integration_tests.rs	End-to-end integration tests running the compiled binary against temp workspaces.
crates/cargo-ensure-no-unused-workspace-deps/docs/design/README.md	Design doc covering motivation, rule, UX, and CI integration plan.
crates/cargo-ensure-no-unused-workspace-deps/README.md	Crate README describing purpose, usage, config, and behavior.
crates/cargo-ensure-no-unused-workspace-deps/CHANGELOG.md	New crate changelog file scaffold.
crates/cargo-ensure-no-unused-workspace-deps/logo.png	New crate logo asset (LFS pointer).
crates/cargo-ensure-no-unused-workspace-deps/favicon.ico	New crate favicon asset (LFS pointer).

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

The round-three rewrite narrowed the trailing-removal note to "sub-table survivor", but the branch still drops for a dotted survivor too: `Item::as_value_mut` returns `None` for a dotted `Item::Table`, which is what `fix_reports_a_drop_when_the_last_survivor_cannot_carry_comments` pins with a trailing `serde.version = "1"`. Restores both forms in the code comment and design §5, and says why the two directions differ: a dotted survivor can be carried *onto* -- the comments go ahead of it, on its first inner key -- and only appending *after* one is impossible. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.

The exit-code section still said an empty catalog has "nothing to be stale", which stopped being true when the stale allow-list report moved ahead of that early return. `a_stale_allow_entry_is_reported_against_an_empty_catalog` pins the behaviour the sentence contradicted. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 14 out of 15 changed files in this pull request and generated no new comments.

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

crates/cargo-ensure-no-unused-workspace-deps/src/fix.rs:16

Outcome::carries includes both comment blocks that are successfully carried onto a survivor and comment blocks that are ultimately dropped (onto: None). The current field doc says "moved", which is misleading given the Dropped ... reporting and Carry::onto semantics.

This issue also appears in the following locations of the same file:

line 20
line 36

    /// Comment blocks that moved off a removed entry.

crates/cargo-ensure-no-unused-workspace-deps/src/fix.rs:23

Carry records both successful carries and drops (onto: None), so the doc comment "Only recorded when comments actually moved" is incorrect. It’s still true that from is never empty, but the condition is that at least one comment line existed on removed entries (carried or dropped).

/// Comments that belonged to removed entries and had to go somewhere else.
///
/// Only recorded when comments actually moved, so `from` is never empty.
///

crates/cargo-ensure-no-unused-workspace-deps/src/fix.rs:36

Carry::lines is also used when comments are dropped (onto: None), so describing them as lines that "moved" is inaccurate.

    /// How many comment lines moved.

Martin Taillefer (geeknoid) · 2026-08-30T04:05:40Z

martin-kolinek Would it be worth expanding the scope of this tool to completely replace cargo-udeps? So simultaneously check whether each project has superfluous dependencies, and then whether the workspace has superfluous dependencies?

martin-kolinek and others added 2 commits August 25, 2026 18:12

martin-kolinek changed the title ~~docs: design for cargo-ensure-no-unused-workspace-deps~~ feat: design and skeleton for cargo-ensure-no-unused-workspace-deps Aug 25, 2026

martin-kolinek changed the title ~~feat: design and skeleton for cargo-ensure-no-unused-workspace-deps~~ feat: add cargo-ensure-no-unused-workspace-deps Aug 26, 2026