diff --git a/CHANGELOG.md b/CHANGELOG.md index 10b1e6a..02a3ef0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and ## [Unreleased] +### Added + +- `EnforceFormRequestToDtoRule` — new optional `formRequestToDtoExemptClasses` PHPStan parameter (default `[]`): a list of fully-qualified class names to exempt from the `toDto()` contract, matched by **exact FQCN**. This is the class-keyed alternative to the existing per-file `phpstan.neon` `ignoreErrors` suppression path (identifier + `path` keyed) — predictable across file moves and, crucially, it ports a retiring local FormRequest→DTO Pest arch test's FQCN exemption list into package config 1:1. Wired through `extension.neon` (`formRequestToDtoExemptClasses: []` parameter + `listOf(string())` schema + `exemptClasses: %formRequestToDtoExemptClasses%` on the rule's service registration), mirroring the `formRequestBaseClass` precedent. No class name is ever hardcoded in the rule body — a consumer-supplied FQCN list is *config*, not a rule-body literal, so the package's "never by name inside the rule" convention is preserved. Seed: war-room enforcement queue #131 — `EnforceFormRequestToDtoRule` now duplicates territory-local `FormRequestsTest` arch tests (entreezuil first live case, PR #274), enforcing the same invariant twice with two exemption ledgers to keep in sync; the class-keyed param lets a territory retire its local arch test and consolidate its exempt list into package config (that retirement is a separate follow-up, not this change). README documents the param + a "Retiring a local FormRequest→DTO arch test" migration recipe. **Versioning: backward-compatible MINOR** (new optional parameter, default empty ⇒ zero new errors in existing consumers — NOT a candidate-major; the default-empty list means no consumer sees behaviour change until it opts in). Proposed release: **v0.6.0**. + ## [0.5.0] — 2026-06-25 **Release-as-a-whole: candidate MAJOR (pre-1.0 minor bump — `^0.4 → ^0.5`)** — ships two new rules (`ForbidHttpExceptionInActionsRule`, `ForbidResourceWrappedInJsonResponseRule`) from the Commander's review of emmie PR #481 (war-room enforcement queue #123 + #124). Both surface new errors in already-clean code wherever a consumer violates, so each consumer adopts on its own `^0.4 → ^0.5` bump PR (the `^0.{minor}` caret means `^0.4` excludes `0.5.0` — tagging auto-adopts nobody). **Blast radius (surveyed 2026-06-25 `origin/development`): ZERO violators on emmie + kendo for both rules** (the #481 offender is on its branch, not merged) — the per-territory bump is expected clean save for un-merged branch work. diff --git a/README.md b/README.md index 9cf5589..06f339b 100644 --- a/README.md +++ b/README.md @@ -96,7 +96,9 @@ parameters: Inheritance is matched via PHPStan reflection (FQCN ancestor traversal), not short-name matching. Abstract classes never fire — a per-territory abstract `BaseFormRequest` intermediate is exempt by shape, not by name. A `toDto()` declared on a parent class or provided by a trait satisfies the contract (mirroring the source-of-truth entreezuil Pest arch test's `method_exists()` matcher). -Legitimately DTO-less requests (e.g. a `LoginRequest` whose auth flow calls `AuthManager::attempt()` directly, or read-only filter/query requests) are suppressed per territory via `phpstan.neon` — never by name inside the rule: +Legitimately DTO-less requests (e.g. a `LoginRequest` whose auth flow calls `AuthManager::attempt()` directly, or read-only filter/query requests) are suppressed per territory in one of two consumer-config-driven ways — never by name inside the rule. + +**Option A — per-file `ignoreErrors` (path-keyed):** ```neon parameters: @@ -108,6 +110,34 @@ parameters: Each ignore should carry a comment with rationale. +**Option B — `formRequestToDtoExemptClasses` (class-keyed):** a list of fully-qualified class names to skip, matched by **exact FQCN**. This is the class-keyed alternative to `ignoreErrors` — predictable across file moves, and it ports a retiring local arch test's exempt-class list into package config 1:1. Default empty ⇒ no exemptions. + +```neon +parameters: + formRequestToDtoExemptClasses: + # login handler: auth flow calls Auth::attempt() directly, no Action DTO + - 'App\Http\Requests\Auth\LoginRequest' +``` + +A consumer-supplied FQCN list is *config*, not a rule-body literal — the "never by name inside the rule" convention is preserved. + +#### Retiring a local FormRequest→DTO arch test + +Where a territory already enforces "every concrete FormRequest exposes `toDto()`" via a local Pest arch test (e.g. entreezuil's `tests/Arch/FormRequestsTest.php`), this rule now duplicates that invariant. To retire the local test cleanly: + +1. Move the arch test's exempt-class list into `formRequestToDtoExemptClasses` as FQCNs. For entreezuil that is: + ```neon + parameters: + formRequestToDtoExemptClasses: + # framework Auth::attempt() path, no Action DTO + - 'App\Http\Requests\Auth\LoginRequest' + # intermediate base (make it `abstract` and it drops out entirely) + - 'App\Http\Requests\BaseFormRequest' + ``` +2. Delete the local arch test — the package rule (identifier `enforceFormRequestToDto.missingToDtoMethod`) is now the single enforcement authority. + +(Territory arch-test retirement is a separate follow-up dispatch, not part of shipping this option.) + ### `EnforceCurrentUserAttributeRule` — false positives `#[\Illuminate\Container\Attributes\CurrentUser]` resolves the authenticated user at **method-entry DI time**. A controller method that resolves the user *after* `Auth::attempt()` succeeds — the canonical **login handler** on a `guest` / throttle-only route — cannot use the attribute: at method entry no user exists yet, so injection yields `null` and breaks login. The rule fires on any `Auth::user()` / `$request->user()` / `auth()->user()` inside the `App\Http\Controllers` namespace and **cannot see routes**, so it will flag these legitimate login handlers. Suppress per territory via `phpstan.neon` — never by name inside the rule: diff --git a/extension.neon b/extension.neon index 404c464..6ca4179 100644 --- a/extension.neon +++ b/extension.neon @@ -15,9 +15,18 @@ parameters: # NEON-quoting note above. formRequestBaseClass: 'Illuminate\Foundation\Http\FormRequest' + # `EnforceFormRequestToDtoRule`: list of fully-qualified class names to + # exempt from the `toDto()` contract (exact-FQCN match). The class-keyed + # alternative to per-file `ignoreErrors` — intended for porting a retiring + # local FormRequest→DTO arch test's exempt-class list into package config + # 1:1. Default empty ⇒ no exemptions, behaviour unchanged. Each FQCN uses + # single backslashes — see the NEON-quoting note above. + formRequestToDtoExemptClasses: [] + parametersSchema: resourceDataBaseClass: string() formRequestBaseClass: string() + formRequestToDtoExemptClasses: listOf(string()) services: - @@ -59,6 +68,7 @@ services: class: ScriptDevelopment\PhpstanWarroomRules\Rules\EnforceFormRequestToDtoRule arguments: formRequestBaseClass: %formRequestBaseClass% + exemptClasses: %formRequestToDtoExemptClasses% tags: [phpstan.rules.rule] - class: ScriptDevelopment\PhpstanWarroomRules\Rules\EnforceCurrentUserAttributeRule diff --git a/src/Rules/EnforceFormRequestToDtoRule.php b/src/Rules/EnforceFormRequestToDtoRule.php index a19fcc7..1ebbec8 100644 --- a/src/Rules/EnforceFormRequestToDtoRule.php +++ b/src/Rules/EnforceFormRequestToDtoRule.php @@ -13,6 +13,7 @@ use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; +use function in_array; use function sprintf; /** @@ -47,8 +48,17 @@ * * Legitimately DTO-less requests (entreezuil precedent: `LoginRequest`, * whose auth flow calls `AuthManager::attempt()` directly) are suppressed - * per consumer `phpstan.neon` `ignoreErrors` keyed on the identifier — - * never by name inside the rule, per the package convention. + * one of two ways, both consumer-config-driven — never by name inside the + * rule, per the package convention: + * 1. Per-file `phpstan.neon` `ignoreErrors` keyed on the identifier + + * path (brittle to file moves). + * 2. The `formRequestToDtoExemptClasses` PHPStan parameter — a list of + * fully-qualified class names to skip (matched by exact FQCN). This is + * the class-keyed alternative to `ignoreErrors`, intended for porting a + * retiring local arch test's FQCN exemption list into package config + * 1:1. A consumer-supplied FQCN list is *config*, not a rule-body + * literal — no class name is ever hardcoded in this rule, so the + * "never by name inside the rule" convention is preserved. * * Implementation note: the constructor default uses `FormRequest::class` * (compile-time constant, never autoloads) instead of an FQCN string @@ -66,8 +76,15 @@ final class EnforceFormRequestToDtoRule implements Rule { private const string DTO_METHOD_NAME = 'toDto'; + /** + * @param list $exemptClasses fully-qualified class names to skip + * (exact-FQCN match); the class-keyed + * alternative to `ignoreErrors`, + * supplied only from consumer config + */ public function __construct( private string $formRequestBaseClass = FormRequest::class, + private array $exemptClasses = [], ) {} public function getNodeType(): string @@ -97,6 +114,14 @@ public function processNode(Node $node, Scope $scope): array return []; } + // Class-keyed consumer exemption: exact-FQCN match against the + // configured list. Predictable (no short-name collisions), and it + // ports a retiring local arch test's exempt-class list 1:1. Names + // live in consumer config, never in this rule body. + if (in_array($classReflection->getName(), $this->exemptClasses, true)) { + return []; + } + return [ RuleErrorBuilder::message(sprintf( '%s extends FormRequest but does not define a toDto() method — raw validated-array handoff risk (ADR-0012 / war-room queue #55 / entreezuil FormRequestsTest opt-in invariant).', diff --git a/tests/Fixtures/FormRequestToDto/SecondViolatorRequest.php b/tests/Fixtures/FormRequestToDto/SecondViolatorRequest.php new file mode 100644 index 0000000..019c3b0 --- /dev/null +++ b/tests/Fixtures/FormRequestToDto/SecondViolatorRequest.php @@ -0,0 +1,23 @@ + + */ + public function rules(): array + { + return [ + 'name' => ['required', 'string'], + ]; + } +} diff --git a/tests/Rules/EnforceFormRequestToDtoRuleTest.php b/tests/Rules/EnforceFormRequestToDtoRuleTest.php index 0a20b04..09d7702 100644 --- a/tests/Rules/EnforceFormRequestToDtoRuleTest.php +++ b/tests/Rules/EnforceFormRequestToDtoRuleTest.php @@ -169,6 +169,90 @@ public function testCustomBaseClassParameterMatchesAlternativeFqcn(): void ); } + public function testViolatorWithEmptyExemptListIsFlagged(): void + { + // Regression: the new exemptClasses param defaults to empty; default + // behaviour must be unchanged (violator still fires). + $this->ruleOverride = new EnforceFormRequestToDtoRule(exemptClasses: []); + + $this->analyse( + [ + __DIR__ . '/../Fixtures/FormRequestToDto/_stubs.php', + __DIR__ . '/../Fixtures/FormRequestToDto/ViolatorRequest.php', + ], + [ + [ + 'App\Http\Requests\ViolatorRequest extends FormRequest but does not define a toDto() method — raw validated-array handoff risk (ADR-0012 / war-room queue #55 / entreezuil FormRequestsTest opt-in invariant).', + 9, + ], + ], + ); + } + + public function testExemptClassByFqcnIsNotFlagged(): void + { + // The violator's exact FQCN is in the exempt list — the class-keyed + // consumer exemption path. No error. + $this->ruleOverride = new EnforceFormRequestToDtoRule( + exemptClasses: ['App\Http\Requests\ViolatorRequest'], + ); + + $this->analyse( + [ + __DIR__ . '/../Fixtures/FormRequestToDto/_stubs.php', + __DIR__ . '/../Fixtures/FormRequestToDto/ViolatorRequest.php', + ], + [], + ); + } + + public function testExemptionIsPreciseNotGlobalOffSwitch(): void + { + // Exempting one FQCN must not silence a DIFFERENT non-exempt violator + // analysed in the same run — the exemption is precise, not a global + // off-switch. + $this->ruleOverride = new EnforceFormRequestToDtoRule( + exemptClasses: ['App\Http\Requests\ViolatorRequest'], + ); + + $this->analyse( + [ + __DIR__ . '/../Fixtures/FormRequestToDto/_stubs.php', + __DIR__ . '/../Fixtures/FormRequestToDto/ViolatorRequest.php', + __DIR__ . '/../Fixtures/FormRequestToDto/SecondViolatorRequest.php', + ], + [ + [ + 'App\Http\Requests\SecondViolatorRequest extends FormRequest but does not define a toDto() method — raw validated-array handoff risk (ADR-0012 / war-room queue #55 / entreezuil FormRequestsTest opt-in invariant).', + 12, + ], + ], + ); + } + + public function testExemptMatchIsExactFqcnNotShortNameOrOtherNamespace(): void + { + // The match is exact-FQCN: neither the bare short name nor an + // unrelated-namespace class of the same short name exempts the real + // `App\Http\Requests\ViolatorRequest` — it must still fire. + $this->ruleOverride = new EnforceFormRequestToDtoRule( + exemptClasses: ['ViolatorRequest', 'App\Other\ViolatorRequest'], + ); + + $this->analyse( + [ + __DIR__ . '/../Fixtures/FormRequestToDto/_stubs.php', + __DIR__ . '/../Fixtures/FormRequestToDto/ViolatorRequest.php', + ], + [ + [ + 'App\Http\Requests\ViolatorRequest extends FormRequest but does not define a toDto() method — raw validated-array handoff risk (ADR-0012 / war-room queue #55 / entreezuil FormRequestsTest opt-in invariant).', + 9, + ], + ], + ); + } + /** * Load the shipped extension.neon so testRuleResolvesFromExtensionNeonAndFires * can pull the rule out of the container with its NEON-configured