refactor(lib)!: mark ParseOutput and UsageErr non_exhaustive - #772
refactor(lib)!: mark ParseOutput and UsageErr non_exhaustive#772JamBalaya56562 wants to merge 1 commit into
Conversation
Ten public types in usage-lib already carry `#[non_exhaustive]` — `Spec`, `SpecArg`, `SpecFlag`, `Parser` among them. These two were missed, and jdx#762 ran into the consequence: adding a variant to `UsageErr` and two fields to `ParseOutput` were each reported as breaking on their own. Neither type is one a caller builds or matches exhaustively. `ParseOutput` is what `parse()` returns; its only struct literal is in `parse.rs` itself, and nothing outside the crate constructs one. `UsageErr` is an error type, where a wildcard arm is the normal shape. Nothing in the workspace matches it — the CLI uses it as a return type and downcasts once. This also restores a signal that was lost. The squash of jdx#762 took its subject from the PR title, which had no `!`, so the `BREAKING CHANGE:` footer on the branch commit never reached main: fix(parse): enforce double_dash="required" for positional args (jdx#762) git-cliff therefore reads main as holding only a `fix:` since v4.1.0 and would compute v4.1.1 for the next release — shipping two breaking API changes as a patch. Marking a type `#[non_exhaustive]` is itself a breaking change, so the `!` here is the honest description of this commit, and it puts the major back within reach of `--bumped-version`. The version in Cargo.toml is deliberately untouched: `tasks/release-plz` publishes and tags as soon as main carries a version with no matching tag, so a hand-written bump in a feature PR would publish on the merge push with no changelog entry and no release PR. `lint:semver` therefore stays red until the release PR bumps it, which is the state main has been in since jdx#762 landed. BREAKING CHANGE: `ParseOutput` can no longer be constructed with a struct literal outside usage-lib, and `UsageErr` can no longer be matched exhaustively outside it — a wildcard arm is now required. Both replace the narrower breaks jdx#762 introduced with the durable form.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR marks the public ChangesPublic API updates
Estimated code review effort: 1 (Trivial) | ~3 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR marks two public usage-lib result types as non-exhaustive to make future additions forward-compatible and restore the intended major-version release signal.
Confidence Score: 5/5The PR appears safe to merge, with no concrete regressions identified in current workspace consumers. The attributes intentionally tighten downstream construction and matching rules, while all current workspace usage remains compatible and runtime behavior is unchanged. Important Files Changed
Reviews (1): Last reviewed commit: "refactor(lib)!: mark ParseOutput and Usa..." | Re-trigger Greptile |
|
CI is red on That is this PR's own change reported back. It cannot go green here: main has been red the same way since #762 landed, because its squash took the subject from a PR title I forgot to mark |
Two attributes, and a repair for the version signal #762 lost.
The consistency part
Ten public types in
usage-libalready carry#[non_exhaustive]—Spec,SpecArg,SpecFlag,SpecCommand'sSpecExample,Parser, and others.ParseOutputandUsageErrwere missed, and #762 ran straight into the consequence: addingnext_arganddouble_dash_seento one, andArgRequiresDoubleDashto the other, were each reported as breaking on their own.Neither is a type a caller builds or matches exhaustively:
ParseOutputis whatparse()returns. Its only struct literal is inparse.rsitself; nothing outside the crate constructs one.UsageErris an error type, where a wildcard arm is the normal shape. Nothing in the workspace matches on it —usage-cliuses it as a return type and downcasts once, incli/src/lib.rs.cargo build --all --all-featurespasses untouched, which is the workspace-level check that neither pattern exists.The repair part
main has been red since #762 merged, and this fixes the cause.
The squash took its subject from the PR title, which had no
!, so theBREAKING CHANGE:footer on the branch commit never reached main. What landed was:git-cliff therefore reads main as holding only a
fix:since v4.1.0, and--bumped-versionwould compute v4.1.1 — shipping two breaking API changes as a patch. Every PR opened against main is red in the meantime, becauselint:semvercompares Cargo.toml (4.1.0) against crates.io (4.1.0), calls itno change; assume patch, and any major finding fails.Marking a type
#[non_exhaustive]is itself a breaking change, so the!on this commit is the honest description of what it does, not a marker added to game the version. And it puts the major back within reach of--bumped-version, so the next release computes v5.0.0 and covers #762 as well.What
lint:semversays nowStill two failures, but different — and better ones:
constructible_struct_adds_fieldandenum_variant_addedare gone: once a struct cannot be constructed externally and an enum cannot be matched exhaustively, adding to either is no longer separately breaking. The attribute replaces two one-off findings with the durable statement, and that is the point of adding it now rather than later.This PR does not turn
lint:semvergreen. It goes green when the release PR sets Cargo.toml to 5.0.0. Merging this makes that release be 5.0.0.Why the version is not bumped here
tasks/release-plzopens withwhich is the publish half of the release-PR flow. A hand-written bump in a feature PR is indistinguishable from a merged release branch, so it would publish to crates.io on the merge push — no CHANGELOG entry, no release PR, outside the cadence guard in
auto-merge-release.yml. I tried exactly that on #762 and backed it out.Verified
cargo build --all --all-features,cargo test --all --all-features,cargo fmt --all -- --checkandcargo clippy --all --all-features --all-targets -- -D warningsall pass on Linux.cargo semver-checks check-release -p usage-libfails with the two findings above, which is the intended outcome.Three more types, if you want them
The same gap exists on
SpecCommand(lib/src/spec/cmd.rs),ParseValue(lib/src/parse.rs) andSpecDoubleDashChoices(lib/src/spec/arg.rs). Since this release already spends a major, folding them in costs nothing extra — say the word and I will.I left them out because one of them is not free.
ParseValueis what consumers match to read a parsed value, so#[non_exhaustive]forces a wildcard arm on mise and anyone else doing the same. That is a judgement about downstream ergonomics rather than about semver hygiene, and it seemed yours to make.SpecCommandandSpecDoubleDashChoicescarry no such cost.This pull request was generated by Claude Code.
Summary by CodeRabbit