Skip to content

Consider class and constant finality in ClassConstantAccessType::getResult() for static::CONST PHPDoc types#5570

Open
phpstan-bot wants to merge 4 commits intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-35asf4c
Open

Consider class and constant finality in ClassConstantAccessType::getResult() for static::CONST PHPDoc types#5570
phpstan-bot wants to merge 4 commits intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-35asf4c

Conversation

@phpstan-bot
Copy link
Copy Markdown
Collaborator

Summary

The ClassConstantAccessType (used for @return static::CONST PHPDoc 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 $foo could be a subclass with a different constant value.

Changes

  • src/Type/ClassConstantAccessType.php:
    • getResult() now checks class/constant finality before resolving:
      • Final class or final constant → returns concrete value via getValueType()
      • Non-final class with typed constant (PHPDoc or native type) → returns declared type via getValueType()
      • Non-final class with untyped, non-final constant → returns MixedType
    • Added isSubTypeOf() override that uses getValueType() directly (not the resolved type), ensuring the ClassConstantAccessType passes through TypehintHelper::decideType() and is preserved in the method's return type for later StaticType resolution
    • Added isAcceptedBy() override with the same approach for consistency
  • tests/PHPStan/Analyser/nsrt/bug-13828.php: Updated 3 assertions that incorrectly expected concrete constant values for non-final classes with untyped constants
  • tests/PHPStan/Analyser/nsrt/bug-6989.php: Updated 2 assertions where static::CONST was used as array shape keys on non-final classes — these now correctly resolve to non-empty-array<string> instead of array{key: string}
  • tests/PHPStan/Analyser/nsrt/bug-14556.php: New regression test from the issue's playground sample

Root cause

ClassConstantAccessType::getResult() unconditionally called getValueType(), 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::CONST could resolve to a different value at runtime, so the type must be the declared type constraint (PHPDoc/native type) or mixed if none exists. This mirrors the existing logic in InitializerExprTypeResolver (lines 2569–2595) which already handles this correctly for static::CONST expressions in PHP code.

An additional subtlety: ClassConstantAccessType must survive TypehintHelper::decideType() to allow later StaticType resolution when the method is called on a specific (potentially final) class. The isSubTypeOf() and isAcceptedBy() overrides use the base class's getValueType() for compatibility checks, ensuring the type is preserved through decideType() while getResult() returns the correctly-constrained type.

Analogous cases probed

  • self::CONST and parent::CONST in PHPDocs: Not affected — TypeNodeResolver doesn't create ClassConstantAccessType for these, resolving them directly to concrete values (correct since self/parent always refer to the lexical class).
  • Wildcard patterns (static::FOO_*): These also resolve concrete values eagerly without ClassConstantAccessType, so they have a similar potential issue for non-final classes. However, fixing them requires a different approach (they don't use ClassConstantAccessType at all) and is out of scope for this PR.
  • Typed constants (PHPDoc or native type): Already correct — getValueType() returns the declared type, which is the right constraint for non-final classes.
  • Final constants: Already correct — the fix preserves the concrete value for final const.

Test

  • bug-14556.php: Regression test with FooBar (non-final, untyped constant → mixed), BarBaz (non-final subclass → mixed), and FinalBarBaz (final subclass → 'bar')
  • Updated assertions in bug-13828.php and bug-6989.php that were asserting the buggy behavior

Fixes phpstan/phpstan#14556

…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.
Copy link
Copy Markdown
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
@phpstan-bot
Copy link
Copy Markdown
Collaborator Author

I processed this review but have nothing to report.

Copy link
Copy Markdown
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests in tests/PHPStan/Analyser/nsrt/bug-13828.php are not really sufficient. We should probe constants with PHPDoc types, constants with native types, final constants and constants in final classes. To see which type is actually used in these cases.

…ypes, final constants and final classes

Probes which type is used for each combination:
- Final class + native type → concrete value
- Final class + PHPDoc type → concrete value
- Final class + both types → concrete value
- Non-final class + final const + PHPDoc type → declared PHPDoc type
- Non-final class + final const + native type → declared native type

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@phpstan-bot
Copy link
Copy Markdown
Collaborator Author

Pushed. Here's a summary of the added test cases in bug-13828.php:

Case Class Constant Expected Type
FinalClassWithNativeType final const string 'foo' (concrete value)
FinalClassWithPhpDocType final @var non-empty-string 'foo' (concrete value)
FinalClassWithBothTypes final both PHPDoc + native 'foo' (concrete value)
WithFinalPhpDocConstant non-final final const + PHPDoc non-empty-string (declared type)
WithFinalNativeConstant non-final final const string string (declared type)

The tests reveal an interesting asymmetry: for final classes, static resolves to the concrete class before ClassConstantAccessType::getResult() runs, so the constant's concrete value is returned regardless of type declarations. For non-final classes with final constants, getResult() sees the finality but getValueType() returns the declared type (not the concrete value) when a PHPDoc or native type is present.

Copy link
Copy Markdown
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still wrong. Final constants should infer their exact value. There are missing tests for non-final class and non-final constants. They should infer to their native/PHPDoc types.

…tResult()

Add getInitializerExprType() to ClassConstantReflection to retrieve the
concrete initializer value type, bypassing PHPDoc/native type declarations.
Use it in ClassConstantAccessType::getResult() for final constants, since
they cannot be overridden and their exact value is always known.

Update test expectations for final typed constants to expect exact values
instead of declared types. Add test cases for non-final class with non-final
typed constants to verify they correctly resolve to PHPDoc/native types.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@phpstan-bot
Copy link
Copy Markdown
Collaborator Author

Done. Here's a summary of what changed:

Code changes:

  • Added getInitializerExprType(): Type to ClassConstantReflection interface and all 3 implementations (RealClassClassConstantReflection, DummyClassConstantReflection, RewrittenDeclaringClassClassConstantReflection). This method returns the concrete value type resolved from the initializer expression, bypassing PHPDoc/native type declarations.
  • Updated ClassConstantAccessType::getResult() to call getInitializerExprType() instead of getValueType() for final constants — since final constants can't be overridden, their exact value is always known.

Test changes:

  • Updated 3 assertions in bug-13828.php for final typed constants (WithFinalTypedConstant, WithFinalPhpDocConstant, WithFinalNativeConstant) to expect the exact value 'foo' instead of the declared type.
  • Added 4 test cases in bug-14556.php: non-final class with native typed constant (string), non-final class with PHPDoc typed constant (non-empty-string), non-final class with final untyped constant ('foo'), and non-final class with final typed constant ('foo').

All 11,964 tests pass and make phpstan reports no errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants