test+docs: cover ConnectionTransactionReturnTypeExtension + document prod illuminate/* deps

CHANGELOG.md

@@ -8,13 +8,18 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and
 
 ### Added
 
+- **Tests:** direct type-inference coverage for `ConnectionTransactionReturnTypeExtension` via a `PHPStan\Testing\TypeInferenceTestCase` (`tests/Type/ConnectionTransactionReturnTypeExtensionTest.php` + fixture `tests/Fixtures/ConnectionTransactionReturnType/transaction-return-type.php`). The extension previously had no direct test — it was only exercised implicitly by audit-snapshot rule fixtures, none of which asserted the resolved return type. The new fixture loads `extension.neon` (same config consumers register) and `assertType()`s the inferred type of `$connection->transaction(...)` for closures returning a constant scalar, an object/DTO, a nullable, an array shape, and a widened (non-constant) scalar — pinning that the extension forwards the closure acceptor's return type rather than `mixed`. Test-only; no consumer-facing surface. Closes Quartermaster F-2. **Versioning:** none (internal test coverage).
 - `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<User>` 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.**
 
 ### 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).
 
+### Documentation
+
+- **README:** added a *Production dependencies* section documenting why the `illuminate/*` chain (`database`, `contracts`, `cache`, `filesystem`, `log`, `mail`) lives in `require`, not `require-dev`. The rules and `ConnectionTransactionReturnTypeExtension` reflect against Illuminate contracts/classes at analysis time, so they are genuine analysis-time (runtime-for-the-extension) dependencies; moving them to `require-dev` would break consumers analysing non-Laravel or partial trees. Documents the architectural intent (Sapper M1 Finding #4, Form A — keep + document, do not move). **Versioning:** none (docs only).
+
 ## [0.3.0] — 2026-05-13
 
 **Release-as-a-whole: MAJOR** — collapses three rule-level contractual widenings into a single Major bump per ADR-0021 §Versioning. Each rule's pre-cascade audit returned 0 violators across all 5 consumer territories (kendo, entreezuil, emmie, ublgenie, brick-inventory-orchestrator), so the Major represents the contract change, not empirical violation count. Consumers upgrading from `^0.2` to `^0.3` accept the broader rule contracts whether or not their existing code trips them. **Phase A pin sweep (`^0.1.x` → `^0.2`) closed pre-release** — all four laggard consumers (kendo, entreezuil, emmie, BIO) bumped between 2026-05-06 and 2026-05-08 via independent dispatches; verified by 4-territory Medic wave 2026-05-13 (all no-op, all `composer phpstan` clean against `EnforceAuditSnapshotOnRetryRule`). Phase B pin sweep (`^0.2` → `^0.3`) follows post-tag as a separate war-room dispatch.

README.md

@@ -90,6 +90,10 @@ Inheritance is matched via PHPStan reflection (FQCN ancestor traversal), not sho
 
 `ConnectionTransactionReturnTypeExtension` is registered alongside the rules. It resolves the return type of `$connection->transaction(fn () => $foo)` to the closure's return type instead of `mixed`, enabling strict typing of transaction call sites.
 
+## Production dependencies
+
+The `illuminate/*` packages (`database`, `contracts`, `cache`, `filesystem`, `log`, `mail`) sit in `require`, not `require-dev`, on purpose. The rules and `ConnectionTransactionReturnTypeExtension` reflect against Illuminate contracts and classes (e.g. `Illuminate\Database\ConnectionInterface`, the cache/mail/queue facades the audit-scope rule reasons about) *at analysis time* — when a consumer runs PHPStan, this package's code resolves those symbols, so they are genuine analysis-time (runtime-for-the-extension) dependencies, not test-only tooling. Moving them to `require-dev` would omit them from a normal `composer require --dev` install and break consumers that analyse non-Laravel or partial trees where the Illuminate symbols are not otherwise present.
+
 ## Versioning
 
 Semantic versioning:

tests/Fixtures/ConnectionTransactionReturnType/transaction-return-type.php

@@ -0,0 +1,68 @@
+<?php
+
+declare(strict_types = 1);
+
+// Fixture for ConnectionTransactionReturnTypeExtension. The extension resolves
+// ConnectionInterface::transaction(fn () => $x) to the closure's return type
+// instead of mixed. Each assertType() below pins the inferred type to the
+// closure's declared/inferred return type. Classmap-autoloaded (not PSR-4) so
+// Pint's psr_autoloading fixer leaves the namespaced classes untouched.
+
+namespace ScriptDevelopment\PhpstanWarroomRules\Tests\Fixtures\ConnectionTransactionReturnType;
+
+use Illuminate\Database\ConnectionInterface;
+
+use function PHPStan\Testing\assertType;
+
+final class TransactionResult
+{
+    public function __construct(
+        public int $id,
+    ) {}
+}
+
+final class TransactionCaller
+{
+    public function returnsInt(ConnectionInterface $connection): void
+    {
+        $result = $connection->transaction(fn(): int => 42);
+
+        // Closure body narrows to the constant 42; the extension forwards the
+        // precise inferred return type (not the widened `int`), proving it
+        // reads the closure acceptor's return type rather than the declaration.
+        assertType('42', $result);
+    }
+
+    public function returnsObject(ConnectionInterface $connection): void
+    {
+        $result = $connection->transaction(fn(): TransactionResult => new TransactionResult(1));
+
+        assertType(TransactionResult::class, $result);
+    }
+
+    public function returnsNullable(ConnectionInterface $connection, ?string $maybe): void
+    {
+        // A captured nullable variable keeps the union intact, so the extension
+        // forwards the genuine `string|null` rather than a narrowed constant.
+        $result = $connection->transaction(fn(): ?string => $maybe);
+
+        assertType('string|null', $result);
+    }
+
+    public function returnsArray(ConnectionInterface $connection): void
+    {
+        $result = $connection->transaction(fn(): array => ['a', 'b']);
+
+        assertType('array{\'a\', \'b\'}', $result);
+    }
+
+    public function returnsWidenedScalar(ConnectionInterface $connection, int $value): void
+    {
+        // A captured variable defeats constant folding, so the closure's
+        // inferred return type is the widened `int` — confirming the extension
+        // forwards whatever the acceptor resolves, constant or not.
+        $result = $connection->transaction(fn(): int => $value);
+
+        assertType('int', $result);
+    }
+}

tests/Type/ConnectionTransactionReturnTypeExtensionTest.php

@@ -0,0 +1,42 @@
+<?php
+
+declare(strict_types = 1);
+
+namespace ScriptDevelopment\PhpstanWarroomRules\Tests\Type;
+
+use PHPStan\Testing\TypeInferenceTestCase;
+use PHPUnit\Framework\Attributes\DataProvider;
+
+/**
+ * Direct type-inference coverage for ConnectionTransactionReturnTypeExtension.
+ *
+ * The extension is registered through extension.neon (same config consumers
+ * load), so the assertType() calls in the fixture are resolved with the
+ * extension active. Without it, ConnectionInterface::transaction() returns
+ * mixed and every assertion below would fail.
+ */
+final class ConnectionTransactionReturnTypeExtensionTest extends TypeInferenceTestCase
+{
+    /**
+     * @return iterable<mixed>
+     */
+    public static function dataFileAsserts(): iterable
+    {
+        yield from self::gatherAssertTypes(
+            __DIR__ . '/../Fixtures/ConnectionTransactionReturnType/transaction-return-type.php',
+        );
+    }
+
+    #[DataProvider('dataFileAsserts')]
+    public function testFileAsserts(string $assertType, string $file, ...$args): void
+    {
+        $this->assertFileAsserts($assertType, $file, ...$args);
+    }
+
+    public static function getAdditionalConfigFiles(): array
+    {
+        return [
+            __DIR__ . '/../../extension.neon',
+        ];
+    }
+}