Close the pre-1.0 wave: what a specification means, and what a refusal says - #129
Merged
Conversation
Nine defects found by migrating the monorepo onto the library and by
probing the engine against its own documentation.
**A specification is read the way the contract reads a call.** An optional
parameter a specification did not spell used to arrive as the contract's
default value, so arity became an implicit part of every specification:
`claimReady(any(), any(), any())` did not match `claimReady($t, 3, [], 100)`,
and the report said "never called" beside a call whose only difference was a
position the author never wrote. Every generated parameter now defaults to
the arity sentinel, optional ones included, and what an omission means
follows the contract: a parameter it declares optional may be left out by
any caller, so a specification that leaves it out says nothing about it; a
required one is present in every real call, so stopping before it still has
to be said with `Arg::rest()`. Dispatch materializes the declared default
for a real call, which keeps `tag('alpha')` and `tag('alpha', 1)` the same
call in the log. `Arg::rest()` therefore works where the remaining
parameters are optional — the docs called it "declared parameters left
unspelled" and the engine refused it. (#123, #125)
**`mixed $v = null` was a fatal error.** It rendered as `mixed|null`, which
PHP refuses at compile time — uncatchable, out of eval(), for a signature
that is neither exotic nor rare.
**A captor inside a combinator matched and recorded nothing.** Refused at
construction now, in `allOf()`, `anyOf()`, `not()` and `containing()`. (#124)
**A matcher inside an array argument matched nothing** and said nothing
about it; it is refused, and `Arg::containing()` reads matchers in its
entries so there is something to be refused *towards*.
**Understudy's own refusals are no longer rewrapped** as "the specification
closure threw before it reached an understudy", which buried the sentence
that says what to change.
**`strict()` and `label()` answer with the double**, so the mode can be
chosen where the double is handed over. (#126)
**`throwsWith()` and `Invocation::arg()`**: an exception built from the call
it answers, and a call read by the contract's parameter names. (#127)
**A protocol step due on another double says so** — two doubles under one
protocol rendered every step by its call alone, so `count()` arriving on the
wrong one read as the step that was due.
Fixes #123, fixes #124, fixes #125, fixes #126, fixes #127.
README, README.ru and llms.txt in one pass: what an unspelled optional parameter means, that a matcher inside an array argument is refused and `Arg::containing()` reads matchers instead, that a captor may not sit in a combinator, `throwsWith()`, `Invocation::arg()`, and the two facade methods that now answer with the double.
The guide pages that describe matching, capturing and modes; the API reference regenerated from the changed docblocks; and a description for every public parameter the wave added, so the completeness budget stays where it is.
`examples/basic-usage.php` runs the unspelled optional parameter, `arg()` and `throwsWith()`, so the additions are executed by `composer build` rather than only described. Rector's changes read as what they are: two locally called statics that became instance methods, one first-class callable, and an assert Psalm no longer needs.
The diff scored 89.5% against a 92 gate, which is the wave diluting a number the package holds to. Three of the escaped mutants were the tests' fault and are now killed: a materialized default whose value happened to equal the position it sits at (`note()` defaults to 7 so one cannot stand in for the other), a position no target declares a default for, and the depth cap on the search for a buried matcher — asserted from both sides, since a cap nobody tests is a number that drifts. `asSpecification()` asked a possibly-missing signature three times through `?->`; it reads the optional positions once instead, which is both simpler and three fewer ways to be defensive about the same thing. Full run: 93% covered MSI, gate 92.
The doubles cookbook the monorepo migration kept re-deriving — the request-capturing client, the strict never-called dependency, the counting spy that is `calls()` rather than four captors, the fault-injecting decorator over a real implementation, and per-object behaviour — plus the boundary that was as useful as the recipes: what is NOT a double's job. It goes in the skill file, which is where an agent reads before writing a test. AGENTS.md says what the sentinel default means now, why `func_num_args()` cannot stand in for it, and that dispatch is the one place `getDefaultValue()` may be called. Fixes #128.
roave reports two breaks — `strict()` and `label()` return the double now — and the heading is what says they are meant: a minor above v0.9.0 while the package is below 1.0 is the compatibility boundary build.yml looks for.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: Warning Your free Security trial is over. An organization admin can upgrade to Advanced for continuous pull request security review or dismiss this notice. 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 |
`combinator` is what `allOf()`/`anyOf()`/`not()` are called throughout the code and the messages; the prose linter had never met it.
`referenceSlot()` asks `hasActionFor()` which expectation will answer, and it asked with the omitted arguments still sentinels. A specification spelling the contract's default therefore answered "nothing configured", so the slot kept what the test had written through the reference instead of being replaced by the configured value — the call answered correctly and the next read did not. Found probing the wave's own change: dispatch materializes defaults, and this is the one other entry point that reads arguments before it.
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.
Nine defects, found by migrating the monorepo onto the library and by probing
the engine against its own documentation. Two of them change what a
specification means, so this is
0.10.0rather than the tag after0.9.0.A specification is read the way the contract reads a call
An optional parameter a specification did not spell used to arrive as the
contract's default value, so arity became an implicit part of every
specification:
claimReady(any(), any(), any())did not matchclaimReady($now, 3, [], 100), and the report saidnever calledbeside acall whose only difference was a position the author never wrote.
Every generated parameter now defaults to the arity sentinel, optional ones
included, and what an omission means follows the contract:
specification that leaves it out says nothing about it and matches whatever
was passed there — rendered as
…in a failure message, which is what tellsit apart from an
any()the test did write;before it still has to be said with
Arg::rest().A real call is unchanged: dispatch materializes the declared default, so
tag('alpha')andtag('alpha', 1)stay one call in the log. Same root causefixed
Arg::rest()where the remaining parameters are optional (#125 — thedocs promised a behaviour the engine refused), and a fatal error:
mixed $v = nullrendered asmixed|null, which PHP refuses at compile time —uncatchable, out of
eval(), for a signature that is neither exotic nor rare.Refusals for what used to be silent
allOf()/anyOf()/not()/containing()matched andrecorded nothing (capture() inside allOf()/anyOf() matches but silently does not record #124);
about it — and
Arg::containing()now reads matchers in its own entries,nested, so there is something to be refused towards;
closure threw before it reached an understudy";
expectSequence()rendered every step by its call alone.Additions
Understudy::strict()andlabel()answer with the double they configured(Understudy::strict() and label() should return the double #126) — the two BC findings roave reports, and what the version heading
declares;
WhenBuilder::throwsWith()builds the exception from the call it answers,one per call (First-class throwing an exception built from the call arguments #127);
Invocation::arg()reads an argument by position or by the contract's ownparameter name (First-class throwing an exception built from the call arguments #127).
Documentation
README (both languages),
llms.txt, the guide pages for matching, capturingand modes, the regenerated API reference, and the five doubles the monorepo
migration kept re-deriving — written into the skill file with the boundary
that was as useful as the recipes: what is not a double's job (#128).
Verification
composer build,composer rectorandmake release-checkgreen (roavereports the two intentional returns); 1026 tests; full mutation run at 93%
covered MSI against the 92 gate, and the wave's own diff carries three new
tests written to kill mutants it had left alive.
Fixes #123, fixes #124, fixes #125, fixes #126, fixes #127, fixes #128.