Repository navigation
feat(#26): report fan-out to distinct callees as a fan-out terminal - #28
Merged
Merged
Conversation
… of fan-out Two existing tests happened to forward one parameter to two distinct callees on one execution path — the exact shape #26's fan-out rule will turn into a terminal. Left as-is they would silently stop pinning what they were written for once that rule lands. - same-callee-two-callsites: Divergent::forwardTwice now calls Terminal::first twice (same callee, two call sites) instead of two different Terminal methods, matching what the fixture's name and the #17 commit that introduced it actually describe. - ChainBuilderTest::testBranchToTwoCalleesYieldsTwoFindings is renamed to testForwardsInExclusiveBranchesYieldOneFindingPerBranch and now puts the two forwards in mutually exclusive if/else arms, so it keeps pinning "two findings from one origin" via branches that can't both execute, rather than via unconditional fan-out.
…fate A method that forwards a parameter to two or more distinct callees on one execution path does not pass the value on — it combines what those callees return. That is consumption, not a hop, so it gets its own ParamFate::FanOut instead of being counted as another pass-through link in a chain. Forwards that reach distinct callees only in mutually exclusive branch arms stay PureForward: a dispatcher picks one callee per run and really does hand the value on untouched. BranchArmPath records, per forward, which arm of each enclosing if/elseif/else, match, switch and ternary it sits in; two forwards are exclusive when some branch names them in different arms. A branch condition is not an arm, and `??`, loops and statements after a dispatcher join every arm, so those all fan out. This amends frozen rule 1 (every occurrence a whole-argument forward => PureForward): whole-argument forwards to distinct callees on one path are now a terminal use. Classifier only — the chain traversal already treats any non-PureForward fate as a terminal, so a fan-out parameter renders as `used` until its own terminal kind and the documentation follow.
… cache format BranchArmPath labelled the whole ElseIf_, MatchArm and Case_ node as an arm of its enclosing branch, so a forward inside `elseif (check($p))`, a match arm's condition or a `case a($p):` expression looked exclusive with the other arms and the parameter was misclassified as a pure forward. A condition is evaluated on the way into the arms that follow it, so it can never be exclusive with any of them — exactly as the `if`, `match`, `switch` and ternary conditions were already treated. Arm labels are now recorded at the arm node, keyed by the enclosing branching node, and only for the arm's body; a condition records nothing and therefore shares the path with every arm. That is also the conservative direction: a shared path can only yield fewer findings. FileIndexCache::FORMAT goes 2 -> 3 because an entry written by an earlier 0.2.0-dev build reports a fan-out parameter as PureForward, and the cache promises drift is a miss, never a stale result. Also: correct the false rationale in BranchArmPathTest (the detector compares every ordered pair, so the test stands on its own as a symmetry pin), rename FanOutDetector::combineDistinctCallees to the predicate name areDistinctOnOnePath, and replace its thrice-repeated array shape with the ForwardOccurrence value object.
…not a hop A parameter that a method forwards to two or more distinct callees on one execution path is already classified as ParamFate::FanOut, but the chain traversal still rendered such a terminal as `used`. It now gets its own TerminalKind::FanOut, printed `(fan-out)` and selectable via `--exclude-terminal fan-out`, so a team can tell "the value arrived at a method that reads it through accessors" apart from an ordinary use. The issue's example — BackupReader::toDto dispatching to SubscriptionLine::fromLine, which builds its DTO from LineField::string/int/date — stops reporting a 3-hop chain through the accessors and reports the one hop that is really there: toDto -> fromLine (fan-out). Both boundaries the issue draws are pinned by fixtures: a `match` dispatcher that forwards to one factory per arm stays a hop (match-dispatcher-is-a-hop), and the same callee twice is still a pure forward (same-callee-two-callsites). Amends frozen rule 1 (documented in docs/plan.md), which now also records the two deliberate simplifications of the syntactic arm test: `switch` fallthrough counts as exclusive arms, and an early-return dispatcher counts as fan-out.
…p FanOut in the smoke index Three gaps the fan-out work left uncovered. `parent::` forwards already count as distinct callees, but nothing pinned it. Rule 4 exempts `parent::` from the hop and class counts because it hands the value to the same object; that exemption deliberately does not extend to fan-out, since the base-class chain is re-reported from the base method as its own origin. Two cases now hold the ruling in place. Infection reported an escaped `continue` -> `break` mutant in ChainTraversal::walk(): with only one forward per method in the suite, stopping after an internal-function use looked identical to skipping it. A hop whose two exclusive arms forward into `count()` and into a user method distinguishes them — the second finding disappears under `break`. The classifier-smoke fixture pinned every fate token in `--dump-index` except FanOut. A fan-out method in its source, with both forwards spelled out, closes that. Its findings output is unchanged: a FanOut node is not a pure-forward node, so it starts no chain and adds no incoming edge.
… fan-out rule The fan-out amendment named the shapes it was designed against; review turned up four more the frozen text did not settle, each now written down with its direction of error. `parent::` forwards count as distinct callees — the reader who knows rule 4 exempts `parent::` from the hop and class counts would otherwise assume the exemption carries over. Callee identity is a `CalleeRef` triple, so it errs in both directions and the record now says where. `self::a($p); Foo::a($p);` over-counts (two callees, one method); two property-fetch receivers under-count, because both hint `raw` and collapse into one callee. The second is the direction that yields more findings than truth, so it is worth naming that it is bounded: the resolver truncates at a `raw` receiver anyway. Every guard-clause dispatcher is fan-out, not just the `if/return` the text already showed — `instanceof` ladders and `try`/`catch` land the same way. The `switch` fallthrough sentence lacked its direction; it is the one simplification that yields more findings than truth. Also adds the missing `FanOut` -> `fan-out` row to the Phase 4 terminal mapping, and disambiguates rule 6's "fan-out count", which means implementations behind a truncated call and predates the terminal kind entirely.
…th match Simplification pass over the fan-out change: - BranchArmPath labelled `elseif`/`else`/match-arm/case arms by their position among siblings, which needed an enclosing-branch lookup and a sibling scan per arm, copied four times with one word changed. Labels are only ever compared between arms of one branch, so the arm node's own object id is a label that needs no scan — one helper replaces four. - ChainTraversal::terminalKind() had grown into three sequential guards with a silent catch-all; an exhaustive match on ParamFate makes the next fate a compile-time omission instead of a silently `used` chain. - ForwardOccurrence wraps the ForwardSite the collector already records instead of re-holding its CalleeRef. - UsageClassifier returns one tuple for the two forward-only fates. - Finding's docblock points at TerminalKind::keepsTerminalNode() instead of restating its list; docs/plan.md stops overloading "fan-out count" for the interface implementation count and records `$this->go($p); $c->go($p);` next to the `self::`/`Foo::` identity note. No behaviour change; composer check green, diff-scoped MSI 99%.
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.
Closes #26
What & why
A parameter forwarded to two or more distinct callees on a single
execution path does not pass the value on — it combines what those callees
return. That is consumption, not a hop. This branch gives that shape its own
ParamFate::FanOutandTerminalKind::FanOut, so a fan-out terminal rendersas
(fan-out)(selectable via--exclude-terminal fan-out) instead of beingmis-reported as a multi-hop chain through accessors.
The issue's example —
BackupReader::toDtodispatching toSubscriptionLine::fromLine, which builds its DTO fromLineField::string/int/date— stops reporting a 3-hop chain through theaccessors and reports the one hop that is really there:
toDto -> fromLine (fan-out).Forwards that reach distinct callees only in mutually exclusive branch arms
stay
PureForward: a dispatcher picks one callee per run and really does handthe value on untouched.
BranchArmPathrecords, per forward, which arm of eachenclosing
if/elseif/else,match,switchand ternary it sits in; twoforwards are exclusive only when some branch names them in different arms. A
branch condition shares the path with every arm, so a forward there fans out.
Amends frozen rule 1 in docs/plan.md, and records the edges
review surfaced:
parent::forwards count as distinct callees; callee identityis a
CalleeReftriple (soself::a();Foo::a()over-counts, tworawreceivers under-count but are bounded by resolver truncation); every
guard-clause dispatcher (
instanceofladders,try/catch, early return) isfan-out;
switchfallthrough counts as exclusive arms. BumpsFileIndexCache::FORMAT2 → 3 since older builds cached a fan-out parameter asPureForward.Checklist
composer checkis green (cs + stan + md + test) — 564 testscomposer infection:diffis green (100% MSI, 112 mutants killed)tests/fixtures/develop(nevermain)