diff --git a/AGENTS.md b/AGENTS.md index 42b8b17..8835fed 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -486,12 +486,12 @@ Commands: `make docs-install`, `make docs-api`, `make docs-dev`, need PHP; the rest are Node, so the site stays buildable without a PHP toolchain. -Since the 2026-09-18 review wave: the null-safe reads in -`Runtime::probeAnswer()` and `Understudy::rejectNestedCalls()` (a generated +Since the 2026-09-18 review wave: the null-safe reads in the recording +branch of `Runtime::dispatch()` and in `Understudy::record()` (a generated class always has a blueprint, and the signal's double is registered unless the specification names a forgotten one — a path the message tolerates with -`understudy`), the `nested: true` flag of the probe's default (the throwaway -context observes nothing either way), the value written into the +`understudy`), the `nested: true` flag of the recording's default (the +throwaway context observes nothing either way), the value written into the `droppedByScope` map (only membership is read), the Fiber branch of `popScope()` passing `byScope` (no Fiber scope test exists — add one if the branch grows), and the trailing `return true` of the intersection loop in diff --git a/CHANGELOG.md b/CHANGELOG.md index 9aa4ef1..b48cfbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,28 @@ # Changelog +## 0.12.0 — 2026-09-19 + +- **Fixed.** A specification closure is run exactly once. Since 0.11.0 the + recording, which ended on the first double call, re-ran the closure a second + time to see whether another call hid in the arguments of the first — and + the closure is user code: a counter in it went up twice, a factory built + twice, an `Understudy::for()` inside it created a second double that stayed + in the context and failed `verifyAll(strictStubs: true)` as a stub never + used. The recording now keeps going past a call instead of ending on it: + the dispatcher retains the call and answers it with the mode's type-safe + default out of a throwaway context, so every call the closure makes is + seen in one pass. A method with no safe default (`: never`, an object + type) still ends the closure on its signal, as every recording used to. + Code after the call now runs during a recording, against that default; if + it does not survive it (a call on the `null` a `?Book` answers with, a + closure return type the default does not fit), the specification stands — + that code was never run before and is not the specification. (#142) +- **Changed.** Two calls side by side — `when(fn () => $r->count() . + $r->describe())` — are refused like a nested pair, and the refusal reads + "calls `count()` … and then `describe()` …" for both shapes; the earlier + wording assumed the nested one. `InvalidCallSpecification::nestedCall()` is + `moreThanOneCall()`. + ## 0.11.0 — 2026-09-18 - **Fixed.** An interface extending `Throwable`, `DateTimeInterface`, diff --git a/README.md b/README.md index 06b9fa8..7735e95 100644 --- a/README.md +++ b/README.md @@ -290,13 +290,17 @@ same as uncalled, so not even `strictStubs` reports it. An exhausted call-count expectation keeps answering the matching call, so use a non-overlapping matcher when a broad fallback should handle later calls. -The closure makes exactly one call on a double. A call nested in the arguments -of another — `when(fn () => $repo->find($repo->count()))` — is refused with -`InvalidCallSpecification` naming both: the inner call would have been -specified silently and the outer one not at all. Stub the inner call in a -`when()` of its own and pass a literal or a matcher. A required parameter has -to be spelled; `Arg::rest()` says the ones after it do not matter, so a wide -signature does not need an `Arg::any()` per position. +The closure makes exactly one call on a double, and runs exactly once. Two +calls — nested in the arguments of another, `when(fn () => $repo->find($repo->count()))`, +or side by side — are refused with `InvalidCallSpecification` naming both: +with two, one of them would be specified and the other not. Stub each call +in a `when()` of its own and pass a literal or a matcher where one fed the +other. A recording answers the call with the type's default (`null` for +`?Book`, `0` for `int`) and lets the closure finish, so code after the call +runs against that default; if it does not survive it, the specification +stands. A required parameter has to be spelled; `Arg::rest()` says the ones +after it do not matter, so a wide signature does not need an `Arg::any()` per +position. `returns()` is checked against the declared return type where it is written: a value on a `: void` method (`returns(null)` is allowed — it is the idiom for diff --git a/README.ru.md b/README.ru.md index 41dad8b..5998487 100644 --- a/README.ru.md +++ b/README.ru.md @@ -288,13 +288,17 @@ catch-all, зарегистрированный *после* точечного, точечный не диагностируется — недостижимый не значит невызванный, и даже `strictStubs` его не покажет. -Замыкание делает ровно один вызов на дубле. Вызов, вложенный в аргументы -другого — `when(fn () => $repo->find($repo->count()))`, — отвергается -`InvalidCallSpecification` с обоими именами: внутренний вызов был бы -застаблен молча, а внешний — не застаблен вовсе. Застабьте внутренний -отдельным `when()` и передайте литерал или матчер. Обязательный параметр надо -записать; `Arg::rest()` говорит, что остальное неважно, и широкой сигнатуре -не нужен `Arg::any()` на каждую позицию. +Замыкание делает ровно один вызов на дубле и выполняется ровно один раз. +Два вызова — вложенный в аргументы другого, `when(fn () => $repo->find($repo->count()))`, +или два подряд — отвергаются `InvalidCallSpecification` с обоими именами: +из двух один был бы застаблен, а другой нет. Застабьте каждый отдельным +`when()`, а там, где один вызов питал аргументы другого, передайте литерал +или матчер. Запись отвечает на вызов дефолтом типа (`null` для `?Book`, `0` +для `int`) и даёт замыканию дойти до конца, так что код после вызова +исполняется с этим дефолтом; если он его не переживает, спецификация +остаётся в силе. Обязательный параметр надо записать; `Arg::rest()` говорит, +что остальное неважно, и широкой сигнатуре не нужен `Arg::any()` на каждую +позицию. `returns()` сверяется с объявленным типом возврата там, где написан: значение на `: void`-методе (`returns(null)` разрешён — это идиома «ничего не diff --git a/llms.txt b/llms.txt index bed30f8..6ff2a12 100644 --- a/llms.txt +++ b/llms.txt @@ -20,10 +20,12 @@ No runtime deps; `ext-mbstring` is not needed. covers it where the contract declares it required. A real call still logs the contract's default for an argument the caller omitted, so `tag('a')` and `tag('a', 1)` are one call. -- The closure must contain **exactly one** direct call on a double. Anything - else raises `InvalidCallSpecification` — including a call nested in the - arguments of another (`fn () => $repo->find($repo->count())`), which is - refused naming both calls rather than silently specifying the inner one. +- The closure must contain **exactly one** direct call on a double, and it + runs exactly once. Two calls — nested in the arguments of another + (`fn () => $repo->find($repo->count())`) or side by side — raise + `InvalidCallSpecification` naming both. The recording answers the call with + the type's default and lets the closure finish; code after the call that + does not survive the default does not fail the specification. - `returns()` is checked against the declared return type at registration: a value on `: void` (`returns(null)` is allowed), anything on `: never`, and a value the type cannot hold (`null` on a non-nullable type, an array or an diff --git a/src/Exception/InvalidCallSpecification.php b/src/Exception/InvalidCallSpecification.php index d22ceff..33dd6b1 100644 --- a/src/Exception/InvalidCallSpecification.php +++ b/src/Exception/InvalidCallSpecification.php @@ -38,26 +38,27 @@ public static function noCallRecorded(): self } /** - * The closure called one double method while evaluating the arguments of - * another. The inner call is what the recording saw; the outer call is - * what the test meant, and neither reading is safe to guess. + * The closure called more than one double method. Nested in the arguments + * of another (`find($r->count())`) or one after the other (`a() + b()`), + * the recording cannot tell which one the test meant, and neither reading + * is safe to guess. * - * @param non-empty-string $innerLabel the double the inner call was made on - * @param non-empty-string $innerMethod the inner call, the one the recording saw - * @param non-empty-string $outerLabel the double the outer call was made on - * @param non-empty-string $outerMethod the outer call, whose arguments held the inner one + * @param non-empty-string $firstLabel the double the first call was made on + * @param non-empty-string $firstMethod the first call dispatched — for a nested pair, the inner one + * @param non-empty-string $lastLabel the double the last call was made on + * @param non-empty-string $lastMethod the last call dispatched — for a nested pair, the outer one */ - public static function nestedCall(string $innerLabel, string $innerMethod, string $outerLabel, string $outerMethod): self + public static function moreThanOneCall(string $firstLabel, string $firstMethod, string $lastLabel, string $lastMethod): self { return new self(sprintf( - 'The specification closure calls `%s()` on understudy `%s` while evaluating the arguments of `%s()` ' - . 'on understudy `%s`. A closure must contain exactly one direct call on a double; the inner call would ' - . 'have been specified silently and the outer one not at all. Stub the inner call in a when() of its own ' - . 'and pass a literal or a matcher here, for example: when(fn () => $repository->find(Arg::any()))', - $innerMethod, - $innerLabel, - $outerMethod, - $outerLabel, + 'The specification closure calls `%s()` on understudy `%s` and then `%s()` on understudy `%s`. ' + . 'A closure must contain exactly one direct call on a double; with two, one of them would be specified ' + . 'silently and the other not at all. Stub each call in a when() of its own, and where one call feeds the ' + . 'arguments of another pass a literal or a matcher instead, for example: when(fn () => $repository->find(Arg::any()))', + $firstMethod, + $firstLabel, + $lastMethod, + $lastLabel, )); } diff --git a/src/Runtime/Runtime.php b/src/Runtime/Runtime.php index 848f299..8d1bc76 100644 --- a/src/Runtime/Runtime.php +++ b/src/Runtime/Runtime.php @@ -95,70 +95,6 @@ final class Runtime */ private static array $live = []; - /** - * The calls a specification closure made while it was re-run in probe - * mode, or `null` outside one. See {@see probe()}. - * - * @var list|null - */ - private static ?array $probed = null; - - /** - * Re-runs a specification closure with every double call answered instead - * of signalled, and reports the calls it made. - * - * The recording proper throws on the FIRST dispatch — that is what lets a - * `: ?Book` or a `: never` method hand its name back without producing a - * value — but the first dispatch is the innermost call, so a closure like - * `fn () => $r->find($r->count())` was abandoned before `find()` was ever - * seen and silently specified `count()`. Here the closure runs again, each - * call answered with the mode's type-safe default out of a throwaway - * context, and the caller refuses the specification when more than one - * call turns up. - * - * Everything about this pass is advisory. A default may not exist (a - * `: never` method, a return type nothing can stand in for) and the code - * after a call may not survive a default (`->title` on the `null` a - * `?Book` answers with): any throwable ends the probe as inconclusive and - * the specification the first pass recorded stands, exactly as before. - * Nothing the probe creates is registered anywhere that verification, - * accounting or `idle()` can see. - * - * @return list|null the calls, in the - * order they were made; `null` when the probe could not finish - */ - public static function probe(callable $call): ?array - { - // Called from inside the recording that the first pass opened, so the - // dispatcher already routes every call here; nothing to open or close. - \assert(self::current()->isRecording()); - self::$probed = []; - - try { - $call(); - - return self::$probed; - } catch (\Throwable) { - return null; - } finally { - self::$probed = null; - } - } - - /** - * The probe's answer to one call: recorded, then the type-safe default the - * mode would give, built in a context nobody else holds. - * - * @param non-empty-string $method - */ - private static function probeAnswer(object $double, string $method): mixed - { - self::$probed[] = [$double, $method]; - $signature = DoubleFactory::blueprintOfGenerated($double::class)?->method($method); - - return TypeDefaultResolver::forSignature('probe', $signature, $method, new RuntimeContext(), nested: true, double: $double); - } - public static function current(): RuntimeContext { $stack = self::stack(); @@ -545,11 +481,31 @@ public static function dispatch(object $double, string $method, array $args): mi $current = self::current(); if ($current->isRecording()) { - if (self::$probed !== null) { - return self::probeAnswer($double, $method); + // The signal is retained, not thrown: the closure goes on, so a + // second call — `fn () => $r->find($r->count())` dispatches + // `count()` first — is seen and refused by the recorder instead + // of being specified silently. The call is answered with the + // mode's type-safe default out of a context nobody else holds, + // `nested` so that no collaborator is invented along the way. + // Where no default exists (`: never`, an object type) the signal + // is thrown after all and the closure ends here, as every + // recording used to. + $signal = new InvocationSignal($double, $method, $args); + $current->recordSpecificationCall($signal); + $signature = DoubleFactory::blueprintOfGenerated($double::class)?->method($method); + + try { + return TypeDefaultResolver::forSignature( + self::stateOf($double)?->label() ?? 'understudy', + $signature, + $method, + new RuntimeContext(), + nested: true, + double: $double, + ); + } catch (\Throwable) { + throw $signal; } - - throw new InvocationSignal($double, $method, $args); } self::rejectLeakedMatchers($method, $args); @@ -982,9 +938,10 @@ public static function referenceSlot(object $double, string $method, array $args /** @var mixed $value */ $value = self::dispatch($double, $method, $args); - if (self::$probed !== null) { - // A probe answers with a throwaway: the per-method slot is state - // the test can observe through the reference it already holds. + if (self::current()->isRecording()) { + // A recording answers with a throwaway: the per-method slot is + // state the test can observe through the reference it already + // holds, and the closure's own value must not land in it. $slot = new ReferenceSlot(); $slot->value = $value; diff --git a/src/Runtime/RuntimeContext.php b/src/Runtime/RuntimeContext.php index 0c707df..8b6ca55 100644 --- a/src/Runtime/RuntimeContext.php +++ b/src/Runtime/RuntimeContext.php @@ -25,6 +25,9 @@ final class RuntimeContext */ private int $recordingDepth = 0; + /** @var list */ + private array $recordingSignals = []; + /** @var \SplObjectStorage */ private \SplObjectStorage $doubles; @@ -78,6 +81,10 @@ public function isRecording(): bool public function beginRecording(): void { + if ($this->recordingDepth === 0) { + $this->recordingSignals = []; + } + $this->recordingDepth++; } @@ -88,6 +95,17 @@ public function endRecording(): void } } + public function recordSpecificationCall(InvocationSignal $signal): void + { + $this->recordingSignals[] = $signal; + } + + /** @return list */ + public function recordingSignals(): array + { + return $this->recordingSignals; + } + public function register(object $double, DoubleState $state): void { $this->doubles[$double] = $state; diff --git a/src/Understudy.php b/src/Understudy.php index 27500e5..d3cba4a 100644 --- a/src/Understudy.php +++ b/src/Understudy.php @@ -1317,8 +1317,14 @@ public static function idle(): bool } /** - * Runs the specification closure with recording on, and catches the signal - * the called method throws instead of returning. + * Runs the specification closure with recording on, in one pass, and + * reads the call it made off the context. + * + * A call made while recording is retained by the dispatcher and answered + * with a default, so the closure runs to its end and every call it makes + * is seen; only a method with no safe default (`: never`, an object type) + * ends it early on its signal. The closure is user code and runs exactly + * once: whatever it counts, creates or writes happens once. */ private static function record(callable $call): InvocationSignal { @@ -1326,59 +1332,58 @@ private static function record(callable $call): InvocationSignal $context->beginRecording(); try { - $call(); - } catch (InvocationSignal $signal) { - self::rejectNestedCalls($call, $signal); + try { + $call(); + } catch (InvocationSignal) { + // Retained before it was thrown; the closure just ended early. + } catch (UnderstudyError $failure) { + // Our own refusals are already about the specification — a + // matcher built with an impossible range, a captor inside a + // combinator, a double the test retired. Wrapping one in "the + // closure threw before it reached an understudy" would bury + // the sentence that says what to change. + throw $failure; + } catch (\Throwable $failure) { + // Before any call: the closure never reached a double, and its + // error is the whole story. After one: the code past the call + // ran against a default it may not survive (`->title` on the + // `null` a `?Book` answers with, a closure return type the + // default does not fit) — that code was never run by a + // recording before, and is not the specification. + if ($context->recordingSignals() === []) { + throw InvalidCallSpecification::closureFailed($failure); + } + } + + $signals = $context->recordingSignals(); + + if ($signals === []) { + throw InvalidCallSpecification::noCallRecorded(); + } + + if (count($signals) > 1) { + // Dispatch order: for `find($r->count())` the inner `count()` + // is first, and for `a() + b()` the first is simply first. + $first = $signals[0]; + $last = $signals[count($signals) - 1]; + + throw InvalidCallSpecification::moreThanOneCall( + Runtime::stateOf($first->double)?->label() ?? 'understudy', + $first->method, + Runtime::stateOf($last->double)?->label() ?? 'understudy', + $last->method, + ); + } // A specification that ended with Arg::rest() physically passed // fewer arguments than the method declares; the generated // parameters answered with their sentinel defaults, and those are // stripped — or the omission is refused — before anything reads // the arguments as a specification. - return $signal->asSpecification(); - } catch (UnderstudyError $failure) { - // Our own refusals are already about the specification — a matcher - // built with an impossible range, a captor inside a combinator, a - // double the test retired. Wrapping one in "the closure threw - // before it reached an understudy" would bury the sentence that - // says what to change. - throw $failure; - } catch (\Throwable $failure) { - throw InvalidCallSpecification::closureFailed($failure); + return $signals[0]->asSpecification(); } finally { $context->endRecording(); } - - throw InvalidCallSpecification::noCallRecorded(); - } - - /** - * Refuses a closure that called more than one double method. - * - * The signal that ended the first pass came from the INNERMOST call: the - * arguments of `$r->find($r->count())` are evaluated before `find()` is - * dispatched, so `count()` signalled and `find()` was never reached. The - * rule says exactly one call; a closure that broke it used to specify the - * inner call silently, with the outer one's `returns()` attached to it. - * {@see Runtime::probe()} re-runs the closure with calls answered instead - * of signalled, which is the only way to see past the first one. - */ - private static function rejectNestedCalls(callable $call, InvocationSignal $signal): void - { - $calls = Runtime::probe($call); - - if ($calls === null || count($calls) < 2) { - return; - } - - $outer = $calls[count($calls) - 1]; - - throw InvalidCallSpecification::nestedCall( - Runtime::stateOf($signal->double)?->label() ?? 'understudy', - $signal->method, - Runtime::stateOf($outer[0])?->label() ?? 'understudy', - $outer[1], - ); } /** diff --git a/tests/UnderstudyTest.php b/tests/UnderstudyTest.php index 5686fc9..d0399e7 100644 --- a/tests/UnderstudyTest.php +++ b/tests/UnderstudyTest.php @@ -600,11 +600,11 @@ public function specificationClosureWithANestedDoubleCallIsRejected(): void $repository = Understudy::for(BookRepository::class); Expect::exception(InvalidCallSpecification::class)->withMessage( - 'The specification closure calls `count()` on understudy `BookRepository` while evaluating the ' - . 'arguments of `find()` on understudy `BookRepository`. A closure must contain exactly one direct ' - . 'call on a double; the inner call would have been specified silently and the outer one not at all. ' - . 'Stub the inner call in a when() of its own and pass a literal or a matcher here, for example: ' - . 'when(fn () => $repository->find(Arg::any()))', + 'The specification closure calls `count()` on understudy `BookRepository` and then `find()` on ' + . 'understudy `BookRepository`. A closure must contain exactly one direct call on a double; with two, ' + . 'one of them would be specified silently and the other not at all. Stub each call in a when() of its ' + . 'own, and where one call feeds the arguments of another pass a literal or a matcher instead, for ' + . 'example: when(fn () => $repository->find(Arg::any()))', ); Understudy::when(fn() => $repository->find($repository->count())); @@ -620,19 +620,31 @@ public function aNestedCallOnAnotherDoubleIsRejectedByBothNames(): void $counter = Understudy::label(Understudy::for(BookRepository::class), 'inner'); Expect::exception(InvalidCallSpecification::class) - ->withMessageContaining('calls `count()` on understudy `inner` while evaluating the arguments of `find()` on understudy `outer`'); + ->withMessageContaining('calls `count()` on understudy `inner` and then `find()` on understudy `outer`'); Understudy::expect(fn() => $repository->find($counter->count())); } /** - * The probe that sees past the first call answers every call with the - * mode's default and treats any failure as inconclusive. Neither the - * nested doubles it builds nor its answers may leave a trace: the context - * is as idle afterwards as before, and a `: never` method — which has no - * default to answer with — is still specifiable. + * Two calls side by side are as ambiguous as two nested ones, and the + * recording sees them in dispatch order. */ - public function theNestedCallProbeLeavesNoTraceAndToleratesNever(): void + public function twoCallsInARowAreRejected(): void + { + $repository = Understudy::for(BookRepository::class); + + Expect::exception(InvalidCallSpecification::class) + ->withMessageContaining('calls `count()` on understudy `BookRepository` and then `describe()` on understudy `BookRepository`'); + + Understudy::when(fn() => $repository->count() . $repository->describe()); + } + + /** + * A `: never` method has no default to answer with, so its signal ends the + * closure — and the specification stands, as does one for a method that + * does have a default. + */ + public function recordingToleratesAMethodWithoutADefault(): void { $repository = Understudy::for(BookRepository::class); @@ -646,6 +658,75 @@ public function theNestedCallProbeLeavesNoTraceAndToleratesNever(): void $repository->abort('x'); } + /** + * The closure is user code, and the second pass that used to look past + * the first call ran all of it again: a counter went up twice, a factory + * built twice. (#142) + */ + public function specificationClosureRunsOnlyOnce(): void + { + $repository = Understudy::for(BookRepository::class); + $runs = 0; + + Understudy::when(function () use ($repository, &$runs): ?Book { + ++$runs; + + return $repository->find($runs); + })->returns(new Book('one')); + + Assert::same($runs, 1); + Assert::same($repository->find(1)?->title, 'one'); + } + + /** + * A double created inside the closure used to be created twice, and the + * second one — stubbed by nobody, called by nobody — stayed in the + * context, where `strictStubs` reported its stub as never used. (#142) + */ + public function specificationClosureDoesNotRegisterASecondDouble(): void + { + $created = []; + + Understudy::when(function () use (&$created): int { + $repository = Understudy::for(BookRepository::class); + $created[] = $repository; + + return $repository->count(); + })->returns(1); + + Assert::same(count($created), 1); + Assert::false(Understudy::idle()); + Assert::same($created[0]->count(), 1); + Understudy::verifyAll(strictStubs: true); + } + + /** + * Code after the call runs against the default the call was answered + * with, and may not survive it — a call on the `null` a `?Book` answers + * with, a closure return type the default does not fit. No + * recording ever ran that code before, and it is not the specification. + */ + #[DataProvider('codeAfterTheCallProvider')] + public function codeAfterTheCallThatFailsOnTheDefaultDoesNotFailTheSpecification(\Closure $specification): void + { + $repository = Understudy::for(BookRepository::class); + + Understudy::when($specification($repository))->returns(new Book('one')); + + Assert::same($repository->find(1)?->title, 'one'); + } + + public static function codeAfterTheCallProvider(): iterable + { + yield 'method call on null' => [fn(BookRepository $r): \Closure => fn() => $r->find(1)->jsonSerialize()]; + yield 'closure return type' => [fn(BookRepository $r): \Closure => fn(): Book => $r->find(1)]; + yield 'explicit throw after the call' => [fn(BookRepository $r): \Closure => function () use ($r): void { + $r->find(1); + + throw new \LogicException('after the call'); + }]; + } + /** * Two doubles of one contract used to be both `BookRepository` in a * report, and "which one?" was the reader's problem. The first keeps the