Skip to content

Resolve WithoutImpurePoints purity transitively through calls to other effect-free callables#5798

Merged
ondrejmirtes merged 1 commit into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-ps9mj59
Jun 3, 2026
Merged

Resolve WithoutImpurePoints purity transitively through calls to other effect-free callables#5798
ondrejmirtes merged 1 commit into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-ps9mj59

Conversation

@phpstan-bot
Copy link
Copy Markdown
Collaborator

Summary

The CallTo*StatementWithoutImpurePointsRule family reports standalone statements whose
only purpose is calling a callable that has no side effects (e.g. foo(); on its own line
where foo() just returns 1 + 1). Previously this only worked for callables that contain
zero 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 now
reported when doBar() only performs other possibly-pure calls, without doBar() needing
an explicit @pure annotation.

Changes

  • New src/Rules/DeadCode/PossiblyPureCallTransitivePurityResolver.php — a shared
    #[AutowiredService] that:
    • resolveDependencies(): turns a declaration's impure points and throw points into the
      set of callable keys it depends on, or null when it can never be effect-free (a
      non-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 and
      computes the transitive closure of effect-free declarations with a fixpoint (memoized
      per CollectedDataNode).
    • resolveCall(): maps a FuncCall / MethodCall / NullsafeMethodCall / StaticCall
      / New_ to the callable key(s) it targets, reusing the same finality guard as
      PossiblyPureMethodCallCollector so overridable methods don't look effect-free.
  • ConstructorWithoutImpurePointsCollector, FunctionWithoutImpurePointsCollector,
    MethodWithoutImpurePointsCollector
    — drop the "zero impure points / zero throw
    points" requirement and instead record each declaration's dependency keys via the
    resolver.
  • CallToConstructorStatementWithoutImpurePointsRule,
    CallToFunctionStatementWithoutImpurePointsRule,
    CallToMethodStatementWithoutImpurePointsRule,
    CallToStaticMethodStatementWithoutImpurePointsRule
    — filter collected declarations
    through getPureCallableKeys() before the existing join with the PossiblyPure*Collector
    call sites.
  • The constructor and function rules used foreach ($node->get(...) as [$x]), which only
    read the first declaration per file. Replaced with proper nested loops, which matters now
    that multiple declarations per file are collected.
  • tests/PHPStan/Analyser/AnalyserIntegrationTest::testBug13982 updated: the now-correct
    Call 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 / new impure point plus (with implicitThrows) 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
WithoutSideEffects rule family and the noop-statement detection in NodeScopeResolver.

Test

  • CallToFunctionStatementWithoutImpurePointsRuleTest::testTransitive — function calling a
    function calling a base pure function (depth-2 transitivity), plus a transitively-impure
    function that stays unreported.
  • CallToMethodStatementWithoutImpurePointsRuleTest::testTransitive — instance method that
    only calls another pure method and a pure function (cross-kind).
  • CallToStaticMethodStatementWithoutImpurePointsRuleTest::testRule — the existing data
    file's self:: / parent:: / static:: pure forwarders are now reported (lines 22–24).
  • CallToConstructorStatementWithoutImpurePointsRuleTest::testTransitive — constructor that
    only calls a pure function and a pure static method.
  • All four rule tests' getCollectors() now register all three WithoutImpurePoints
    collectors so cross-kind transitivity is exercised the way it is in production at level 4.
  • Each new test was confirmed to fail when the resolver is reverted to the old
    "zero impure points" semantics.

Fixes phpstan/phpstan#14759

…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
@ondrejmirtes ondrejmirtes merged commit e634e59 into phpstan:2.2.x Jun 3, 2026
662 of 670 checks passed
@ondrejmirtes ondrejmirtes deleted the create-pull-request/patch-ps9mj59 branch June 3, 2026 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

These collectors could work transitively

2 participants