Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
32 changes: 31 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Comment thread
Goosterhof marked this conversation as resolved.

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:
Expand Down
10 changes: 10 additions & 0 deletions extension.neon
Original file line number Diff line number Diff line change
Expand Up @@ -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:
-
Expand Down Expand Up @@ -59,6 +68,7 @@ services:
class: ScriptDevelopment\PhpstanWarroomRules\Rules\EnforceFormRequestToDtoRule
arguments:
formRequestBaseClass: %formRequestBaseClass%
exemptClasses: %formRequestToDtoExemptClasses%
tags: [phpstan.rules.rule]
-
class: ScriptDevelopment\PhpstanWarroomRules\Rules\EnforceCurrentUserAttributeRule
Expand Down
29 changes: 27 additions & 2 deletions src/Rules/EnforceFormRequestToDtoRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;

use function in_array;
use function sprintf;

/**
Expand Down Expand Up @@ -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
Expand All @@ -66,8 +76,15 @@ final class EnforceFormRequestToDtoRule implements Rule
{
private const string DTO_METHOD_NAME = 'toDto';

/**
* @param list<string> $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
Expand Down Expand Up @@ -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).',
Expand Down
23 changes: 23 additions & 0 deletions tests/Fixtures/FormRequestToDto/SecondViolatorRequest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php

declare(strict_types = 1);

namespace App\Http\Requests;

use Illuminate\Foundation\Http\FormRequest;

// A second concrete violator, analysed alongside ViolatorRequest to prove
// that exempting one FQCN does NOT globally silence the rule — the other
// non-exempt violator must still fire.
final class SecondViolatorRequest extends FormRequest
{
/**
* @return array<string, mixed>
*/
public function rules(): array
{
return [
'name' => ['required', 'string'],
];
}
}
84 changes: 84 additions & 0 deletions tests/Rules/EnforceFormRequestToDtoRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading