[ChrisAbner]

Closes phpstan/phpstan#15377

Scope::getFunctionCallStack() and getFunctionCallStackWithParameters() were emptied inside a closure passed as an argument because many scope derivations rebuilt the scope with [] in the $inFunctionCallsStack slot. The issue's cases go through invalidateExpression() and invalidateMethodsOnExpression(), but the same [] is in mergeWith(), enter/exitExpressionAssign(), set/unsetAllowedUndefinedExpression(), processFinallyScope(), processAlwaysIterableForeachScopeWithoutPollute(), generalizeWithVariableState(), withRecordedStatementDelta() and the Closure::bind()/call() derivations. So after any if/else, ternary, ??, isset(), loop or try/finally in the closure the stack was gone for the rest of it. This affects extensions that read the stack (the issue's transaction check, phpstan-drupal's DeprecationHelperScope) and the core consumers, isInClassExists() and UnusedPrivatePropertyRule::isPropertySelfWrite(). With this PR the stack only changes when entering or leaving a call, a class, a function or a namespace. The closure-inference walk (enter*WithoutReflection()) keeps starting from [], because enterAnonymousFunction() puts the stack back on top of it.

Keeping the stack in mergeWith() is only correct if both merged scopes carry the same stack, and they did not: a throw point records its scope in the middle of a call, and TryCatchHandler built the catch and finally scopes from those. That is also a separate false negative on 2.3.x: in try { class_exists(name()); } catch (...) { new Missing(); } the catch block counted as inside class_exists() and the missing class was not reported. So the second commit gives the throw point scopes the stack of the try statement (MutatingScope::withFunctionCallStackOf()) before the catch and finally scopes are built from them. To check the invariant, I temporarily logged (on c539adc52) every mergeWith() call whose two scopes had different stacks: 851 such merges in make phpstan on 2.3.x, and 0 with this PR in make phpstan, NodeScopeResolverTest, tests/PHPStan/Rules and the rest of tests/PHPStan/Analyser. The same [] → $this->inFunctionCallsStack change was made for entering closures in be279db. The [] here goes back to 17bde93, which added a positional argument, and 5f91da6, and neither says why the stack would be cleared. MutatingScope and TryCatchHandler have turbo mirrors, so every change is ported. The private scopeWith(..., bool emptyCallStack) helper in MutatingScope.cpp lost its flag, because both of its callers passed true. generated/MutatingScope.h is regenerated with PHP 8.5.

The PR has three commits: the two invalidations; catch/finally taking the stack of try; and the remaining derivations. Each commit has its own tests that fail without it.

Verification. The issue's reproduction project (its rule prints getFunctionCallStackWithParameters() at each probe()), run with bin/phpstan from this branch, before (2.3.x at c539adc52) and after, with the same output with and without the extension:

Line	Call before probe()	Before	After
10	none	[inTransaction]	[inTransaction]
12	interface method returning int	[inTransaction]	[inTransaction]
14	void interface method	[]	[inTransaction]
25	void method of a concrete class	[]	[inTransaction]
26	void function	[inTransaction]	[inTransaction]
27	@phpstan-impure method returning int	[]	[inTransaction]

And one construct per closure, stack after it:

Inside the closure passed to inTransaction()	Before	After
if/else, ternary	[]	[inTransaction]
$n ?? 1, isset($n)	[]	[inTransaction]
foreach, while, foreach with break	[]	[inTransaction]
after try { ... } finally { ... }	[]	[inTransaction]
in catch, one throw point inside var_dump()	[inTransaction, var_dump]	[inTransaction]
in catch, two throw points	[]	[inTransaction]
takesTwo($b ? 1 : 2, <here>)	[takesTwo]	[inTransaction, takesTwo]
after \Closure::bind(static function () {}, null, null)	[]	[inTransaction]
try { class_exists(name()); } catch (...) { new Missing(); } (not in a closure)	no error	Instantiated class Missing not found.

The new tests (testBug15377 in both ScopeFunctionCallStack*RuleTests, InstantiationRuleTest::testClassExistsInCatch) fail on the commit before theirs and pass after, with and without the extension (I checked the failing side with the extension loaded for the first commit). With and without the extension, these pass: tests/PHPStan/Rules (3620 tests), NodeScopeResolverTest (1893), the rest of tests/PHPStan/Analyser (1181), tests/PHPStan/Type (6599), tests/PHPStan/Reflection and the remaining directories. The one error, ReflectionProviderGoldenTest with no reflection-8.4.test data file, only shows up when the directory is passed by hand, since phpunit.xml excludes it. make phpstan and phpcs are clean. For the extension: strict build without warnings, smoke.php ALL OK, side-by-side.php (PHP 8.5) and signature-parity.php OK, walk-trace.php identical over all NSRT files, src/Analyser and src/Type/Constant, and, before the rebase, make sanitize OK (smoke, arena-smoke, signature-parity and parser-corpus under UBSan). The issue's project and the construct cases give the same --error-format=raw output with and without the extension.

Performance: before rebasing onto the current 2.3.x, I ran seven interleaved A/B pairs of bin/phpstan analyse -c build/phpstan.neon src without the extension (user CPU, result cache cleared), comparing 2.3.x with this branch. The output was identical, and this branch was 1.1% slower on average. The difference between pairs ranged from -5.8% to +7.3% on this machine, so the difference is within the noise. The closure-type cache, whose key includes the stack, had 552 misses instead of 550 out of 572 lookups.

Not covered: I did not run the levels and exec groups or the full make lint-turbo. Closures written in a callback body but not passed anywhere now keep the context of the enclosing call after an if, a loop and so on, as they already did before the first such statement (ClosureTypeResolver::buildParametersAndAcceptors() reads the top of the stack). No NSRT result changed. Code in a catch block after a call inside class_exists() and similar is now analysed as code outside that call, so it can report errors that were hidden before.

If you'd rather review this in smaller pieces, the first commit alone fixes the cases from the issue, and the catch/finally change can go in its own PR.