Report functions and methods that unconditionally call themselves on every code path#5889
Open
phpstan-bot wants to merge 1 commit into
Open
Report functions and methods that unconditionally call themselves on every code path#5889phpstan-bot wants to merge 1 commit into
phpstan-bot wants to merge 1 commit into
Conversation
…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.
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
A function or method whose body unconditionally calls itself — the classic
public function getWorld(): string { return $this->getWorld(); }produced by anautocompletion 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,matcharm, a ternary/
??/&&/||short-circuit, or a self-call hidden inside aclosure) 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 bodystatement-by-statement and only descends into sub-expressions that are
guaranteed to be evaluated (e.g. both operands of
+, but only the leftoperand 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 onMethodReturnStatementsNode(level 4). Matches$this->m()for instancemethods,
self::m()/static::m()/Class::m()for static methods, andnew self()/new static()/new Class()inside a constructor. Skipsgenerators and first-class callable syntax (
$this->m(...)).src/Rules/Functions/InfiniteFunctionRecursionRule.php— new rule onFunctionReturnStatementsNode(level 4). Resolves the called function name viaReflectionProviderso namespaced self-calls match.tests/PHPStan/Levels/data/methodCalls-4.json— regenerated; three existinglevel-test fixtures genuinely start with an unconditional self-call.
tests/PHPStan/Analyser/traits/trait-aliases.php— added a base case to afixture whose
fooMethod()incidentally recursed unconditionally (it exists totest 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
InfiniteRecursionFindercentralises 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:
$this->m()(the reported case).self::m(),static::m(),Self\Class::m().new self()/new static()re-enter the constructor.echo '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):
if, ternary,??right operand).$this->m(...)(creates aClosure, no call).yield $this->m()does not run the body).get => $this->x) — adistinct, non-call construct, out of scope for self-calls.
Test
tests/PHPStan/Rules/Methods/InfiniteMethodRecursionRuleTestwithdata/infinite-method-recursion.php— covers the reported getter plus everyanalogous shape above and a batch of non-recursive counter-examples that must
stay silent.
tests/PHPStan/Rules/Functions/InfiniteFunctionRecursionRuleTestwithdata/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