fix: EnforceFormRequestToDtoRule accepts toDtos() bulk convention (v0.6.1)

[Goosterhof]

Summary

EnforceFormRequestToDtoRule checked a single DTO_METHOD_NAME = 'toDto' (singular) and false-positived on the canonical ADR-0020 bulk-list pattern — a FormRequest that converts its validated input to list<…Data> via toDtos() (plural). The Level-2 PHPStan rule was stricter than the Level-1 arch test it mirrors: codebook's own tests/Arch/FormRequestsTest.php already accepts toDtos() and is green.

Fix: a concrete FormRequest that declares or inherits toDto() OR toDtos() now satisfies the contract; only a class with neither is flagged (DTO_METHOD_NAME → DTO_METHOD_NAMES = ['toDto', 'toDtos'], pass if hasNativeMethod() is true for any). The abstract-skip gate, the FormRequest-base inheritance gate, and the formRequestToDtoExemptClasses exemption param are unchanged. Error message updated to toDto()/toDtos().

Fleet blast radius

Surveyed 2026-07-01 (origin/development): codebook (5 requests) + kendo (ReorderEpicsRequest, 1) define toDtos(); entreezuil / ublgenie / emmie / isms have none. Both codebook and kendo were about to suppress this false positive per-consumer — the wrong level. Fixed here.

Versioning

PATCH — v0.6.1 (false-positive narrowing per ADR-0021 §Versioning: removes errors, adds none). Base main is already at v0.6.0, so this ships as 0.6.1, not the 0.5.1 originally envisioned (a 0.5.1 tag on a tree already containing 0.6.0's exemption-param code would violate SemVer ordering). ^0.6 consumers inherit 0.6.1 on a plain composer update — no constraint bump. Consumers still pinned ^0.5 / ^0.3 reach it only when they bump to ^0.6.

Tests / fixture

New CompliantToDtosRequest fixture (defines only toDtos(): array, must NOT flag) + testCompliantToDtosBulkListIsNotFlagged, alongside the unchanged neither-method positive case. Three-way sanity: toDto() passes / toDtos() passes / neither errors.

Gates green locally: composer test (124), phpstan, format:check (Pint), coverage:check (88.38%), mutation:ci (MSI ~84.34%, floor 75%).

🤖 Generated with Claude Code

https://claude.ai/code/session_0123shnHWMoS1GeWjiEc4KpM

[Goosterhof]

⏳ Code approve-worthy — but CI is PENDING, verdict held (not green until it resolves)

0 blockers · 1 observation (latent, non-blocking) · 2 praise · independent first look · CI pending — conditional

This is the fix for the toDtos() false-positive I flagged on codebook #438 — loop closed. I reviewed the code and it's sound; I'm explicitly not calling CI green because both check (8.4)/check (8.5) are still pending. (I called "CI green" over a pending check twice today — #579 and #438 — and was wrong both times; not a third. Finalize this to APPROVE only once the two checks pass.)

The code (approve-worthy on its own)

DTO_METHOD_NAMES = ['toDto', 'toDtos'] + a loop over hasNativeMethod($method) — a class satisfies the contract if it declares/inherits either. Faithful to the existing method-existence approach and the source-of-truth FormRequestsTest matcher (which the docblock notes already accepts both). The CompliantToDtosRequest fixture pins the plural leg. This is exactly the enhancement #438's six suppressions called for — once this ships (v0.6.1) codebook can drop those @phpstan-ignore lines.

Observation — name-only, not return-type (latent, pre-existing, non-blocking)

Because acceptance is by method name (hasNativeMethod), a toDtos(): array that returns raw $this->validated() would pass the rule — the same hole toDto(): array already had before this PR. So #47 doesn't introduce the gap; it extends an existing design property (mirroring the Pest method_exists() matcher). Flagging only because the rule's whole purpose is preventing raw-array handoff, and "the method exists" is weaker than "the method returns a DTO/list<Dto>". PHPStan can assert the return type (getReturnType → is it a DTO / list-of-DTO?), which would make both legs airtight. Worth a follow-up enforcement-queue item — NOT this PR (it would change the singular leg's contract too, and the fixture's toDtos(): array + @return list<StoreUserData> phpdoc shape would need the docblock-type read). Ship the name-based fix now; tighten to return-type later if we want it airtight.

Praise

1. Closes the #438 false-positive at the source (rule) rather than per-consumer suppression — the right level on the enforcement ladder.
2. Consistent extension: one const list, one loop, error message + docblock + fixture all updated together; no special-casing.

Verdict: code approve-worthy, verdict conditional on CI — I'll finalize (and the thread can resolve) once check (8.4)/(8.5) go green. If either fails, it's a blocker until fixed. Own-PR → COMMENT regardless.

Automated war-room agent review — posted because this PR carries the Agent Review Requested label.

[jasperboerhof]

Town Crier Review · 8/10 · PASS · 🤝 Confirm — 🟡 2

phpstan-warroom-rules #47 · AC anchor: PR description (no kendo issue linked; bug fix described in CHANGELOG entry) · head 2255fa4de2 · via the town-crier bus (request #173)

Tip

Reviewed the EnforceFormRequestToDtoRule toDtos() bulk-convention fix (DTO_METHOD_NAMES loop, message, fixture, CHANGELOG); it corroborates the-general's read that the rule-contract change correctly closes codebook #​438's false positive and confirms their latent name-only/no-return-type-check gap as a pre-existing, non-blocking MINOR — and finds their CI-pending hold has since cleared (checks 8.4/8.5 both SUCCESS at this head). One addition: README.md's rule table and dedicated section still describe the rule as toDto()-only even though the code, docblock, and error message now accept toDto()/toDtos().

1 finding(s) posted inline:

• 🟡 MINOR · src/Rules/EnforceFormRequestToDtoRule.php:128 — Acceptance check is name-only (hasNativeMethod), no return-type assertion on toDto()/toDtos()

Bus thread · 1 prior review(s):

• the-general (independent): Code approve-worthy (closes my #​438 toDtos() false-positive concern at the rule level) BUT CI PENDING — verdict HELD, not calling green (applying today's #​579/#​438 lesson: pending != green). Finalize …

Findings outside the diff (not inline-postable)

• 🟡 MINOR — README still documents EnforceFormRequestToDtoRule as toDto()-only after the toDtos() acceptance ships (README.md:50)
  • Fix: Update README.md's rule table row and the EnforceFormRequestToDtoRule section to say toDto() or toDtos(), mirroring the docblock's language.

[jasperboerhof]

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.

[jasperboerhof]

Town Crier Review · 8/10 · PASS · 🤝 Confirm — 🟡 1

phpstan-warroom-rules #47 · AC anchor: PR description (bug symptom + fleet blast-radius claims — no linked kendo issue) · head 1461f654bc · via the town-crier bus (request #173)

Tip

Reviewed EnforceFormRequestToDtoRule's DTO_METHOD_NAMES widening (accepts toDtos() alongside toDto()) plus its fixture/test/CHANGELOG updates — a sound, well-tested, PATCH-classified false-positive fix per ADR-0021 that confirms the-general's and Goosterhof's read of the code. We refute dispatch's "CI cleared" note: a later unrelated merge-from-main moved the head and check (8.4)/(8.5) are pending again at 1461f65, so the-general/Goosterhof's hold-for-CI caution is still the one that applies; we also surface one new MINOR (README doc drift) alongside the already-tracked name-only-acceptance gap.

1 finding(s) — detailed below (not anchorable to diff lines).

Bus thread · 2 prior review(s):

• the-general (independent): Code approve-worthy (closes my #​438 toDtos() false-positive concern at the rule level) BUT CI PENDING — verdict HELD, not calling green (applying today's #​579/#​438 lesson: pending != green). Finalize …
• dispatch (confirm): confirm — corroborates the-general's toDtos() fix (DTO_METHOD_NAMES loop + fixture, closes codebook #​438) and their latent name-only/no-return-type gap (MINOR, pre-existing, non-blocking). Their …

Findings outside the diff (not inline-postable)

• 🟡 MINOR — README still documents toDto()-only after toDtos() bulk acceptance shipped (README.md:50)
  • Fix: Update README.md:50 and :97 to also name toDtos() as an accepted method.

[jasperboerhof]

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]

Confirm/deny prior thread + finalize CI hold

0 blockers · 1 confirmed non-blocking gap (pre-existing, tracked) · 1 confirmed MINOR (doc drift) · CI green — finalizing my earlier hold to approve-worthy

Finalizing my 2026-07-01 hold (review-233 @ 2255fa4d): CI was PENDING then; at live head 1461f654 both check (8.4) and check (8.5) are pass (gh pr checks 47). Per my own prior note — "finalize this to APPROVE only once the two checks pass" — done. Nothing new on substance; the code was already sound.

Confirm dispatch's name-only/no-return-type observation (rounds 1–3): still accurate at 1461f654 — src/Rules/EnforceFormRequestToDtoRule.php:128-131 loops hasNativeMethod($method) over toDto/toDtos; a toDtos(): array that returns raw $this->validated() would still pass. Pre-existing property (the singular leg had the identical hole before this PR), not introduced here. Correctly left open as a follow-up, not a blocker.

Confirm README.md doc-drift (dispatch rounds 2–3) — verified independently at 1461f654: README.md:50 (rule table row: "If the class neither declares nor inherits a toDto() method, error") and README.md:97 ("A toDto() declared on a parent class or provided by a trait satisfies the contract") both still describe the singular-only contract — zero mention of toDtos() — even though the rule's docblock, error message, and CHANGELOG.md were all updated in this same diff. This PR touches CHANGELOG.md + the rule + tests but not README.md. Genuine drift, MINOR (README isn't the enforcement surface — rule + arch test are — but a reader trusting the docs would miss the plural leg entirely). Should be a fast follow-up, doesn't block this PATCH.

dispatch round-3's CI-flip-flop is now moot: round 2 said CI cleared, round 3 caught a merge-from-main that re-pended it — both were accurate at time of writing. Live head 1461f654 (same head round 3 cites) is green on both checks now; no open CI concern remains.

Rule↔arch-test parity, independently verified: codebook's tests/Arch/FormRequestsTest.php:147-157 (hasMethod('toDto') || hasMethod('toDtos'), both keyed to the declaring class) matches this rule's DTO_METHOD_NAMES loop exactly. The PR's claim of restoring Level-2/Level-1 parity holds — this isn't just narrative, the two matchers are structurally equivalent.

Verdict: approve-worthy. 0 blockers, 1 known/tracked non-blocking gap (return-type assertion — file as follow-up, not this PR), 1 confirmed doc-drift MINOR (README.md:50/97 — fast-follow). Own-PR → COMMENT per policy; Jasper's auto-approval at this head stands as the binding approval.

Automated war-room agent review — posted because this PR carries the Agent Review Requested label.

[jasperboerhof]

Town Crier Review · 8/10 · PASS · 🤝 Confirm — 🟡 1

phpstan-warroom-rules #47 · AC anchor: PR description / CHANGELOG (bug, no kendo issue link) · head 1461f654bc · via the town-crier bus (request #173)

Tip

Reviewed the toDtos() acceptance fix (DTO_METHOD_NAMES loop + CompliantToDtosRequest fixture) end-to-end and cross-examined the full bus + off-bus thread — every prior claim (the-general, dispatch, Goosterhof) holds at head 1461f65, the fix is sound and tested, and CI is green. We corroborate the thread's PASS and formally file the README.md:50/97 doc-drift MINOR that dispatch/the-general/Goosterhof already flagged on the bus but never inlined; the pre-existing name-only acceptance gap stays on its own already-open badge thread.

1 finding(s) — detailed below (not anchorable to diff lines).

Bus thread · 4 prior review(s):

• the-general (independent): Code approve-worthy (closes my #​438 toDtos() false-positive concern at the rule level) BUT CI PENDING — verdict HELD, not calling green (applying today's #​579/#​438 lesson: pending != green). Finalize …
• dispatch (confirm): confirm — corroborates the-general's toDtos() fix (DTO_METHOD_NAMES loop + fixture, closes codebook #​438) and their latent name-only/no-return-type gap (MINOR, pre-existing, non-blocking). Their …
• dispatch (confirm): confirm - corroborates the-general/Goosterhof: toDtos() fix is sound (DTO_METHOD_NAMES loop + fixture, 14/14 local phpunit at 1461f65). Refutes dispatch's "CI cleared": an unrelated merge-from-main …
• the-general (confirm): confirm — corroborates dispatch's return-type-gap (pre-existing, non-blocking) and README.md:50/97 doc-drift MINOR (verified independently, both lines still toDto()-only). Finalizing my own prior …

Findings outside the diff (not inline-postable)

• 🟡 MINOR — README rule table still says toDto()-only after the rule started accepting toDtos() (README.md:50)
  • Fix: Update README.md:50 and :97 to read toDto()/toDtos(), matching the docblock and CHANGELOG wording.

[jasperboerhof]

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.