Report variable writes whose value never reaches a use, per array offset - #6334
Open
ondrejmirtes wants to merge 14 commits into
Open
Report variable writes whose value never reaches a use, per array offset#6334ondrejmirtes wants to merge 14 commits into
ondrejmirtes wants to merge 14 commits into
Conversation
Dead stores found by the upcoming unused-variable check: null initialisations overwritten on every path, unused foreach values and destructured items, an assignment from the never-returning fail(), and catch variables that are never read. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CbZPnVnRJDLbnqmtD3sYSy
Every source-level write of a local variable (plain and compound assignment, inc/dec, array-offset write on an array or string, list() item, foreach value/key, catch variable) becomes a VariableWrite with a VariableWrittenExpr marker in the scope. A new write of the same variable kills the earlier markers explicitly; merges keep them as Maybe, so at any point the markers say which writes still reach it. The one source-level read of a variable (VariableHandler) records the reaching writes as read in an immutable per-function-like VariableWritesFrame held by NodeScopeResolver; compact(), get_defined_vars(), extract(), eval and include read everything, goto makes the frame opaque, and by-ref parameters/uses, global, static and reference aliases are untracked. The frame is emitted as a VariableWritesNode after each ReturnStatementsNode. generalizeWith() now carries markers planted only by the newer loop pass, and createConditionalExpressions() no longer records certainty-No conditionals for virtual nodes (a later narrowing could otherwise erase a marker). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CbZPnVnRJDLbnqmtD3sYSy
…e never read Level 4, behind the unusedVariable bleeding-edge toggle. A write is reported when no path from it reaches a read of the written value - including writes overwritten before being read and writes on only one branch. Catch variables are reported only where non-capturing catches exist; $_-prefixed names are exempt. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CbZPnVnRJDLbnqmtD3sYSy
A nullsafe call's plain twin re-enters the already-walked receiver in consume-stored mode; its arguments are walked there for the first time, so their variable reads are genuine and must not be treated like on-demand synthetic pricing. Found on slevomat: every variable read only inside the arguments of $x?->m(...) was reported as never read. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CbZPnVnRJDLbnqmtD3sYSy
Reads are recorded on walk scopes, never on a promoted one, so the marker is only needed in the phpDoc-typed map; keeping it out of the native map halves its share of every merge, generalization, equality check and invalidation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CbZPnVnRJDLbnqmtD3sYSy
So that CI and the downstream dogfooding projects surface false positives before the rule ships behind bleeding edge only. Revert before merging. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CbZPnVnRJDLbnqmtD3sYSy
Closes phpstan/phpstan#12789 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CbZPnVnRJDLbnqmtD3sYSy
Closes phpstan/phpstan#12012 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CbZPnVnRJDLbnqmtD3sYSy
Closes phpstan/phpstan#11483 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CbZPnVnRJDLbnqmtD3sYSy
Closes phpstan/phpstan#10202 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CbZPnVnRJDLbnqmtD3sYSy
ondrejmirtes
force-pushed
the
unused-variables
branch
from
September 7, 2026 15:49
2441298 to
805b8ab
Compare
ondrejmirtes
force-pushed
the
unused-variables-followups
branch
from
September 7, 2026 15:58
890334d to
6041802
Compare
simple-downgrader cannot downgrade named arguments that skip an optional parameter, so the write:/supersededMarkerExprs: assignVariable() call sites and the writeSiteKind: processVirtualAssign() call sites left named arguments in the PHP 7.4 build and its lint failed to parse them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CbZPnVnRJDLbnqmtD3sYSy
The rule already skips KIND_CATCH writes when PhpVersion::supportsNoncapturingCatches() is false - a catch clause cannot drop the variable before PHP 8.0, so there is nothing actionable to report. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CbZPnVnRJDLbnqmtD3sYSy
A read no longer counts as a use by itself: a write is used iff its value reaches a sink (a call argument, a condition, a return, echo, throw, a property write...) directly or through the writes it is computed into. The right side of an assignment, a compound assignment and the operand of an increment are walked with the write as the ExpressionContext's value-flow target; pure combinators (arithmetic, concat, casts, unary ops, ternary and match arms, literal arrays, interpolated strings, ??'s right side) keep it, everything else drops it (enterDeep()/withoutValueFlow()). VariableHandler then records a dependency edge instead of a read, VariableWritesNode computes the used set as a fixpoint over the edges and the rule reports `$b = $b + 1` chains, `$s .= ...` chains and `$i++` runs that never reach a sink, as Psalm does. A nested assignment or compound assignment copies its dependencies to the enclosing target. Array offsets are tracked as their own writes of the variable: a literal assigned straight to a variable registers one write per constant offset (items of `$a = ['x' => 1, 'y' => 2]`, up to TRACKED_LITERAL_ITEMS_LIMIT), `$a['k'] = ...` is an offset write that kills only the earlier writes of offset 'k', `$a['k']['j'] = ...` extends offset 'k' (reads it, kills nothing). The receiver of an offset access is read as a container (its whole-variable writes only), `$a['k']` reads offset 'k' plus the writes of unknown offsets, a dynamic offset reads everything, `$a['k'] OP=` and `$a['k'] ??=` read the offset first. unset() discards the reaching writes of a variable or a constant offset without reading them (MutatingScope:: withoutVariableWriteMarkers()), so `$x = 1; unset($x);` is reported too. Messages: "Value assigned to variable $x is never used.", "Value assigned to $a['k'] is never used." and "Offset 'k' of array assigned to variable $a is never used." (an item is reported on its own only when the array as a whole is used). The stronger rule found two more genuine bugs in fixtures: bug-10847's loop appends to the iterated $overloads instead of $processedOverloads, and array-destructuring-array-dim-fetch builds $barcodes for nothing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CbZPnVnRJDLbnqmtD3sYSy
ondrejmirtes
force-pushed
the
unused-variables-followups
branch
from
September 7, 2026 16:33
6041802 to
8e260fa
Compare
ondrejmirtes
force-pushed
the
unused-variables
branch
2 times, most recently
from
September 8, 2026 09:30
2152a04 to
88d0188
Compare
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.
Follow-up to #6330: the two refinements listed there, plus the
unset()false negative.Value flow (Psalm's "better unused variable detection")
A read is no longer a use by itself. A write is used iff its value reaches a sink — a call argument, a condition, a return, echo, throw, a property write… — directly or through the writes it is computed into. So
$a = 5; $a = $a + 1;,$s .= …chains,$i++runs and Psalm's article example are reported at every write, while$a = 5; $a = $a + 1; sink($a);stays quiet.ExpressionContextcarries a value-flow target.AssignHandler,AssignOpHandlerand the inc/dec handlers register the write before walking its value (registerWriteSite()); pure combinators (arithmetic/concat/comparisons, casts, unary ops, ternary and match arms, literal arrays, interpolated strings,??'s right side) keep the target viaenterDeepKeepingValueFlow(), everything else drops it —enterDeep()for nested sub-expressions,withoutValueFlow()for same-depth sinks (&&/||right operands, piped calls, closure uses, assignment-target sub-walks).VariableHandler::composeResult()records a dependency edge in theVariableWritesFrameinstead of a read when a target is set;VariableWritesNoderesolves the used set as a fixpoint over the edges.$a = $b = $c + 1and$a = ($b OP= …)copy the inner write's dependencies to the outer target;$a = $b = 1; sink($a);still reports$b(its variable is never read).Array offsets — literals and
$a['k'] = …TRACKED_LITERAL_ITEMS_LIMIT = 32). Their markers are planted together with the whole write's.$a['k'] = …is an offset write that kills only the earlier writes of offset'k';$a['k']['j'] = …extends offset'k'(reads it, kills nothing);$a[$i] = …/$a[] = …are unknown-offset writes.$a['k']reads offset'k'plus unknown-offset writes; a dynamic offset reads everything;$a['k'] OP=and$a['k'] ??=read the offset first.unset($x)/unset($a['k'])discard the reaching writes without reading them (MutatingScope::withoutVariableWriteMarkers()), so$x = 1; unset($x);is reported too.Messages:
Value assigned to variable $x is never used.,Value assigned to $a['k'] is never used.,Offset 'k' of array assigned to variable $a is never used.— an item is reported on its own only when the array as a whole is used, otherwise the assignment is.Verification
unused-variable-value-flow.phpandunused-variable-offsets.phpare new); the #12012 case now reports exactly the lines the reporter asked for.make phpstanclean (it caught a real false positive on the way —$cache[$k] ??= …inScopeOps, sincePreparedAssignTarget::isAssignOp()is false for coalesce mode; fixed and covered),make testsgreen turbo-off and turbo-on, cs/lint clean.$overloadsinstead of$processedOverloads, andarray-destructuring-array-dim-fetchbuilds$barcodesfor nothing.foreachfilling$tags,$headerTablefeeding an already-dead offset write, four literal offsets always overwritten or never read).src/Typewas too noisy to resolve a difference (−0.4 %, t ≈ 0); no large regression.The levels expectation JSONs (
tests/PHPStan/Levels/data/*-4.json) are not part of this PR — the levels config includes bleeding edge, so both branches need them once the base PR settles.🤖 Generated with Claude Code
https://claude.ai/code/session_01CbZPnVnRJDLbnqmtD3sYSy