fix: NEON double-backslash no-op in formRequestBaseClass + resourceDataBaseClass defaults

Goosterhof · claude · Goosterhof · commit ef0c5b1d9b3b · 2026-06-12T11:28:10.000+02:00
The shipped extension.neon parameter defaults used double-backslash single-quoted FQCNs. NEON only unescapes \\ inside double-quoted strings; in single quotes the backslashes stay literal, so the value decoded to a class name matching nothing and ClassReflection::isSubclassOf() returned false for every analysed class — both EnforceFormRequestToDtoRule and (pre-existing, since PR #20) EnforceResourceDataValidatorOptInRule were silent no-ops for any consumer using the default. CI stayed green because every test constructs the rule directly via the PHP ::class constructor default, never exercising the NEON registration path. Fixes (PR #33 review — jasperboerhof BLOCKER + MAJOR + MINOR, General-review concerns): - extension.neon: single-backslash defaults + inline NEON-quoting warning. - container-resolved regression test per rule (getAdditionalConfigFiles + getByType) pinning the NEON default and %parameter% wiring; both confirmed to fail when the double-backslash defect is reintroduced. - TraitProvidedToDtoRequest fixture + test (trait-provided toDto() satisfies the contract via hasNativeMethod trait flattening). - TransitiveViolatorRequest fixture + test (concrete leaf extends abstract base without toDto() — transitive omission must fire at the leaf). Gates: composer test (107), phpstan, format:check, coverage (88.06% >= 83) green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -18,6 +18,12 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and
 
 - `EnforceCurrentUserAttributeRule` — corrected the controller-detection gate from an ancestry check (`ClassReflection::isSubclassOf(Illuminate\Routing\Controller)`) to a namespace prefix check (`$scope->getNamespace()` starts with `App\Http\Controllers`). The ancestry gate was a **silent no-op**: every consumer territory (kendo, ublgenie, entreezuil) ships base-less `final` controllers with no `extends Controller`, so the ancestry walk matched **zero** controllers and the rule enforced nothing. The namespace gate mirrors the sibling `ForbidEloquentMutationInControllersRule` and the canonical "controllers are identified by the `App\Http\Controllers` namespace" convention. Caught by the agent-review sweep on PR #26 (review `pullrequestreview-4401182606`). Regression-proofed by a base-less `final` controller fixture (`RequestUserInBaselessController` — flagged; `CurrentUserAttributeInBaselessController` — clean) that reproduces the exact real-world shape the rule missed. **Versioning:** false-negative closure — the rule now actually fires where it always intended to; this is part of the same `[Unreleased]` Major bump the rule's addition already carries (consumers with un-migrated controllers will now see the errors). No new identifier; no consumer `ignoreErrors` migration shape change.
 
+- `EnforceFormRequestToDtoRule` / `EnforceResourceDataValidatorOptInRule` — corrected the shipped `extension.neon` parameter defaults from double-backslash single-quoted FQCNs (`'Illuminate\\Foundation\\Http\\FormRequest'`, `'App\\Http\\Resources\\ResourceData'`) to single-backslash (`'Illuminate\Foundation\Http\FormRequest'`, `'App\Http\Resources\ResourceData'`). NEON only unescapes `\\` inside **double**-quoted strings; in single quotes (and unquoted) `\\` stays two literal characters, so the parameter decoded to a 4-segment double-backslash class name that matches no real class. The effect: `ClassReflection::isSubclassOf()` returned false for every analysed class and **both rules were silent no-ops for any consumer registering `extension.neon` without an explicit parameter override** — CI stayed green because every PHPUnit test, the coverage gate, and Infection construct the rule directly via the PHP `::class` constructor default (PHP single-quoted strings *do* collapse `\\`), never exercising the NEON registration path. Verified empirically end-to-end: shipped default → `[OK] No errors` on the `ViolatorRequest` / `ViolatorResource` fixtures; single-backslash → fires with the exact expected message/line. The `resourceDataBaseClass` defect is pre-existing (shipped in PR #20, v0.3.0) — `EnforceResourceDataValidatorOptInRule` has been a no-op for default-configured consumers since release; fixed here in the same edit rather than deferred since `EnforceFormRequestToDtoRule` introduced the identical defect on the line above. Caught by the agent-review sweep on PR #33 (jasperboerhof BLOCKER). Regression-proofed by a container-resolved test per rule (`testRuleResolvesFromExtensionNeonAndFires` — resolves the rule from the PHPStan container via `getAdditionalConfigFiles()` + `getByType()`, exercising the NEON default and `%parameter%` wiring, then asserts the violator fires; both tests confirmed to fail when the double-backslash defect is reintroduced). An inline NEON-quoting warning comment now guards both defaults in `extension.neon`. **Versioning:** for `EnforceFormRequestToDtoRule`, this is part of the same `[Unreleased]` candidate-Major the rule's addition carries (the rule now actually fires in consumers). For `EnforceResourceDataValidatorOptInRule`, the v0.3.0 release that shipped the rule was a no-op for default consumers; restoring enforcement surfaces previously-undetected violations on `^0.3` consumers without a parameter override — **the pre-cascade audit demanded for both rules must now treat the resource-data rule as effectively un-enforced until this fix ships.**
+
+### Added
+
+- **Tests:** `EnforceFormRequestToDtoRule` gains two fixture-backed coverage additions closing documented-but-unpinned semantics: `TraitProvidedToDtoRequest` (a concrete request whose `toDto()` arrives via a trait — pins the trait leg of the `method_exists()` parity promise, which routes through PHPStan's `hasNativeMethod()` trait flattening) and `TransitiveViolatorRequest` (a concrete leaf extending the abstract `AbstractBaseRequest` with no `toDto()` anywhere in the chain — pins transitive-violation detection through an intermediate abstract layer, the inverse of the existing inherited-compliant case). Both raised in the PR #33 review (jasperboerhof MINOR + General-review concern). Test-only; no consumer-facing surface. **Versioning:** none (test coverage).
+
 ### Changed
 
 - **CI:** pinned `symfony/console` to `^7.2` in `require-dev`. `symfony/console` 8.x (v8.1.0, released 2026-05-29) breaks Infection 0.33.x's mutation runner — its DI container references `Symfony\Component\Console\Helper\QuestionHelper` as a service Symfony Console 8 no longer registers that way, so `composer mutation:ci` aborts with `Unknown service` and exits 1. Because the package's `composer.lock` is gitignored, CI resolves dependencies fresh on every run; `illuminate/*` v13 permits Symfony 8, so the resolver began pulling v8.1.0 and the mutation gate went red fleet-wide (PRs green on 2026-05-28 turned red on 2026-05-29 with no source change). The pin holds the dev toolchain at `symfony/console` v7.4.x — verified mutation gate green (Covered Code MSI 81% ≥ 75) — until Infection ships Symfony Console 8 support, at which point this constraint should be widened or removed. **Versioning:** none (dev-only test-infra; no consumer-facing surface).
diff --git a/extension.neon b/extension.neon
@@ -3,13 +3,17 @@ parameters:
     # base class. Default matches the kendo / emmie / ublgenie / entreezuil
     # convention `App\Http\Resources\ResourceData`. Override per consumer
     # `phpstan.neon` if a territory ships the base under a different FQCN.
-    resourceDataBaseClass: 'App\\Http\\Resources\\ResourceData'
+    # NOTE: single backslashes — NEON only unescapes `\\` inside DOUBLE quotes;
+    # in single quotes (and unquoted) `\\` stays two literal characters, which
+    # decodes to a class name that matches nothing and silently no-ops the rule.
+    resourceDataBaseClass: 'App\Http\Resources\ResourceData'
 
     # `EnforceFormRequestToDtoRule`: FQCN of the FormRequest base class.
     # Default matches the Laravel framework base every territory's requests
     # ultimately extend. Override per consumer `phpstan.neon` to narrow the
-    # contract to a territory-local base FQCN.
-    formRequestBaseClass: 'Illuminate\\Foundation\\Http\\FormRequest'
+    # contract to a territory-local base FQCN. Single backslashes — see the
+    # NEON-quoting note above.
+    formRequestBaseClass: 'Illuminate\Foundation\Http\FormRequest'
 
 parametersSchema:
     resourceDataBaseClass: string()
diff --git a/tests/Fixtures/FormRequestToDto/TraitProvidedToDtoRequest.php b/tests/Fixtures/FormRequestToDto/TraitProvidedToDtoRequest.php
@@ -0,0 +1,39 @@
+<?php
+
+declare(strict_types = 1);
+
+namespace App\Http\Requests;
+
+use App\DataTransferObjects\StoreUserData;
+use Illuminate\Foundation\Http\FormRequest;
+
+// Trait-provided toDto() must satisfy the contract — the rule routes through
+// PHPStan's hasNativeMethod(), which flattens trait-composed methods, mirroring
+// the source-of-truth Pest test's method_exists() matcher. Pins the trait leg
+// of the promise documented in the rule docblock, README, and CHANGELOG, which
+// otherwise rests on an untested assumption about PHPStan internals.
+trait ProvidesToDto
+{
+    public function toDto(): StoreUserData
+    {
+        /** @var string $name */
+        $name = $this->validated()['name'];
+
+        return new StoreUserData($name);
+    }
+}
+
+final class TraitProvidedToDtoRequest extends FormRequest
+{
+    use ProvidesToDto;
+
+    /**
+     * @return array<string, mixed>
+     */
+    public function rules(): array
+    {
+        return [
+            'name' => ['required', 'string'],
+        ];
+    }
+}
diff --git a/tests/Fixtures/FormRequestToDto/TransitiveViolatorRequest.php b/tests/Fixtures/FormRequestToDto/TransitiveViolatorRequest.php
@@ -0,0 +1,24 @@
+<?php
+
+declare(strict_types = 1);
+
+namespace App\Http\Requests;
+
+// Concrete leaf extending the abstract intermediate `AbstractBaseRequest`,
+// which declares no toDto() anywhere in the chain. Transitive-violation
+// detection must fire HERE, at the leaf — the inverse of
+// CompliantInheritedToDtoRequest (where the abstract parent supplies toDto()).
+// Proves the FQCN ancestor traversal detects omission through an intermediate
+// abstract layer, not only direct framework-base extension.
+final class TransitiveViolatorRequest extends AbstractBaseRequest
+{
+    /**
+     * @return array<string, mixed>
+     */
+    public function rules(): array
+    {
+        return [
+            'name' => ['required', 'string'],
+        ];
+    }
+}
diff --git a/tests/Rules/EnforceFormRequestToDtoRuleTest.php b/tests/Rules/EnforceFormRequestToDtoRuleTest.php
@@ -61,6 +61,71 @@ public function testCompliantInheritedToDtoIsNotFlagged(): void
         );
     }
 
+    public function testTraitProvidedToDtoIsNotFlagged(): void
+    {
+        // A concrete request whose toDto() arrives via a trait. The rule
+        // routes through PHPStan's hasNativeMethod(), which flattens
+        // trait-composed methods — pins the trait leg of the contract that
+        // the docblock, README, and CHANGELOG all promise but no fixture
+        // previously exercised.
+        $this->analyse(
+            [
+                __DIR__ . '/../Fixtures/FormRequestToDto/_stubs.php',
+                __DIR__ . '/../Fixtures/FormRequestToDto/TraitProvidedToDtoRequest.php',
+            ],
+            [],
+        );
+    }
+
+    public function testConcreteLeafExtendingAbstractBaseWithoutToDtoIsFlagged(): void
+    {
+        // The inverse of testCompliantInheritedToDtoIsNotFlagged: a concrete
+        // leaf extends the abstract intermediate `AbstractBaseRequest`, which
+        // declares no toDto() anywhere in the chain. Transitive-violation
+        // detection must fire at the leaf — proving the ancestor traversal
+        // catches omission through an intermediate abstract layer, not only
+        // direct framework-base extension.
+        $this->analyse(
+            [
+                __DIR__ . '/../Fixtures/FormRequestToDto/_stubs.php',
+                __DIR__ . '/../Fixtures/FormRequestToDto/AbstractBaseRequest.php',
+                __DIR__ . '/../Fixtures/FormRequestToDto/TransitiveViolatorRequest.php',
+            ],
+            [
+                [
+                    'App\Http\Requests\TransitiveViolatorRequest 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).',
+                    13,
+                ],
+            ],
+        );
+    }
+
+    public function testRuleResolvesFromExtensionNeonAndFires(): void
+    {
+        // End-to-end pin on the extension.neon registration path consumers
+        // actually use: resolve the rule from the PHPStan container so the
+        // shipped `formRequestBaseClass` default and the `%formRequestBaseClass%`
+        // argument wiring are exercised — NOT the PHP constructor default.
+        // A NEON quoting regression in the shipped default (e.g. the
+        // double-backslash single-quoted form) silently no-ops the rule while
+        // every direct-instantiation test stays green; this is the only gate
+        // that catches it.
+        $this->ruleOverride = self::getContainer()->getByType(EnforceFormRequestToDtoRule::class);
+
+        $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 testAbstractBaseWithoutToDtoIsNotFlagged(): void
     {
         // The per-territory `BaseFormRequest` shape: abstract, extends the
@@ -104,6 +169,20 @@ public function testCustomBaseClassParameterMatchesAlternativeFqcn(): void
         );
     }
 
+    /**
+     * Load the shipped extension.neon so testRuleResolvesFromExtensionNeonAndFires
+     * can pull the rule out of the container with its NEON-configured
+     * `formRequestBaseClass` parameter applied.
+     *
+     * @return array<int, string>
+     */
+    public static function getAdditionalConfigFiles(): array
+    {
+        return [
+            __DIR__ . '/../../extension.neon',
+        ];
+    }
+
     protected function getRule(): Rule
     {
         return $this->ruleOverride ?? new EnforceFormRequestToDtoRule;
diff --git a/tests/Rules/EnforceResourceDataValidatorOptInRuleTest.php b/tests/Rules/EnforceResourceDataValidatorOptInRuleTest.php
@@ -136,6 +136,45 @@ public function testCustomBaseClassParameterMatchesAlternativeFqcn(): void
         );
     }
 
+    public function testRuleResolvesFromExtensionNeonAndFires(): void
+    {
+        // End-to-end pin on the extension.neon registration path consumers
+        // actually use: resolve the rule from the PHPStan container so the
+        // shipped `resourceDataBaseClass` default and the
+        // `%resourceDataBaseClass%` argument wiring are exercised — NOT the PHP
+        // constructor default. The shipped default carried a NEON
+        // double-backslash quoting defect that silently no-op'd this rule for
+        // every default consumer since PR #20; this gate catches it.
+        $this->ruleOverride = self::getContainer()->getByType(EnforceResourceDataValidatorOptInRule::class);
+
+        $this->analyse(
+            [
+                __DIR__ . '/../Fixtures/ResourceDataValidatorOptIn/_stubs.php',
+                __DIR__ . '/../Fixtures/ResourceDataValidatorOptIn/ViolatorResource.php',
+            ],
+            [
+                [
+                    'App\Http\Resources\ViolatorResource declares EAGER_LOAD_COUNT but does not call validateRelationsLoaded() — silent-zero bug risk (ADR-0009 / war-room queue #55 / kendo PR #1084 opt-in invariant).',
+                    9,
+                ],
+            ],
+        );
+    }
+
+    /**
+     * Load the shipped extension.neon so testRuleResolvesFromExtensionNeonAndFires
+     * can pull the rule out of the container with its NEON-configured
+     * `resourceDataBaseClass` parameter applied.
+     *
+     * @return array<int, string>
+     */
+    public static function getAdditionalConfigFiles(): array
+    {
+        return [
+            __DIR__ . '/../../extension.neon',
+        ];
+    }
+
     protected function getRule(): Rule
     {
         return $this->ruleOverride ?? new EnforceResourceDataValidatorOptInRule;
