fix(fix): decline to rewrite a wildcard constraint - #89
Merged
Conversation
`rewrite_constraint` has three guards -- a comma, a space or `|` after the operator prefix, and a leading letter -- and none of them sees a wildcard. For `1.x` the operator prefix is empty, the rest holds no space or pipe, and it starts with a digit, so the function returns the target version and the range the author wrote collapses to a pin. The witness in the core checker proves the path is live rather than dead code: `1.x` against `1.0.0, 1.9.0, 2.0.0` is `UpdateAvailable` with a compatible target of `1.9.0`, which is exactly what default `fix` hands to `rewrite_constraint`. The behavioural case then shows a `package.json` pinned to `1.9.0` where `1.x` was asked for -- in npm a bare version is an exact match. The two existing asserts that a bare `*` is pinned are reversed here. Issue the reversal is the issue's intent.
A wildcard is a range the author chose, not a version, so substituting a concrete release narrows their manifest. Decline it beside the dist-tag guard: `rest` is `*`, or any of its dot-segments is `*`, `x`, `X`, or `+`. None of those is legal in a concrete version, so nothing rewritable is caught, and Gradle's `1.+` has the same shape as npm's `1.x`. Decline rather than widen. Widening `1.x` to `2.x` needs to know how the ecosystem spells a wildcard, and this layer sees only the raw string with no `Ecosystem` to ask. It would also be wrong for NuGet, where `1.*` and a bare `2.0.0` mean genuinely different things -- a bare NuGet version is an inclusive minimum, not a pin. Declining leaves the floating constraint the author wrote, which is what they asked for.
The guard matched the whole dot-segment, so a Composer stability flag walked straight past it: `2.8.*@dev` splits into `["2", "8", "*@dev"]`, no segment equals a wildcard, and `--fix --all` rewrote the constraint to `7.0.0` — an exact pin in Composer that loses both the wildcard and the flag. That is the Cut the stability flag off before splitting, and match `*` and `+` by their leading character, since neither can legitimately begin a version segment: semver build metadata and Go's `+incompatible` attach their `+` to the end of a numeric segment (`0+incompatible`), never the start. `x`/`X` stay an exact whole-segment match, because a letter can legitimately lead an identifier inside a prerelease or build (`1.0.0-alpha+exp.sha.5114f85`). Every concrete form the shipped parsers emit is asserted still rewritable — Go pseudo-versions, `+incompatible`, semver build metadata and prereleases, the spec's own `1.0.0-x.7.z.92`, NuGet four-part versions and bracketed ranges, Python epochs and `~=`, Hex's `~>` — so the widened guard cannot quietly stop `fix` from working on ordinary dependencies. The `--all` target-selection branch had no coverage at all, which is where this lived: `results_for` never set `latest_available`, so no test could reach it. It does now, and the wildcard case is asserted under `--all` alongside a non-wildcard neighbour that must still be fixed.
This was referenced Sep 1, 2026
`rewrite_constraint` split a Composer stability flag off before testing for
a wildcard, but the rewrite emits only `{prefix}{new_version}` — nothing
carries the flag forward. A flag hung off a plain version therefore passed
every guard and was rewritten flag-free: `">=2.8@dev"` became `">=7.0.0"`,
and the unbounded `"@dev"` ("any version, dev stability") became the exact
pin `"7.0.0"` — issue #87's harm reached without a wildcard.
A stability flag qualifies a range this layer cannot reconstruct, so decline
every `@` form outright, as a dist-tag is already declined. That subsumes the
`split('@')` inside `is_wildcard`, which is simplified away, and makes the
npm alias decline (`npm:pkg@1.0.0`) explicit rather than an accident of the
alphabetic dist-tag guard.
No concrete form the shipped parsers emit contains an `@`, so nothing that
rewrote before stops rewriting.
This was referenced Sep 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #87.
The gap
rewrite_constraintincrates/dependable/src/fix.rsdeclines the forms it cannot rewrite without changing their meaning: anything containing,, anything with a space or|after the operator prefix, and anything starting with a letter once that prefix is stripped (the dist-tag guard, which keepslatestandnextfrom being pinned).A wildcard falls through all three. For
1.xthe operator prefix is empty, sorestis1.x— no comma, no space, no pipe, and it starts with a digit. The function returns the target version, and the range the author wrote becomes a pin.Reachable by default
The issue recorded the path as read from source but not reproduced, so the first commit is a witness rather than a patch.
check_version("1.x", &["1.0.0", "1.9.0", "2.0.0"], None)isUpdateAvailablewithlatest_compatible == 1.9.0: the wildcard matches1.0.0and1.9.0but not2.0.0, so the newest release sits outside the range. Defaultfix— no--all, no lockfile — takeslatest_compatibleand hands1.xtorewrite_constraint, which rewrites"lodash": "1.x"to"lodash": "1.9.0". In npm a bare version is an exact match, so the floating constraint collapses to one release.1.x,1.*, and1.Xare reachable this way;*needs a lockfile behind the newest, and1.+needs--all. Barexandanywere caught only by accident, because they happen to start with a letter.The change
Decline, beside the dist-tag guard:
restis*, or any dot-segment ofrestis*,x,X, or+.Declining rather than widening (
1.x→2.x) is deliberate. Widening needs per-ecosystem knowledge of how a wildcard is spelled, and this layer sees only the raw string with noEcosystem. It would also be wrong for NuGet, where1.*and a bare2.0.0mean different things — a bare NuGet version is an inclusive minimum. Declining leaves the author with the floating constraint they asked for, exactly as a dist-tag is left alone.One documented behaviour is reversed
Two existing asserts required a bare
*to be pinned, one of them with a comment justifying it. Both now assert the opposite, and the comment says so rather than being deleted. Issue #87 names*alongside1.xand1.*as "ranges, not versions", so this is the issue's intent. Nothing else depends on it — the PRD andSCOPE.mddocument onlylatest.Tests
rewrite_never_narrows_a_wildcard_to_a_pin—1.x,1.*,1.X,1.+,^1.x,1.2.x,*all decline.a_wildcard_dependency_is_left_untouched_by_fix— apackage.jsonwith"lodash": "1.x"yields no fix record and a byte-identical manifest.wildcard_constraint_is_reported_as_upgradable— the reachability witness in the core checker.All four failed on the first commit and pass on the second.
Folded in on review: a constraint carrying an
@is declinedrewrite_constraintsplit a Composer stability flag off before testing for a wildcard (let versionish = rest.split('@').next()...), which was right for the wildcard test but not for the rewrite: the function emits onlyformat!("{prefix}{new_version}"), so nothing carries the flag forward. A flag hung off a plain version therefore passed every guard and was rewritten flag-free. Verified end to end against the real parser andcheck_versionunderdependable fix --all, newest7.0.0:"@dev""7.0.0"">=2.8@dev"">=7.0.0""2.8@dev""7.0.0""^1.0@beta""^7.0.0"The
"@dev"row is #87's own harm class — a range collapsed to a pin — reached without a wildcard. It is pre-existing: the old guard also returnedSome(">=7.0.0")for">=2.8@dev". The@handling on this branch made it visible and cheap to close.rewrite_constraintnow declines anyrestcontaining'@'outright, as a dist-tag is already declined: a stability flag qualifies a range this layer cannot reconstruct. That subsumes thesplit('@')insideis_wildcard, which is simplified away, and makes the npm-alias decline (npm:pkg@1.0.0) explicit rather than an accident of the alphabetic dist-tag guard.No false positive: none of the concrete forms the shipped parsers emit contains an
@— Go pseudo-versions,v2.0.0+incompatible,1.2.3+build.5,1.0.0-alpha+exp.sha.5114f85,1.0.0-x.7.z.92, NuGet1.0.0.4, Python1!2.0and~=1.4, Hex~> 1.0— andrewrite_leaves_every_concrete_version_form_rewritableasserts each by name so it stays that way.rewrite_declines_a_stability_flag_on_a_plain_versioncovers"@dev",">=2.8@dev","2.8@dev","^1.0@beta", and"npm:pkg@1.0.0". Reverting the guard makes it fail on the first four (the alias was already caught incidentally);"*@dev"and the rest of the wildcard suite stay green either way.Note
output::tree::tests::ascii_points_a_member_at_its_own_treefails in some local shells, on this branch and onmasteralike. It is not a regression and not related to this change: the test asserts on plain substrings, and the renderer emits ANSI colour whenFORCE_COLORis set in the environment, so the assertion fails on the escape codes.env -u FORCE_COLOR cargo test --bin dependablepasses onmasterand on this branch. CI does not setFORCE_COLOR, which is why it stays green there.Worth pinning colour off in that test so it cannot depend on the caller's environment, but that is unrelated to #87 and is left alone here.
Unresolved review notes
Review of this branch raised five further findings. None is repaired here; each is tracked or recorded below.
Per-ecosystem wildcard handling — Medium —
crates/dependable/src/fix.rs,rewrite_constraint— tracked as #92Claim. The decline is blanket because
rewrite_constraintcarries noEcosystemin its signature — but the ecosystem is reachable one call up:apply_fixestakesmanifest: &Path, andManifestKind::detect(path).ecosystem()is two lines away. So declining everywhere is conservatism, not necessity. An earlier version of the guard comment claimed the layer could not see the ecosystem; that was wrong and has been corrected.Scenario. Cargo
serde = "1.*", locked at1.0.100,1.0.219published.checkreportsPatchAvailable. Before this branch,fixwroteserde = "1.0.219"— which Cargo reads as^1.0.219, still a range and the idiomatic form. Now the manifest is left alone. The npm harm this branch fixes is real, but the same substitution was correct for Rust, Go, and Dart, where a bare version is a caret or an inclusive minimum rather than a pin.Direction. Thread
Ecosystem(or a narrowerbare_version_is_exact: bool) fromapply_fixesintorewrite_constraintand decline only where a bare version pins — npm, Composer, Hex. NuGet stays declined on its own merits.Why not here. It contradicts the approved plan for #87, which chose the blanket decline deliberately. The maintainer elected to ship the conservative guard and refine it separately.
npm partial versions are still narrowed — Low —
crates/dependable/src/fix.rs,rewrite_constraint— tracked as #92Claim. npm treats a partial version as an X-range:
"16"≡16.x,"1.0"≡1.0.x. Neither contains a wildcard character, sois_wildcarddoes not see them and the rewrite proceeds.Scenario.
"react": "16"with versions up to16.14.0and18.2.0published becomes"react": "16.14.0"— an exact pin, from a constraint that floated across all of 16.x. The same class of harm as #87, reached by a different spelling. It is pre-existing rather than introduced here.Direction. Cannot be fixed without the ecosystem, because for Cargo
"1.0"→"1.5.0"is correct. Depends on the finding above."Everything is already up to date." after declined wildcards — Low —
crates/dependable/src/runner.rs,run_fix— tracked as #93Claim.
run_fixreports success from an empty record list, so a declined-but-updatable item is indistinguishable from nothing to do.Scenario. A
package.jsonwhose only dependency is"lodash": "1.x", with1.9.0in range:checkreports an available update,fixprints "Everything is already up to date." and exits 0, and--dry-runprints nothing. Not new in kind — dist-tags and compound ranges were already silent — but this branch widens the silent set.Direction. Have
plan_fixesreturn the declined-but-updatable items alongside its records, and emit anote:line in the registerreport_inherited_skipsalready uses for exactly this class of contradiction.Why not here. It lives in
runner.rsand inplan_fixes' return type, both outside this branch's manifest.An interior
xsegment is declined when it should not be — Low —crates/dependable/src/fix.rs,is_wildcardClaim.
is_wildcardmatchesx/Xas a whole dot-segment, which a semver prerelease or build identifier can legitimately be:1.0.0-alpha.x.1splits to["1", "0", "0-alpha", "x", "1"]and1.0.0+build.xto["1", "0", "0+build", "x"]. Both are declined as wildcards though neither floats.Why not here. Pre-existing — the same whole-segment match shipped in the first commit of this branch — contrived, and unrelated to the
@change. No shipped parser has been observed emitting either shape. Recorded so the next person touchingis_wildcardknows the case exists.NuGet
[1.0]is widened to a floating minimum — Low —crates/dependable/src/fix.rs,rewrite_constraintClaim. NuGet's bracketed exact form
[1.0]holds no comma, no space, no|, no letter, and no wildcard, so it is rewritten to[1.5.0— or, depending on how the span is recorded, to a bare version, which NuGet reads as an inclusive minimum. Either way the author's exact pin is not preserved.Why not here. Pre-existing, outside this diff, and in a different guard than the one this branch touches.
[1.0,2.0)is already declined by the comma guard; only the comma-free bracketed form falls through. Recorded, not fixed.