test(compat): gate the ADR 0005 rename table against the deprecation registry - #557
Merged
Conversation
…registry DeprecationRegistryTest scans src/ for Deprecations::notice() calls and requires each one to be registered. That catches a deprecation nobody registered; it cannot catch the opposite and likelier failure, a rename that ships no notice at all. Nothing is emitted, so nothing is scanned, and the registry stays as correct — and as empty — as it was. Today it holds one entry against roughly forty renames ADR 0005 fixes. Add the reverse direction. v3-renames.json transcribes every old spelling ADR 0005's two tables name, with the channel it uses and the deprecation id once staged; V3RenameRegistryTest parses those tables and requires the fixture to cover them, so a row nobody transcribed fails rather than going unnoticed. unstaged_count is a ratchet asserted in both directions: an unstaged addition and a stale number each fail with their own message. The ordering this protects is unrecoverable. The first breaking commit on main turns the pending release into 3.0.0, after which no further v2 release can be cut, so a rename whose deprecation was forgotten before that point cannot be given one without spending another major. #499 assigned entry ownership to the individual v3 issues and left the cross-check as a review rule; this makes it arithmetic. No src/ change. Both conformance baselines unchanged. Closes #556
…without staging Review found the gate accepted three shapes it exists to reject, all reproduced before fixing. An entry counted as staged on the strength of an id that existed, so pointing spec_base_path at laravel.config.auto_inject_dummy_bearer passed: both carry removed_in 3.0 and nothing compared anything else. The registry entry's surface must now name the spelling being deprecated and its replacement must name the v3 name ADR 0005 gives it, compared with the backticks taken out because both fields are prose. One id may stage only one spelling. unchanged-spelling asked only for a null removed_in, so relabelling --json-output and lowering the count passed — the one channel that stages nothing and counts toward nothing was also the easiest to claim. It now has to hold twice: the entry replaces itself, and ADR 0005's own row names the same spelling in its v3 column. Four rows keep their name; only --output-file keeps its meaning, so the derivation is permission rather than instruction and the other three stay on deprecation. The accepted-spelling check ran fixture to LegacyIdentity only, so deleting OPENAPI_VALIDATION_OUTPUT passed while the spelling kept working and kept needing a removal plan. The two are now compared as one set. Parsing moved from strtok() to explode(): strtok() returns false for a token that is only the delimiter, which PHPStan's stub does not model, and explode() reports the same case as a value the guard can see. Nine mutations checked, each failing its intended assertion; src/ untouched and both conformance baselines unchanged.
…er fixture Second review round found three more shapes the gate accepted, all reproduced first. acknowledged_unvalidatable_schemes was listed by hand because ADR 0005's cell describes it — "same name, both surfaces" — instead of naming it, so the ADR scan could not see it and deleting the entry passed. Rather than add a hand-maintained list of hand-maintained entries, the ADR now names the spelling in backticks with the same prose kept as an aside. That is editorial: it contradicts no decision, spells out one the ADR already made, and leaves every fixture entry answerable to either the ADR tables or LegacyIdentity's maps. A new test asserts exactly that, so the next entry belonging to neither source fails instead of being deletable in silence. unchanged-spelling admitted any row whose v3 column repeated the spelling, which is four rows, not one. Three of them keep the name while replacing the value it accepts — a removal to anyone who wrote the old value down — and the difference lives in the value grammar the parser deliberately discards. The derivation is deleted for a one-name list: this channel stages nothing and counts toward nothing, so entry should cost a deliberate edit rather than a property a relabelled entry can acquire. Neither fixture ever checked its replacement against the ADR, so they could agree with each other about a v3 target that does not exist — including a staged pair moved wholesale by editing both files together. Replacements are now matched against the v3 names of their own ADR row, and a "— removed —" row must say it has no successor rather than invent one. Fourteen mutations checked, each failing its intended assertion; src/ untouched and both conformance baselines unchanged.
…5 gives it Third review round. All three findings reproduced before fixing. A grouped ADR row maps several spellings onto several members of one v3 key, and the check accepted a replacement containing ANY of the row's tokens. So min_endpoint_coverage could point at coverage.min_coverage['response'] and still pass: the key was right, only the member was wrong. The pairing was never in the ADR in machine-readable form, so the fix puts it there. The thirteen grouped rows now write `old` → `new` per spelling — the arrow the ADR already uses for this meaning at :51 and :255 — and the comparison becomes exact. Three fixture replacements change to match what the ADR now states literally; the ADR's decisions do not, and the spelling set it yields is identical. UNCHANGED_SPELLINGS was checked one way, so moving --output-file onto deprecation and raising the count read as progress on the ratchet while nothing was staged. Constant and fixture are now the same set. surface and owner were any non-empty string, which accepts surface: cli-flag on a configuration key because the wrong value is a legal one. A closed enum would not have helped for the same reason, so surface is derived instead: from which ADR table the row sits under, and from which LegacyIdentity map the spelling appears in. owner must look like an issue reference. Fourteen mutations checked, plus four that tamper with the ADR rather than the fixture. What still gets through is an in-row swap performed in the ADR and the fixture together, which is a change to the source of truth rather than a drift away from it. src/ untouched; both conformance baselines unchanged.
…prose Fourth review round. All three findings reproduced before fixing. An id counted as staging whichever spelling the registry's prose happened to mention, so moving auto_inject_dummy_bearer's id onto its sibling key and naming both in one sentence passed while the notice went on announcing the old key. The registry now carries `spelling` and `v3_target` beside the two prose fields — the same two facts in the form a test can compare by equality — and the prose still has to agree with them. That alone would only move the lie one field along, so DeprecationRegistryTest reads the `subject` argument of each notice out of src/ and requires the entry to name the surface its own call announces. The scenario the review demonstrated now fails even when the fixtures are edited together, because the call site disagrees. removed_in accepted any non-empty string, so a deprecation re-dated to 4.0 kept its notice, kept its entry, and quietly stopped being something v3 has to finish. Each channel now has exactly one removal: 3.0 for a deprecation, LegacyIdentity's own version for an accepted spelling, none for a spelling that survives. The member check searched for a member inside the target, so coverage.min_coverage['response_typo'] passed on the strength of containing 'response'. Members are now parsed out of the target — array subscript or collapsed grammar, the two forms a target can take — and compared as a set against the ones the row enumerates. Twelve mutations checked this round plus six regressions from earlier rounds; src/ untouched and both conformance baselines unchanged.
…ment The registry only ever had the notice's id checked against `src/`, so a notice could be re-pointed at another target, re-dated off the v3 milestone, or moved to a different surface while the ledger a reviewer reads went on describing the notice that used to be there. Entries now carry the call's three prose arguments verbatim and are compared to the source one by one. The ADR target check split its key from its members but compared the key with `assertStringStartsWith`, which accepted `coverage.min_coverage_typo['response']` under `coverage.min_coverage`. Targets are parsed whole into a key and its members, and both halves are compared exactly.
A second Deprecations::notice() call under an id already seen overwrote the
first in the scan, so an incorrect notice could be parked in front of a
correct one — and the channel dedups per id, meaning the incorrect one is the
only one a process ever emits. All of src/ is now scanned in one pass and a
reused id fails, keeping the call a consumer actually reads.
A grouped `use A\{B}` import was not parsed, so a call through one resolved
against the current namespace, missed the emitter, and left the scan without
ever demanding a registry entry.
An ADR row that collapses no key enumerates no members, and the member check
skipped exactly those rows, so any subscript could be added to their targets.
The comparison now runs on every row, with the empty set as the answer.
… every structure Four reported holes: `namespace\Deprecations::notice()` is a T_NAME_RELATIVE the scanner did not list, so the call resolved to nothing and left the gate. `stripslashes()` matched neither of PHP's quoting rules — it ate the backslash out of `'C:\new'` and left `\x41` as text — so the registry could record a string no consumer ever reads. Literals are decoded by PHP's own rules now, and what cannot be decoded faithfully fails. A target's members were flattened to a set of names, losing the depth of an array subscript chain and whether a CLI grammar member carries a value. Targets are parsed into a key, a notation and a member => carries-a-value map, and all three are compared to what the row declares. A notice's surface was a substring test, which "The CLI flag 'x' (not a config key)" passed while saying the opposite. The subject now has one shape, built from the surface the ADR assigns and the spelling being deprecated. Nine more found by an adversarial sweep of the same gate: `bin/gesso` was never scanned though it owns the CLI surface; a direct `trigger_error()` or a PHP 8.4 `#[\Deprecated]` bypassed the emitter; `use A, B;` clauses after the first, second namespace blocks, dynamic dispatch and callable construction all hid a call; ADR rows without a leading pipe, without backticks, with an arrow inside a parenthetical, or delimited with ASCII dashes all left the scan; and the ratchet, the spelling count and `owner` each certified themselves.
`Deprecations::$method(...)` selects the method at runtime and reached the emitter with nothing to resolve. It joins the other escapes; the trailing `(` is what tells it from `self::$counts`, which the emitter reads three of. Both halves of `trigger_error(..., E_USER_DEPRECATED)` can be renamed by an import, and the check compared the written spelling of each. Function and constant imports are resolved to their fully qualified names, so an alias onto the channel is found and an alias off it is not mistaken for one. `#[\Deprecated]` was read as the one token after `#[`, so it was invisible second in a group and under an alias. Attribute groups are read to their `]` and each name resolved as the class reference it is. `owner` only had to be some issue the ADR mentions, so two entries could swap theirs and still look routed. It is derived per spelling now, applying ADR 0005's rule 4 to the division of labour the ADR sets out; `console_output`, which section C gives to #502 in prose, gains the same inline marker the removals already carry.
…an attach to
The severity of a trigger_error() call was matched by the constant's spelling
among all its arguments, so a numeric 16384 — the value that spelling has —
raised the channel with nothing to see it. The error_level argument is read as
an argument now, by name or by position, and one this cannot evaluate counts
as the channel rather than being waved through. A string naming the function
counts too: call_user_func('trigger_error', …) raises the same notice.
Attribute names are class names, and PHP resolves those case-insensitively.
`#[\deprecated]` is the built-in attribute and was compared as though it
were not.
Listing the ways a member can be selected at runtime let each new syntax
through in turn — `::${'notice'}` after `::{'notice'}` after `::$method(`. The
rule is inverted: a literal member name is the one form the scan can read, and
everything else on a reference to the emitter is reported. A string naming the
emitter is reported for the same reason, since it leaves no class token at all.
A severity the scan could not evaluate was read as "not the deprecation channel", which is the one thing it cannot know: `const LEVEL = E_USER_DEPRECATED` passed as a fourth name for something else. Only the four severities trigger_error() accepts can answer no; anything else is unreadable and counts. Integer literals are read in every base PHP writes them in, so 0x4000 is no longer zero. `user_error()` is PHP's own alias for the same function, and a relative `namespace\trigger_error()` resolves to it in a file with no namespace, which is what bin/gesso is. Calls resolve through the same name resolution the notice scan uses, so both reach the channel. A nowdoc body is a static string that is not a quoted literal, so a callable spelled out in one was invisible. Heredoc and nowdoc bodies are read too. A removed row's replacement only had to start with `none`, leaving the rest of the field free to name the successor the row denies having. It is the bare sentinel now; the reason lives in the entry's $comment.
A heredoc body arrives as one T_ENCAPSED_AND_WHITESPACE and was compared as written, so a call built from \x53tudio\Gesso\Internal\Deprecations::notice named the emitter at runtime and nothing at scan time. The opening T_START_HEREDOC tells the two bodies apart: a nowdoc stays literal, a heredoc gets the escapes a double-quoted string gets, minus \" which PHP keeps.
wadakatu
force-pushed
the
feat/v3-rename-registry-gate
branch
from
August 14, 2026 02:15
0701be1 to
0d5db32
Compare
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
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.
Summary
Adds
tests/fixtures/compatibility/v3-renames.json— every old spelling ADR 0005's two tables name, the v3 spelling that replaces it, the channel it uses, and its deprecation id once staged — andV3RenameRegistryTest, which parses those tables and holds the fixture to them.v2-deprecations.jsongains the machine-readable fields that let an id be matched to the rename it stages by equality, and now records the notice its own call emits.No
src/change.Why
Fixes #556.
DeprecationRegistryTestscanssrc/forDeprecations::notice()calls and requires each one to be registered. That catches a deprecation nobody registered. It cannot catch the opposite and likelier failure: a rename that ships no notice at all. Nothing is emitted, so nothing is scanned, and the registry stays as correct — and as empty — as it was. It holds one entry today, against roughly forty renames ADR 0005 fixes.#499 anticipated the distribution ("each entry is owned and landed by its own v3 issue, not by this one") and left the cross-check as a review rule: "a v3 PR that removes a surface with no entry has nothing to delete and is caught in review against this list." Spread across #501, #502, #503, #505, #507, #508 and #524, that is a lot of reviews for a mistake that cannot be repaired afterwards —
docs/versioning.md:216-221notes the first breaking commit onmainturns the pending release into3.0.0, after which no further v2 release can be cut. A forgotten deprecation then costs another major.What the gate checks
→gives itsurfaceis rightLegacyIdentitymap the spelling is inowneris the issue that owns that nameremoved_inis rightLegacyIdentity::REMOVED_IN, or nonev2-deprecations.jsonby equalityDeprecations::notice()insrc/, argument by argumentsrc/andbin/in one pass; a reused id failstrigger_error(E_USER_DEPRECATED)and#[\Deprecated]sites, counted per file against a named allowlist, with imports and relative names resolved,user_errorincluded, the severity evaluated in every base, and an unevaluable one countedDeprecations::notice(— computed class, computed member,::classcallable, any static string naming the emitter (heredoc and nowdoc included), a second namespace, an unreadable literalsurface, against the notice's ownsubjectLegacyIdentity::ENV_NAMES+COMMAND_NAMES, as a setunchanged-spellingmembershipVerification
Ten review rounds, each finding reproduced before it was fixed and re-run afterwards. Round seven also ran an adversarial sweep of the gate — three independent lenses (token scanner, ADR parser, fixture semantics), each candidate reproduced or refuted by a separate agent against a pristine checkout. 16 candidates, 15 reproduced; 6 were already closed by the round-seven fixes, the other 9 are fixed here.
The current battery is 86 mutations, each failing its intended assertion:
surfaceon three surface kinds, a malformedowner, a re-datedremoved_inon two channels, the wrong member of the right key, an in-row swap, a cross-row key move, a key differing from the row's only by a suffix, a subscript and a grammar member added to rows that enumerate neither, a nested and a repeated subscript, a repeated grammar member, a declared value dropped and an undeclared one added, an array row answered in the CLI grammar, a shipped notice unlinked by nulling its id, a row deleted from the ADR and the fixture together, anownerpointing at no issue, anownerswapped to a different real ADR issue on three surfaces, a removed row recording a successor, and a config key smuggled intoLegacyIdentityto leave the deprecation channel.subject,replacementandremovedIneach changed insrc/alone, then with the registry edited to agree, then withv3-renames.jsonedited to agree as well; plus a notice naming the sibling spelling, and a target with prose trailing it.use A\{B}import, a comma-separateduse A, B;, anamespace\Deprecations::reference, a second namespace block,$class::notice(),::{'notice'}(), a[Deprecations::class, 'notice']callable, orbin/gesso; atrigger_error(E_USER_DEPRECATED)and a#[\Deprecated]beside the emitter; the method name held in a variable;trigger_errorandE_USER_DEPRECATEDeach reached through an import alias;#[\Deprecated]written second in a group, under an alias, and in lower case; a numeric16384severity, one held in a variable, andtrigger_errorreached throughcall_user_func;::${'notice'}and a string callable naming the emitter; a severity held in a user-defined constant and written in hex, binary, octal,0oand digit-separated form;user_error()and a relativenamespace\trigger_error()inbin/gesso; a nowdoc spelling out the callable, and a heredoc whose escapes build the emitter's name andtrigger_error's (\x53tudio\Gesso\…).stripslashes()decoded to a string the notice never prints.|.The parsers are guarded by their own tests, so a drifted ADR table or a silently-stopped scan cannot make the coverage checks pass vacuously; the target split is pinned against all five target shapes ADR 0005 writes and the declaration side against all three, the literal decoder against fourteen escape spellings PHP defines and the one a heredoc leaves alone, and the call scan against eleven import and call spellings that all reach the same emitter.
ADR 0004 reduction-PR check:
git diff origin/main...HEADover both conformance baselines is empty.src/,bin/andcomposer.jsonare untouched.lycheeand the docs toolchain are not installed locally, sonpm run docs:links/docs:buildwere not run; CI covers both and they pass.composer testpasses — 3368 tests, 28111 assertions (CI, PHP 8.3 / PHPUnit 12)composer stanpassescomposer cs-checkpassesWhat still gets through
An in-row swap performed in ADR 0005 and the fixture together. The ADR is the source of truth, so editing it is a change to the answer rather than a drift away from it, and no test comparing the two can tell those apart. Reading the ADR diff is the only control, which is why the arrow rows are worth reviewing on their own.
Deleting a row from both is no longer in that category: the number of spellings the ADR names is pinned in the test, so a spelling leaving the gate costs an edit there too.
The equivalent trick on the registry side is closed. Every prose argument a notice passes is compared to
src/verbatim, and the facts that give those arguments meaning — the target, the removal version, the surface — are each pinned to the ADR throughv3-renames.json. A coordinated edit across both fixtures and the call site still fails, on whichever of those three it contradicts. What a notice may still add freely is a trailing parenthetical aside; everything before it is the v3 name exactly.Notes for reviewers
This branch edits ADR 0005, which is Accepted. Fifteen rows, all editorial: they spell out pairings the ADR already decided and change no v3 key name, member name, flag name, or removal. One cell read
same name, both surfacesand now names the spelling; thirteen grouped rows now write`old` → `new`per spelling instead of listing old spellings and new members in two parallel columns; and theconsole_reportrow gains(#502), the marker the removed rows already use, recording the owner the ADR's own section C decides in prose. The→is the ADR's own notation for this meaning (:51,:255). The spelling set the parser derives is identical before and after; three fixture replacements change to match what the ADR now states literally. Under ADR 0004 a contradicting change needs a superseding record — none of these contradict anything, but the diff is worth reading as a decision rather than as a detail.v2-deprecations.jsonwas restructured. Its two prose fields are now onenoticeobject holding the call'ssubject,replacementandremovedInverbatim, compared tosrc/argument by argument, alongside the barespellingandv3_targetthe rename gate compares by equality. Changing the wording, the target or the date of a notice is now a change to this fixture too — which is the point, since the fixture is the list a major deletes from.The ratchet is hard from day one, not a reported count that flips later. #556 proposed starting soft on the grounds that a red gate gets skipped. It is green today — 53 unstaged,
unstaged_count: 53— so there is nothing to flip. It is asserted in both directions: a stale number left high after staging leaves room for the next omission to slip in under the old ceiling.unchanged-spellingmembership is hand-listed, deliberately. Four ADR rows keep their spelling, but three of them (baseline_staleand the two--strict-*flags) replace the value that spelling accepts, which is a removal to anyone who wrote the old value down. Telling that apart from a placeholder like--output-file=<path>is a guess, so the channel that stages nothing and counts toward nothing costs a deliberate edit to enter. The constant's docblock records the rejected derivation.The ADR checks live inside the scan, not in a test of their own, so a test that only wants the spelling list still cannot read a malformed table. The same now holds for the registry's shape, which is validated where it is loaded. The cost is that their failures are attributed to whichever test called first; the message names the row and the reason.