From 3759f55f3f0334ec11fd78c0a57b7f43ef5da96a Mon Sep 17 00:00:00 2001 From: "v.razuvaev" Date: Fri, 18 Sep 2026 02:14:31 +0300 Subject: [PATCH 1/3] Fix the 2026-09-18 review findings: fatal-free refusals, nested calls, returns() checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Refuse an interface extending Throwable, DateTimeInterface, UnitEnum or Traversable (without Iterator/IteratorAggregate), and an interface declaring __construct, with UnsupportedTarget before eval(); the guard compared the contract's own name and let ClientExceptionInterface die in the compiler as an uncatchable fatal. Fixes #133 - Refuse a specification closure that calls one double method while evaluating the arguments of another, naming both: the recording ended on the innermost call and stubbed it silently with the outer call's returns(). The closure is re-run once in a probe that answers calls with defaults out of a throwaway context; where the probe cannot finish, the specification stands as before. Fixes #134 - Check returns() against the declared return type at registration — a value on void (null stays allowed), anything on never, a value the type cannot hold — naming the double instead of a TypeError naming the generated class from inside the code under test. No stricter than the engine: generated methods are not under strict_types. Fixes #135 - Number the second and later unlabelled doubles of one contract (Repo, Repo#2) so a report says which one. Fixes #137 - Say "scope() closed" rather than "reset()" in ForgottenDouble after a scope dropped the double. Fixes #138 - Document rather than change: a later catch-all stub is not diagnosed (#136), the depth-1 default double is fresh per call, no partial double of an abstract class, readonly promoted properties, matcher leaks into real objects, Arg::rest() for wide signatures. --- AGENTS.md | 12 + CHANGELOG.md | 46 ++++ README.md | 52 ++++- README.ru.md | 49 +++- llms.txt | 41 +++- src/Codegen/DoubleFactory.php | 112 ++++++--- src/Exception/ForgottenDouble.php | 16 ++ src/Exception/InvalidCallSpecification.php | 59 +++++ .../InvalidSpecificationArgument.php | 21 ++ src/Expectation/ReturnContract.php | 134 +++++++++++ src/Runtime/DoubleState.php | 17 +- src/Runtime/Runtime.php | 145 +++++++++++- src/Runtime/RuntimeContext.php | 22 ++ src/Understudy.php | 53 ++++- src/WhenBuilder.php | 36 ++- tests/ClassDoubleTest.php | 80 ++++++- tests/Fixture/Ret/PlainRepository.php | 59 +++++ tests/Fixture/Ret/RepositoryAndLibrarian.php | 17 ++ tests/Fixture/Ret/Stringy.php | 14 ++ .../Fixture/Undoublable/AggregateContract.php | 7 + tests/Fixture/Undoublable/ClockContract.php | 7 + .../Undoublable/CollectionContract.php | 7 + .../Undoublable/ConstructedContract.php | 12 + tests/Fixture/Undoublable/EnumContract.php | 7 + .../Fixture/Undoublable/ThrowableContract.php | 7 + tests/LedgerTest.php | 4 +- tests/ReturnContractTest.php | 118 ++++++++++ tests/UnderstudyTest.php | 212 +++++++++++++++++- tests/VerificationFailureTest.php | 2 +- 29 files changed, 1305 insertions(+), 63 deletions(-) create mode 100644 src/Expectation/ReturnContract.php create mode 100644 tests/Fixture/Ret/PlainRepository.php create mode 100644 tests/Fixture/Ret/RepositoryAndLibrarian.php create mode 100644 tests/Fixture/Ret/Stringy.php create mode 100644 tests/Fixture/Undoublable/AggregateContract.php create mode 100644 tests/Fixture/Undoublable/ClockContract.php create mode 100644 tests/Fixture/Undoublable/CollectionContract.php create mode 100644 tests/Fixture/Undoublable/ConstructedContract.php create mode 100644 tests/Fixture/Undoublable/EnumContract.php create mode 100644 tests/Fixture/Undoublable/ThrowableContract.php create mode 100644 tests/ReturnContractTest.php diff --git a/AGENTS.md b/AGENTS.md index 0fcd982..42b8b17 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -486,6 +486,18 @@ 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 +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 +`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 +`ReturnContract::acceptsType()` (falling through lands on the `match` default, +which accepts a name `class_exists()` does not know). + ## When you finish - If the change touches the public API, run `make docs-api` and commit the diff --git a/CHANGELOG.md b/CHANGELOG.md index fcb1b39..803fc04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,51 @@ # Changelog +## Unreleased + +- **Fixed.** An interface extending `Throwable`, `DateTimeInterface`, + `UnitEnum`/`BackedEnum` or `Traversable` (without `Iterator`/ + `IteratorAggregate`), and an interface declaring `__construct`, are refused + with `UnsupportedTarget` before generation. The guard compared the + contract's own name, so `Psr\Http\Client\ClientExceptionInterface` and + its kin walked past it into `eval()` and died there as a fatal error no + test, adapter or `try` can catch — ending the whole suite run. The + `Throwable` advice no longer points at doubling an exception class, which + is refused too; construct a real exception for `throws()`. (#133) +- **Fixed.** A specification closure that calls one double method while + evaluating the arguments of another — `when(fn () => $r->find($r->count()))` + — is refused with `InvalidCallSpecification` naming both calls. The + recording ends on the first dispatch, which is the innermost call, so the + inner call was specified silently with the outer call's `returns()` and the + outer one not at all. The closure is now re-run once with calls answered + instead of signalled to see past the first; where that probe cannot finish + (a `: never` method, a return type with no default, code after the call that + does not survive a default) the specification stands as before. (#134) +- **Fixed.** `returns()` is checked against the declared return type where it + is written: a value on a `: void` method (`returns(null)` stays allowed as + the idiom for "answer nothing"), anything on a `: never` method, and a value + the type cannot hold — `null` where the type is not nullable, an array or an + object where a scalar is declared, an object of the wrong class — raise + `InvalidCallSpecification`/`InvalidSpecificationArgument` naming the double, + instead of a `TypeError` naming the generated class from inside the code + under test. No stricter than the engine: a generated method is not under + `strict_types`, so `returns('5')` on `: int` still answers `5`. + `WhenBuilder::__construct()` gained an optional trailing parameter. (#135) +- **Changed.** The second and later unlabelled doubles of one contract in a + context get a numbered default label — `Repo`, `Repo#2` — so a report says + which one; the first keeps the bare name and an explicit `label()` still + wins. (#137) +- **Changed.** `ForgottenDouble` after a `scope()` closed says so, instead of + blaming a `reset()` the test never wrote. (#138) +- **Documentation.** Written down rather than changed: a catch-all stub + registered after a specific one is not diagnosed (unreachable is not + uncalled, so `strictStubs` stays quiet — register the broad stub first, + #136); the depth-1 default double is a fresh instance per call; a class + double is never a partial double of an abstract class; a `readonly` + promoted property of a class target stays uninitialized where a plain typed + one starts empty; `MatcherLeaked` is raised only by a double, a matcher + handed to a real object is out of the engine's sight; `Arg::rest()` is the + way to stop spelling a wide signature. + ## 0.10.0 — 2026-09-06 The wave the 1.0 candidate turned out to still need: ten defects found by diff --git a/README.md b/README.md index 3333d25..06b9fa8 100644 --- a/README.md +++ b/README.md @@ -173,6 +173,21 @@ that is not the first target. A double that cannot intercept every method would run the target's real code against an object whose constructor never ran, which is worse than not building it at all. +The same goes for what PHP itself forbids a class to implement, checked on the +contract and on everything it extends: an interface extending `Throwable` +(`Psr\Http\Client\ClientExceptionInterface`, `Psr\Cache\CacheException`), +`DateTimeInterface`, `UnitEnum`, or `Traversable` without `Iterator` or +`IteratorAggregate` — and an interface declaring a constructor, which a double +never runs. Each is refused with `UnsupportedTarget` naming the ancestor, +rather than dying in the compiler as a fatal error no test can catch. A double +cannot be thrown: construct a real exception and hand it to `throws()`. + +A class double is never a partial double. A concrete method of an abstract +class target is dispatched like every other — its body does not run, so a +template method returns the mode's default rather than calling the abstract +step you stubbed. Running real code takes a real instance and +`Understudy::forwarding()`; `Invocation::callOriginal()` needs the same. + ### Property hooks (PHP 8.4+) A contract declaring a property is doublable: @@ -268,9 +283,31 @@ when(fn () => $repository->mode())->returns('fast', 'slow'); ``` A later stub for the same call wins; earlier ones stay reachable as fallbacks -when their arguments do not match. An exhausted call-count expectation keeps -answering the matching call, so use a non-overlapping matcher when a broad -fallback should handle later calls. +when their arguments do not match. Register the broad stub first and the +specific one after it: a catch-all registered *after* a specific stub takes +every call, and the specific one is not diagnosed — unreachable is not the +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. + +`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 +"answer nothing"), anything on a `: never` method, and a value the type cannot +hold — `null` where the type is not nullable, an array or an object where a +scalar is declared, an object of the wrong class — are refused with the +double's label instead of surfacing later as a `TypeError` naming the generated +class from inside the code under test. The check is no stricter than the +engine: a generated method is not under `strict_types`, so `returns('5')` on +`: int` still answers `5`. What `answers()` produces can only be judged at the +call, and there a wrong type is still PHP's own `TypeError`. | Matcher | Matches | |---|---| @@ -668,7 +705,10 @@ A loose double never invents a value by running someone else's constructor, and never hands back an unconstructed instance of a real class. What it can hand back is another understudy: a return type that can itself be doubled becomes one, one level deep, which the same test can configure. That double is a -generated stand-in, not the target with its constructor skipped. +generated stand-in, not the target with its constructor skipped — and a fresh +one on every call: `$outer->inner() === $outer->inner()` is false. A test that +takes one and verifies against it sees only the calls made on that instance; +stub the method with `returns()` when the collaborator has to be stable. A built-in interface is the exception: a method declared `: Stringable`, `: Countable`, `: JsonSerializable`, `: ArrayAccess` or `: IteratorAggregate` @@ -826,7 +866,9 @@ The following calls to `tag` were made during this test: The asterisks mark the argument that differed — borrowed from [NSubstitute](https://nsubstitute.github.io). `Understudy::label($double, '…')` -names a double when several of the same contract are in play. +names a double when several of the same contract are in play; without a label +the second and later doubles of one contract are numbered — `BookRepository`, +`BookRepository#2` — so a report still says which one. An object argument is matched by identity, so two instances never match however equally they read — and the message has to be able to show which of the two diff --git a/README.ru.md b/README.ru.md index 0954854..41dad8b 100644 --- a/README.ru.md +++ b/README.ru.md @@ -174,6 +174,21 @@ trait, internal-класс, анонимный класс и любой клас который не может перехватить все методы, исполнил бы настоящий код цели над объектом с невыполненным конструктором — это хуже, чем не собраться вовсе. +То же с тем, что PHP запрещает реализовывать классу, — проверяется и на самом +контракте, и на всём, что он расширяет: интерфейс, расширяющий `Throwable` +(`Psr\Http\Client\ClientExceptionInterface`, `Psr\Cache\CacheException`), +`DateTimeInterface`, `UnitEnum` или `Traversable` без `Iterator`/ +`IteratorAggregate`, а также интерфейс, объявляющий конструктор, которого дубль +никогда не выполняет. Каждый отвергается `UnsupportedTarget` с именем предка, +а не умирает в компиляторе fatal-ошибкой, которую не поймать ни одним тестом. +Дубль нельзя бросить: соберите настоящее исключение и отдайте его в `throws()`. + +Класс-дубль никогда не бывает частичным. Конкретный метод абстрактной цели +диспатчится как любой другой — его тело не выполняется, поэтому +template-метод отвечает дефолтом режима, а не вызывает застабленный +абстрактный шаг. Для настоящего кода нужен настоящий экземпляр и +`Understudy::forwarding()`; `Invocation::callOriginal()` требует того же. + ### Property hooks (PHP 8.4+) Контракт, объявляющий свойство, дублируется: @@ -268,7 +283,29 @@ when(fn () => $repository->mode())->returns('fast', 'slow'); ``` Более поздний стаб на тот же вызов выигрывает, а ранние остаются запасными — -поэтому широкий стаб с `Arg::any()` может лежать под точечным. +поэтому широкий стаб с `Arg::any()` может лежать под точечным. Именно под: +catch-all, зарегистрированный *после* точечного, забирает каждый вызов, а +точечный не диагностируется — недостижимый не значит невызванный, и даже +`strictStubs` его не покажет. + +Замыкание делает ровно один вызов на дубле. Вызов, вложенный в аргументы +другого — `when(fn () => $repo->find($repo->count()))`, — отвергается +`InvalidCallSpecification` с обоими именами: внутренний вызов был бы +застаблен молча, а внешний — не застаблен вовсе. Застабьте внутренний +отдельным `when()` и передайте литерал или матчер. Обязательный параметр надо +записать; `Arg::rest()` говорит, что остальное неважно, и широкой сигнатуре +не нужен `Arg::any()` на каждую позицию. + +`returns()` сверяется с объявленным типом возврата там, где написан: значение +на `: void`-методе (`returns(null)` разрешён — это идиома «ничего не +отвечать»), что угодно на `: never`, и значение, которое тип не вмещает — +`null` на ненуллабельном типе, массив или объект там, где объявлен скаляр, +объект не того класса, — отвергаются с меткой дубля вместо того, чтобы позже +всплыть `TypeError` с именем сгенерированного класса из глубины тестируемого +кода. Проверка не строже движка: сгенерированный метод не под `strict_types`, +поэтому `returns('5')` на `: int` по-прежнему отвечает `5`. Что произведёт +`answers()`, судить можно только на вызове, и там неверный тип остаётся +`TypeError` самого PHP. | Матчер | Что принимает | |---|---| @@ -668,7 +705,11 @@ Loose-дубль никогда не выдумывает значение, за никогда не отдаёт экземпляр настоящего класса с пропущенным конструктором. Отдать он может другой дубль: возвращаемый тип, который сам можно дублировать, становится дублем — на один уровень, и тот же тест может его настроить. Это -сгенерированный заменитель, а не цель с пропущенным конструктором. +сгенерированный заменитель, а не цель с пропущенным конструктором — и новый +на каждый вызов: `$outer->inner() === $outer->inner()` ложно. Тест, который +взял такой дубль и верифицирует по нему, видит только вызовы на этом +экземпляре; когда коллаборатор должен быть стабильным, застабьте метод через +`returns()`. Исключение — встроенные интерфейсы: метод, объявленный `: Stringable`, `: Countable`, `: JsonSerializable`, `: ArrayAccess` или `: IteratorAggregate`, @@ -822,7 +863,9 @@ The following calls to `tag` were made during this test: Звёздочками помечен разошедшийся аргумент — приём заимствован у [NSubstitute](https://nsubstitute.github.io). `Understudy::label($double, '…')` -даёт дублю имя, когда их в тесте несколько на один контракт. +даёт дублю имя, когда их в тесте несколько на один контракт; без метки второй +и последующие дубли одного контракта нумеруются — `BookRepository`, +`BookRepository#2`, — и отчёт всё равно говорит, какой именно. Объектный аргумент сравнивается по идентичности, поэтому два экземпляра не совпадут, как бы одинаково они ни читались, — и сообщение обязано показать, diff --git a/llms.txt b/llms.txt index 4ac00b8..bed30f8 100644 --- a/llms.txt +++ b/llms.txt @@ -21,7 +21,17 @@ No runtime deps; `ext-mbstring` is not needed. 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`. + 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. +- `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 + object on a scalar, an object of the wrong class) raise + `InvalidCallSpecification`/`InvalidSpecificationArgument` with the double's + label. No stricter than the engine: generated methods are not under + `strict_types`, so `returns('5')` on `: int` answers `5`. `answers()` is + judged only at the call; a wrong type there is PHP's own `TypeError`. - A double exposes nothing beyond its contract. All operations are static methods on `Understudy`, or the free functions `when()` / `expect()` / `expectSequence()` / `verify()`. @@ -34,7 +44,17 @@ No runtime deps; `ext-mbstring` is not needed. `clone` is a double of its own. - Refused before generation, each with the reason: a `final` class, a class with a non-private `final` instance method, an enum, a trait, an internal class, an - anonymous class, a class that is not the first target. + anonymous class, a class that is not the first target; and — checked on the + contract and every interface it extends — `Throwable`, `DateTimeInterface`, + `UnitEnum`/`BackedEnum`, `Traversable` without `Iterator`/`IteratorAggregate`, + plus an interface declaring `__construct`. All `UnsupportedTarget`, never a + fatal out of `eval()`. A double cannot be thrown: construct a real exception + for `throws()`. +- A class double is never partial: a concrete method of an abstract target is + dispatched, its body does not run. Real code needs a real instance and + `forwarding()`; `Invocation::callOriginal()` needs the same. A `readonly` + promoted property of a class target stays uninitialized (reading it raises + PHP's own error); a plain typed one starts at the empty value of its type. - An abstract hooked property (PHP 8.4+: an interface property, or an `abstract` one on a class) is rendered: the generated class declares it with the dispatcher inside the hook. Exactly the declared hooks; a get-only @@ -84,7 +104,9 @@ No runtime deps; `ext-mbstring` is not needed. `null` on a nullable return: `?Book` answers with the registered Book, and is `null` only when nothing was registered for it. - Without a registration, a doublable return type becomes a double of its own, - one level deep, adopted into the context that owns the outer double. + one level deep, adopted into the context that owns the outer double — a + fresh one per call (`$o->inner() !== $o->inner()`); stub the method when the + collaborator must be stable. - `Understudy::wire(Sut::class, $overrides)` returns `array{sut: object, doubles: array}`. It reads the constructor only. Object parameters become doubles; a nullable object still does; an @@ -123,6 +145,8 @@ No runtime deps; `ext-mbstring` is not needed. `checkpoint()` span EVERY context still holding doubles, not just the caller's. A test body run in a Fiber owns a context the adapter never stands in, and skipping it let an unmet `expect()` pass unnoticed. +- Without `label()`, the second and later doubles of one contract in a context + are numbered in reports: `Repo`, `Repo#2`. An explicit label outranks it. - A failure message renders an object argument as `App\Book#1 {title: 'Dune'}`. The `#N` is an alias numbered within that one message in order of first appearance — never an object id, which is reused after a collection — so the @@ -179,7 +203,9 @@ when(fn () => $repo->find(Arg::any()))->answers( Matching: most recently registered wins, earlier stubs remain as fallbacks on argument mismatch — a matching call-count expectation keeps answering after its count is exhausted. Register the broad stub FIRST and the specific one -after it. Arguments match by `===`, or by matcher. +after it: a catch-all registered after a specific stub takes every call and +the specific one is not diagnosed (unreachable ≠ uncalled, so `strictStubs` +stays quiet). Arguments match by `===`, or by matcher. ### Matchers — `Arg` @@ -218,7 +244,9 @@ the test's own code, so an exception in it travels. A matcher that could never match is refused at construction with `InvalidSpecificationArgument`: an inverted range (`int`/`float`/`count`), a pattern PCRE cannot compile, and `instanceOf()` naming a type that is not loadable. A matcher that reaches a -real call raises `MatcherLeaked`. +real call on a *double* raises `MatcherLeaked`; one handed to a real object is +out of the engine's sight and is simply a wrong argument (the PHPStan plugin's +`understudy.matcherLeak` is the textual guard). `Arg::captor(?class-string $class = null): Captor` builds a typed argument captor — the replacement for reading `args[N]` out of the call log: @@ -415,7 +443,8 @@ Understudy::forget($replaced); For a double the test built and then replaced: verification (including `strictStubs`), accounting and reset stop seeing it. Any call on it — or any -question about its calls — raises `ForgottenDouble` naming `forget()`. +question about its calls — raises `ForgottenDouble` naming `forget()`. The +same guard after a `scope()` closed names the scope; after `reset()`, the reset. One-way; a replacement is a different object. ### Modes and naming diff --git a/src/Codegen/DoubleFactory.php b/src/Codegen/DoubleFactory.php index 6e16bad..8fe0592 100644 --- a/src/Codegen/DoubleFactory.php +++ b/src/Codegen/DoubleFactory.php @@ -19,15 +19,9 @@ final class DoubleFactory /** Where every generated double's class lives, and nothing else does. */ public const string GENERATED_NAMESPACE = __NAMESPACE__ . '\\Generated\\'; - /** - * Whether this object is a double this factory made — asked by the facade - * methods, which are handed an object rather than a closure and otherwise - * cannot tell "never was a double" from "was one before a reset". - */ - public static function isGenerated(object $double): bool - { - return str_starts_with($double::class, self::GENERATED_NAMESPACE); - } + private const string ENUM_REASON = 'only an enum may implement it, and an enum cannot be doubled at all — ' + . 'its cases are the values themselves. Pass the case you need, or double an interface the enum implements.'; + /** @var array */ private static array $blueprints = []; @@ -38,8 +32,20 @@ public static function isGenerated(object $double): bool /** @var array> */ private static array $reflections = []; + + private function __construct() {} + /** + * Whether this object is a double this factory made — asked by the facade + * methods, which are handed an object rather than a closure and otherwise + * cannot tell "never was a double" from "was one before a reset". + */ + public static function isGenerated(object $double): bool + { + return str_starts_with($double::class, self::GENERATED_NAMESPACE); + } + /** * One reflection per generated class rather than one per double: building * it costs more the more members the class has, and every double of a @@ -130,7 +136,7 @@ private static function reflect(string $contract, bool $primary): \ReflectionCla $reflection = new \ReflectionClass($contract); if ($reflection->isInterface()) { - self::rejectUndoublableInterface($contract); + self::rejectUndoublableInterface($reflection, $contract); } else { self::rejectUndoublableClass($reflection, $contract, $primary); } @@ -139,7 +145,8 @@ private static function reflect(string $contract, bool $primary): \ReflectionCla } /** - * The five interfaces the language forbids a userland class to implement. + * The five interfaces the language forbids a userland class to implement, + * checked on the contract AND on everything it extends. * * Every other refusal in this file is a considered `UnsupportedTarget`; * these used to walk past all of them and be answered by the compiler @@ -149,30 +156,79 @@ private static function reflect(string $contract, bool $primary): \ReflectionCla * contracts anybody reaches for, a clock and an error, so the fatal was * not a corner. * + * The first version of this check compared the contract's own name, and + * an interface *extending* one of the five walked past it into the same + * fatal — `Psr\Http\Client\ClientExceptionInterface`, which is what a + * consumer stubs an HTTP client to throw, extends `Throwable`. The + * ancestry is what the compiler looks at, so it is what is checked here. + * * Not a property of being built in: `Iterator`, `IteratorAggregate`, - * `Stringable` and `Countable` double perfectly well. It is these five - * specifically, and each of them has a way through. + * `Stringable` and `Countable` double perfectly well — the first two + * extend `Traversable` and are exactly the way through it, so the + * `Traversable` arm asks for the presence of one of them rather than the + * absence of the ancestor. * - * @param class-string $contract + * An interface may also declare a constructor. Nothing renders one — the + * generated class must never run a constructor — so the class would stay + * abstract, which is the same uncatchable fatal one step later. + * + * @param \ReflectionClass $reflection + * @param class-string $contract */ - private static function rejectUndoublableInterface(string $contract): void + private static function rejectUndoublableInterface(\ReflectionClass $reflection, string $contract): void { - $reason = match (ltrim(strtolower($contract), '\\')) { - 'throwable' => 'PHP forbids a userland class to implement Throwable directly. Double a ' - . 'concrete exception class instead, or an interface of your own that extends none of it.', - 'unitenum', 'backedenum' => 'only an enum may implement it, and an enum cannot be doubled at all — ' - . 'its cases are the values themselves. Pass the case you need, or double an interface the ' - . 'enum implements.', - 'datetimeinterface' => 'PHP forbids a userland class to implement DateTimeInterface. Pass a real ' + $forbidden = [ + \Throwable::class => 'PHP forbids a userland class to implement Throwable. A double cannot be thrown; ' + . 'construct a real exception and hand it to throws(), or double an interface of your own that ' + . 'extends none of it.', + // BackedEnum first: it extends UnitEnum, and naming the nearer + // ancestor is what the reader wrote. + \BackedEnum::class => self::ENUM_REASON, + \UnitEnum::class => self::ENUM_REASON, + \DateTimeInterface::class => 'PHP forbids a userland class to implement DateTimeInterface. Pass a real ' . '\DateTimeImmutable, or put a clock interface of your own in front of it and double that.', - 'traversable' => 'PHP requires it to be reached through Iterator or IteratorAggregate. Double ' - . 'one of those — both work here — or an interface of yours that extends one.', - default => null, - }; + ]; + + foreach ($forbidden as $ancestor => $reason) { + if (is_a($contract, $ancestor, allow_string: true)) { + throw UnsupportedTarget::notDoublable($contract, self::viaAncestor($contract, $ancestor) . $reason); + } + } - if ($reason !== null) { - throw UnsupportedTarget::notDoublable($contract, $reason); + if ( + is_a($contract, \Traversable::class, allow_string: true) + && !is_a($contract, \Iterator::class, allow_string: true) + && !is_a($contract, \IteratorAggregate::class, allow_string: true) + ) { + throw UnsupportedTarget::notDoublable( + $contract, + self::viaAncestor($contract, \Traversable::class) + . 'PHP requires it to be reached through Iterator or IteratorAggregate. Double one of those — ' + . 'both work here — or an interface of yours that extends one.', + ); } + + if ($reflection->hasMethod('__construct')) { + throw UnsupportedTarget::notDoublable( + $contract, + 'the interface declares a constructor, and a double never runs one — the generated class would ' + . 'be left abstract. Double an interface that leaves construction to the implementation.', + ); + } + } + + /** + * Names the ancestor when the refusal comes from one the contract merely + * extends: the reader wrote `ClientExceptionInterface`, and "cannot + * implement Throwable" alone sends them looking for a word that is not in + * their file. + * + * @param class-string $contract + * @param class-string $ancestor + */ + private static function viaAncestor(string $contract, string $ancestor): string + { + return strcasecmp(ltrim($contract, '\\'), $ancestor) === 0 ? '' : sprintf('it extends %s, and ', $ancestor); } /** diff --git a/src/Exception/ForgottenDouble.php b/src/Exception/ForgottenDouble.php index 3936638..73ea46a 100644 --- a/src/Exception/ForgottenDouble.php +++ b/src/Exception/ForgottenDouble.php @@ -30,6 +30,22 @@ public static function afterReset(string $method): self )); } + /** + * The double was built inside `Understudy::scope()` and the scope has + * closed since. There is no reset() to look for. + * + * @param non-empty-string $member the method called, or the facade verb asked + */ + public static function afterScope(string $member): self + { + return new self(sprintf( + "This understudy is no longer known to Understudy, but `%s()` was called on it.\n" + . 'It was created inside a scope() that has since closed, and a scope drops its doubles when ' + . 'it ends; build the double in the scope that will use it, or outside the scope altogether.', + $member, + )); + } + /** * Builds the error for calling a deliberately retired double. * diff --git a/src/Exception/InvalidCallSpecification.php b/src/Exception/InvalidCallSpecification.php index 3646f1c..3197c2d 100644 --- a/src/Exception/InvalidCallSpecification.php +++ b/src/Exception/InvalidCallSpecification.php @@ -37,6 +37,65 @@ 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. + * + * @param non-empty-string $innerLabel + * @param non-empty-string $innerMethod + * @param non-empty-string $outerLabel + * @param non-empty-string $outerMethod + */ + public static function nestedCall(string $innerLabel, string $innerMethod, string $outerLabel, string $outerMethod): 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, + )); + } + + /** + * `returns()` on a method declared `: void`: the value would never be + * observed, and a test that relies on it is checking nothing. + * + * @param non-empty-string $label + * @param non-empty-string $method + */ + public static function returnsOnVoid(string $label, string $method): self + { + return new self(sprintf( + 'Understudy `%s`: `%s()` is declared `: void`, so returns() has nothing to answer with — the value ' + . 'would never be observed. Drop the value (returns(null) is allowed), or use answers()/throws() if the ' + . 'call should do something.', + $label, + $method, + )); + } + + /** + * `returns()` on a method declared `: never`, which cannot return at all. + * + * @param non-empty-string $label + * @param non-empty-string $method + */ + public static function returnsOnNever(string $label, string $method): self + { + return new self(sprintf( + 'Understudy `%s`: `%s()` is declared `: never` and cannot return. Configure what it throws: ' + . 'when(fn () => $double->%s(...))->throws(new SomeException())', + $label, + $method, + $method, + )); + } + /** * Handed the object directly, so there is no closure to blame — naming * one sends the reader looking for a mistake they have not made. diff --git a/src/Exception/InvalidSpecificationArgument.php b/src/Exception/InvalidSpecificationArgument.php index 7ace1b0..5fb9b63 100644 --- a/src/Exception/InvalidSpecificationArgument.php +++ b/src/Exception/InvalidSpecificationArgument.php @@ -75,6 +75,27 @@ public static function noReturnValues(): self return new self('returns() needs at least one value'); } + /** + * A `returns()` value the declared return type cannot hold. Refused where + * it was written: left to the engine it surfaced as a `TypeError` naming + * the generated class, from wherever in the code under test the call was. + * + * @param non-empty-string $label + * @param non-empty-string $method + * @param non-empty-string $declared the return type as the contract declares it + * @param non-empty-string $given the value's type, as `get_debug_type()` reports it + */ + public static function returnValueOfWrongType(string $label, string $method, string $declared, string $given): self + { + return new self(sprintf( + 'Understudy `%s`: returns() was given %s, but `%s()` is declared `: %s` and cannot answer with it.', + $label, + $given, + $method, + $declared, + )); + } + /** * `Arg::instanceOf()` naming a class or interface that is not loadable — * a matcher nothing can ever satisfy, which would otherwise report itself diff --git a/src/Expectation/ReturnContract.php b/src/Expectation/ReturnContract.php new file mode 100644 index 0000000..c3b561f --- /dev/null +++ b/src/Expectation/ReturnContract.php @@ -0,0 +1,134 @@ +acceptsType($value, $this->signature->returnType); + } + + private function acceptsType(mixed $value, string $type): bool + { + if ($type === 'mixed') { + return true; + } + + if (str_starts_with($type, '?')) { + return $value === null || $this->acceptsType($value, substr($type, 1)); + } + + if (str_contains($type, '|')) { + foreach ($this->branches($type) as $branch) { + if ($this->acceptsType($value, $branch)) { + return true; + } + } + + return false; + } + + if (str_contains($type, '&')) { + foreach (explode('&', $type) as $member) { + if (!$this->acceptsType($value, $member)) { + return false; + } + } + + return true; + } + + // A class name arrives absolute (`\App\Book`), which `class_exists()` + // and `instanceof` both read as written. + return match ($type) { + // `returns(null)` on a void method is the idiom for "answer + // nothing"; a value is refused before this table is consulted. + 'null', 'void' => $value === null, + 'never' => false, + 'int', 'float', 'bool', 'true', 'false' => is_scalar($value), + 'string' => is_scalar($value) || $value instanceof \Stringable, + 'array' => is_array($value), + 'iterable' => is_iterable($value), + 'callable' => is_callable($value), + 'object' => is_object($value), + 'self', 'static' => $value instanceof $this->selfContract, + default => !class_exists($type) && !interface_exists($type) || $value instanceof $type, + }; + } + + /** + * The top-level members of a union, with a parenthesised intersection + * kept whole. + * + * @return list + */ + private function branches(string $type): array + { + $branches = []; + $depth = 0; + $current = ''; + + foreach (str_split($type) as $character) { + if ($character === '(') { + $depth++; + } elseif ($character === ')') { + $depth--; + } + + if ($character === '|' && $depth === 0) { + $branches[] = $current; + $current = ''; + + continue; + } + + $current .= $character; + } + + $branches[] = $current; + + // A DNF member is parenthesised on the wire, `(A&B)|null`; the + // intersection rule reads it without the parentheses. + return array_map(static fn(string $branch): string => trim($branch, '()'), $branches); + } +} diff --git a/src/Runtime/DoubleState.php b/src/Runtime/DoubleState.php index 670d1d2..41f5289 100644 --- a/src/Runtime/DoubleState.php +++ b/src/Runtime/DoubleState.php @@ -41,6 +41,9 @@ final class DoubleState /** @var non-empty-string|null */ private ?string $label = null; + /** @var non-empty-string|null */ + private ?string $defaultLabel = null; + private ?object $forwardingTarget = null; /** @var array */ @@ -102,7 +105,19 @@ public function setMode(Mode $mode): void */ public function label(): string { - return $this->label ?? $this->blueprint->displayName(); + return $this->label ?? $this->defaultLabel ?? $this->blueprint->displayName(); + } + + /** + * The label a report uses when the test set none: the contract's short + * name, numbered by the owning context once it holds more than one double + * of that contract. An explicit `setLabel()` still outranks it. + * + * @param non-empty-string $label + */ + public function setDefaultLabel(string $label): void + { + $this->defaultLabel = $label; } /** diff --git a/src/Runtime/Runtime.php b/src/Runtime/Runtime.php index 2e593ae..848f299 100644 --- a/src/Runtime/Runtime.php +++ b/src/Runtime/Runtime.php @@ -56,6 +56,16 @@ final class Runtime */ private static ?\WeakMap $forgotten = null; + /** + * The forgotten doubles a closing `scope()` dropped, as opposed to a + * `reset()`. Only the failure message tells them apart: "created before a + * reset()" sends the reader looking for a reset() they never wrote when + * what ended the double was the scope it was built in. + * + * @var \WeakMap|null + */ + private static ?\WeakMap $droppedByScope = null; + /** * Doubles retired on purpose through `Understudy::forget()`. The * distinction is for the failure message only: both a reset and a @@ -85,6 +95,70 @@ 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(); @@ -126,7 +200,7 @@ public static function popScope(): void $context = array_pop(self::$main); if ($context instanceof RuntimeContext) { - self::retire($context); + self::retire($context, byScope: true); } return; @@ -138,7 +212,7 @@ public static function popScope(): void $context = array_pop($stack); if ($context instanceof RuntimeContext) { - self::retire($context); + self::retire($context, byScope: true); } $fibers->offsetSet($fiber, $stack); @@ -324,6 +398,34 @@ public static function isForgotten(object $double): bool return self::$forgotten?->offsetExists($double) ?? false; } + /** + * Whether a double was forgotten by the `scope()` it was created in + * closing, rather than by a `reset()`. + */ + public static function isDroppedByScope(object $double): bool + { + return self::$droppedByScope?->offsetExists($double) ?? false; + } + + /** + * The refusal for a call or a question that reached a forgotten double, + * worded by what forgot it: `forget()`, a closing scope, or a reset. + * + * @param non-empty-string $member + */ + public static function forgottenDouble(object $double, string $member): ForgottenDouble + { + if (self::isRetiredOnPurpose($double)) { + return ForgottenDouble::onPurpose($member); + } + + if (self::isDroppedByScope($double)) { + return ForgottenDouble::afterScope($member); + } + + return ForgottenDouble::afterReset($member); + } + /** * Whether a double was retired on purpose through `Understudy::forget()`. */ @@ -388,6 +490,20 @@ private static function owners(): \WeakMap return self::$owners; } + /** + * @return \WeakMap + */ + private static function droppedByScope(): \WeakMap + { + if (self::$droppedByScope === null) { + /** @var \WeakMap $dropped */ + $dropped = new \WeakMap(); + self::$droppedByScope = $dropped; + } + + return self::$droppedByScope; + } + /** * @return \WeakMap */ @@ -429,6 +545,10 @@ 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); + } + throw new InvocationSignal($double, $method, $args); } @@ -451,11 +571,7 @@ public static function dispatch(object $double, string $method, array $args): mi // message differs for a double retired through `Understudy::forget()`: // sending the reader looking for a stray reset() they never wrote // is worse than the plain truth. - if (self::isRetiredOnPurpose($double)) { - throw ForgottenDouble::onPurpose($method); - } - - throw ForgottenDouble::afterReset($method); + throw self::forgottenDouble($double, $method); } // A by-reference argument is live: whatever answers the call can write @@ -866,6 +982,15 @@ 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. + $slot = new ReferenceSlot(); + $slot->value = $value; + + return $slot; + } + $context = self::ownerOf($double) ?? self::current(); $state = $context->stateOf($double); @@ -1175,7 +1300,7 @@ private static function forgetOrphans(): void * live until collection and the map they sit in keeps growing. Eager * removal here is what keeps that map the size of one test. */ - private static function retire(RuntimeContext $context): void + private static function retire(RuntimeContext $context, bool $byScope = false): void { self::unremember($context); @@ -1193,6 +1318,10 @@ private static function retire(RuntimeContext $context): void foreach ($context->allDoubles() as $double) { self::forgotten()[$double] = true; unset(self::$owners[$double]); + + if ($byScope) { + self::droppedByScope()[$double] = true; + } } } } diff --git a/src/Runtime/RuntimeContext.php b/src/Runtime/RuntimeContext.php index 22e1cb0..0c707df 100644 --- a/src/Runtime/RuntimeContext.php +++ b/src/Runtime/RuntimeContext.php @@ -91,6 +91,28 @@ public function endRecording(): void public function register(object $double, DoubleState $state): void { $this->doubles[$double] = $state; + $this->numberDefaultLabel($state); + } + + /** + * Gives the second and later doubles of one contract a numbered default + * label — `Repo#2` — so a report naming one of them can be read without + * relabelling. The first keeps the bare name: with one double per contract, + * the overwhelmingly common shape, nothing changes. + * + * @var array + */ + private array $labelCounts = []; + + private function numberDefaultLabel(DoubleState $state): void + { + $name = $state->blueprint->displayName(); + $count = ($this->labelCounts[$name] ?? 0) + 1; + $this->labelCounts[$name] = $count; + + if ($count > 1) { + $state->setDefaultLabel($name . '#' . $count); + } } public function knows(object $double): bool diff --git a/src/Understudy.php b/src/Understudy.php index 80569cc..27500e5 100644 --- a/src/Understudy.php +++ b/src/Understudy.php @@ -18,6 +18,7 @@ use Rasuvaeff\Understudy\Exception\VerificationFailed; use Rasuvaeff\Understudy\Expectation\ArgumentFormatter; use Rasuvaeff\Understudy\Expectation\Expectation; +use Rasuvaeff\Understudy\Expectation\ReturnContract; use Rasuvaeff\Understudy\Runtime\ArmedSequence; use Rasuvaeff\Understudy\Runtime\DoubleState; use Rasuvaeff\Understudy\Runtime\InvocationSignal; @@ -133,7 +134,7 @@ public static function when(callable $call): WhenBuilder $state->addExpectation($expectation); - return new WhenBuilder($expectation); + return new WhenBuilder($expectation, self::returnContract($state, $signal->method)); } /** @@ -155,7 +156,22 @@ public static function expect(callable $call): ExpectBuilder $state->addExpectation($expectation); - return new ExpectBuilder($expectation); + return new ExpectBuilder($expectation, self::returnContract($state, $signal->method)); + } + + /** + * What `returns()` may be given for this call, or `null` for a method the + * blueprint does not know — which only a hand-built double can present. + * + * @param non-empty-string $method + */ + private static function returnContract(DoubleState $state, string $method): ?ReturnContract + { + $signature = $state->blueprint->method($method); + + return $signature === null + ? null + : new ReturnContract($state->label(), $method, $signature, $state->blueprint->contracts[0]); } /** @@ -1312,6 +1328,8 @@ private static function record(callable $call): InvocationSignal try { $call(); } catch (InvocationSignal $signal) { + self::rejectNestedCalls($call, $signal); + // 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 @@ -1334,6 +1352,35 @@ private static function record(callable $call): InvocationSignal 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], + ); + } + /** * What `verify()`'s named arguments mean together, or a refusal. * @@ -1391,7 +1438,7 @@ private static function stateOf(object $double, string $facade): DoubleState // same problem — and neither used to be named at all, because // both were reported as a specification closure gone wrong. throw DoubleFactory::isGenerated($double) - ? ForgottenDouble::afterReset($facade) + ? Runtime::forgottenDouble($double, $facade) : InvalidCallSpecification::notADouble($facade); } diff --git a/src/WhenBuilder.php b/src/WhenBuilder.php index c66ece2..26ccbbc 100644 --- a/src/WhenBuilder.php +++ b/src/WhenBuilder.php @@ -4,9 +4,11 @@ namespace Rasuvaeff\Understudy; +use Rasuvaeff\Understudy\Exception\InvalidCallSpecification; use Rasuvaeff\Understudy\Exception\InvalidSpecificationArgument; use Rasuvaeff\Understudy\Expectation\ComputeAnswer; use Rasuvaeff\Understudy\Expectation\Expectation; +use Rasuvaeff\Understudy\Expectation\ReturnContract; use Rasuvaeff\Understudy\Expectation\ReturnValue; use Rasuvaeff\Understudy\Expectation\ThrowComputed; use Rasuvaeff\Understudy\Expectation\ThrowError; @@ -46,7 +48,14 @@ class WhenBuilder /** * @internal */ - public function __construct(protected readonly Expectation $expectation) {} + /** + * @param ReturnContract|null $contract what the specified method may + * answer with; `null` only where no signature is known + */ + public function __construct( + protected readonly Expectation $expectation, + private readonly ?ReturnContract $contract = null, + ) {} /** * Returns each value in turn across successive calls, then keeps returning @@ -62,6 +71,31 @@ public function returns(mixed ...$values): static throw InvalidSpecificationArgument::noReturnValues(); } + if ($this->contract !== null) { + // `returns(null)` on a void method is the idiom for "answer + // nothing", and PHP itself reads a void method as answering null; + // only a value that would be silently dropped is refused. + if ($this->contract->signature->returnsVoid && $list !== array_fill(0, count($list), null)) { + throw InvalidCallSpecification::returnsOnVoid($this->contract->label, $this->contract->method); + } + + if ($this->contract->signature->returnsNever) { + throw InvalidCallSpecification::returnsOnNever($this->contract->label, $this->contract->method); + } + + /** @var mixed $value */ + foreach ($list as $value) { + if (!$this->contract->accepts($value)) { + throw InvalidSpecificationArgument::returnValueOfWrongType( + $this->contract->label, + $this->contract->method, + $this->contract->signature->returnType, + get_debug_type($value), + ); + } + } + } + // Several values ARE a chain: `returns($a, $b)` is exactly // `returns($a)->then()->returns($b)`. Keeping them as one action with // a position of its own would give the expectation two competing diff --git a/tests/ClassDoubleTest.php b/tests/ClassDoubleTest.php index e38fbcd..7702318 100644 --- a/tests/ClassDoubleTest.php +++ b/tests/ClassDoubleTest.php @@ -29,6 +29,12 @@ use Rasuvaeff\Understudy\Tests\Fixture\Cls\SealedLedger; use Rasuvaeff\Understudy\Tests\Fixture\Cls\Stamp; use Rasuvaeff\Understudy\Tests\Fixture\Suit; +use Rasuvaeff\Understudy\Tests\Fixture\Undoublable\AggregateContract; +use Rasuvaeff\Understudy\Tests\Fixture\Undoublable\ClockContract; +use Rasuvaeff\Understudy\Tests\Fixture\Undoublable\CollectionContract; +use Rasuvaeff\Understudy\Tests\Fixture\Undoublable\ConstructedContract; +use Rasuvaeff\Understudy\Tests\Fixture\Undoublable\EnumContract; +use Rasuvaeff\Understudy\Tests\Fixture\Undoublable\ThrowableContract; use Rasuvaeff\Understudy\Tests\Fixture\Unify\AbstractStaticFromInterface; use Rasuvaeff\Understudy\Tests\Fixture\Unify\StaticPingContract; use Rasuvaeff\Understudy\Tests\Fixture\Unify\StaticPingWiderParameter; @@ -260,8 +266,8 @@ public static function undoublableInterfaceProvider(): iterable { yield 'throwable' => [ \Throwable::class, - 'PHP forbids a userland class to implement Throwable directly. Double a concrete exception ' - . 'class instead, or an interface of your own that extends none of it.', + 'PHP forbids a userland class to implement Throwable. A double cannot be thrown; construct a ' + . 'real exception and hand it to throws(), or double an interface of your own that extends none of it.', ]; yield 'unit enum' => [ \UnitEnum::class, @@ -292,11 +298,79 @@ public static function undoublableInterfaceProvider(): iterable */ public function aLeadingBackslashDoesNotHideTheRefusal(): void { - Expect::exception(UnsupportedTarget::class)->withMessageContaining('Double a concrete exception class'); + // The full message: the leading backslash must not read as "extends + // Throwable" either — it is the same name, not an ancestor. + Expect::exception(UnsupportedTarget::class)->withMessage( + 'Cannot create an understudy for `\\Throwable`: PHP forbids a userland class to implement Throwable. ' + . 'A double cannot be thrown; construct a real exception and hand it to throws(), or double an interface ' + . 'of your own that extends none of it.', + ); Understudy::for('\\Throwable'); } + /** + * The compiler looks at the whole ancestry, so the guard has to as well. + * The first version compared the contract's own name, and an interface + * *extending* one of the five — `ClientExceptionInterface extends + * Throwable` is the everyday case — walked past it into the same + * uncatchable fatal. An interface declaring a constructor is a fourth way + * to the fatal: nothing renders one, so the generated class stays abstract. + * + * @param class-string $contract + */ + #[DataProvider('undoublableByAncestryProvider')] + public function anInterfaceExtendingAForbiddenOneIsRefusedBeforeEval(string $contract, string $reason): void + { + Expect::exception(UnsupportedTarget::class) + ->withMessage(sprintf('Cannot create an understudy for `%s`: %s', $contract, $reason)); + + Understudy::for($contract); + } + + public static function undoublableByAncestryProvider(): iterable + { + yield 'extends Throwable' => [ + ThrowableContract::class, + 'it extends Throwable, and PHP forbids a userland class to implement Throwable. A double cannot be ' + . 'thrown; construct a real exception and hand it to throws(), or double an interface of your own ' + . 'that extends none of it.', + ]; + yield 'extends DateTimeInterface' => [ + ClockContract::class, + 'it extends DateTimeInterface, and PHP forbids a userland class to implement DateTimeInterface. Pass ' + . 'a real \\DateTimeImmutable, or put a clock interface of your own in front of it and double that.', + ]; + yield 'extends Traversable without Iterator' => [ + CollectionContract::class, + 'it extends Traversable, and PHP requires it to be reached through Iterator or IteratorAggregate. ' + . 'Double one of those — both work here — or an interface of yours that extends one.', + ]; + yield 'extends BackedEnum' => [ + EnumContract::class, + 'it extends BackedEnum, and only an enum may implement it, and an enum cannot be doubled at all — its ' + . 'cases are the values themselves. Pass the case you need, or double an interface the enum implements.', + ]; + yield 'declares a constructor' => [ + ConstructedContract::class, + 'the interface declares a constructor, and a double never runs one — the generated class would be ' + . 'left abstract. Double an interface that leaves construction to the implementation.', + ]; + } + + /** + * The way through Traversable is an interface that extends Iterator or + * IteratorAggregate — which also extends Traversable, and a guard that + * only asked about the ancestor would refuse the very thing it recommends. + */ + public function anInterfaceExtendingIteratorAggregateStillDoubles(): void + { + $double = Understudy::for(AggregateContract::class); + + Assert::instanceOf($double, AggregateContract::class); + Assert::instanceOf($double, \Traversable::class); + } + /** * The neighbours: being built in is not the reason those five are * refused, and a rule written that way would take these with it. diff --git a/tests/Fixture/Ret/PlainRepository.php b/tests/Fixture/Ret/PlainRepository.php new file mode 100644 index 0000000..a7e5139 --- /dev/null +++ b/tests/Fixture/Ret/PlainRepository.php @@ -0,0 +1,59 @@ +withMessage( "This understudy is no longer known to Understudy, but `count()` was called on it.\n" - . 'It was created before a reset(); create doubles inside the test that uses them ' - . 'rather than sharing one across tests.', + . 'It was created inside a scope() that has since closed, and a scope drops its doubles when ' + . 'it ends; build the double in the scope that will use it, or outside the scope altogether.', ); $escaped->count(); diff --git a/tests/ReturnContractTest.php b/tests/ReturnContractTest.php new file mode 100644 index 0000000..5daa2df --- /dev/null +++ b/tests/ReturnContractTest.php @@ -0,0 +1,118 @@ +contract($type)->accepts($value), $accepted); + } + + public static function verdictProvider(): iterable + { + $book = new Book('x'); + $stringable = new Stringy(); + $repository = new PlainRepository(); + + yield 'mixed takes anything' => ['mixed', $book, true]; + yield 'mixed takes null' => ['mixed', null, true]; + yield 'nullable takes null' => ['?int', null, true]; + yield 'nullable takes the type' => ['?int', 5, true]; + yield 'nullable refuses the wrong type' => ['?int', [], false]; + yield 'int takes int' => ['int', 5, true]; + yield 'int takes a numeric string (engine coerces)' => ['int', '5', true]; + yield 'int takes bool (engine coerces)' => ['int', true, true]; + yield 'int refuses null' => ['int', null, false]; + yield 'int refuses array' => ['int', [], false]; + yield 'int refuses object' => ['int', $book, false]; + yield 'float takes int' => ['float', 1, true]; + yield 'float refuses array' => ['float', [1], false]; + yield 'bool takes string' => ['bool', 'x', true]; + yield 'bool refuses object' => ['bool', $book, false]; + yield 'true takes scalar' => ['true', 1, true]; + yield 'true refuses null' => ['true', null, false]; + yield 'false takes scalar' => ['false', 0, true]; + yield 'false refuses array' => ['false', [], false]; + yield 'string takes string' => ['string', 'x', true]; + yield 'string takes int' => ['string', 1, true]; + yield 'string takes Stringable' => ['string', $stringable, true]; + yield 'string refuses a plain object' => ['string', $book, false]; + yield 'string refuses null' => ['string', null, false]; + yield 'array takes array' => ['array', [], true]; + yield 'array refuses object' => ['array', $book, false]; + yield 'array refuses string' => ['array', 'x', false]; + yield 'iterable takes array' => ['iterable', [], true]; + yield 'iterable takes Traversable' => ['iterable', new \ArrayIterator([]), true]; + yield 'iterable refuses string' => ['iterable', 'x', false]; + yield 'callable takes closure' => ['callable', static fn(): int => 1, true]; + yield 'callable refuses int' => ['callable', 1, false]; + yield 'object takes object' => ['object', $book, true]; + yield 'object refuses array' => ['object', [], false]; + yield 'null takes null' => ['null', null, true]; + yield 'null refuses int' => ['null', 0, false]; + yield 'void takes null' => ['void', null, true]; + yield 'void refuses a value' => ['void', 1, false]; + yield 'never refuses everything' => ['never', null, false]; + yield 'class takes instance' => ['\\' . Book::class, $book, true]; + yield 'class refuses another class' => ['\\' . Book::class, $stringable, false]; + yield 'class refuses scalar' => ['\\' . Book::class, 'x', false]; + yield 'interface takes implementation' => ['\\' . BookRepository::class, $repository, true]; + yield 'self takes the contract' => ['self', $repository, true]; + yield 'self refuses another object' => ['self', $book, false]; + yield 'static takes the contract' => ['static', $repository, true]; + yield 'static refuses another object' => ['static', $book, false]; + yield 'union takes either branch' => ['int|string', 'x', true]; + yield 'union takes null branch' => ['int|null', null, true]; + yield 'union refuses what no branch takes' => ['int|string', [], false]; + yield 'union with class branch' => ['\\' . Book::class . '|null', $book, true]; + yield 'intersection takes both' => ['\\' . BookRepository::class . '&\\' . Librarian::class, new RepositoryAndLibrarian(), true]; + yield 'intersection refuses one of two' => ['\\' . BookRepository::class . '&\\' . Librarian::class, $repository, false]; + yield 'dnf: parenthesised intersection branch' => ['(\\' . BookRepository::class . '&\\' . Librarian::class . ')|null', null, true]; + yield 'dnf: refuses one half of the intersection' => ['(\\' . BookRepository::class . '&\\' . Librarian::class . ')|null', $repository, false]; + yield 'unknown type accepts anything' => ['\\Nope\\Missing', 'x', true]; + yield 'unknown type accepts null' => ['\\Nope\\Missing', null, true]; + } + + private function contract(string $type): ReturnContract + { + return new ReturnContract( + 'BookRepository', + 'method', + new MethodSignature( + name: 'method', + parameters: '', + arguments: '[]', + returnType: $type, + returnsNever: $type === 'never', + returnsVoid: $type === 'void', + returnsReference: false, + ), + BookRepository::class, + ); + } +} diff --git a/tests/UnderstudyTest.php b/tests/UnderstudyTest.php index cc7ad3b..5686fc9 100644 --- a/tests/UnderstudyTest.php +++ b/tests/UnderstudyTest.php @@ -19,6 +19,8 @@ use Rasuvaeff\Understudy\Exception\UnsupportedTarget; use Rasuvaeff\Understudy\Exception\VerificationFailed; use Rasuvaeff\Understudy\Expectation\Expectation; +use Rasuvaeff\Understudy\Expectation\ReturnContract; +use Rasuvaeff\Understudy\ExpectBuilder; use Rasuvaeff\Understudy\FailureReport; use Rasuvaeff\Understudy\Invocation; use Rasuvaeff\Understudy\Outcome; @@ -32,6 +34,7 @@ use Rasuvaeff\Understudy\Tests\Fixture\Clock; use Rasuvaeff\Understudy\Tests\Fixture\HashedContract; use Rasuvaeff\Understudy\Tests\Fixture\HashedContractToo; +use Rasuvaeff\Understudy\Tests\Fixture\Librarian; use Rasuvaeff\Understudy\Tests\Fixture\Named; use Rasuvaeff\Understudy\Tests\Fixture\Unify\IntersectedPair; use Rasuvaeff\Understudy\Tests\Fixture\Unify\IntersectionAlpha; @@ -48,9 +51,11 @@ use Rasuvaeff\Understudy\Tests\Fixture\VariadicSink; use Rasuvaeff\Understudy\Tests\Support\GoldenMessage; use Rasuvaeff\Understudy\Understudy; +use Rasuvaeff\Understudy\WhenBuilder; use Testo\Assert; use Testo\Assert\ExpectNoAssertions; use Testo\Codecov\Covers; +use Testo\Data\DataProvider; use Testo\Expect; use Testo\Lifecycle\AfterTest; use Testo\Test; @@ -61,6 +66,9 @@ #[Test] #[Covers(Understudy::class)] +#[Covers(WhenBuilder::class)] +#[Covers(ExpectBuilder::class)] +#[Covers(ReturnContract::class)] #[Covers(Runtime::class)] #[Covers(RuntimeContext::class)] #[Covers(DoubleState::class)] @@ -581,6 +589,94 @@ public function failureNamesTheContractWhenNoLabelWasSet(): void Understudy::verify(fn() => $repository->count()); } + /** + * The arguments of `find()` are evaluated before `find()` is dispatched, + * so the recording used to see `count()` alone: the closure was abandoned + * on that first signal, `count()` was stubbed with the `returns()` meant + * for `find()`, and `find()` was never specified at all. + */ + 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()))', + ); + + Understudy::when(fn() => $repository->find($repository->count())); + } + + /** + * Across two doubles the inner call belonged to the other one: the stub + * landed on the wrong object with a value its return type could not hold. + */ + public function aNestedCallOnAnotherDoubleIsRejectedByBothNames(): void + { + $repository = Understudy::label(Understudy::for(BookRepository::class), 'outer'); + $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`'); + + 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. + */ + public function theNestedCallProbeLeavesNoTraceAndToleratesNever(): void + { + $repository = Understudy::for(BookRepository::class); + + Understudy::when(fn() => $repository->abort('x'))->throws(new \RuntimeException('aborted')); + Understudy::when(fn() => $repository->find(1))->returns(new Book('one')); + + Assert::same(count(Understudy::calls(fn() => $repository->find(Arg::any()))), 0); + Assert::same(count(Understudy::calls(fn() => $repository->count())), 0); + Assert::same($repository->find(1)?->title, 'one'); + Expect::exception(\RuntimeException::class)->withMessage('aborted'); + $repository->abort('x'); + } + + /** + * 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 + * bare name — one double per contract is the common shape and nothing + * changes for it — the second and later are numbered, and an explicit + * label still outranks the number. + */ + public function laterDoublesOfTheSameContractGetNumberedDefaultLabels(): void + { + $first = Understudy::for(BookRepository::class); + $second = Understudy::for(BookRepository::class); + $third = Understudy::label(Understudy::for(BookRepository::class), 'archive'); + $other = Understudy::for(Librarian::class); + + expect(fn() => $first->count()); + expect(fn() => $second->count()); + expect(fn() => $third->count()); + expect(fn() => $other->pick()); + + try { + Understudy::verifyAll(); + Assert::true(actual: false, message: 'Expected VerificationFailed'); + } catch (VerificationFailed $failure) { + Assert::string($failure->getMessage()) + ->contains('Understudy `BookRepository` expected `count()`') + ->contains('Understudy `BookRepository#2` expected `count()`') + ->contains('Understudy `archive` expected `count()`') + ->contains('Understudy `Librarian` expected `pick()`'); + } + } + public function specificationClosureWithoutACallIsRejected(): void { Expect::exception(InvalidCallSpecification::class)->withMessage( @@ -702,10 +798,12 @@ public function aByReferenceMethodDispatchesWithoutANotice(): void public function aNeverMethodConfiguredToReturnIsRejected(): void { // Returning from a `: never` method is a TypeError by language rule; - // the message has to name the real mistake instead. + // the message has to name the real mistake instead. `returns()` is + // refused where it is written; `answers()` can only be judged by what + // it produces, at the call. $repository = Understudy::for(BookRepository::class); - when(fn() => $repository->abort('stop'))->returns('nope'); + when(fn() => $repository->abort('stop'))->answers(static fn(): string => 'nope'); Expect::exception(NeverMethodCalled::class)->withMessage( "Understudy `BookRepository` has `abort()` configured to return, but the method is declared `: never` and cannot.\n" @@ -715,6 +813,88 @@ public function aNeverMethodConfiguredToReturnIsRejected(): void $repository->abort('stop'); } + /** + * The signature is known when `returns()` is written, so a value the + * declared type cannot hold is refused there — with the label — rather + * than surfacing later as a `TypeError` naming the generated class from + * inside the code under test. No stricter than the engine: a generated + * method is not under `strict_types`, so `returns('5')` on `: int` stays + * the coercion it always was, and `returns(null)` on `: void` stays the + * idiom for "answer nothing". + */ + #[DataProvider('refusedReturnValueProvider')] + public function aReturnValueTheDeclaredTypeCannotHoldIsRefusedAtRegistration(callable $specify, string $exception, string $message): void + { + $repository = Understudy::for(BookRepository::class); + + Expect::exception($exception)->withMessage($message); + + $specify($repository); + } + + public static function refusedReturnValueProvider(): iterable + { + yield 'value on void' => [ + static fn(BookRepository $r) => when(fn() => $r->save(Arg::any()))->returns(true), + InvalidCallSpecification::class, + 'Understudy `BookRepository`: `save()` is declared `: void`, so returns() has nothing to answer with — ' + . 'the value would never be observed. Drop the value (returns(null) is allowed), or use answers()/throws() ' + . 'if the call should do something.', + ]; + yield 'value on never' => [ + static fn(BookRepository $r) => when(fn() => $r->abort('x'))->returns('nope'), + InvalidCallSpecification::class, + 'Understudy `BookRepository`: `abort()` is declared `: never` and cannot return. Configure what it ' + . 'throws: when(fn () => $double->abort(...))->throws(new SomeException())', + ]; + yield 'string on nullable object' => [ + static fn(BookRepository $r) => when(fn() => $r->find(1))->returns('not a book'), + InvalidSpecificationArgument::class, + 'Understudy `BookRepository`: returns() was given string, but `find()` is declared `: ?\\' + . Book::class . '` and cannot answer with it.', + ]; + yield 'null on int' => [ + static fn(BookRepository $r) => when(fn() => $r->count())->returns(null), + InvalidSpecificationArgument::class, + 'Understudy `BookRepository`: returns() was given null, but `count()` is declared `: int` and cannot ' + . 'answer with it.', + ]; + yield 'object on array' => [ + static fn(BookRepository $r) => when(fn() => $r->titles())->returns(new Book('x')), + InvalidSpecificationArgument::class, + 'Understudy `BookRepository`: returns() was given ' . Book::class . ', but `titles()` is declared ' + . '`: array` and cannot answer with it.', + ]; + yield 'second value of a chain' => [ + static fn(BookRepository $r) => when(fn() => $r->count())->returns(1, []), + InvalidSpecificationArgument::class, + 'Understudy `BookRepository`: returns() was given array, but `count()` is declared `: int` and cannot ' + . 'answer with it.', + ]; + } + + #[DataProvider('acceptedReturnValueProvider')] + public function aReturnValueTheEngineWouldCoerceOrAcceptIsNotRefused(callable $specify): void + { + $repository = Understudy::for(BookRepository::class); + + $specify($repository); + + Assert::true(actual: true); + } + + public static function acceptedReturnValueProvider(): iterable + { + yield 'null on void' => [static fn(BookRepository $r) => when(fn() => $r->save(Arg::any()))->returns(null)]; + yield 'numeric string on int' => [static fn(BookRepository $r) => when(fn() => $r->count())->returns('5')]; + yield 'bool on int' => [static fn(BookRepository $r) => when(fn() => $r->count())->returns(true)]; + yield 'null on nullable object' => [static fn(BookRepository $r) => when(fn() => $r->find(1))->returns(null)]; + yield 'object on nullable object' => [static fn(BookRepository $r) => when(fn() => $r->find(1))->returns(new Book('x'))]; + yield 'generator on Generator' => [static fn(BookRepository $r) => when(fn() => $r->stream())->returns((static function (): \Generator { + yield 1; + })())]; + } + public function aDoubleUsedAfterResetSaysSo(): void { // Answering with null would violate the declared return type and @@ -1079,6 +1259,34 @@ public function configuringAForgottenDoubleIsRejected(): void Understudy::label($repository, 'catalogue'); } + /** + * A scope drops its doubles when it closes; the message used to blame a + * `reset()` the test never wrote. Both the call and the facade verb say + * which it was. + */ + public function aDoubleFromAClosedScopeSaysSo(): void + { + $repository = Understudy::scope(static fn(): BookRepository => Understudy::for(BookRepository::class)); + + try { + $repository->count(); + Assert::true(actual: false, message: 'Expected ForgottenDouble'); + } catch (ForgottenDouble $failure) { + Assert::same( + $failure->getMessage(), + "This understudy is no longer known to Understudy, but `count()` was called on it.\n" + . 'It was created inside a scope() that has since closed, and a scope drops its doubles when ' + . 'it ends; build the double in the scope that will use it, or outside the scope altogether.', + ); + } + + Expect::exception(ForgottenDouble::class) + ->withMessageContaining('`label()` was called on it') + ->withMessageContaining('created inside a scope() that has since closed'); + + Understudy::label($repository, 'catalogue'); + } + public function anExpectationWithoutAnActionStopsTheSearch(): void { // The expectation matched first and has no action of its own, so the diff --git a/tests/VerificationFailureTest.php b/tests/VerificationFailureTest.php index a6e489b..5cea1cf 100644 --- a/tests/VerificationFailureTest.php +++ b/tests/VerificationFailureTest.php @@ -112,7 +112,7 @@ public function nothingElseReportsUnaccountedCalls(): void { $repository = Understudy::for(BookRepository::class); $book = new Book('title'); - when(fn() => $repository->save(Arg::any()))->returns(true); + when(fn() => $repository->save(Arg::any()))->returns(null); $repository->save($book); $record = $this->firstFailureOf(fn() => Understudy::nothingElse($repository)); From 391d5c83e25ad80fdc2420b3819b3335bb83d61a Mon Sep 17 00:00:00 2001 From: "v.razuvaev" Date: Fri, 18 Sep 2026 02:19:02 +0300 Subject: [PATCH 2/3] Describe the new refusal parameters; read fixtures the way the newer rector spells it --- src/Exception/ForgottenDouble.php | 2 +- src/Exception/InvalidCallSpecification.php | 16 +++++------ .../InvalidSpecificationArgument.php | 4 +-- tests/Bypass/FileWrapperTest.php | 28 +++++++++---------- .../BypassFinalsIntegrationTest.php | 4 +-- 5 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/Exception/ForgottenDouble.php b/src/Exception/ForgottenDouble.php index 73ea46a..ce811af 100644 --- a/src/Exception/ForgottenDouble.php +++ b/src/Exception/ForgottenDouble.php @@ -34,7 +34,7 @@ public static function afterReset(string $method): self * The double was built inside `Understudy::scope()` and the scope has * closed since. There is no reset() to look for. * - * @param non-empty-string $member the method called, or the facade verb asked + * @param non-empty-string $member the method called on the double, or the facade verb it was handed to */ public static function afterScope(string $member): self { diff --git a/src/Exception/InvalidCallSpecification.php b/src/Exception/InvalidCallSpecification.php index 3197c2d..d22ceff 100644 --- a/src/Exception/InvalidCallSpecification.php +++ b/src/Exception/InvalidCallSpecification.php @@ -42,10 +42,10 @@ public static function noCallRecorded(): self * another. The inner call is what the recording saw; the outer call is * what the test meant, and neither reading is safe to guess. * - * @param non-empty-string $innerLabel - * @param non-empty-string $innerMethod - * @param non-empty-string $outerLabel - * @param non-empty-string $outerMethod + * @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 */ public static function nestedCall(string $innerLabel, string $innerMethod, string $outerLabel, string $outerMethod): self { @@ -65,8 +65,8 @@ public static function nestedCall(string $innerLabel, string $innerMethod, strin * `returns()` on a method declared `: void`: the value would never be * observed, and a test that relies on it is checking nothing. * - * @param non-empty-string $label - * @param non-empty-string $method + * @param non-empty-string $label the double, as a refusal names it + * @param non-empty-string $method the method declared `: void` */ public static function returnsOnVoid(string $label, string $method): self { @@ -82,8 +82,8 @@ public static function returnsOnVoid(string $label, string $method): self /** * `returns()` on a method declared `: never`, which cannot return at all. * - * @param non-empty-string $label - * @param non-empty-string $method + * @param non-empty-string $label the double, as a refusal names it + * @param non-empty-string $method the method declared `: never` */ public static function returnsOnNever(string $label, string $method): self { diff --git a/src/Exception/InvalidSpecificationArgument.php b/src/Exception/InvalidSpecificationArgument.php index 5fb9b63..8c97bc6 100644 --- a/src/Exception/InvalidSpecificationArgument.php +++ b/src/Exception/InvalidSpecificationArgument.php @@ -80,8 +80,8 @@ public static function noReturnValues(): self * it was written: left to the engine it surfaced as a `TypeError` naming * the generated class, from wherever in the code under test the call was. * - * @param non-empty-string $label - * @param non-empty-string $method + * @param non-empty-string $label the double, as a refusal names it + * @param non-empty-string $method the method whose return type refused the value * @param non-empty-string $declared the return type as the contract declares it * @param non-empty-string $given the value's type, as `get_debug_type()` reports it */ diff --git a/tests/Bypass/FileWrapperTest.php b/tests/Bypass/FileWrapperTest.php index 5785d79..8827829 100644 --- a/tests/Bypass/FileWrapperTest.php +++ b/tests/Bypass/FileWrapperTest.php @@ -71,7 +71,7 @@ public function installingTwiceWidensOneAllowList(): void FileWrapper::install([['namespace' => $namespace, 'class' => 'OpenedNeighbour']]); FileWrapper::install([['namespace' => $namespace, 'class' => 'SealedNeighbour']]); - $read = $this->read(\dirname(__DIR__) . '/Fixture/Bypass/Neighbours.php'); + $read = $this->read(__DIR__ . '/../Fixture/Bypass/Neighbours.php'); // By declaration, not by the phrase: the file's own docblock says // "final classes", and a substring assertion would read that as one. @@ -90,7 +90,7 @@ public function theGlobalModeIsNotNarrowedByALaterTargetedCall(): void FileWrapper::install(null); FileWrapper::install([['namespace' => 'Nowhere', 'class' => 'Nothing']]); - $read = $this->read(\dirname(__DIR__) . '/Fixture/Bypass/Neighbours.php'); + $read = $this->read(__DIR__ . '/../Fixture/Bypass/Neighbours.php'); Assert::false(str_contains($read, 'final class OpenedNeighbour')); Assert::false(str_contains($read, 'final class SealedNeighbour')); @@ -105,7 +105,7 @@ public function aTargetedListIsWidenedIntoTheGlobalMode(): void FileWrapper::install([['namespace' => 'Nowhere', 'class' => 'Nothing']]); FileWrapper::install(null); - $read = $this->read(\dirname(__DIR__) . '/Fixture/Bypass/Neighbours.php'); + $read = $this->read(__DIR__ . '/../Fixture/Bypass/Neighbours.php'); Assert::false(str_contains($read, 'final class OpenedNeighbour')); Assert::false(str_contains($read, 'final class SealedNeighbour')); @@ -124,7 +124,7 @@ public function aPhpSourceIsHandedOverTransformed(): void { FileWrapper::targetOnly([['namespace' => 'Rasuvaeff\\Understudy\\Tests\\Fixture\\Bypass', 'class' => 'SealedGate']]); - $read = $this->read(\dirname(__DIR__) . '/Fixture/Bypass/SealedGate.php'); + $read = $this->read(__DIR__ . '/../Fixture/Bypass/SealedGate.php'); Assert::string($read)->contains('class SealedGate'); Assert::false(str_contains($read, 'final class SealedGate')); @@ -134,7 +134,7 @@ public function aPhpSourceNobodyAskedAboutIsHandedOverAsItIs(): void { FileWrapper::targetOnly([['namespace' => 'Other', 'class' => 'Gate']]); - $read = $this->read(\dirname(__DIR__) . '/Fixture/Bypass/SealedGate.php'); + $read = $this->read(__DIR__ . '/../Fixture/Bypass/SealedGate.php'); Assert::string($read)->contains('final class SealedGate'); } @@ -170,7 +170,7 @@ public function oneInstallCanNameSeveralClasses(): void ['namespace' => $namespace, 'class' => 'SealedNeighbour'], ]); - $read = $this->read(\dirname(__DIR__) . '/Fixture/Bypass/Neighbours.php'); + $read = $this->read(__DIR__ . '/../Fixture/Bypass/Neighbours.php'); Assert::false(str_contains($read, 'final class OpenedNeighbour')); Assert::false(str_contains($read, 'final class SealedNeighbour')); @@ -183,7 +183,7 @@ public function oneInstallCanNameSeveralClasses(): void */ public function aRegisteredWrapperTransformsWhatPhpItselfReads(): void { - $path = \dirname(__DIR__) . '/Fixture/Bypass/SealedGate.php'; + $path = __DIR__ . '/../Fixture/Bypass/SealedGate.php'; Assert::string((string) file_get_contents($path))->contains('final class SealedGate'); @@ -205,7 +205,7 @@ public function aRegisteredWrapperTransformsWhatPhpItselfReads(): void */ public function theWrapperIsStillInPlaceAfterADelegatedOperation(): void { - $path = \dirname(__DIR__) . '/Fixture/Bypass/SealedGate.php'; + $path = __DIR__ . '/../Fixture/Bypass/SealedGate.php'; FileWrapper::install([[ 'namespace' => 'Rasuvaeff\\Understudy\\Tests\\Fixture\\Bypass', @@ -223,7 +223,7 @@ public function anUppercaseExtensionIsStillPhp(): void { FileWrapper::targetOnly(null); - $read = $this->read(\dirname(__DIR__) . '/Fixture/Bypass/Uppercase.PHP'); + $read = $this->read(__DIR__ . '/../Fixture/Bypass/Uppercase.PHP'); Assert::false(str_contains($read, 'final class Uppercase')); Assert::string($read)->contains('class Uppercase'); @@ -237,7 +237,7 @@ public function aReadWriteHandleIsNotTransformed(): void { FileWrapper::targetOnly(null); - Assert::string($this->read(\dirname(__DIR__) . '/Fixture/Bypass/SealedGate.php', 'r+')) + Assert::string($this->read(__DIR__ . '/../Fixture/Bypass/SealedGate.php', 'r+')) ->contains('final class SealedGate'); } @@ -273,7 +273,7 @@ public function aWriteHandleIsNotTransformed(): void */ public function readingThroughAnUninstalledWrapperDoesNotInstallIt(): void { - $path = \dirname(__DIR__) . '/Fixture/Bypass/SealedGate.php'; + $path = __DIR__ . '/../Fixture/Bypass/SealedGate.php'; FileWrapper::targetOnly(null); $this->read($path); @@ -289,7 +289,7 @@ public function theStreamReportsPositionSizeAndEnd(): void FileWrapper::targetOnly([]); $wrapper = new FileWrapper(); $opened = null; - $path = \dirname(__DIR__) . '/Fixture/Bypass/SealedGate.php'; + $path = __DIR__ . '/../Fixture/Bypass/SealedGate.php'; Assert::true($wrapper->stream_open($path, 'r', STREAM_USE_PATH, $opened)); Assert::same($opened, $path); @@ -361,7 +361,7 @@ public function lockedFilePutContentsStillWrites(): void public function statAnswersForTheRealPath(): void { $wrapper = new FileWrapper(); - $path = \dirname(__DIR__) . '/Fixture/Bypass/SealedGate.php'; + $path = __DIR__ . '/../Fixture/Bypass/SealedGate.php'; $stat = $wrapper->url_stat($path, 0); @@ -377,7 +377,7 @@ public function statAnswersForTheRealPath(): void public function statFollowsOrDoesNotFollowALinkAsAsked(): void { $wrapper = new FileWrapper(); - $target = \dirname(__DIR__) . '/Fixture/Bypass/SealedGate.php'; + $target = __DIR__ . '/../Fixture/Bypass/SealedGate.php'; $link = sys_get_temp_dir() . '/understudy-link-' . getmypid() . '.php'; if (file_exists($link)) { diff --git a/tests/Integration/BypassFinalsIntegrationTest.php b/tests/Integration/BypassFinalsIntegrationTest.php index 7fa0a7e..4b98830 100644 --- a/tests/Integration/BypassFinalsIntegrationTest.php +++ b/tests/Integration/BypassFinalsIntegrationTest.php @@ -91,7 +91,7 @@ public function aPreloadedClassIsAlreadyLoaded(): void { $ini = [ 'opcache.enable_cli=1', - 'opcache.preload=' . dirname(__DIR__) . '/Fixture/Bypass/preload.php', + 'opcache.preload=' . __DIR__ . '/../Fixture/Bypass/preload.php', ]; if (\DIRECTORY_SEPARATOR === '\\') { @@ -195,7 +195,7 @@ private function execute(string $scenario, array $ini): array '%s %s%s %s 2>&1', escapeshellarg(PHP_BINARY), $flags, - escapeshellarg(dirname(__DIR__) . '/Fixture/Bypass/scenario.php'), + escapeshellarg(__DIR__ . '/../Fixture/Bypass/scenario.php'), escapeshellarg($scenario), ); From 627780100e190e8c234a984a74f7d12faccf1f20 Mon Sep 17 00:00:00 2001 From: "v.razuvaev" Date: Fri, 18 Sep 2026 02:27:15 +0300 Subject: [PATCH 3/3] Pin both adapter readings of a swallowed override in the phpunit smoke leg; one more fixture path for the newer rector --- bin/consumer-smoke | 25 +++++++++++++++++++------ tests/Bypass/FileWrapperTest.php | 2 +- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/bin/consumer-smoke b/bin/consumer-smoke index 910c0b1..d453cb9 100755 --- a/bin/consumer-smoke +++ b/bin/consumer-smoke @@ -1394,7 +1394,7 @@ PHP check_local phpunit phpunit || return 1 local out out="$(run phpunit './vendor/bin/phpunit --no-progress 2>&1' || true)" - if printf '%s' "$out" | grep -q 'Tests: 4' && printf '%s' "$out" | grep -q 'Failures: 2' \ + if printf '%s' "$out" | grep -q 'Tests: 4' && printf '%s' "$out" | grep -qE 'Failures: [23]' \ && printf '%s' "$out" | grep -q 'open(2)'; then printf ' ok the trait turns an unmet expectation into a runner failure\n' else @@ -1404,13 +1404,26 @@ PHP fi # `open(40)` is the class that composed explicitly: its unmet expectation - # is one of the two failures. `open(30)` is the class that overrode - # without composing: it must NOT appear, because nothing reported it — - # which is the hazard, stated as a fact rather than as a warning. + # is one of the failures. `open(30)` is the class that overrode without + # composing: its expectation is never verified, so `open(30)` must NOT + # appear. What the adapter does about that depends on its version — since + # understudy-phpunit 0.5 the `#[After]` hook notices the swallowed + # verification and fails the test with the aliasing recipe (the third + # failure); before it, the hazard was silent, and both readings are pinned + # here because a published adapter behind that release still installs. if printf '%s' "$out" | grep -q 'open(40)' && ! printf '%s' "$out" | grep -q 'open(30)'; then - printf ' ok the documented alias recipe verifies, and a bare override loses it silently\n' + if printf '%s' "$out" | grep -q 'Failures: 3' \ + && printf '%s' "$out" | grep -q 'SilentOverrideTest' \ + && printf '%s' "$out" | grep -q 'expectations were never verified'; then + printf ' ok the documented alias recipe verifies, and a bare override is reported as unverified\n' - return 0 + return 0 + fi + if printf '%s' "$out" | grep -q 'Failures: 2'; then + printf ' ok the documented alias recipe verifies, and a bare override loses it silently (adapter predates the guard)\n' + + return 0 + fi fi printf ' FAIL the override composition did not behave as documented:\n%s\n' "$out" diff --git a/tests/Bypass/FileWrapperTest.php b/tests/Bypass/FileWrapperTest.php index 8827829..fb0a07f 100644 --- a/tests/Bypass/FileWrapperTest.php +++ b/tests/Bypass/FileWrapperTest.php @@ -413,7 +413,7 @@ public function statFollowsOrDoesNotFollowALinkAsAsked(): void public function directoriesAreListedThroughTheNativeWrapper(): void { $wrapper = new FileWrapper(); - $directory = \dirname(__DIR__) . '/Fixture/Bypass'; + $directory = __DIR__ . '/../Fixture/Bypass'; Assert::true($wrapper->dir_opendir($directory));