Fix the 2026-09-18 review findings: fatal-free refusals, nested calls, returns() checks - #139
Merged
Merged
Conversation
…, returns() checks - 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.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…e leg; one more fixture path for the newer rector
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
One PR for the whole wave of the 2026-09-18 family review: the two class A defects, the class B ones the engine could answer, and the divergences that are written down rather than changed.
Throwable,DateTimeInterface,UnitEnum/BackedEnumorTraversable(withoutIterator/IteratorAggregate), and an interface declaring__construct, are refused withUnsupportedTargetbeforeeval(). The guard compared the contract's own name, soPsr\Http\Client\ClientExceptionInterfacewalked past it and died in the compiler as a fatal no test, adapter ortrycan catch.Iterator/IteratorAggregatekeep doubling; the Throwable advice no longer points at doubling an exception class (refused too).when(fn () => $r->find($r->count()))is refused naming both calls. Recording ends on the first dispatch — the innermost call — socount()was stubbed silently withfind()'sreturns(). The closure is re-run once in a probe mode that answers every call with the mode's default out of a throwaway context; any throwable ends the probe as inconclusive (a: nevermethod, a return type without a default, code after the call that does not survive a default) and the specification stands as before. Reference-returning methods get a throwaway slot in probe mode so the per-double slot is untouched.returns()is checked against the declared return type where it is written: a value on: void(returns(null)stays allowed — it is the idiom for "answer nothing" and the package's own tests use it), anything on: never, and a value the type cannot hold (nullon a non-nullable type, array/object on a scalar, wrong class) raiseInvalidCallSpecification/InvalidSpecificationArgumentnaming the double. No stricter than the engine: generated methods are not understrict_types, soreturns('5')on: intstill answers5.answers()is judged only at the call.WhenBuilder::__construct()gains an optional trailing parameter.Repo,Repo#2; the first keeps the bare name,label()still wins.ForgottenDoubleafter ascope()closed names the scope instead of areset()the test never wrote.readonlypromoted properties stay uninitialized;MatcherLeakedis raised only by a double;Arg::rest()is the way to stop spelling a wide signature.Checks:
composer build(1107 tests),composer rector,git diff --check;make mutation— 3415 mutants (was 3244;ReturnContractand the builders joined the#[Covers]map), Covered Code MSI 93 % at gate 92, seven new escapes classified as equivalent inAGENTS.md.