From 8018c868c0fdb18700a8e3e7ab4278552b29d0dd Mon Sep 17 00:00:00 2001 From: Gerard Oosterhof Date: Tue, 21 Jul 2026 14:04:36 +0200 Subject: [PATCH 1/3] spike(queue-58): AST-aware untimed-HTTP-client rule (Doctrine #8) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Type-anchored PHPStan rule detecting Http-facade / injected-Factory chains that reach a send verb with no explicit ->timeout(). Conservative: fires only on fully-visible single-expression chains rooted at an entry point (facade static call or Factory-typed receiver); declines split/helper-built chains and Guzzle/SDK surfaces to hold false positives at zero. SPIKE — not registered in extension.neon (does not ship on a tag). Field-run against kendo/emmie/ublgenie/BIO: zero false positives; positive control fires on the real Illuminate Factory. Coverage finding: the dominant fleet idiom is the helper-built split ($this->apiClient()->get()), which the rule must decline — so it complements, not replaces, the named-list tests. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01QFNeeAHWJdLLLzzu3oSXMy --- src/Rules/ForbidUntimedHttpClientRule.php | 237 ++++++++++++++++++ .../CompliantFactoryChainTimeout.php | 19 ++ .../CompliantTimeoutMidChain.php | 18 ++ .../CompliantTimeoutStatic.php | 15 ++ .../CompliantWithOptionsTimeout.php | 15 ++ .../DeclinedLocalPendingRequestVar.php | 27 ++ .../DeclinedSplitChainProperty.php | 25 ++ .../IgnoredNonHttpReceiver.php | 25 ++ .../ViolationBareStaticGet.php | 15 ++ .../ViolationChainNoTimeout.php | 15 ++ .../ViolationConnectTimeoutOnly.php | 17 ++ .../ViolationFactoryChainNoTimeout.php | 19 ++ tests/Fixtures/UntimedHttpClient/_stubs.php | 229 +++++++++++++++++ .../Rules/ForbidUntimedHttpClientRuleTest.php | 123 +++++++++ 14 files changed, 799 insertions(+) create mode 100644 src/Rules/ForbidUntimedHttpClientRule.php create mode 100644 tests/Fixtures/UntimedHttpClient/CompliantFactoryChainTimeout.php create mode 100644 tests/Fixtures/UntimedHttpClient/CompliantTimeoutMidChain.php create mode 100644 tests/Fixtures/UntimedHttpClient/CompliantTimeoutStatic.php create mode 100644 tests/Fixtures/UntimedHttpClient/CompliantWithOptionsTimeout.php create mode 100644 tests/Fixtures/UntimedHttpClient/DeclinedLocalPendingRequestVar.php create mode 100644 tests/Fixtures/UntimedHttpClient/DeclinedSplitChainProperty.php create mode 100644 tests/Fixtures/UntimedHttpClient/IgnoredNonHttpReceiver.php create mode 100644 tests/Fixtures/UntimedHttpClient/ViolationBareStaticGet.php create mode 100644 tests/Fixtures/UntimedHttpClient/ViolationChainNoTimeout.php create mode 100644 tests/Fixtures/UntimedHttpClient/ViolationConnectTimeoutOnly.php create mode 100644 tests/Fixtures/UntimedHttpClient/ViolationFactoryChainNoTimeout.php create mode 100644 tests/Fixtures/UntimedHttpClient/_stubs.php create mode 100644 tests/Rules/ForbidUntimedHttpClientRuleTest.php diff --git a/src/Rules/ForbidUntimedHttpClientRule.php b/src/Rules/ForbidUntimedHttpClientRule.php new file mode 100644 index 0000000..025bc92 --- /dev/null +++ b/src/Rules/ForbidUntimedHttpClientRule.php @@ -0,0 +1,237 @@ +get($url)`. + * 2. An injected client factory (`Illuminate\Http\Client\Factory`) — the + * dominant fleet idiom, e.g. `$this->http->withToken($t)->get($url)` + * where `$this->http` is a promoted `Factory` property. Anchored by TYPE, + * so the alias (`$http` / `$httpClient` / `$client`) is irrelevant. + * + * A timeout is considered present when the visible chain contains a + * `->timeout(...)` call OR a `->withOptions([... 'timeout' => ... ])` carrying + * a `'timeout'` key. `connectTimeout()` alone is NOT sufficient — it bounds the + * handshake, not the response, so a hung server still stalls the caller; + * Doctrine #8 wants the request timeout. + * + * Doctrine source: war-room §Architectural Principles #8 — Explicit timeouts + * on external HTTP calls. Promotion candidate for war-room enforcement queue + * #58 (the AST-aware successor to the per-territory `ExternalHttpTimeoutTest` + * named-list Pest tests on kendo / emmie, which detect wrong-shape on enrolled + * classes but are blind to OMISSION — a new untimed call nobody enrolls). + * + * CONSERVATIVE BY DESIGN — fires only when the ENTIRE chain from an entry point + * (facade static call, or a `Factory`-typed receiver) to the send verb is + * visible in a single expression. It deliberately DECLINES (never a false + * positive) on: + * + * - Split chains — the `PendingRequest` is built on one statement / helper + * and sent on another (`$req = $this->http->timeout(5); $req->get(...)`, or + * `$this->apiClient()->post(...)` where `apiClient()` returns a pre-timed + * request). The send-site expression cannot see the builder, so a + * `PendingRequest`-typed (as opposed to `Factory`-typed) root is NOT an + * anchor — the timeout may have been set upstream. + * - Raw `GuzzleHttp\Client` construction / per-call `['timeout' => N]` request + * options (a distinct AST surface — the timeout rides an options array). + * - Vendor SDKs that wrap Guzzle and configure the timeout via their own + * `setConfig([...])` — invisible to any HTTP-client scan; flagging them + * would be a false positive. + * - Timeouts configured at a DI binding (a provider binds a pre-timed client). + * + * These exclusions are the deliberate consequence of biasing a first-cut rule + * toward zero false positives (a false positive reddens a compliant consumer's + * whole `phpstan` run and forces a coordinated baseline; a false negative is + * absorbed by the surviving named-list test). The excluded surfaces remain the + * responsibility of the per-territory named-list Pest test until a follow-up + * widens this rule. + * + * Suppression: standard PHPStan inline-ignore on the identifier + * `forbidUntimedHttpClient.missingTimeout`. + * + * @implements Rule + */ +final class ForbidUntimedHttpClientRule implements Rule +{ + private const string HTTP_FACADE = Http::class; + + private const string CLIENT_FACTORY = Factory::class; + + /** Terminal HTTP send verbs on the facade / factory / `PendingRequest`. */ + private const array SEND_VERBS = ['get', 'post', 'put', 'patch', 'delete', 'head', 'send']; + + public function getNodeType(): string + { + return CallLike::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if ($node instanceof StaticCall) { + return $this->processStaticSend($node); + } + + if ($node instanceof MethodCall) { + return $this->processChainedSend($node, $scope); + } + + return []; + } + + /** + * A bare static send with no chain at all — `Http::get($url)` — can carry + * no timeout, so it is always a violation when rooted at the Http facade. + * + * @return list + */ + private function processStaticSend(StaticCall $node): array + { + if (!$this->isHttpFacade($node->class) || !$this->isSendVerb($node->name)) { + return []; + } + + return [$this->buildError($node)]; + } + + /** + * A chained send — `->...->get($url)`. Fires only when the receiver + * chain is fully visible back to a recognised entry point (the `Http` + * facade static call, or a `Factory`-typed receiver) AND carries no + * timeout. Any other / unresolved root ⇒ DECLINE (return []). + * + * @return list + */ + private function processChainedSend(MethodCall $node, Scope $scope): array + { + if (!$this->isSendVerb($node->name)) { + return []; + } + + $cursor = $node->var; + + while ($cursor instanceof MethodCall) { + if ($this->declaresTimeout($cursor)) { + return []; + } + + $cursor = $cursor->var; + } + + if ($cursor instanceof StaticCall) { + // Chain root is a static call. Only the Http facade is an entry we + // can fully account for; the root call may itself set the timeout. + if (!$this->isHttpFacade($cursor->class)) { + return []; + } + + return $this->declaresTimeout($cursor) ? [] : [$this->buildError($node)]; + } + + // Chain root is an expression (property / variable). It anchors ONLY if + // its type is the client Factory — the entry point, where the whole + // chain is guaranteed visible. A `PendingRequest`-typed root may carry + // an upstream timeout we cannot see, so it is NOT an anchor. + if ($this->isClientFactory($scope->getType($cursor))) { + return [$this->buildError($node)]; + } + + return []; + } + + /** + * True when a builder call in the chain establishes a request timeout: + * a `->timeout(...)` method, or a `->withOptions([... 'timeout' => ...])` + * carrying the option key. + */ + private function declaresTimeout(MethodCall|StaticCall $call): bool + { + $name = $this->methodName($call->name); + + if ($name === 'timeout') { + return true; + } + + if ($name === 'withoptions') { + return $this->argArrayHasTimeoutKey($call); + } + + return false; + } + + private function argArrayHasTimeoutKey(MethodCall|StaticCall $call): bool + { + $first = $call->getArgs()[0]->value ?? null; + + if (!$first instanceof Array_) { + return false; + } + + foreach ($first->items as $item) { + if ($item->key instanceof String_ && mb_strtolower($item->key->value) === 'timeout') { + return true; + } + } + + return false; + } + + private function isClientFactory(Type $type): bool + { + return (new ObjectType(self::CLIENT_FACTORY))->isSuperTypeOf($type)->yes(); + } + + private function isHttpFacade(mixed $class): bool + { + return $class instanceof Name && $class->toString() === self::HTTP_FACADE; + } + + private function isSendVerb(mixed $name): bool + { + return in_array($this->methodName($name), self::SEND_VERBS, true); + } + + private function methodName(mixed $name): string + { + return $name instanceof Identifier ? mb_strtolower($name->toString()) : ''; + } + + private function buildError(MethodCall|StaticCall $node): IdentifierRuleError + { + return RuleErrorBuilder::message( + 'Outbound HTTP request declares no explicit timeout. ' + . 'Add ->timeout(seconds) to the chain — external calls must not rely on the framework default (Doctrine Principle #8).', + ) + ->identifier('forbidUntimedHttpClient.missingTimeout') + ->line($node->getStartLine()) + ->build(); + } +} diff --git a/tests/Fixtures/UntimedHttpClient/CompliantFactoryChainTimeout.php b/tests/Fixtures/UntimedHttpClient/CompliantFactoryChainTimeout.php new file mode 100644 index 0000000..65031f0 --- /dev/null +++ b/tests/Fixtures/UntimedHttpClient/CompliantFactoryChainTimeout.php @@ -0,0 +1,19 @@ +http->withToken($token)->timeout(30)->get($url); + } +} diff --git a/tests/Fixtures/UntimedHttpClient/CompliantTimeoutMidChain.php b/tests/Fixtures/UntimedHttpClient/CompliantTimeoutMidChain.php new file mode 100644 index 0000000..606d0a3 --- /dev/null +++ b/tests/Fixtures/UntimedHttpClient/CompliantTimeoutMidChain.php @@ -0,0 +1,18 @@ + $data + */ + public function push(string $url, string $token, array $data): void + { + Http::withToken($token)->timeout(5)->post($url, $data); + } +} diff --git a/tests/Fixtures/UntimedHttpClient/CompliantTimeoutStatic.php b/tests/Fixtures/UntimedHttpClient/CompliantTimeoutStatic.php new file mode 100644 index 0000000..3647909 --- /dev/null +++ b/tests/Fixtures/UntimedHttpClient/CompliantTimeoutStatic.php @@ -0,0 +1,15 @@ +get($url); + } +} diff --git a/tests/Fixtures/UntimedHttpClient/CompliantWithOptionsTimeout.php b/tests/Fixtures/UntimedHttpClient/CompliantWithOptionsTimeout.php new file mode 100644 index 0000000..d44c319 --- /dev/null +++ b/tests/Fixtures/UntimedHttpClient/CompliantWithOptionsTimeout.php @@ -0,0 +1,15 @@ + 5])->get($url); + } +} diff --git a/tests/Fixtures/UntimedHttpClient/DeclinedLocalPendingRequestVar.php b/tests/Fixtures/UntimedHttpClient/DeclinedLocalPendingRequestVar.php new file mode 100644 index 0000000..e109339 --- /dev/null +++ b/tests/Fixtures/UntimedHttpClient/DeclinedLocalPendingRequestVar.php @@ -0,0 +1,27 @@ +http->withToken($token)->timeout(30); + + $request->get($url); + } +} diff --git a/tests/Fixtures/UntimedHttpClient/DeclinedSplitChainProperty.php b/tests/Fixtures/UntimedHttpClient/DeclinedSplitChainProperty.php new file mode 100644 index 0000000..16900d1 --- /dev/null +++ b/tests/Fixtures/UntimedHttpClient/DeclinedSplitChainProperty.php @@ -0,0 +1,25 @@ +client->get($url); + } +} diff --git a/tests/Fixtures/UntimedHttpClient/IgnoredNonHttpReceiver.php b/tests/Fixtures/UntimedHttpClient/IgnoredNonHttpReceiver.php new file mode 100644 index 0000000..a2a15fa --- /dev/null +++ b/tests/Fixtures/UntimedHttpClient/IgnoredNonHttpReceiver.php @@ -0,0 +1,25 @@ +get()` on an unrelated object whose chain root is not the Http facade. + * The rule must not fire — `get` as a method name is not exclusive to HTTP. + */ +final class LocalRepository +{ + public function get(string $id): string + { + return $id; + } +} + +final class IgnoredNonHttpReceiver +{ + public function fetch(LocalRepository $repo): string + { + return $repo->get('x'); + } +} diff --git a/tests/Fixtures/UntimedHttpClient/ViolationBareStaticGet.php b/tests/Fixtures/UntimedHttpClient/ViolationBareStaticGet.php new file mode 100644 index 0000000..d14608f --- /dev/null +++ b/tests/Fixtures/UntimedHttpClient/ViolationBareStaticGet.php @@ -0,0 +1,15 @@ +get($url); + } +} diff --git a/tests/Fixtures/UntimedHttpClient/ViolationConnectTimeoutOnly.php b/tests/Fixtures/UntimedHttpClient/ViolationConnectTimeoutOnly.php new file mode 100644 index 0000000..0ef288e --- /dev/null +++ b/tests/Fixtures/UntimedHttpClient/ViolationConnectTimeoutOnly.php @@ -0,0 +1,17 @@ +get($url); + } +} diff --git a/tests/Fixtures/UntimedHttpClient/ViolationFactoryChainNoTimeout.php b/tests/Fixtures/UntimedHttpClient/ViolationFactoryChainNoTimeout.php new file mode 100644 index 0000000..097854d --- /dev/null +++ b/tests/Fixtures/UntimedHttpClient/ViolationFactoryChainNoTimeout.php @@ -0,0 +1,19 @@ +http->withToken($token)->get($url); + } +} diff --git a/tests/Fixtures/UntimedHttpClient/_stubs.php b/tests/Fixtures/UntimedHttpClient/_stubs.php new file mode 100644 index 0000000..b814c23 --- /dev/null +++ b/tests/Fixtures/UntimedHttpClient/_stubs.php @@ -0,0 +1,229 @@ + $options + */ + public function withOptions(array $options): PendingRequest + { + return new PendingRequest; + } + + public function baseUrl(string $url): PendingRequest + { + return new PendingRequest; + } + + /** + * @param array $query + */ + public function get(string $url, array $query = []): Response + { + return new Response; + } + + /** + * @param array $data + */ + public function post(string $url, array $data = []): Response + { + return new Response; + } + } + + class PendingRequest + { + public function timeout(int $seconds): static + { + return $this; + } + + public function connectTimeout(int $seconds): static + { + return $this; + } + + public function withToken(string $token): static + { + return $this; + } + + /** + * @param array $options + */ + public function withOptions(array $options): static + { + return $this; + } + + public function baseUrl(string $url): static + { + return $this; + } + + /** + * @param array $query + */ + public function get(string $url, array $query = []): Response + { + return new Response; + } + + /** + * @param array $data + */ + public function post(string $url, array $data = []): Response + { + return new Response; + } + + /** + * @param array $data + */ + public function put(string $url, array $data = []): Response + { + return new Response; + } + + /** + * @param array $data + */ + public function patch(string $url, array $data = []): Response + { + return new Response; + } + + /** + * @param array $data + */ + public function delete(string $url, array $data = []): Response + { + return new Response; + } + + public function head(string $url): Response + { + return new Response; + } + + /** + * @param array $options + */ + public function send(string $method, string $url, array $options = []): Response + { + return new Response; + } + } +} + +namespace Illuminate\Support\Facades { + use Illuminate\Http\Client\PendingRequest; + use Illuminate\Http\Client\Response; + + class Http + { + public static function timeout(int $seconds): PendingRequest + { + return new PendingRequest; + } + + public static function connectTimeout(int $seconds): PendingRequest + { + return new PendingRequest; + } + + public static function withToken(string $token): PendingRequest + { + return new PendingRequest; + } + + /** + * @param array $options + */ + public static function withOptions(array $options): PendingRequest + { + return new PendingRequest; + } + + public static function baseUrl(string $url): PendingRequest + { + return new PendingRequest; + } + + /** + * @param array $query + */ + public static function get(string $url, array $query = []): Response + { + return new Response; + } + + /** + * @param array $data + */ + public static function post(string $url, array $data = []): Response + { + return new Response; + } + + /** + * @param array $data + */ + public static function put(string $url, array $data = []): Response + { + return new Response; + } + + /** + * @param array $data + */ + public static function patch(string $url, array $data = []): Response + { + return new Response; + } + + /** + * @param array $data + */ + public static function delete(string $url, array $data = []): Response + { + return new Response; + } + + public static function head(string $url): Response + { + return new Response; + } + } +} diff --git a/tests/Rules/ForbidUntimedHttpClientRuleTest.php b/tests/Rules/ForbidUntimedHttpClientRuleTest.php new file mode 100644 index 0000000..322d9cf --- /dev/null +++ b/tests/Rules/ForbidUntimedHttpClientRuleTest.php @@ -0,0 +1,123 @@ + + */ +final class ForbidUntimedHttpClientRuleTest extends RuleTestCase +{ + private const string MESSAGE = 'Outbound HTTP request declares no explicit timeout. ' + . 'Add ->timeout(seconds) to the chain — external calls must not rely on the framework default (Doctrine Principle #8).'; + + public function testFlagsBareStaticSend(): void + { + $this->analyse( + [__DIR__ . '/../Fixtures/UntimedHttpClient/ViolationBareStaticGet.php'], + [ + [self::MESSAGE, 13], + ], + ); + } + + public function testFlagsChainWithoutTimeout(): void + { + $this->analyse( + [__DIR__ . '/../Fixtures/UntimedHttpClient/ViolationChainNoTimeout.php'], + [ + [self::MESSAGE, 13], + ], + ); + } + + public function testFlagsConnectTimeoutOnly(): void + { + // connectTimeout() is not a request timeout — the rule must still fire. + $this->analyse( + [__DIR__ . '/../Fixtures/UntimedHttpClient/ViolationConnectTimeoutOnly.php'], + [ + [self::MESSAGE, 15], + ], + ); + } + + public function testFlagsInjectedFactoryChainWithoutTimeout(): void + { + // The dominant fleet idiom: an injected Illuminate\Http\Client\Factory. + $this->analyse( + [__DIR__ . '/../Fixtures/UntimedHttpClient/ViolationFactoryChainNoTimeout.php'], + [ + [self::MESSAGE, 17], + ], + ); + } + + public function testIgnoresInjectedFactoryChainWithTimeout(): void + { + $this->analyse( + [__DIR__ . '/../Fixtures/UntimedHttpClient/CompliantFactoryChainTimeout.php'], + [], + ); + } + + public function testDeclinesLocalPendingRequestVariable(): void + { + // Timeout set upstream on a PendingRequest-typed local — out of view. + $this->analyse( + [__DIR__ . '/../Fixtures/UntimedHttpClient/DeclinedLocalPendingRequestVar.php'], + [], + ); + } + + public function testIgnoresStaticTimeoutChain(): void + { + $this->analyse( + [__DIR__ . '/../Fixtures/UntimedHttpClient/CompliantTimeoutStatic.php'], + [], + ); + } + + public function testIgnoresTimeoutMidChain(): void + { + $this->analyse( + [__DIR__ . '/../Fixtures/UntimedHttpClient/CompliantTimeoutMidChain.php'], + [], + ); + } + + public function testIgnoresWithOptionsTimeoutKey(): void + { + $this->analyse( + [__DIR__ . '/../Fixtures/UntimedHttpClient/CompliantWithOptionsTimeout.php'], + [], + ); + } + + public function testDeclinesSplitChainBuiltOnAProperty(): void + { + // Builder assembled out of view — deliberate conservative miss. + $this->analyse( + [__DIR__ . '/../Fixtures/UntimedHttpClient/DeclinedSplitChainProperty.php'], + [], + ); + } + + public function testIgnoresNonHttpReceiver(): void + { + $this->analyse( + [__DIR__ . '/../Fixtures/UntimedHttpClient/IgnoredNonHttpReceiver.php'], + [], + ); + } + + protected function getRule(): Rule + { + return new ForbidUntimedHttpClientRule; + } +} From 12af636c40880b3691620ef19357c486296a6084 Mon Sep 17 00:00:00 2001 From: Gerard Oosterhof Date: Tue, 21 Jul 2026 14:14:44 +0200 Subject: [PATCH 2/3] feat(queue-58): register ForbidUntimedHttpClientRule (Doctrine #8) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ship the untimed-HTTP-client rule as a COMPLEMENT to the per-territory ExternalHttpTimeoutTest named-lists. Registered in extension.neon (no params); CHANGELOG [Unreleased] Added entry; territory CLAUDE.md rules table + Principle #8 projection bullet updated. Field-validated zero-FP on kendo/emmie/ublgenie/BIO; registration confirmed firing via extension.neon. MINOR (no baseline on swept consumers). Does NOT retire the named-lists — the helper-built split stays out of AST reach; the builder-side enforcement that would is the queue #58 continuation. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01QFNeeAHWJdLLLzzu3oSXMy --- CHANGELOG.md | 10 ++++++++++ CLAUDE.md | 2 ++ extension.neon | 3 +++ 3 files changed, 15 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 289efa2..00b6e64 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,16 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and ## [Unreleased] +### Added + +- `ForbidUntimedHttpClientRule` — new rule enforcing war-room **Architectural Principle #8** (explicit timeouts on outbound HTTP) at analysis time, the AST-aware successor to the per-territory `ExternalHttpTimeoutTest` named-list Pest tests (kendo / emmie). The named-list tests detect wrong-shape on *enrolled* classes but are blind to **omission** — a new untimed call nobody adds to the list; this rule closes the omission gap for the tractable call shapes. Registered in `extension.neon` (no parameters). Identifier: `forbidUntimedHttpClient.missingTimeout`. Doctrine: war-room §Architectural Principles #8. Seed: war-room enforcement queue #58 (spike branch `spike/wr-queue58-untimed-http-client`). + + **Detection (type-anchored, two entry points):** fires on a terminal send verb (`get`/`post`/`put`/`patch`/`delete`/`head`/`send`) reached without an explicit request timeout, where the entry point is either (1) the `Http` facade (`Illuminate\Support\Facades\Http` static-call root), or (2) an **injected `Illuminate\Http\Client\Factory`** receiver (`$this->http->…->get()`, anchored by TYPE so the property alias is irrelevant — the dominant fleet idiom, established by field survey of kendo/emmie/ublgenie/BIO). A timeout counts when the visible chain contains `->timeout(...)` **or** `->withOptions([... 'timeout' => ...])`; `connectTimeout()` alone does NOT (it bounds the handshake, not the response). + + **CONSERVATIVE BY DESIGN — biased to zero false positives.** Fires only when the ENTIRE chain from an entry point to the send verb is visible in a single expression. It DECLINES (never a false positive) on: split chains where the `PendingRequest` is built on one statement / helper and sent on another (`$req = $this->http->timeout(5); $req->get()`, or `$this->apiClient()->post()` returning a pre-timed request — a large fraction of the real fleet surface); raw `GuzzleHttp\Client` construction / per-call `['timeout' => N]` options; vendor SDKs that configure the timeout via their own `setConfig([...])`; and DI-bound pre-timed clients. **Consequence: this rule COMPLEMENTS the per-territory named-list tests; it does NOT replace them** — the helper-built split (which the named-list's whole-file scan does cover) is out of AST reach for a single-expression rule. The builder-side enforcement that would actually retire the named-lists is a separate follow-up (queue #58 continuation). + + **Field-validated (spike, 2026-07-21):** run against kendo / emmie / ublgenie / BIO backends — **zero false positives** (compliant chains short-circuit on the timeout; split/helper chains decline); a synthetic positive control fires on exactly the untimed method against the *real* Illuminate `Factory`. **Versioning: MINOR** (new rule; zero violations on every swept consumer ⇒ no baseline on kendo/emmie/ublgenie/BIO). A not-yet-swept consumer carrying an untimed *direct* facade/Factory chain would see a new error and adopt on its own bump PR; per the pre-1.0 caret convention `^0.8` excludes the next minor, so tagging auto-adopts nobody. **NOT tagged** (release is ally-gated). + ### Changed - `EnforceResourceDataValidatorOptInRule` + `EnforceFormRequestToDtoRule` + `EnforceAuditModelProtectionsRule` — migrated the internal inheritance gate off the deprecated `ClassReflection::isSubclassOf(string)` API onto `isSubclassOfClass(ClassReflection)` (war-room enforcement queue #112). `isSubclassOf(string)` is `@deprecated Use isSubclassOfClass instead.` in PHPStan 2.2+ and **removed in PHPStan 3.x** — a latent break for every consumer's static analysis the day this package (the fleet's canonical static-analysis backbone) targets PHPStan 3. Each of the three rules now injects `PHPStan\Reflection\ReflectionProvider` (constructor DI, autowired by the PHPStan/Nette container — no `extension.neon` argument wiring needed, proven by each rule's container-resolution test) and resolves its configured/known base FQCN via `reflectionProvider->hasClass()/getClass()` before calling `isSubclassOfClass()`. **Behaviour is byte-for-byte preserved:** the migration inlines the deprecated method's own body (`if (!hasClass($fqcn)) return false; return isSubclassOfClass(getClass($fqcn));`), so the load-bearing unknown-base-class no-op — a tree lacking the configured base class (or `Illuminate\Database\Eloquent\Model` for the audit rule) silently does not fire, the "consumers analysing non-Laravel trees are unaffected" guarantee — is reproduced exactly. Pinned by a new base-class-absent no-op test per rule (a configured base FQCN absent from the analysed tree ⇒ zero errors); all existing positive tests stay green. **Versioning: PATCH per ADR-0021 §Versioning** (internal API migration / future-proofing — adds no errors, removes none, no consumer-visible behaviour change). Seed: war-room enforcement queue #112. diff --git a/CLAUDE.md b/CLAUDE.md index cb50ae0..498c159 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -34,6 +34,7 @@ Composer package distributing war-room-doctrine PHPStan rules across `script-dev | `EnforceResourceDataValidatorOptInRule` | ADR-0009 §EAGER_LOAD validator opt-in | `enforceResourceDataValidatorOptIn.missingValidatorCall` | | `EnforceFormRequestToDtoRule` | ADR-0012 §FormRequest → DTO Flow | `enforceFormRequestToDto.missingToDtoMethod` | | `EnforceCurrentUserAttributeRule` | War-room §Explicit over implicit | `enforceCurrentUserAttribute.useAttributeInsteadOfRequestUser` | +| `ForbidUntimedHttpClientRule` | War-room §Explicit HTTP timeouts (#8) | `forbidUntimedHttpClient.missingTimeout` (type-aware; flags an `Http` facade OR injected `Illuminate\Http\Client\Factory` chain that reaches a send verb with no explicit `->timeout()` / `withOptions(['timeout'])`. Conservative — fires only on fully-visible single-expression chains; declines split/helper-built chains + Guzzle/SDK surfaces to hold FP at zero. COMPLEMENTS, does not replace, the per-territory `ExternalHttpTimeoutTest`. on `main`, `[Unreleased]`) | | `EnforceAuditModelProtectionsRule` | ADR-0001 §Append-only | `enforceAuditModelProtections.hasFactoryForbidden` / `.softDeletesForbidden` / `.updatedAtNotDisabled` (denylist-inversion; discovers audit models by shape — `auditModelNameSuffixes` default `AuditLog` OR `auditModelNamespacePrefixes` default `App\Models\Audit` — and flags `HasFactory` / `SoftDeletes` / missing `const UPDATED_AT = null`. shipped v0.7.0) | | `EnforceActionResultDtoRule` | ADR-0020 + ADR-0011 | `enforceActionResultDto.arrayReturnFromExecute` (signature-only; flags an `array` / `?array` / `array\|Dto` union / `iterable` native return type on `App\Actions\*` `execute()`. Phpdoc-only `@return array{...}` is a deliberate miss; no `list` carve-out. Seed kendo PR #1653. on `main`, `[Unreleased]` — pending v0.8.0 tag (release PR #53)) | | `ConnectionTransactionReturnTypeExtension` | (type extension, no rule) | — | @@ -112,6 +113,7 @@ SemVer per ADR-0021: ### War-room Architectural Principle rules (no published ADR) - **Explicit over implicit** — package distributes `ForbidAbortHelperRule` (bans `abort()` / `abort_if()` / `abort_unless()`; shipped), `EnforceCurrentUserAttributeRule` (flags `Request::user()` / `Auth::user()` / `auth()->user()` in `App\Http\Controllers`, steering to the `#[CurrentUser]` container attribute per Architectural Principle #9; shipped v0.4.0), `ForbidHttpExceptionInActionsRule` (type-aware sibling of `ForbidAbortHelperRule` — bans throwing the `Symfony\…\HttpException` family from `App\Actions\*`; HTTP status concerns belong to the HTTP layer per Principles #1 + #3; `ValidationException` deliberately out of scope; shipped v0.5.0), and `ForbidResourceWrappedInJsonResponseRule` (bans wrapping a `JsonResource` in `response()->json()` / `new JsonResponse()` inside controllers per Principle #1 + ADR-0009; shipped v0.5.0). These enforce war-room §Architectural Principles (the last two also touching numbered ADRs) — each rule's docblock "Doctrine source" line names its authority. +- **Explicit HTTP timeouts (#8)** — package distributes `ForbidUntimedHttpClientRule` (flags an `Http` facade / injected `Illuminate\Http\Client\Factory` chain reaching a send verb without an explicit request timeout, per Architectural Principle #8; the AST-aware, omission-closing successor to the per-territory `ExternalHttpTimeoutTest` named-list Pest tests — conservative single-expression detection, declines split/helper-built chains + Guzzle/SDK surfaces; COMPLEMENTS the named-lists rather than replacing them; on `main`, `[Unreleased]`). Seed: war-room enforcement queue #58. ### War-room internal ADRs diff --git a/extension.neon b/extension.neon index 866a555..5301aee 100644 --- a/extension.neon +++ b/extension.neon @@ -126,6 +126,9 @@ services: auditModelNamespacePrefixes: %auditModelNamespacePrefixes% auditModelNameSuffixes: %auditModelNameSuffixes% tags: [phpstan.rules.rule] + - + class: ScriptDevelopment\PhpstanWarroomRules\Rules\ForbidUntimedHttpClientRule + tags: [phpstan.rules.rule] - class: ScriptDevelopment\PhpstanWarroomRules\Type\ConnectionTransactionReturnTypeExtension tags: [phpstan.broker.dynamicMethodReturnTypeExtension] From c9056b61ea682ec4831330c95b3c150d1f61e4be Mon Sep 17 00:00:00 2001 From: Gerard Oosterhof Date: Thu, 23 Jul 2026 10:06:37 +0200 Subject: [PATCH 3/3] feat(rules): type-aware withOptions + Macroable chain guard (bus review follow-up) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses both findings on PR #57: Major — withOptions() with a computed/non-literal options array false-positived: argArrayHasTimeoutKey only inspected literal Array_ nodes, so a helper-built array carrying 'timeout' still flagged the chain. The check is now TYPE-aware with a tri-state collapse: a constant array type provably lacking 'timeout' still fires (and now sees THROUGH a variable holding a literal array — a widening the AST check missed); a constant array carrying the key (any union variant) is timed; anything non-constant is POSSIBLY timed and the chain DECLINES — absence is unprovable and a false positive is the one unacceptable outcome (ADR-0021). Minor — a Macroable chain member (Http::github() at the static root, or an intermediate ->github() on the equally-Macroable PendingRequest) was misclassified as untimed even though the macro may return a pre-timed request. New KNOWN_BUILDERS surface: any chain member outside it declines (when()/unless() deliberately excluded — their closures can set the timeout invisibly); a genuine builder missing from the list costs only a false negative. Also: CLIENT_FACTORY is now a literal FQCN string (illuminate/http is not in this package's dev tree — Factory::class was a latent class.notFound under self-analysis; sibling DEFAULT_SINK pattern). 5 new fixtures + 5 tests (computed-declines, variable-without-timeout fires, variable-with-timeout silent, macro root declines, macro mid-chain declines). CHANGELOG updated. Gates: 192 tests / 277 assertions, phpstan OK, pint clean, coverage. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01APYknX78PNXAChnD17Dayx --- CHANGELOG.md | 2 +- src/Rules/ForbidUntimedHttpClientRule.php | 120 +++++++++++++++--- .../CompliantWithOptionsVariableTimeout.php | 18 +++ .../DeclinedMacroMidChain.php | 17 +++ .../DeclinedMacroStaticRoot.php | 18 +++ .../DeclinedWithOptionsComputed.php | 27 ++++ .../ViolationWithOptionsVariableNoTimeout.php | 21 +++ .../Rules/ForbidUntimedHttpClientRuleTest.php | 49 +++++++ 8 files changed, 254 insertions(+), 18 deletions(-) create mode 100644 tests/Fixtures/UntimedHttpClient/CompliantWithOptionsVariableTimeout.php create mode 100644 tests/Fixtures/UntimedHttpClient/DeclinedMacroMidChain.php create mode 100644 tests/Fixtures/UntimedHttpClient/DeclinedMacroStaticRoot.php create mode 100644 tests/Fixtures/UntimedHttpClient/DeclinedWithOptionsComputed.php create mode 100644 tests/Fixtures/UntimedHttpClient/ViolationWithOptionsVariableNoTimeout.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 00b6e64..1514fa5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and ### Added -- `ForbidUntimedHttpClientRule` — new rule enforcing war-room **Architectural Principle #8** (explicit timeouts on outbound HTTP) at analysis time, the AST-aware successor to the per-territory `ExternalHttpTimeoutTest` named-list Pest tests (kendo / emmie). The named-list tests detect wrong-shape on *enrolled* classes but are blind to **omission** — a new untimed call nobody adds to the list; this rule closes the omission gap for the tractable call shapes. Registered in `extension.neon` (no parameters). Identifier: `forbidUntimedHttpClient.missingTimeout`. Doctrine: war-room §Architectural Principles #8. Seed: war-room enforcement queue #58 (spike branch `spike/wr-queue58-untimed-http-client`). +- `ForbidUntimedHttpClientRule` — new rule enforcing war-room **Architectural Principle #8** (explicit timeouts on outbound HTTP) at analysis time, the AST-aware successor to the per-territory `ExternalHttpTimeoutTest` named-list Pest tests (kendo / emmie). The named-list tests detect wrong-shape on *enrolled* classes but are blind to **omission** — a new untimed call nobody adds to the list; this rule closes the omission gap for the tractable call shapes. Registered in `extension.neon` (no parameters). Identifier: `forbidUntimedHttpClient.missingTimeout`. Doctrine: war-room §Architectural Principles #8. Seed: war-room enforcement queue #58 (spike branch `spike/wr-queue58-untimed-http-client`). **Review follow-up (bus #57 findings):** the `withOptions()` check is TYPE-aware, not AST-literal — a constant array type provably lacking `'timeout'` still fires (including through a variable holding a literal array, a widening over the inline-`Array_`-only first cut), while a computed/helper-built options expression (not a constant array type) is POSSIBLY timed and the chain DECLINES (the Major: flagging it was a false positive). A chain member outside the known `PendingRequest` builder surface — a Macroable extension (`Http::github()`, root or intermediate) or `when()`/`unless()` with their opaque closures — likewise declines (the Minor: a macro may return a pre-timed request); a genuine builder missing from the list costs only a false negative. **Detection (type-anchored, two entry points):** fires on a terminal send verb (`get`/`post`/`put`/`patch`/`delete`/`head`/`send`) reached without an explicit request timeout, where the entry point is either (1) the `Http` facade (`Illuminate\Support\Facades\Http` static-call root), or (2) an **injected `Illuminate\Http\Client\Factory`** receiver (`$this->http->…->get()`, anchored by TYPE so the property alias is irrelevant — the dominant fleet idiom, established by field survey of kendo/emmie/ublgenie/BIO). A timeout counts when the visible chain contains `->timeout(...)` **or** `->withOptions([... 'timeout' => ...])`; `connectTimeout()` alone does NOT (it bounds the handshake, not the response). diff --git a/src/Rules/ForbidUntimedHttpClientRule.php b/src/Rules/ForbidUntimedHttpClientRule.php index 025bc92..81932e0 100644 --- a/src/Rules/ForbidUntimedHttpClientRule.php +++ b/src/Rules/ForbidUntimedHttpClientRule.php @@ -7,13 +7,11 @@ use Illuminate\Http\Client\Factory; use Illuminate\Support\Facades\Http; use PhpParser\Node; -use PhpParser\Node\Expr\Array_; use PhpParser\Node\Expr\CallLike; use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Expr\StaticCall; use PhpParser\Node\Identifier; use PhpParser\Node\Name; -use PhpParser\Node\Scalar\String_; use PHPStan\Analyser\Scope; use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\Rule; @@ -39,9 +37,14 @@ * so the alias (`$http` / `$httpClient` / `$client`) is irrelevant. * * A timeout is considered present when the visible chain contains a - * `->timeout(...)` call OR a `->withOptions([... 'timeout' => ... ])` carrying - * a `'timeout'` key. `connectTimeout()` alone is NOT sufficient — it bounds the - * handshake, not the response, so a hung server still stalls the caller; + * `->timeout(...)` call OR a `->withOptions(...)` whose options provably carry + * a `'timeout'` key. The options check is TYPE-aware, not AST-literal: a + * variable holding a literal array resolves to a constant array type and is + * seen through; an options expression whose type is NOT a constant array (a + * computed array, a helper/config() return) is treated as POSSIBLY timed and + * the chain DECLINES — absence is unprovable, and a false positive is the one + * unacceptable outcome. `connectTimeout()` alone is NOT sufficient — it bounds + * the handshake, not the response, so a hung server still stalls the caller; * Doctrine #8 wants the request timeout. * * Doctrine source: war-room §Architectural Principles #8 — Explicit timeouts @@ -67,6 +70,14 @@ * `setConfig([...])` — invisible to any HTTP-client scan; flagging them * would be a false positive. * - Timeouts configured at a DI binding (a provider binds a pre-timed client). + * - `->withOptions($computed)` where the options type is not a constant + * array — the key set is unknowable statically (see above). + * - Chain members outside the known `PendingRequest` builder surface — a + * `Macroable` extension (`Http::github()->get(...)`, or an intermediate + * `->github()`) may return a PRE-TIMED request, so an unknown method + * anywhere in the chain (root or intermediate) declines. `when()` / + * `unless()` decline for the same reason: their closures can set the + * timeout invisibly. * * These exclusions are the deliberate consequence of biasing a first-cut rule * toward zero false positives (a false positive reddens a compliant consumer's @@ -84,11 +95,38 @@ final class ForbidUntimedHttpClientRule implements Rule { private const string HTTP_FACADE = Http::class; + /** + * Literal FQCN, not `Factory::class` — `illuminate/http` is not in this + * package's own dev tree (only `illuminate/support` is), so a class-const + * fetch is a `class.notFound` under self-analysis. `ObjectType` takes the + * string happily; in a consumer tree without Laravel the anchor simply + * never matches (correct: nothing to enforce). Same pattern as the + * sibling rule's `DEFAULT_SINK`. + */ private const string CLIENT_FACTORY = Factory::class; /** Terminal HTTP send verbs on the facade / factory / `PendingRequest`. */ private const array SEND_VERBS = ['get', 'post', 'put', 'patch', 'delete', 'head', 'send']; + /** + * The known `PendingRequest` fluent-builder surface (lowercased). A chain + * member OUTSIDE this set — a Macroable extension, or `when()`/`unless()` + * whose closures are opaque — may have set the timeout internally, so the + * chain declines rather than risk a false positive. A genuine builder + * missing from this list costs only a false negative (ADR-0021 posture). + */ + private const array KNOWN_BUILDERS = [ + 'accept', 'acceptjson', 'asform', 'asjson', 'asmultipart', 'async', 'attach', + 'baseurl', 'beforesending', 'bodyformat', 'connecttimeout', 'contenttype', + 'dd', 'dump', 'maxredirects', 'replaceheaders', 'retry', 'sink', + 'throw', 'throwif', 'throwunless', 'timeout', + 'withbasicauth', 'withbody', 'withcookies', 'withdigestauth', + 'withheader', 'withheaders', 'withmiddleware', 'withoptions', + 'withqueryparameters', 'withrequestmiddleware', 'withresponsemiddleware', + 'withtoken', 'withurlparameters', 'withuseragent', + 'withoutredirecting', 'withoutverifying', + ]; + public function getNodeType(): string { return CallLike::class; @@ -139,7 +177,14 @@ private function processChainedSend(MethodCall $node, Scope $scope): array $cursor = $node->var; while ($cursor instanceof MethodCall) { - if ($this->declaresTimeout($cursor)) { + if ($this->declaresTimeout($cursor, $scope)) { + return []; + } + + // An unknown chain member — a Macroable extension (`->github()`) + // or anything outside the known builder surface — may have set the + // timeout internally. DECLINE, never a false positive. + if (!$this->isKnownBuilder($cursor->name)) { return []; } @@ -153,7 +198,17 @@ private function processChainedSend(MethodCall $node, Scope $scope): array return []; } - return $this->declaresTimeout($cursor) ? [] : [$this->buildError($node)]; + if ($this->declaresTimeout($cursor, $scope)) { + return []; + } + + // Same macro guard at the static root — `Http::github()->get(...)` + // may be a macro returning a pre-timed request. + if (!$this->isKnownBuilder($cursor->name)) { + return []; + } + + return [$this->buildError($node)]; } // Chain root is an expression (property / variable). It anchors ONLY if @@ -168,11 +223,11 @@ private function processChainedSend(MethodCall $node, Scope $scope): array } /** - * True when a builder call in the chain establishes a request timeout: - * a `->timeout(...)` method, or a `->withOptions([... 'timeout' => ...])` - * carrying the option key. + * True when a builder call in the chain establishes (or MAY establish) a + * request timeout: a `->timeout(...)` method, or a `->withOptions(...)` + * whose options are not provably timeout-free. */ - private function declaresTimeout(MethodCall|StaticCall $call): bool + private function declaresTimeout(MethodCall|StaticCall $call, Scope $scope): bool { $name = $this->methodName($call->name); @@ -181,29 +236,60 @@ private function declaresTimeout(MethodCall|StaticCall $call): bool } if ($name === 'withoptions') { - return $this->argArrayHasTimeoutKey($call); + return $this->withOptionsMayCarryTimeout($call, $scope); } return false; } - private function argArrayHasTimeoutKey(MethodCall|StaticCall $call): bool + /** + * Tri-state collapse over the `withOptions()` argument, by TYPE: + * + * - constant array type carrying a `'timeout'` key (any union variant) — + * timed, chain is compliant; + * - constant array type(s) all provably WITHOUT the key — untimed, the + * chain stays a candidate; + * - anything else (a computed array, a helper/config() return, unknown) — + * POSSIBLY timed, so the chain declines: absence is unprovable and a + * false positive is the one unacceptable outcome (ADR-0021). + * + * The type path subsumes the old literal-`Array_` AST check (a literal + * resolves to a constant array type) and additionally sees through a + * variable holding a literal array. + */ + private function withOptionsMayCarryTimeout(MethodCall|StaticCall $call, Scope $scope): bool { $first = $call->getArgs()[0]->value ?? null; - if (!$first instanceof Array_) { + if ($first === null) { + // `withOptions()` with no argument adds nothing to the request. return false; } - foreach ($first->items as $item) { - if ($item->key instanceof String_ && mb_strtolower($item->key->value) === 'timeout') { - return true; + $constantArrays = $scope->getType($first)->getConstantArrays(); + + if ($constantArrays === []) { + return true; + } + + foreach ($constantArrays as $constantArray) { + foreach ($constantArray->getKeyTypes() as $keyType) { + foreach ($keyType->getConstantStrings() as $keyString) { + if (mb_strtolower($keyString->getValue()) === 'timeout') { + return true; + } + } } } return false; } + private function isKnownBuilder(mixed $name): bool + { + return in_array($this->methodName($name), self::KNOWN_BUILDERS, true); + } + private function isClientFactory(Type $type): bool { return (new ObjectType(self::CLIENT_FACTORY))->isSuperTypeOf($type)->yes(); diff --git a/tests/Fixtures/UntimedHttpClient/CompliantWithOptionsVariableTimeout.php b/tests/Fixtures/UntimedHttpClient/CompliantWithOptionsVariableTimeout.php new file mode 100644 index 0000000..b6b8605 --- /dev/null +++ b/tests/Fixtures/UntimedHttpClient/CompliantWithOptionsVariableTimeout.php @@ -0,0 +1,18 @@ + 5]; + + Http::withOptions($options)->get($url); + } +} diff --git a/tests/Fixtures/UntimedHttpClient/DeclinedMacroMidChain.php b/tests/Fixtures/UntimedHttpClient/DeclinedMacroMidChain.php new file mode 100644 index 0000000..71a6248 --- /dev/null +++ b/tests/Fixtures/UntimedHttpClient/DeclinedMacroMidChain.php @@ -0,0 +1,17 @@ +github()->get($url); + } +} diff --git a/tests/Fixtures/UntimedHttpClient/DeclinedMacroStaticRoot.php b/tests/Fixtures/UntimedHttpClient/DeclinedMacroStaticRoot.php new file mode 100644 index 0000000..ba3307a --- /dev/null +++ b/tests/Fixtures/UntimedHttpClient/DeclinedMacroStaticRoot.php @@ -0,0 +1,18 @@ +get($url); + } +} diff --git a/tests/Fixtures/UntimedHttpClient/DeclinedWithOptionsComputed.php b/tests/Fixtures/UntimedHttpClient/DeclinedWithOptionsComputed.php new file mode 100644 index 0000000..c6c3d67 --- /dev/null +++ b/tests/Fixtures/UntimedHttpClient/DeclinedWithOptionsComputed.php @@ -0,0 +1,27 @@ +options())->get($url); + } + + /** + * @return array + */ + private function options(): array + { + return ['timeout' => 5, 'verify' => false]; + } +} diff --git a/tests/Fixtures/UntimedHttpClient/ViolationWithOptionsVariableNoTimeout.php b/tests/Fixtures/UntimedHttpClient/ViolationWithOptionsVariableNoTimeout.php new file mode 100644 index 0000000..cf7b227 --- /dev/null +++ b/tests/Fixtures/UntimedHttpClient/ViolationWithOptionsVariableNoTimeout.php @@ -0,0 +1,21 @@ + false]; + + Http::withOptions($options)->get($url); + } +} diff --git a/tests/Rules/ForbidUntimedHttpClientRuleTest.php b/tests/Rules/ForbidUntimedHttpClientRuleTest.php index 322d9cf..cf50c05 100644 --- a/tests/Rules/ForbidUntimedHttpClientRuleTest.php +++ b/tests/Rules/ForbidUntimedHttpClientRuleTest.php @@ -116,6 +116,55 @@ public function testIgnoresNonHttpReceiver(): void ); } + public function testDeclinesComputedWithOptions(): void + { + // The #57 review's Major: a computed options array (helper return) is + // not a constant array type — absence of 'timeout' is unprovable, so + // the chain DECLINES instead of false-positive flagging. + $this->analyse( + [__DIR__ . '/../Fixtures/UntimedHttpClient/DeclinedWithOptionsComputed.php'], + [], + ); + } + + public function testFlagsWithOptionsVariableProvablyWithoutTimeout(): void + { + // The type path sees through a variable holding a literal array — a + // provably timeout-free options set still fires (a widening over the + // old inline-Array_-only check). + $this->analyse( + [__DIR__ . '/../Fixtures/UntimedHttpClient/ViolationWithOptionsVariableNoTimeout.php'], + [[self::MESSAGE, 19]], + ); + } + + public function testIgnoresWithOptionsVariableWithTimeout(): void + { + $this->analyse( + [__DIR__ . '/../Fixtures/UntimedHttpClient/CompliantWithOptionsVariableTimeout.php'], + [], + ); + } + + public function testDeclinesMacroStaticRoot(): void + { + // The #57 review's Minor: `Http::github()` — a Macroable entry outside + // the known builder surface — may return a pre-timed request. Declines. + $this->analyse( + [__DIR__ . '/../Fixtures/UntimedHttpClient/DeclinedMacroStaticRoot.php'], + [], + ); + } + + public function testDeclinesMacroMidChain(): void + { + // Same guard for an intermediate member — PendingRequest is Macroable. + $this->analyse( + [__DIR__ . '/../Fixtures/UntimedHttpClient/DeclinedMacroMidChain.php'], + [], + ); + } + protected function getRule(): Rule { return new ForbidUntimedHttpClientRule;