Skip to content

Commit 1bbdf80

Browse files
Goosterhofclaude
andcommitted
engineer: add EnforceResourceDataValidatorOptInRule (queue #55 Phase-2 promotion)
Promotes kendo PR #1084's Pest arch test (third instance of the "arch test detects misuse but not omission" enforcement shape) from Level-1 territory- local to Level-2 cross-territory PHPStan rule. Fires on classes extending `App\Http\Resources\ResourceData` that declare a non-empty `EAGER_LOAD_COUNT` / `EAGER_LOAD_SUM` constant but never call `validateRelationsLoaded()` — the silent-zero bug closed by kendo PR #1079 (KD-0494) re-introduces silently without the opt-in call. Doctrine: ADR-0009 §EAGER_LOAD validator opt-in. Identifier: enforceResourceDataValidatorOptIn.missingValidatorCall. Configurable base FQCN: `resourceDataBaseClass` parameter (default `App\Http\Resources\ResourceData`). Source-of-truth Pest arch test: kendo `tests/Arch/ResourcesTest.php` lines 157-265 at commit `db20ea9cf` (merged 2026-05-07). War-room enforcement queue: #55. /intel campaign: campaigns/war-room/2026-05-07-intel-pattern-analysis.md. Commander disposition (2026-05-07): extract now, do not wait for a fourth instance — criterion was procedural, not principled; rule shape is namespace- agnostic and PHPStan-portable today. This dispatch covers ONLY the ResourceData shape. Sister extractions for the FormRequest `toDto()` omission shape (queue #55 instance #2 — entreezuil already has the stronger Pest arch test form) and the routes `->can()` omission shape (queue #55 instance #1) are deferred to separate dispatches. Implementation notes: - Registers on `InClassNode` (not bare `Class_`) so `getClassReflection()` is reliably available — at the bare `Class_` node, scope has not yet entered the class so reflection may be null. - Inheritance gate uses PHPStan's `ClassReflection::isSubclassOf()` — FQCN-based, not short-name, so `App\Unrelated\ResourceData` collisions do not match. - Empty-array constants (`EAGER_LOAD_COUNT = []`) are no-ops and do not fire. - Compliant call shapes: `self::`, `static::`, and `$this->` (instance form accepted for liberal compatibility with kendo Pest matcher even though the production base method is `protected static`). CHANGELOG entry filed under `[Unreleased] / Added` flagging this as a candidate Major bump per ADR-0021 §Versioning. Release PR (separate dispatch) will determine whether this collapses into the same Major as the staged LogRule static-call expansion or cuts a separate Major. Verification gate (all green): - composer test: 41/41 (added 9 new test cases on this rule) - composer phpstan (self-analysis at level max): 0 errors - composer format:check: pass - composer coverage:check: 85.67% > 83% threshold (up from 83.92% baseline) - composer mutation:ci: 79.57% MSI > 75% threshold (up from 78.5% baseline) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 5c3e792 commit 1bbdf80

15 files changed

Lines changed: 657 additions & 1 deletion

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and
66

77
## [Unreleased]
88

9+
### Added
10+
11+
- `EnforceResourceDataValidatorOptInRule` — flags classes extending `App\Http\Resources\ResourceData` that declare a non-empty `EAGER_LOAD_COUNT` or `EAGER_LOAD_SUM` constant but do not call `validateRelationsLoaded()` anywhere in their method bodies. Without the call, missing eager-load aggregates fail open as `0` / `null` instead of throwing — silently re-introducing the silent-zero bug closed by kendo PR #1079 (KD-0494). Doctrine: ADR-0009 §EAGER_LOAD validator opt-in. Identifier: `enforceResourceDataValidatorOptIn.missingValidatorCall`. Promoted from kendo PR #1084's Pest arch test (Armorer, merged 2026-05-07 at `db20ea9cf`) — the third instance of the "arch test detects misuse but not omission" enforcement shape, dispositioned for Phase-2 promotion under war-room enforcement queue #55 by the Commander on 2026-05-07. Inheritance is matched via PHPStan reflection (FQCN ancestor traversal) — short-name collisions in unrelated namespaces do NOT match. The base FQCN is parameterizable via the `resourceDataBaseClass` PHPStan parameter (default: `App\Http\Resources\ResourceData`); territories whose `ResourceData` lives elsewhere can override per consumer `phpstan.neon`. Compliant call shapes: `self::validateRelationsLoaded($model)`, `static::validateRelationsLoaded($model)`, `$this->validateRelationsLoaded($model)` (instance form accepted for liberal compatibility with the source-of-truth Pest matcher, even though the base method is `protected static`). Empty-array constants (`EAGER_LOAD_COUNT = []`) do NOT fire — they are no-ops. **Versioning: per ADR-0021 §Versioning, candidate Major bump (the rule surfaces new errors in already-clean code wherever a consumer territory has a `ResourceData` subclass declaring the aggregate constants without the validator call). The release PR will determine whether v0.3.0 collapses this rule into the same Major bump as the staged `LogRule` static-call expansion, or cuts a separate Major.** **Pre-cascade audit required across emmie, kendo, entreezuil, ublgenie, brick-inventory before tagging** — the kendo arch test already closed kendo's standing proof point (`ProjectGithubRepoResourceData`) in PR #1084, but other consumer territories may carry undetected violators. Sister extractions for the FormRequest `toDto()` omission shape (queue #55 instance #2) and the routes `->can()` middleware omission shape (queue #55 instance #1) are deferred to separate dispatches.
12+
913
### Changed
1014

1115
- **Doctrine:** corrected publish-channel framing in `CLAUDE.md` (L11 and the Release process section) and the `release.yml` header comment. Public packagist.org has no OIDC Trusted Publishing option today — OIDC is a Private Packagist–only feature (`packagist/artifact-publish-github-action`, GA February 2026). The package's actual publish channel is the standard `https://packagist.org/api/github` push-event webhook (`dev-*` aliases on branch push, versioned releases on tag push via `release.yml`). Migration to Private Packagist would change ally-side Composer consumption (private repo URL + token in `composer.json`) and is a commercial decision; tracking continues on Issue #11. Closes Sapper M1 Finding #2 (doctrine drift on publish channel) and resolves Issue #11 audit. **Versioning:** none (doctrine alignment, no consumer-visible behaviour).

CLAUDE.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,10 @@ Composer package distributing war-room-doctrine PHPStan rules across `script-dev
2525
| `ForbidAbortHelperRule` | War-room §Explicit over implicit | `forbidAbortHelper.abortUsed` |
2626
| `LogRule` | ADR-0001 §Append-only | `logRule.logModification` |
2727
| `EnforceAuditSnapshotOnRetryRule` | ADR-0001 §Snapshot-on-Retry Safety | `enforceAuditSnapshotOnRetry.firstStatementMustResetState` |
28+
| `EnforceResourceDataValidatorOptInRule` | ADR-0009 §EAGER_LOAD validator opt-in | `enforceResourceDataValidatorOptIn.missingValidatorCall` |
2829
| `ConnectionTransactionReturnTypeExtension` | (type extension, no rule) ||
2930

30-
Phase 2 expands the rule set: `EnforceAuditSnapshotOnRetryRule` (ADR-0001 §Snapshot-on-Retry Safety) is the first Phase 2 addition, promoted from cross-territory Pest arch tests (emmie PR #187, entreezuil PR #139, ublgenie PR #166, kendo PR #1029). `EnforceExplicitHydrationRule` (ADR-0019) is the next Phase 2 candidate.
31+
Phase 2 expands the rule set: `EnforceAuditSnapshotOnRetryRule` (ADR-0001 §Snapshot-on-Retry Safety) was the first Phase 2 addition, promoted from cross-territory Pest arch tests (emmie PR #187, entreezuil PR #139, ublgenie PR #166, kendo PR #1029). `EnforceResourceDataValidatorOptInRule` (ADR-0009 §EAGER_LOAD validator opt-in) is the second Phase 2 addition, promoted from kendo PR #1084 under war-room enforcement queue #55. `EnforceExplicitHydrationRule` (ADR-0019) is the next Phase 2 candidate.
3132

3233
## Conventions
3334

README.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ includes:
4141
| `ForbidDatabaseManagerInActionsRule` | `forbidDatabaseManager.inAction` | Action constructors | Constructor parameter typed `DatabaseManager` is an error. Inject `ConnectionInterface` instead. |
4242
| `ForbidAbortHelperRule` | `forbidAbortHelper.abortUsed` | Function calls | `abort()`, `abort_if()`, `abort_unless()` are errors. Throw an explicit `HttpException` subclass instead. |
4343
| `LogRule` | `logRule.logModification` | `update()` / `delete()` calls | If the receiver type's class name contains `"Log"` or `"logs"` (case-insensitive), error. |
44+
| `EnforceResourceDataValidatorOptInRule` | `enforceResourceDataValidatorOptIn.missingValidatorCall` | Classes extending `App\Http\Resources\ResourceData` | If the class declares a non-empty `EAGER_LOAD_COUNT` / `EAGER_LOAD_SUM` constant but never calls `validateRelationsLoaded()` in any method, error. |
4445

4546
### `EnforceActionTransactionsRule` — write-method list
4647

@@ -64,6 +65,17 @@ parameters:
6465

6566
Each ignore should carry a comment with rationale. Future versions may add an explicit allow-list parameter — file an issue if you have a recurring need.
6667

68+
### `EnforceResourceDataValidatorOptInRule` — configurable base class
69+
70+
The rule scopes to classes extending `App\Http\Resources\ResourceData` by default. If a territory ships its abstract resource base under a different FQCN, override the `resourceDataBaseClass` parameter in `phpstan.neon`:
71+
72+
```neon
73+
parameters:
74+
resourceDataBaseClass: 'App\Resources\BaseResource'
75+
```
76+
77+
Inheritance is matched via PHPStan reflection (FQCN ancestor traversal), not short-name matching — a class named `ResourceData` in an unrelated namespace will not be matched. Compliant call shapes are `self::validateRelationsLoaded($model)`, `static::validateRelationsLoaded($model)`, and `$this->validateRelationsLoaded($model)` — the production base method is `protected static`, but the instance form is also accepted for compatibility with the source-of-truth Pest arch test's permissive matcher. Empty-array constants (`EAGER_LOAD_COUNT = []`) do not fire — they are no-ops.
78+
6779
### Action namespace assumption
6880

6981
`EnforceActionTransactionsRule` and `ForbidDatabaseManagerInActionsRule` only fire on classes whose namespace starts with `App\Actions`. This matches the Laravel convention used in every `script-development` territory. Territories using a different actions namespace should open a PR to make this configurable.

extension.neon

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,13 @@
1+
parameters:
2+
# `EnforceResourceDataValidatorOptInRule`: FQCN of the abstract resource
3+
# base class. Default matches the kendo / emmie / ublgenie / entreezuil
4+
# convention `App\Http\Resources\ResourceData`. Override per consumer
5+
# `phpstan.neon` if a territory ships the base under a different FQCN.
6+
resourceDataBaseClass: 'App\\Http\\Resources\\ResourceData'
7+
8+
parametersSchema:
9+
resourceDataBaseClass: string()
10+
111
services:
212
-
313
class: ScriptDevelopment\PhpstanWarroomRules\Rules\EnforceActionTransactionsRule
@@ -14,6 +24,11 @@ services:
1424
-
1525
class: ScriptDevelopment\PhpstanWarroomRules\Rules\EnforceAuditSnapshotOnRetryRule
1626
tags: [phpstan.rules.rule]
27+
-
28+
class: ScriptDevelopment\PhpstanWarroomRules\Rules\EnforceResourceDataValidatorOptInRule
29+
arguments:
30+
resourceDataBaseClass: %resourceDataBaseClass%
31+
tags: [phpstan.rules.rule]
1732
-
1833
class: ScriptDevelopment\PhpstanWarroomRules\Type\ConnectionTransactionReturnTypeExtension
1934
tags: [phpstan.broker.dynamicMethodReturnTypeExtension]
Lines changed: 262 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,262 @@
1+
<?php
2+
3+
declare(strict_types = 1);
4+
5+
namespace ScriptDevelopment\PhpstanWarroomRules\Rules;
6+
7+
use PhpParser\Node;
8+
use PhpParser\Node\Expr\Array_;
9+
use PhpParser\Node\Expr\MethodCall;
10+
use PhpParser\Node\Expr\StaticCall;
11+
use PhpParser\Node\Identifier;
12+
use PhpParser\Node\Stmt\Class_;
13+
use PhpParser\Node\Stmt\ClassConst;
14+
use PhpParser\Node\Stmt\ClassMethod;
15+
use PHPStan\Analyser\Scope;
16+
use PHPStan\Node\InClassNode;
17+
use PHPStan\Reflection\ClassReflection;
18+
use PHPStan\Rules\Rule;
19+
use PHPStan\Rules\RuleErrorBuilder;
20+
21+
use function array_filter;
22+
use function implode;
23+
use function in_array;
24+
use function is_array;
25+
use function sprintf;
26+
27+
/**
28+
* Enforces ADR-0009 §EAGER_LOAD validator opt-in: a `ResourceData` subclass
29+
* declaring a non-empty `EAGER_LOAD_COUNT` or `EAGER_LOAD_SUM` constant must
30+
* call `validateRelationsLoaded()` somewhere in its method bodies. Without
31+
* the call, missing eager-load aggregates fail open as `0` / `null` rather
32+
* than throwing — silently re-introducing the silent-zero bug closed by
33+
* kendo PR #1079 (KD-0494).
34+
*
35+
* Doctrine source: ADR-0009 §EAGER_LOAD validator opt-in. Codified after
36+
* kendo PR #1084 (Armorer, merged 2026-05-07 at db20ea9cf) added a Pest
37+
* arch test of this shape; war-room enforcement queue #55 promoted the
38+
* Level-1 territory-local arch test to a Level-2 cross-territory PHPStan
39+
* rule on 2026-05-07.
40+
*
41+
* Scope: classes whose ancestor chain includes the configured base FQCN
42+
* (default: `App\Http\Resources\ResourceData`). Inheritance is matched
43+
* via PHPStan reflection — short-name collisions in unrelated namespaces
44+
* do not fire.
45+
*
46+
* Detection (all three must hold):
47+
* 1. Class transitively extends the configured base class.
48+
* 2. Class declares at least one of `EAGER_LOAD_COUNT` / `EAGER_LOAD_SUM`
49+
* as an own constant with a non-empty array value (empty array `[]`
50+
* is a no-op and does not fire).
51+
* 3. Class body does NOT contain a method-call or static-call with name
52+
* `validateRelationsLoaded` anywhere in any method (recursive walk).
53+
*
54+
* Compliant call shapes (any one suffices):
55+
* - `self::validateRelationsLoaded($model);`
56+
* - `static::validateRelationsLoaded($model);`
57+
* - `$this->validateRelationsLoaded($model);` (the base method is
58+
* `protected static`; the instance form is also accepted for
59+
* liberal compatibility with the source-of-truth Pest arch test).
60+
*
61+
* Implementation note: registers on `InClassNode` rather than `Class_` to
62+
* guarantee `$scope->getClassReflection()` is available — at the bare
63+
* `Class_` node, the scope has not yet entered the class so reflection
64+
* may be null. The AST walk is local; we only need to confirm presence
65+
* anywhere in the class body, not type-resolve receivers.
66+
*
67+
* @implements Rule<InClassNode>
68+
*/
69+
final class EnforceResourceDataValidatorOptInRule implements Rule
70+
{
71+
private const array TARGET_CONSTANTS = [
72+
'EAGER_LOAD_COUNT',
73+
'EAGER_LOAD_SUM',
74+
];
75+
76+
private const string VALIDATOR_METHOD_NAME = 'validateRelationsLoaded';
77+
78+
public function __construct(
79+
private string $resourceDataBaseClass = 'App\Http\Resources\ResourceData',
80+
) {}
81+
82+
public function getNodeType(): string
83+
{
84+
return InClassNode::class;
85+
}
86+
87+
public function processNode(Node $node, Scope $scope): array
88+
{
89+
$classNode = $node->getOriginalNode();
90+
91+
if (!$classNode instanceof Class_) {
92+
return [];
93+
}
94+
95+
if ($classNode->isAbstract()) {
96+
return [];
97+
}
98+
99+
$classReflection = $node->getClassReflection();
100+
101+
if (!$this->extendsResourceDataBase($classReflection)) {
102+
return [];
103+
}
104+
105+
$declaredConstants = $this->declaredAggregateConstants($classNode);
106+
107+
if ($declaredConstants === []) {
108+
return [];
109+
}
110+
111+
if ($this->classBodyCallsValidator($classNode)) {
112+
return [];
113+
}
114+
115+
return [
116+
RuleErrorBuilder::message(sprintf(
117+
'%s declares %s but does not call validateRelationsLoaded() — silent-zero bug risk (ADR-0009 / war-room queue #55 / kendo PR #1084 opt-in invariant).',
118+
$classReflection->getName(),
119+
implode(', ', $declaredConstants),
120+
))
121+
->identifier('enforceResourceDataValidatorOptIn.missingValidatorCall')
122+
->line($classNode->getStartLine())
123+
->build(),
124+
];
125+
}
126+
127+
/**
128+
* Inheritance gate: the class must be a (transitive) subclass of the
129+
* configured base FQCN. Uses PHPStan reflection — handles intermediate
130+
* abstract layers and namespace-relative `extends` clauses. Short-name
131+
* collisions in unrelated namespaces do not match.
132+
*/
133+
private function extendsResourceDataBase(ClassReflection $classReflection): bool
134+
{
135+
if ($classReflection->getName() === $this->resourceDataBaseClass) {
136+
return false;
137+
}
138+
139+
return $classReflection->isSubclassOf($this->resourceDataBaseClass);
140+
}
141+
142+
/**
143+
* Returns short names of TARGET_CONSTANTS declared on this class with a
144+
* non-empty array value. Inherited constants are skipped — only own
145+
* declarations on this class create the obligation.
146+
*
147+
* @return list<string>
148+
*/
149+
private function declaredAggregateConstants(Class_ $node): array
150+
{
151+
$declared = [];
152+
153+
foreach ($node->stmts as $stmt) {
154+
if (!$stmt instanceof ClassConst) {
155+
continue;
156+
}
157+
158+
foreach ($stmt->consts as $const) {
159+
$name = $const->name->toString();
160+
161+
if (!in_array($name, self::TARGET_CONSTANTS, true)) {
162+
continue;
163+
}
164+
165+
if (!$const->value instanceof Array_) {
166+
continue;
167+
}
168+
169+
if ($const->value->items === []) {
170+
continue;
171+
}
172+
173+
$declared[] = $name;
174+
}
175+
}
176+
177+
return $declared;
178+
}
179+
180+
/**
181+
* Walks every method in the class body looking for a method-call or
182+
* static-call to `validateRelationsLoaded`. Only the call's *name* is
183+
* checked — any receiver shape (`self::`, `static::`, `$this->`,
184+
* `parent::`, `Foo::`) suffices, mirroring the source-of-truth kendo
185+
* arch test's permissive matcher.
186+
*/
187+
private function classBodyCallsValidator(Class_ $node): bool
188+
{
189+
$found = false;
190+
191+
foreach ($node->stmts as $stmt) {
192+
if (!$stmt instanceof ClassMethod) {
193+
continue;
194+
}
195+
196+
if ($stmt->stmts === null) {
197+
continue;
198+
}
199+
200+
$this->walkNodes($stmt->stmts, function(Node $inner) use (&$found): void {
201+
if ($found) {
202+
return;
203+
}
204+
205+
if (
206+
$inner instanceof MethodCall
207+
&& $inner->name instanceof Identifier
208+
&& $inner->name->toString() === self::VALIDATOR_METHOD_NAME
209+
) {
210+
$found = true;
211+
212+
return;
213+
}
214+
215+
if (
216+
$inner instanceof StaticCall
217+
&& $inner->name instanceof Identifier
218+
&& $inner->name->toString() === self::VALIDATOR_METHOD_NAME
219+
) {
220+
$found = true;
221+
}
222+
});
223+
224+
if ($found) {
225+
return true;
226+
}
227+
}
228+
229+
return false;
230+
}
231+
232+
/**
233+
* Recursively walk a list of nodes, invoking `$callback` on each one.
234+
* Mirrors `EnforceActionTransactionsRule::walkNodes()` /
235+
* `EnforceAuditSnapshotOnRetryRule::walkNodes()` for parity.
236+
*
237+
* @param array<int|string, Node|null> $nodes
238+
*/
239+
private function walkNodes(array $nodes, callable $callback): void
240+
{
241+
foreach ($nodes as $node) {
242+
if (!$node instanceof Node) {
243+
continue;
244+
}
245+
246+
$callback($node);
247+
248+
foreach ($node->getSubNodeNames() as $name) {
249+
$subNode = $node->{$name};
250+
251+
if ($subNode instanceof Node) {
252+
$this->walkNodes([$subNode], $callback);
253+
} elseif (is_array($subNode)) {
254+
$this->walkNodes(
255+
array_filter($subNode, static fn(mixed $item): bool => $item instanceof Node),
256+
$callback,
257+
);
258+
}
259+
}
260+
}
261+
}
262+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?php
2+
3+
declare(strict_types = 1);
4+
5+
namespace App\Http\Resources;
6+
7+
use App\Models\Project;
8+
9+
final class CompliantInstanceCallResource extends ResourceData
10+
{
11+
public const array EAGER_LOAD_COUNT = ['issues'];
12+
13+
public function hydrate(Project $project): void
14+
{
15+
// Instance form is accepted for liberal compatibility with the
16+
// source-of-truth kendo arch test's permissive matcher.
17+
$this->validateRelationsLoaded($project);
18+
}
19+
}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
3+
declare(strict_types = 1);
4+
5+
namespace App\Http\Resources;
6+
7+
use App\Models\Project;
8+
9+
final class CompliantSelfCallResource extends ResourceData
10+
{
11+
public const array EAGER_LOAD_SUM = ['timeEntries:minutes_spent'];
12+
13+
public static function fromModel(Project $project): self
14+
{
15+
self::validateRelationsLoaded($project);
16+
17+
return new self([
18+
'id' => $project->id,
19+
'name' => $project->name,
20+
]);
21+
}
22+
}

0 commit comments

Comments
 (0)