Consider class and constant finality in ClassConstantAccessType::getResult() for static::CONST PHPDoc types#5570
Open
phpstan-bot wants to merge 2 commits intophpstan:2.1.xfrom
Open
Conversation
…Result()` for `static::CONST` PHPDoc types - Fix `ClassConstantAccessType::getResult()` to check whether the class is final or the constant is final before returning the concrete constant value. For non-final classes with non-final, untyped constants, return `MixedType` instead of the concrete value, matching the behavior of `InitializerExprTypeResolver` for `static::CONST` expressions in code. - Override `isSubTypeOf()` and `isAcceptedBy()` to use `getValueType()` directly, ensuring `ClassConstantAccessType` survives `TypehintHelper::decideType()` and can be resolved correctly after `StaticType` is substituted with the caller's concrete type. - Update test assertions in `bug-13828.php` and `bug-6989.php` that incorrectly expected concrete constant values for non-final classes. - Add regression test `bug-14556.php` for the reported issue.
ondrejmirtes
requested changes
Apr 29, 2026
Member
ondrejmirtes
left a comment
There was a problem hiding this comment.
Also toPhpDocNode is wrong and assumes static but the actual type is $this->type and might be different. Write some tests for TypeToPhpDocNodeTest.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Collaborator
Author
|
I processed this review but have nothing to report. |
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
The
ClassConstantAccessType(used for@return static::CONSTPHPDoc types) was always resolving to the concrete constant value from the declaring class, regardless of whether the class or constant was final. This meant subclasses that override the constant would still show the parent class's value — e.g.,$foo->test()returning'foo'even though$foocould be a subclass with a different constant value.Changes
src/Type/ClassConstantAccessType.php:getResult()now checks class/constant finality before resolving:getValueType()getValueType()MixedTypeisSubTypeOf()override that usesgetValueType()directly (not the resolved type), ensuring theClassConstantAccessTypepasses throughTypehintHelper::decideType()and is preserved in the method's return type for laterStaticTyperesolutionisAcceptedBy()override with the same approach for consistencytests/PHPStan/Analyser/nsrt/bug-13828.php: Updated 3 assertions that incorrectly expected concrete constant values for non-final classes with untyped constantstests/PHPStan/Analyser/nsrt/bug-6989.php: Updated 2 assertions wherestatic::CONSTwas used as array shape keys on non-final classes — these now correctly resolve tonon-empty-array<string>instead ofarray{key: string}tests/PHPStan/Analyser/nsrt/bug-14556.php: New regression test from the issue's playground sampleRoot cause
ClassConstantAccessType::getResult()unconditionally calledgetValueType(), which for untyped constants returns the concrete initializer value. This is correct only when the class is final (no subclass can override the constant) or the constant is final (PHP 8.2+). For non-final classes with non-final constants,static::CONSTcould resolve to a different value at runtime, so the type must be the declared type constraint (PHPDoc/native type) ormixedif none exists. This mirrors the existing logic inInitializerExprTypeResolver(lines 2569–2595) which already handles this correctly forstatic::CONSTexpressions in PHP code.An additional subtlety:
ClassConstantAccessTypemust surviveTypehintHelper::decideType()to allow laterStaticTyperesolution when the method is called on a specific (potentially final) class. TheisSubTypeOf()andisAcceptedBy()overrides use the base class'sgetValueType()for compatibility checks, ensuring the type is preserved throughdecideType()whilegetResult()returns the correctly-constrained type.Analogous cases probed
self::CONSTandparent::CONSTin PHPDocs: Not affected —TypeNodeResolverdoesn't createClassConstantAccessTypefor these, resolving them directly to concrete values (correct sinceself/parentalways refer to the lexical class).static::FOO_*): These also resolve concrete values eagerly withoutClassConstantAccessType, so they have a similar potential issue for non-final classes. However, fixing them requires a different approach (they don't useClassConstantAccessTypeat all) and is out of scope for this PR.getValueType()returns the declared type, which is the right constraint for non-final classes.final const.Test
bug-14556.php: Regression test withFooBar(non-final, untyped constant →mixed),BarBaz(non-final subclass →mixed), andFinalBarBaz(final subclass →'bar')bug-13828.phpandbug-6989.phpthat were asserting the buggy behaviorFixes phpstan/phpstan#14556