diff --git a/CHANGELOG.md b/CHANGELOG.md index 59b9774..947956a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,9 +8,15 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and ### Added +- `EnforceCurrentUserAttributeRule` — flags calls to `Request::user()` / `Auth::user()` / `auth()->user()` inside classes extending `Illuminate\Routing\Controller`. The canonical fix is Laravel's `#[\Illuminate\Container\Attributes\CurrentUser]` container attribute on the method parameter — eliminates the implicit-nullable-then-assert dance (`$user = $request->user(); assert($user instanceof User);`) introduced by emmie PR #263 (EMMIE-0197) and recurring across the war-room fleet. Doctrine: war-room §Architectural Principles — Explicit over implicit. Identifier: `enforceCurrentUserAttribute.useAttributeInsteadOfRequestUser`. Detection branches on three call shapes via `CallLike` registration (mirrors `LogRule` v0.3.0 shape): `MethodCall` on `Illuminate\Http\Request` subtype receiver (type-based via `ObjectType::isSuperTypeOf()`); `MethodCall` whose receiver is a `FuncCall('auth')` (AST-shape match — the helper's return type is unloaded in stub-only environments); `StaticCall` resolving to `Illuminate\Support\Facades\Auth` (FQCN comparison via `$scope->resolveName()`). Scoped to controllers via the `App\Http\Controllers` namespace prefix (`$scope->getNamespace()` + `str_starts_with`) — mirrors `ForbidEloquentMutationInControllersRule` and the canonical "controllers are identified by the `App\Http\Controllers` namespace" convention. FormRequest (`App\Http\Requests`, where `$this->user()` is canonical because container-attribute injection does not apply to `FormRequest::rules()` / `toDto()` / `authorize()` invocations), middleware (`App\Http\Middleware`), services, Actions (`App\Actions`), jobs, and console commands are silent because their namespaces do not start with the controller prefix. Origin: war-room cross-territory recon 2026-05-22 (50+ violations across codebook, ublgenie, entreezuil, emmie; kendo already clean with 30 adopted sites). **Versioning: per ADR-0021 §Versioning, candidate Major bump (the rule surfaces new errors in already-clean code wherever a consumer territory has un-migrated controllers). The release PR will determine whether this collapses into the existing v0.3.0 [Unreleased] block (already a Major) or cuts as a separate Major (v0.4.0) after v0.3.0 ships.** **Pre-cascade audit required before tagging** — consuming territories will need either Medic dispatches to migrate (ublgenie 6 sites, entreezuil 3 sites, emmie 2 sites) or PHPStan-baseline staging (codebook ~40+ sites — safer than mass-edit on lightly-staffed AVG/NEN-7510-downstream territory). Kendo gets a constraint bump only (zero violations). Multi-guard ergonomics (`#[CurrentUser] Client $client` resolves via client guard, `#[CurrentUser] User $user` via user guard — verified in emmie's `ClientController::me`) work as expected: Laravel dispatches by typed parameter. Out of scope v1: `Auth::guard('name')->user()` and other guard-specific resolution paths — rare, substitution is more nuanced (`#[CurrentUser(guard: 'name')]`), do not appear in the recon yield. + - `ForbidEloquentMutationInControllersRule` — bans Eloquent persistence APIs (`save`, `update`, `delete`, `create`, `destroy`, `forceDelete`, `forceFill`, `push`, `restore`, `touch`, and their `*OrFail` / `*Quietly` / `*OrCreate` variants — 24-method blocklist) on `Illuminate\Database\Eloquent\Model` subclasses and `Illuminate\Database\Eloquent\Builder` chains when the call site is inside an `App\Http\Controllers\*` class (including sub-namespaces like kendo's `App\Http\Controllers\Central\*`, matched via `str_starts_with`). Reads (`find`, `where`, `get`, `first`, `paginate`, `pluck`, `count`, `exists`, `query`) are deliberately permitted — route-model binding, ResourceData hydration, and policy checks need controller-level Model access; the doctrine line is "Controllers may READ Models, but MUST NOT mutate them." Identifier: `forbidEloquentMutationInControllers.eloquentMutationInController`. Doctrine: ADR-0011 (Action Class Architecture) — Actions are the chokepoint for mutations — combined with ADR-0019 (Explicit Model Hydration) — `Model::create()` / `fill()` / `forceFill()` / `update()` banned application-wide; this rule enforces the controller surface where the violations have been historically common. Algorithm: namespace gate (`App\Http\Controllers`) → recursively walk every `ClassMethod` body collecting `MethodCall` + `StaticCall` nodes → for `MethodCall`, fire if `ObjectType::isSuperTypeOf()` against `Model` OR `Builder` matches the receiver type and the method name is in the blocklist; for `StaticCall`, fire if `Scope::resolveName()` resolves to a Model subclass FQCN and the method name is in the blocklist. Builder coverage is type-aware (`User::query()->where(...)->update([...])` fires) — the generic parameter is not unwrapped because `ObjectType` matches `Builder` as a subtype of the unparameterized `Builder` cleanly, no brittle generic introspection needed. Supersedes the consumer-side string-match Pest arch tests in kendo (`backend/tests/Arch/ControllersTest.php` `controllers must not call Eloquent write methods directly`), ublgenie + entreezuil (`tests/Arch/ControllersTest.php` of the same shape), and the bridge subset in ISMS (`backend/tests/Architecture/ControllerCurrentUserTest.php` from PR #10, 2026-05-28). The string-match shape catches `->save(`, `->update([`, `->delete(`, `->forceDelete(` but cannot discriminate `Model::create()` from `Response::create()`, `Collection::push()` from `Model::push()`, or `->update($vars)` without an inline array literal — the type-aware AST inspection here closes those gaps. Cross-territory cascade post-merge: consumer Pest tests deleted; emmie + brick-inventory-orchestrator pick up coverage automatically on next composer update. Out of scope: non-`App\Http\Controllers\*` namespaces (Actions/Services/Jobs/Middleware are allowed to call persistence APIs), non-Eloquent receivers, dynamic method names (`$model->{$var}()` — value-flow analysis), variable class names in static calls (`$class::create(...)`). Closes war-room enforcement queue #87. **Versioning: per ADR-0021 §Versioning, candidate Major bump (the rule surfaces new errors in already-clean code wherever a consumer territory has a controller calling Eloquent persistence APIs directly — the three territories currently running the string-match Pest test caught the bulk of these, but the type-aware shape will surface additional violations the string-match shape missed: `Model::create()`, `Model::destroy()`, chained Builder mutations, `*Quietly` variants, etc.). Pre-cascade audit required across ISMS, kendo, emmie, entreezuil, ublgenie, brick-inventory before tagging — three territories' Pest tests already closed the string-match-visible violators; the type-aware additional surface (Builder chains, `Model::create()`, `*Quietly` variants) may carry undetected violators.** - `EnforceAuditTransactionScopeRule` — enforces ADR-0029 (Audit Row Durability Contract) §Decision rule 3. Flags non-transactional state mutations (`StatefulGuard` / `Session` / `Cache` / `Bus` / `Queue` / `Mailer` / `Notification` / `Broadcaster` / `Filesystem` and their `Illuminate\Support\Facades\*` counterparts, mutation methods only) inside `transaction(...)` closures in `App\Actions\*` classes. Identifier: `enforceAuditTransactionScope.nonTransactionalMutationInClosure`. Doctrine: ADR-0029 §Decision rule 3. Seed: ISMS-0003 PR #7 commit `f1d357b` (2026-05-28) — three Auth Actions (`AuthenticateWorkerAction`, `VerifyTwoFactorChallengeAction`, `LogoutWorkerAction`) mutated `StatefulGuard` + `Session` state inside the transaction closure before the audit row write; an audit-write failure would have rolled back the audit row while leaving the session/guard mutation intact (A.8.15 / A.5.33 violation). Reads (`Auth::user()`, `Session::get()`, `Cache::get()`, etc.) are deliberately permitted — only mutations carry the rollback-vs-side-effect asymmetry. Instance-call detection matches the constructor-property's declared FQCN against the blocklist keys; static-call detection resolves the facade name via `Scope::resolveName()`. Nested `transaction(...)` calls inside an outer closure are walked transitively — a nested mutation is still inside the outermost transaction's rollback scope; top-level transaction discovery deduplicates so each violation reports exactly once. Out of scope: manual transaction management (`DB::beginTransaction()` / `commit()`); non-`App\Actions\*` namespaces; the failure-side discipline (sentinel-return; throw-inside-closure detection) which lives as per-territory Pest arch tests under enforcement queue #85. **Versioning: per ADR-0021 §Versioning, candidate Major bump (the rule surfaces new errors in already-clean code wherever a consumer territory has an `App\Actions\*` class mutating non-transactional state inside a `transaction(...)` closure). Pre-cascade audit required across ISMS, kendo, emmie, entreezuil, ublgenie, brick-inventory before tagging — ISMS-0003 PR #7 commit `f1d357b` already closed ISMS's known violators; other consumer territories may carry undetected violators.** +### Fixed + +- `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. + ### 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/CLAUDE.md b/CLAUDE.md index d133930..4103457 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -29,6 +29,7 @@ Composer package distributing war-room-doctrine PHPStan rules across `script-dev | `EnforceAuditTransactionScopeRule` | ADR-0029 | `enforceAuditTransactionScope.nonTransactionalMutationInClosure` | | `ForbidEloquentMutationInControllersRule` | ADR-0011 + ADR-0019 | `forbidEloquentMutationInControllers.eloquentMutationInController` | | `EnforceResourceDataValidatorOptInRule` | ADR-0009 §EAGER_LOAD validator opt-in | `enforceResourceDataValidatorOptIn.missingValidatorCall` | +| `EnforceCurrentUserAttributeRule` | War-room §Explicit over implicit | `enforceCurrentUserAttribute.useAttributeInsteadOfRequestUser` | | `ConnectionTransactionReturnTypeExtension` | (type extension, no rule) | — | 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. diff --git a/README.md b/README.md index ac86fb0..55ffc77 100644 --- a/README.md +++ b/README.md @@ -46,6 +46,7 @@ includes: | `EnforceAuditTransactionScopeRule` | `enforceAuditTransactionScope.nonTransactionalMutationInClosure` | `App\Actions\*` whose `execute()` calls `transaction(...)` with a literal closure | Mutating `StatefulGuard` / `Session` / `Cache` / `Bus` / `Queue` / `Mailer` / `Notification` / `Broadcaster` / `Filesystem` state (or their `Illuminate\Support\Facades\*` counterparts) inside the closure is an error. Reads (`Auth::user()`, `Session::get()`, `Cache::get()`) are permitted. Doctrine: ADR-0029 (Audit Row Durability Contract) §Decision rule 3. | | `ForbidEloquentMutationInControllersRule` | `forbidEloquentMutationInControllers.eloquentMutationInController` | `App\Http\Controllers\*` (including sub-namespaces) | Calling Eloquent persistence APIs (`save`, `update`, `delete`, `create`, `destroy`, `forceDelete`, `forceFill`, `push`, `restore`, `touch`, and their `*OrFail` / `*Quietly` / `*OrCreate` variants — 24-method blocklist) on `Illuminate\Database\Eloquent\Model` subclasses or `Illuminate\Database\Eloquent\Builder` chains is an error. Reads (`find`, `where`, `get`, `first`, `paginate`, `pluck`, `count`, `exists`, `query`) are permitted. Delegate mutations to an Action. Doctrine: ADR-0011 (Action Class Architecture) + ADR-0019 (Explicit Model Hydration). | | `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. | +| `EnforceCurrentUserAttributeRule` | `enforceCurrentUserAttribute.useAttributeInsteadOfRequestUser` | `Request::user()` / `Auth::user()` / `auth()->user()` calls inside Controller descendants | Use `#[\Illuminate\Container\Attributes\CurrentUser] User $user` on the method parameter. FormRequest descendants, middleware, services, Actions, jobs, and console commands are silent — container-attribute injection does not apply to FormRequest methods, and the other contexts have their own canonical resolution paths. | ### `EnforceActionTransactionsRule` — write-method list diff --git a/extension.neon b/extension.neon index 0034fb9..4c936b5 100644 --- a/extension.neon +++ b/extension.neon @@ -38,6 +38,9 @@ services: arguments: resourceDataBaseClass: %resourceDataBaseClass% tags: [phpstan.rules.rule] + - + class: ScriptDevelopment\PhpstanWarroomRules\Rules\EnforceCurrentUserAttributeRule + tags: [phpstan.rules.rule] - class: ScriptDevelopment\PhpstanWarroomRules\Type\ConnectionTransactionReturnTypeExtension tags: [phpstan.broker.dynamicMethodReturnTypeExtension] diff --git a/src/Rules/EnforceCurrentUserAttributeRule.php b/src/Rules/EnforceCurrentUserAttributeRule.php new file mode 100644 index 0000000..b26d567 --- /dev/null +++ b/src/Rules/EnforceCurrentUserAttributeRule.php @@ -0,0 +1,204 @@ +user() calls inside controller methods. + * Use the #[\Illuminate\Container\Attributes\CurrentUser] container attribute on the + * method parameter instead — eliminates the nullable-then-assert dance. + * + * Scoped to classes in the `App\Http\Controllers` namespace. FormRequest + * (`App\Http\Requests`) is excluded by design — container-attribute injection + * does not apply to FormRequest::rules() / toDto() / authorize() invocations. + * Middleware (`App\Http\Middleware`), services, Actions (`App\Actions`), jobs, + * and console commands are likewise out of scope: each context has its own + * canonical resolution path (constructor DI for Actions, authenticated payload + * threading for jobs, etc.). + * + * Detection (three call shapes branch in `processNode`): + * 1. `MethodCall` — receiver type is a (subtype of) `Illuminate\Http\Request` + * and method name is `user`. Type-based via `ObjectType::isSuperTypeOf()`, + * mirroring `EnforceAuditSnapshotOnRetryRule::receiverIsConnectionInterface()`. + * 2. `MethodCall` — receiver is a `FuncCall` to `auth()` and method name is + * `user`. AST-shape match (the helper has no class to type-check against + * in a stub-only fixture environment). + * 3. `StaticCall` — class name resolves to `Illuminate\Support\Facades\Auth` + * and method name is `user`. FQCN comparison via `$scope->resolveName()` + * handles aliased imports. + * + * Containing-class gate (applied to all three branches): the rule fires only + * when `$scope->getNamespace()` starts with `App\Http\Controllers`. This + * mirrors `ForbidEloquentMutationInControllersRule` and the canonical + * "controllers are identified by the `App\Http\Controllers` namespace" + * convention — consumer controllers are base-less `final` classes with no + * `extends Controller`, so an ancestry walk catches nothing. Out-of-scope + * namespaces (FormRequest in `App\Http\Requests`, middleware in + * `App\Http\Middleware`, Actions in `App\Actions`, jobs, services) return + * silently because their namespaces do not start with the controller prefix. + * + * Implementation note: `getNodeType()` returns `CallLike::class` so the rule + * sees both `MethodCall` and `StaticCall` in a single registration — mirrors + * the `LogRule` v0.3.0 expansion shape. + * + * Doctrine source: war-room §Architectural Principles — Explicit over implicit. + * + * @implements Rule + */ +final class EnforceCurrentUserAttributeRule implements Rule +{ + private const string TARGET_METHOD = 'user'; + + private const string CONTROLLER_NAMESPACE_PREFIX = 'App\Http\Controllers'; + + private const string REQUEST_FQCN = Request::class; + + private const string AUTH_FACADE_FQCN = Auth::class; + + public function getNodeType(): string + { + return CallLike::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if (!$this->insideControllerMethod($scope)) { + return []; + } + + if ($node instanceof MethodCall) { + return $this->processMethodCall($node, $scope); + } + + if ($node instanceof StaticCall) { + return $this->processStaticCall($node, $scope); + } + + return []; + } + + /** + * Containing-class gate: the rule fires only inside methods of a class + * whose namespace starts with `App\Http\Controllers`. FormRequest + * (`App\Http\Requests`), middleware (`App\Http\Middleware`), services, + * Actions (`App\Actions`), jobs, and console commands are silent because + * their namespaces do not start with the controller prefix. Mirrors + * `ForbidEloquentMutationInControllersRule` — consumer controllers are + * base-less `final` classes, so an ancestry walk to a base Controller + * catches nothing. + */ + private function insideControllerMethod(Scope $scope): bool + { + $namespace = $scope->getNamespace(); + + if ($namespace === null) { + return false; + } + + return str_starts_with($namespace, self::CONTROLLER_NAMESPACE_PREFIX); + } + + /** + * @return list + */ + private function processMethodCall(MethodCall $node, Scope $scope): array + { + if (!$node->name instanceof Identifier || $node->name->toString() !== self::TARGET_METHOD) { + return []; + } + + // Shape 1: $request->user() — receiver type is Illuminate\Http\Request + if ($this->receiverIsRequest($node, $scope)) { + return [$this->buildError('$request->user()')]; + } + + // Shape 2: auth()->user() — receiver is FuncCall('auth') + if ($this->receiverIsAuthHelper($node)) { + return [$this->buildError('auth()->user()')]; + } + + return []; + } + + /** + * @return list + */ + private function processStaticCall(StaticCall $node, Scope $scope): array + { + if (!$node->name instanceof Identifier || $node->name->toString() !== self::TARGET_METHOD) { + return []; + } + + if (!$node->class instanceof Name) { + return []; + } + + if ($scope->resolveName($node->class) !== self::AUTH_FACADE_FQCN) { + return []; + } + + return [$this->buildError('Auth::user()')]; + } + + /** + * Type-based receiver gate for shape 1: `$scope->getType($node->var)` must + * be a (subtype of) `Illuminate\Http\Request`. Mirrors the canonical type + * pattern in `EnforceAuditSnapshotOnRetryRule::receiverIsConnectionInterface()`. + */ + private function receiverIsRequest(MethodCall $node, Scope $scope): bool + { + $receiverType = $scope->getType($node->var); + $requestType = new ObjectType(self::REQUEST_FQCN); + + return $requestType->isSuperTypeOf($receiverType)->yes(); + } + + /** + * AST-shape gate for shape 2: the receiver is a `FuncCall` to `auth()`. + * No type-based check because the `auth()` helper's return type + * (AuthManager / Guard) is not loaded in stub-only fixture environments; + * the AST shape is unambiguous on its own. + */ + private function receiverIsAuthHelper(MethodCall $node): bool + { + if (!$node->var instanceof FuncCall) { + return false; + } + + if (!$node->var->name instanceof Name) { + return false; + } + + return $node->var->name->toString() === 'auth'; + } + + private function buildError(string $callShape): IdentifierRuleError + { + return RuleErrorBuilder::message(sprintf( + 'Authenticated-user resolution in controller methods uses the #[CurrentUser] container attribute. ' + . 'Add `#[\Illuminate\Container\Attributes\CurrentUser] User $user` to the method signature instead of calling %s inside the body.', + $callShape, + )) + ->identifier('enforceCurrentUserAttribute.useAttributeInsteadOfRequestUser') + ->build(); + } +} diff --git a/tests/Fixtures/CurrentUserAttribute/AuthFacadeInController.php b/tests/Fixtures/CurrentUserAttribute/AuthFacadeInController.php new file mode 100644 index 0000000..3f930af --- /dev/null +++ b/tests/Fixtures/CurrentUserAttribute/AuthFacadeInController.php @@ -0,0 +1,16 @@ +user(); + } +} diff --git a/tests/Fixtures/CurrentUserAttribute/AuthInAction.php b/tests/Fixtures/CurrentUserAttribute/AuthInAction.php new file mode 100644 index 0000000..67b9308 --- /dev/null +++ b/tests/Fixtures/CurrentUserAttribute/AuthInAction.php @@ -0,0 +1,17 @@ +user(); + } +} diff --git a/tests/Fixtures/CurrentUserAttribute/RequestUserInController.php b/tests/Fixtures/CurrentUserAttribute/RequestUserInController.php new file mode 100644 index 0000000..b18b20c --- /dev/null +++ b/tests/Fixtures/CurrentUserAttribute/RequestUserInController.php @@ -0,0 +1,16 @@ +user(); + } +} diff --git a/tests/Fixtures/CurrentUserAttribute/RequestUserInControllerWithAssert.php b/tests/Fixtures/CurrentUserAttribute/RequestUserInControllerWithAssert.php new file mode 100644 index 0000000..61a636a --- /dev/null +++ b/tests/Fixtures/CurrentUserAttribute/RequestUserInControllerWithAssert.php @@ -0,0 +1,24 @@ +user() call fires. + $user = $request->user(); + assert($user instanceof User); + + return $user; + } +} diff --git a/tests/Fixtures/CurrentUserAttribute/RequestUserInFormRequest.php b/tests/Fixtures/CurrentUserAttribute/RequestUserInFormRequest.php new file mode 100644 index 0000000..521b2d1 --- /dev/null +++ b/tests/Fixtures/CurrentUserAttribute/RequestUserInFormRequest.php @@ -0,0 +1,27 @@ +user() !== null; + } + + /** + * @return array + */ + public function payload(Request $request): array + { + return ['user' => $request->user()]; + } +} diff --git a/tests/Fixtures/CurrentUserAttribute/TopLevelAuthCall.php b/tests/Fixtures/CurrentUserAttribute/TopLevelAuthCall.php new file mode 100644 index 0000000..1043877 --- /dev/null +++ b/tests/Fixtures/CurrentUserAttribute/TopLevelAuthCall.php @@ -0,0 +1,10 @@ +getClassReflection()` returns +// null, so the rule must short-circuit at the no-class-reflection gate. +// Silent. Kills the FalseValue mutant on the null-reflection guard. +Auth::user(); diff --git a/tests/Fixtures/CurrentUserAttribute/UsesCurrentUserAttribute.php b/tests/Fixtures/CurrentUserAttribute/UsesCurrentUserAttribute.php new file mode 100644 index 0000000..a1e2b60 --- /dev/null +++ b/tests/Fixtures/CurrentUserAttribute/UsesCurrentUserAttribute.php @@ -0,0 +1,19 @@ + + */ +final class EnforceCurrentUserAttributeRuleTest extends RuleTestCase +{ + private const string EXPECTED_REQUEST_USER = 'Authenticated-user resolution in controller methods uses the #[CurrentUser] container attribute. Add `#[\Illuminate\Container\Attributes\CurrentUser] User $user` to the method signature instead of calling $request->user() inside the body.'; + + private const string EXPECTED_AUTH_FACADE = 'Authenticated-user resolution in controller methods uses the #[CurrentUser] container attribute. Add `#[\Illuminate\Container\Attributes\CurrentUser] User $user` to the method signature instead of calling Auth::user() inside the body.'; + + private const string EXPECTED_AUTH_HELPER = 'Authenticated-user resolution in controller methods uses the #[CurrentUser] container attribute. Add `#[\Illuminate\Container\Attributes\CurrentUser] User $user` to the method signature instead of calling auth()->user() inside the body.'; + + public function testFlagsRequestUserInController(): void + { + $this->analyse( + [ + __DIR__ . '/../Fixtures/CurrentUserAttribute/_stubs.php', + __DIR__ . '/../Fixtures/CurrentUserAttribute/RequestUserInController.php', + ], + [ + [self::EXPECTED_REQUEST_USER, 14], + ], + ); + } + + public function testFlagsAuthFacadeInController(): void + { + $this->analyse( + [ + __DIR__ . '/../Fixtures/CurrentUserAttribute/_stubs.php', + __DIR__ . '/../Fixtures/CurrentUserAttribute/AuthFacadeInController.php', + ], + [ + [self::EXPECTED_AUTH_FACADE, 14], + ], + ); + } + + public function testFlagsAuthHelperInController(): void + { + $this->analyse( + [ + __DIR__ . '/../Fixtures/CurrentUserAttribute/_stubs.php', + __DIR__ . '/../Fixtures/CurrentUserAttribute/AuthHelperInController.php', + ], + [ + [self::EXPECTED_AUTH_HELPER, 13], + ], + ); + } + + public function testFlagsRequestUserInControllerWithAssertOnceOnTheUserCall(): void + { + // The exact PR #263 shape — `$user = $request->user(); assert($user instanceof User);` + // fires once on the `$request->user()` call. The downstream assert is noise the rule + // does not care about. + $this->analyse( + [ + __DIR__ . '/../Fixtures/CurrentUserAttribute/_stubs.php', + __DIR__ . '/../Fixtures/CurrentUserAttribute/RequestUserInControllerWithAssert.php', + ], + [ + [self::EXPECTED_REQUEST_USER, 19], + ], + ); + } + + public function testFlagsRequestUserInBaselessFinalController(): void + { + // Regression proof for the namespace-gate fix (PR #26 blocker). A + // base-less `final` controller with NO `extends Controller` — the exact + // shape kendo / ublgenie / entreezuil ship. The prior ancestry gate + // (`isSubclassOf(Controller)`) matched zero such classes, so the rule + // was a silent no-op. The namespace gate must flag this. + $this->analyse( + [ + __DIR__ . '/../Fixtures/CurrentUserAttribute/_stubs.php', + __DIR__ . '/../Fixtures/CurrentUserAttribute/RequestUserInBaselessController.php', + ], + [ + [self::EXPECTED_REQUEST_USER, 18], + ], + ); + } + + public function testIgnoresCurrentUserAttributeInBaselessFinalController(): void + { + // Compliant base-less `final` controller — container-attribute injection, + // no body call. The rule must not fire even though the namespace gate + // now matches the class. + $this->analyse( + [ + __DIR__ . '/../Fixtures/CurrentUserAttribute/_stubs.php', + __DIR__ . '/../Fixtures/CurrentUserAttribute/CurrentUserAttributeInBaselessController.php', + ], + [], + ); + } + + public function testIgnoresCurrentUserAttribute(): void + { + $this->analyse( + [ + __DIR__ . '/../Fixtures/CurrentUserAttribute/_stubs.php', + __DIR__ . '/../Fixtures/CurrentUserAttribute/UsesCurrentUserAttribute.php', + ], + [], + ); + } + + public function testIgnoresFormRequestUserResolution(): void + { + $this->analyse( + [ + __DIR__ . '/../Fixtures/CurrentUserAttribute/_stubs.php', + __DIR__ . '/../Fixtures/CurrentUserAttribute/RequestUserInFormRequest.php', + ], + [], + ); + } + + public function testIgnoresMiddlewareUserResolution(): void + { + $this->analyse( + [ + __DIR__ . '/../Fixtures/CurrentUserAttribute/_stubs.php', + __DIR__ . '/../Fixtures/CurrentUserAttribute/AuthInMiddleware.php', + ], + [], + ); + } + + public function testIgnoresActionUserResolution(): void + { + $this->analyse( + [ + __DIR__ . '/../Fixtures/CurrentUserAttribute/_stubs.php', + __DIR__ . '/../Fixtures/CurrentUserAttribute/AuthInAction.php', + ], + [], + ); + } + + public function testIgnoresJobUserResolution(): void + { + $this->analyse( + [ + __DIR__ . '/../Fixtures/CurrentUserAttribute/_stubs.php', + __DIR__ . '/../Fixtures/CurrentUserAttribute/AuthInJob.php', + ], + [], + ); + } + + public function testIgnoresTopLevelCallOutsideAnyClass(): void + { + // Top-level call outside any class — `$scope->getClassReflection()` + // returns null, the no-class-reflection gate must short-circuit. + // Kills the FalseValue mutant on `insideControllerMethod()`'s + // null-reflection branch. + $this->analyse( + [ + __DIR__ . '/../Fixtures/CurrentUserAttribute/_stubs.php', + __DIR__ . '/../Fixtures/CurrentUserAttribute/TopLevelAuthCall.php', + ], + [], + ); + } + + protected function getRule(): Rule + { + return new EnforceCurrentUserAttributeRule; + } +}