Repository navigation
Do not cache isGeneric() or the class PHPDoc while the class name scope is created - #6726
Open
GErpeldinger wants to merge 2 commits into
Open
GErpeldinger wants to merge 2 commits into
GErpeldinger wants to merge 2 commits into
Conversation
…pe is created A template bound that names the class itself (@template T of Error|ErrorIterator) leads FileTypeMapper::getNameScope() back to ClassReflection::isGeneric() for the same class, through TypeCombinator::union() and ObjectType::getEnumCases(). The template tags are not resolved yet at that moment, so isGeneric() cached false, and getResolvedPhpDoc() cached a PHPDoc without template tags. Both stayed wrong for the rest of the process. FileTypeMapper now tracks the classes whose name scope is being created, and ClassReflection does not cache these two values while its class is in that set. The same change is made in turbo-ext/src/ClassReflection.cpp.
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
1 participant
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.
Closes phpstan/phpstan#15448
A template bound that names the class itself (
@template T of Error|ErrorIterator) leadsFileTypeMapper::getNameScope()back toClassReflection::isGeneric()for the same class, throughTypeCombinator::union()andObjectType::getEnumCases(). The template tags are not resolved yet at that moment, soisGeneric()cachedfalse, andgetResolvedPhpDoc()cached a PHPDoc without template tags. Both stayed wrong for the rest of the process.FileTypeMappernow tracks the classes whose name scope is being created.ClassReflectiondoes not cache these two values while its class is in that set. The same change is made inturbo-ext/src/ClassReflection.cpp.Tests: the two new cases in
WrongVariableNameInVarTagRuleTestfail before the fix: class declared in the analysed file, and class only used from it. The full suite passes with and without the extension.make phpstan,make cs, and the turbo smoke test, signature parity and side-by-side pass.testBug10049: one more error, a missing type onSimpleEntity. It comes from how PHPStan approximates@template SELF of SimpleEntity<SELF>, a bound that refers to its own template: the result isSimpleEntity<SimpleEntity>, and the innerSimpleEntityhas no type. The wrong cache hid it before.Performance: no slowdown. Symfony 8.2
src/, cold cache, before and after in alternating order, 4 to 5 runs each (the first run after a container start is left out), same 2,113 errors in every run:phpbench: +1.0% total time, smaller than the noise between two runs of the same code (+1.9%).
I do not know C++. Claude (AI) wrote the turbo change, and I could not review it. It was checked with: the regression tests failing with the unmodified extension and passing with the new one, the full suite with the extension loaded, the smoke test, signature parity, side-by-side, and the Symfony runs above.
🤖 AI-written description.