Skip to content

Report functions and methods that unconditionally call themselves on every code path#5889

Open
phpstan-bot wants to merge 1 commit into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-zu9qo8f
Open

Report functions and methods that unconditionally call themselves on every code path#5889
phpstan-bot wants to merge 1 commit into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-zu9qo8f

Conversation

@phpstan-bot

Copy link
Copy Markdown
Collaborator

Summary

A function or method whose body unconditionally calls itself — the classic
public function getWorld(): string { return $this->getWorld(); } produced by an
autocompletion mistake — compiles and lints cleanly but blows the stack at
runtime. This adds analysis that reports such functions and methods when a
self-call is guaranteed to be reached on every code path, which always leads to
infinite recursion.

The check is deliberately conservative, matching the issue's own observation
that these mistakes have "no branching for the return statement": as soon as the
analysis sees any branching construct (an if, switch, loop, try, match
arm, a ternary/??/&&/|| short-circuit, or a self-call hidden inside a
closure) it gives up, because a base case might live there. This avoids false
positives on legitimate recursion with a base case.

Changes

  • src/Rules/InfiniteRecursionFinder.php — new autowired helper. Walks the body
    statement-by-statement and only descends into sub-expressions that are
    guaranteed to be evaluated (e.g. both operands of +, but only the left
    operand of ??/&&; the condition of a ternary/match, but not its arms;
    call arguments, but not closure bodies). Returns the offending call so the rule
    can point at the right line. Stops at the first statement that ends normal
    control flow without recursing first (return/throw/exit).
  • src/Rules/Methods/InfiniteMethodRecursionRule.php — new rule on
    MethodReturnStatementsNode (level 4). Matches $this->m() for instance
    methods, self::m()/static::m()/Class::m() for static methods, and
    new self()/new static()/new Class() inside a constructor. Skips
    generators and first-class callable syntax ($this->m(...)).
  • src/Rules/Functions/InfiniteFunctionRecursionRule.php — new rule on
    FunctionReturnStatementsNode (level 4). Resolves the called function name via
    ReflectionProvider so namespaced self-calls match.
  • tests/PHPStan/Levels/data/methodCalls-4.json — regenerated; three existing
    level-test fixtures genuinely start with an unconditional self-call.
  • tests/PHPStan/Analyser/traits/trait-aliases.php — added a base case to a
    fixture whose fooMethod() incidentally recursed unconditionally (it exists to
    test trait-method aliasing, not recursion).

Root cause

This is a missing analysis rather than a bug. The pattern is "a self-call that
dominates every normal exit of the body". The new InfiniteRecursionFinder
centralises the unconditional-evaluation logic so both the function rule and the
method rule (and all the call shapes on the callable axis) share one correct,
conservative implementation.

Analogous cases covered

The "callable" axis was swept rather than fixing only the reported instance:

  • Instance methods$this->m() (the reported case).
  • Static methodsself::m(), static::m(), Self\Class::m().
  • Plain functions — namespaced self-calls resolved through reflection.
  • Constructorsnew self() / new static() re-enter the constructor.
  • Guaranteed-but-not-tail self-callsecho 'x'; return $this->m();,
    $x = $this->m(); return $x;, return $this->m() . 'x',
    return $this->m() ?? 'd', return strtoupper($this->m()) are all reported.

Probed and intentionally left out (verified they are not unconditional
infinite recursion, so reporting them would be a false positive):

  • Recursion with a base case (if, ternary, ?? right operand).
  • First-class callable syntax $this->m(...) (creates a Closure, no call).
  • Generators (yield $this->m() does not run the body).
  • Self-calls inside a nested closure/arrow function.
  • Property hooks recursing via the backing property (get => $this->x) — a
    distinct, non-call construct, out of scope for self-calls.

Test

  • tests/PHPStan/Rules/Methods/InfiniteMethodRecursionRuleTest with
    data/infinite-method-recursion.php — covers the reported getter plus every
    analogous shape above and a batch of non-recursive counter-examples that must
    stay silent.
  • tests/PHPStan/Rules/Functions/InfiniteFunctionRecursionRuleTest with
    data/infinite-function-recursion.php — the function-level mirror.

Both tests fail before the change (the finder finds nothing) and pass after.

Fixes phpstan/phpstan#3068

…every code path

- Add InfiniteRecursionFinder which walks a body's statements in order and
  reports a self-call only when it is guaranteed to be evaluated, bailing out
  on any branching construct (if/switch/loop/try/match-arms/short-circuit), so a
  hidden base case never produces a false positive.
- Add InfiniteMethodRecursionRule (level 4) for MethodReturnStatementsNode:
  detects `$this->m()` in instance methods, `self::m()`/`static::m()`/`C::m()`
  in static methods, and `new self()`/`new static()`/`new C()` inside a
  constructor (re-entering itself), while ignoring first-class callable syntax,
  generators and self-calls hidden inside closures.
- Add InfiniteFunctionRecursionRule (level 4) for FunctionReturnStatementsNode,
  resolving the called function name through ReflectionProvider so namespaced
  self-calls are matched correctly.
- Generate methodCalls-4.json for the levels integration test (three existing
  fixtures contain genuine unconditional self-calls) and add a base case to the
  incidental infinite recursion in the trait-aliases fixture.
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.

Detect infinite self-referencing function

2 participants