Repository navigation
Keep the template-argument constraints of a static variable's default and resolve the markers in inferred static variable types - #6812
Open
phpstan-bot wants to merge 1 commit into
Conversation
…lt and resolve the markers in inferred `static` variable types - StaticVariableHandler: carry the template-argument constraints of the scope the default expression returned onto the statement's scope, the way CoalesceHandler carries those of its conditionally evaluated right side. Before this, the `new` / generic call sites in `static $x = new Foo(...)` recorded no facts, so their template arguments were never resolved. - TemplateArgumentFrame::withStaticVariableTypes(): on a resolved frame, replace the UnresolvedTemplateArgumentType markers in the inferred types (collected by the observation walks) with the frame's resolution, the initial type, or the unconstrained default/bound - the same order GenericTypeTemplateTraverser uses at the site. Closure-signature markers are left alone. - Ported both changes to the turbo-ext mirrors (StaticVariableHandler.cpp, TemplateArgumentFrame.cpp). - Analogous cases covered by the regression test: a site created in the body (`static $c = null; $c = new ...`), a generic function call as the default, a run of two `static` variables, a send to a typed parameter instead of a return, a function, and a closure body. A closure stored in a `static` variable was probed too: it stays `Closure(mixed): mixed` with no marker leak, so it is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
On bleeding edge (
unresolvedTemplateArguments+staticVariablesFromUsages), astaticlocal variable initialised with a genericnewexpression kept its "unresolved" template-argument marker. Rules then saw it, soreturn $cache;reportedshould return Cache<string> but returns Cache<unresolved(string)>. Without thestatickeyword, there was no error.This PR makes the
staticstatement keep the constraints of its default expression, and resolves the markers left in the inferredstaticvariable types once the template arguments are resolved.Changes
src/Analyser/StmtHandler/StaticVariableHandler.php: add the default expression's template-argument constraints to the statement's scope withaddTemplateArgumentConstraints(). This is the same patternCoalesceHandleruses for its conditionally evaluated right side.src/Analyser/Generics/TemplateArgumentFrame.php: on a resolved frame,withStaticVariableTypes()now maps each inferred type through a new privateresolveMarkers(). Each non-closure-signatureUnresolvedTemplateArgumentTypebecomes the frame's resolution, falling back to the initial type and then to the unconstrained default/bound. This is the same orderGenericTypeTemplateTraverseruses at a site.turbo-ext/src/StaticVariableHandler.cppandturbo-ext/src/TemplateArgumentFrame.cpp: native ports of both changes.Analogous cases probed:
static $c = null; if ($c === null) { $c = new Cache(...); })staticvariablesmixed)staticvariable staysClosure(mixed): mixedand leaks no marker, so it is unchanged.Root cause
There were two gaps, and both have to be fixed for the reported case:
StaticVariableHandlerdiscarded the scope returned by processing the default expression. ThewithSite/withSendfacts that thenew Cache(...)site recorded there never reached the body's constraints, so the resolver had nothing to resolve.staticvariables are collected during the observation walks, so they contain markers. They were installed into the resolved frame as they were, and the second pass assigned them to the variable at thestaticstatement. Unlike anewexpression, nothing on that path ever substituted the resolutions. This also broke the case where the site is in the body, because the type at thestaticstatement is joined from the body's observed types.Test
tests/PHPStan/Analyser/nsrt/bug-15416.phpcontains the issue's reproducer verbatim, plus the analogous cases listed above. It fails before the fix and passes after. The test also passes with only the native port active (PHP twins reverted, extension loaded).Verification:
make testspasses, both without and withphpstan_turboloaded.make phpstanpasses.smoke.php,side-by-side.php,signature-parity.phpandwalk-trace.phpall pass; the walk traces are identical.make name-collisionaborts on a parse error intests/PHPStan/Build/data/final-class-rule-pipe.php. This also happens on a clean tree; the new test namespace is unique.make lint-turbowas not run: clang-tidy is pinned to 21 and only 18 is installed.Fixes phpstan/phpstan#15416
🤖 Generated with Claude Code