feat(FormRequestToDto): consumer-configurable class-keyed exemption list [queue #131]#45
Conversation
…ist [queue #131] Add an optional `formRequestToDtoExemptClasses` PHPStan parameter (default empty) to `EnforceFormRequestToDtoRule` — a list of fully-qualified class names to skip, matched by exact FQCN. This is the class-keyed alternative to the existing per-file `ignoreErrors` suppression path, letting a territory port a retiring local FormRequest->DTO arch test's exempt-class list into package config 1:1. - Rule: new `array $exemptClasses = []` constructor param; exact-FQCN skip after the existing gates. No class name hardcoded in the rule body — names come only from consumer config, preserving the "never by name in the rule" convention. - extension.neon: `formRequestToDtoExemptClasses: []` parameter + `listOf(string())` schema + `exemptClasses` argument wiring, mirroring the `formRequestBaseClass` precedent. - Tests: empty-list regression, exempt-by-FQCN, precise-not-global-off-switch, exact-FQCN-not-short-name/other-namespace. New SecondViolatorRequest fixture. - Docs: README param + "Retiring a local FormRequest->DTO arch test" recipe; CHANGELOG Added entry (backward-compatible MINOR, proposed v0.6.0). Backward-compatible MINOR: default-empty list ⇒ zero new errors in existing consumers. Territory arch-test retirement is a separate follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01YRSGQLQVRGArBssqbvWqxg
Town Crier Review · 8/10 · PASS · 🔎 Independent — 🟡 1phpstan-warroom-rules #45 · AC anchor: PR description / CHANGELOG (no linked kendo issue, no plan dir) · head Tip Reviewed the new consumer-configurable 1 finding(s) posted inline:
|
jasperboerhof
left a comment
There was a problem hiding this comment.
Auto-approved — Town Crier verdict PASS @Head, CI green, no open MAJOR+ thread. Our approval is our independent vote (approve-alongside): a peer's review / CHANGES_REQUESTED never withholds it — we verify every blocker ourselves, and a real one drops our own verdict below PASS. See the verdict comment + inline notes.
Goosterhof
left a comment
There was a problem hiding this comment.
✅ Approve-worthy — confirms dispatch's PASS; 1 MINOR extended + 1 sequencing note
0 blockers · 1 concern (sequencing, non-blocking) · confirms @dispatch's 8/10 PASS · engaged 1 ally thread
Round 2 — confirm dispatch's PASS. I verified the rule change end-to-end against the package's "never by name in the rule" convention, and it holds:
- Exact-FQCN match, not prefix.
in_array($classReflection->getName(), $this->exemptClasses, true)(EnforceFormRequestToDtoRule.php:119) — strict, FQCN, no substring/short-name over-match. TheSecondViolatorRequestfixture is the right adversarial test: it proves exempting one FQCN does not globally silence the rule. - Default-preserving.
[]default in both the ctor andextension.neon;testViolatorWithEmptyExemptListIsFlaggedpins that an empty list = unchanged behaviour. Correctly a backward-compatible MINOR — no consumer sees a change until it opts in. - Placement is correct. The exemption short-circuits only an otherwise-firing error (it sits after the abstract / base-class / has-
toDto()early-outs), so it can never suppress a different diagnostic. - Convention preserved. The FQCN list is consumer config (
extension.neonparam →%formRequestToDtoExemptClasses%→ service arg), never a rule-body literal — mirrors theformRequestBaseClassprecedent exactly.
Engaged dispatch's MINOR (README:126) — confirmed + corrected
dispatch is right that the migration recipe cites a nonexistent path. Extended in-thread with the verified fix: the real path is backend/tests/Architecture/FormRequestsTest.php (post-ADR-0027 monorepo backend/, dir Architecture/ not Arch/) — tests/Arch/ doesn't exist on development. On a recipe whose whole value is a copy-paste-accurate retirement, the wrong path defeats the purpose; worth the one-line fix before merge.
Concern (sequencing, non-blocking — a precondition, not a defect)
This ships as v0.6.0, and per the package's own pre-1.0 caret doctrine (^0.{minor} excludes the next minor — the 0.5.0 CHANGELOG says "tagging auto-adopts nobody") the new formRequestToDtoExemptClasses param is dark to every consumer still pinned ^0.3/^0.4/^0.5 until each does an adoption bump. That makes the migration recipe's precondition explicit: the live case — entreezuil PR #274, which is currently RED on Rector — must first land its ^0.5→^0.6 bump before it can use this param to retire backend/tests/Architecture/FormRequestsTest.php. Not a defect in this PR (it's the intended caret behaviour), but the retirement follow-up sequences bump-then-retire, and #274 is the blocker in that chain. Worth a one-line note in the recipe so the sequencing isn't a surprise.
Verdict: ship it (own-PR → COMMENT). The code is a correct, well-tested, default-preserving contract addition; the two notes are a doc-path fix and a sequencing caveat, neither a merge blocker. CI green (8.4 + 8.5).
Automated war-room agent review — posted because this PR carries the Agent Review Requested label.
Town Crier Review · 8/10 · PASS · 🤝 Confirmphpstan-warroom-rules #45 · AC anchor: PR description / CHANGELOG entry (no kendo issue key, no plan dir, no PR AC section) · head Tip Reviewed the new consumer-configurable formRequestToDtoExemptClasses parameter end-to-end and it corroborates the prior thread in full: the change is additive and default-preserving (correctly versioned MINOR), the exact-FQCN exemption gate is ordered so it can never suppress a different diagnostic, and fixtures back both the precision and default-list behavior. No new defect surfaces — the only prior finding, a wrong entreezuil test path in the new README migration recipe, is already an open badge thread from our own earlier run and stays open rather than being re-filed. No findings — clean against the review checklist. Bus thread · 2 prior review(s):
|
jasperboerhof
left a comment
There was a problem hiding this comment.
Auto-approved — Town Crier verdict PASS @Head, CI green, no open MAJOR+ thread. Our approval is our independent vote (approve-alongside): a peer's review / CHANGES_REQUESTED never withholds it — we verify every blocker ourselves, and a real one drops our own verdict below PASS. See the verdict comment + inline notes.
…m [queue #131] (#46) Cuts v0.6.0 (backward-compatible MINOR): moves the [Unreleased] Added entry (EnforceFormRequestToDtoRule `formRequestToDtoExemptClasses`, #45) to [0.6.0] — 2026-07-01, adds the [0.6.0] release-tag link-ref, and bumps the [Unreleased] compare base to v0.6.0. Also folds in Jasper's #45 MINOR: the retirement-recipe cited entreezuil's `tests/Arch/FormRequestsTest.php`; post-ADR-0027 the correct path is `backend/tests/Architecture/FormRequestsTest.php`. Corrected here (rather than re-pushing #45 under dismiss-stale) so v0.6.0 ships accurate docs. Claude-Session: https://claude.ai/code/session_01YRSGQLQVRGArBssqbvWqxg Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
Why (enforcement queue #131)
EnforceFormRequestToDtoRulenow duplicates territory-localFormRequestsTestPest arch tests — the same "every concreteFormRequestmust exposetoDto()" invariant enforced twice, with two exemption ledgers to keep in sync (entreezuil is the first live case, PR #274). The path to consolidation is to let territories retire their local arch test and lean on the package rule — but the package's only exemption surface today is per-consumerphpstan.neonignoreErrors, which is path-keyed (brittle to file moves) and doesn't port an arch test's FQCN exemption list 1:1.This PR builds the class-keyed exemption surface so that retirement becomes clean. The retirement itself is a separate follow-up, not this change.
What
array $exemptClasses = []constructor param onEnforceFormRequestToDtoRule, property-promoted likeformRequestBaseClass. After the existing gates (abstract-skip →isSubclassOf→hasNativeMethod('toDto')), a final skip on exact FQCN match against the list. No class name is hardcoded in the rule body — names come only from consumer config, so the documented "never by name inside the rule" convention holds (a consumer-supplied FQCN list is config, not a rule-body literal; called out in the docblock).extension.neon—formRequestToDtoExemptClasses: []underparameters(with the same single-backslash NEON-quoting caveat the FQCN params already carry),listOf(string())underparametersSchema, andexemptClasses: %formRequestToDtoExemptClasses%on the rule's service registration.SecondViolatorRequestfixture.formRequestBaseClassplus a "Retiring a local FormRequest→DTO arch test" recipe using entreezuil's real exemptions (App\Http\Requests\Auth\LoginRequest,App\Http\Requests\BaseFormRequest) as the worked example. CHANGELOGAddedentry.Class-keyed vs
ignoreErrorsignoreErrorsstays valid (path-keyed, per-file). The new param is the class-keyed alternative: predictable across file moves, and it maps an arch test's FQCN exempt-list into package config one-for-one. Both remain consumer-config-driven; neither hardcodes names in the rule.Versioning
Backward-compatible MINOR — new optional parameter, default empty ⇒ zero new errors in existing consumers. NOT a candidate-major. Proposed release: v0.6.0 (not tagged in this PR — the General/Commander cuts the release).
Scope
Package-only. No consumer territory touched, no local arch test retired, no consumer dependency bumped. Generalizing the same param shape to sibling rules (e.g.
EnforceResourceDataValidatorOptInRule) is noted as a follow-up, not built here.Verification (all green locally)
composer format:check— passedcomposer phpstan—[OK] No errorscomposer test— 123 tests, 189 assertions OKcomposer test:coverage+coverage:check— 88.38% (threshold 83%) PASScomposer mutation:ci— MSI 9.32pts over the 75% threshold, exit 0🤖 Generated with Claude Code
https://claude.ai/code/session_01YRSGQLQVRGArBssqbvWqxg