Resolve WithoutImpurePoints purity transitively through calls to other effect-free callables#5798
Merged
ondrejmirtes merged 1 commit intoJun 3, 2026
Conversation
…her effect-free callables - Add PossiblyPureCallTransitivePurityResolver: a shared service that resolves a declaration's impure/throw points into the callable keys it depends on, and computes the transitive closure of effect-free declarations via a fixpoint (base case = no dependencies). Implicit throw points are ignored, mirroring how noop expression statements are detected in NodeScopeResolver; explicit throw points and non-call impure points still mark a declaration as having effects. - ConstructorWithoutImpurePointsCollector, FunctionWithoutImpurePointsCollector and MethodWithoutImpurePointsCollector no longer require zero impure/throw points; they now record the declaration's dependency keys (the possibly-pure callables it calls) so purity can be inferred transitively. - CallToConstructor/Function/Method/StaticMethodStatementWithoutImpurePointsRule now filter collected declarations through the transitive pure set before reporting, so a standalone call to a callable whose body only performs other possibly-pure calls is reported as having no effect (e.g. `Foo::doBar();`). - Fix a latent bug in the constructor and function rules where per-file collector data was destructured with `as [$x]`, only reading the first declaration per file; replaced with proper nested loops now that more declarations are collected per file. - Method-call dependencies reuse the same finality guard as PossiblyPureMethodCallCollector so overridable methods do not make a caller look effect-free. - Update AnalyserIntegrationTest::testBug13982: the anonymous-class `$v->test2()` call is now correctly reported, since test2() only calls the pure private static test(). Closes phpstan/phpstan#14759
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.
Summary
The
CallTo*StatementWithoutImpurePointsRulefamily reports standalone statements whoseonly purpose is calling a callable that has no side effects (e.g.
foo();on its own linewhere
foo()just returns1 + 1). Previously this only worked for callables that containzero impure points directly. A callable whose body only calls other possibly-pure
callables was never reported, because each of those inner calls registers an impure point
(and an implicit throw point) on the enclosing callable.
This change makes the detection transitive: a declaration is effect-free when every
callable it depends on is itself effect-free. So a standalone
Foo::doBar();is nowreported when
doBar()only performs other possibly-pure calls, withoutdoBar()needingan explicit
@pureannotation.Changes
src/Rules/DeadCode/PossiblyPureCallTransitivePurityResolver.php— a shared#[AutowiredService]that:resolveDependencies(): turns a declaration's impure points and throw points into theset of callable keys it depends on, or
nullwhen it can never be effect-free (anon-call impure point, or an explicit throw point). Implicit throw points are ignored,
matching how noop expression statements are detected in
NodeScopeResolver.getPureCallableKeys(): builds the dependency graph from all three collectors andcomputes the transitive closure of effect-free declarations with a fixpoint (memoized
per
CollectedDataNode).resolveCall(): maps aFuncCall/MethodCall/NullsafeMethodCall/StaticCall/
New_to the callable key(s) it targets, reusing the same finality guard asPossiblyPureMethodCallCollectorso overridable methods don't look effect-free.ConstructorWithoutImpurePointsCollector,FunctionWithoutImpurePointsCollector,MethodWithoutImpurePointsCollector— drop the "zero impure points / zero throwpoints" requirement and instead record each declaration's dependency keys via the
resolver.
CallToConstructorStatementWithoutImpurePointsRule,CallToFunctionStatementWithoutImpurePointsRule,CallToMethodStatementWithoutImpurePointsRule,CallToStaticMethodStatementWithoutImpurePointsRule— filter collected declarationsthrough
getPureCallableKeys()before the existing join with thePossiblyPure*Collectorcall sites.
foreach ($node->get(...) as [$x]), which onlyread the first declaration per file. Replaced with proper nested loops, which matters now
that multiple declarations per file are collected.
tests/PHPStan/Analyser/AnalyserIntegrationTest::testBug13982updated: the now-correctCall to method ...::test2() ... has no effect.finding is asserted.Root cause
Inside a callable body, every call to a not-explicitly-pure callable creates a
functionCall/
methodCall/newimpure point plus (withimplicitThrows) an implicit throw point.The old collectors rejected any declaration with a non-zero impure-point or throw-point
count, so purity could never propagate across call boundaries. The pattern affected the
entire family on the callable axis (functions ↔ methods ↔ static methods ↔
constructors), so the fix is applied uniformly across all four collectors/rules through one
shared resolver. Implicit throw points are treated as non-effects, consistent with the
WithoutSideEffectsrule family and the noop-statement detection inNodeScopeResolver.Test
CallToFunctionStatementWithoutImpurePointsRuleTest::testTransitive— function calling afunction calling a base pure function (depth-2 transitivity), plus a transitively-impure
function that stays unreported.
CallToMethodStatementWithoutImpurePointsRuleTest::testTransitive— instance method thatonly calls another pure method and a pure function (cross-kind).
CallToStaticMethodStatementWithoutImpurePointsRuleTest::testRule— the existing datafile's
self::/parent::/static::pure forwarders are now reported (lines 22–24).CallToConstructorStatementWithoutImpurePointsRuleTest::testTransitive— constructor thatonly calls a pure function and a pure static method.
getCollectors()now register all threeWithoutImpurePointscollectors so cross-kind transitivity is exercised the way it is in production at level 4.
"zero impure points" semantics.
Fixes phpstan/phpstan#14759