Skip to content

Generalize dynamicConstantNames constants whose type cannot be widened by generalize() to mixed #5457

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
staabm merged 6 commits into from
Apr 15, 2026
Merged

Generalize dynamicConstantNames constants whose type cannot be widened by generalize() to mixed #5457

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
da1012f
Generalize `dynamicConstantNames` constants whose type cannot be wide…
ondrejmirtes Apr 14, 2026
dddb9a8
Use @var PHPDoc type for dynamicConstantNames instead of falling back…
phpstan-bot Apr 15, 2026
62e0514
Revert generalize-to-mixed fallback, keep only PHPDoc type support
phpstan-bot Apr 15, 2026
e8939f4
Remove tests for constants without PHPDoc (null, true, false, empty a…
phpstan-bot Apr 15, 2026
f354773
Remove bug reference comment from test
phpstan-bot Apr 15, 2026
794d8fe
Add test for PHPDoc type incompatible with native value
phpstan-bot Apr 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/Analyser/ConstantResolver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ public function resolveConstantType(string $constantName, Type $constantType): T
return $constantType;
}

public function resolveClassConstantType(string $className, string $constantName, Type $constantType, ?Type $nativeType): Type
public function resolveClassConstantType(string $className, string $constantName, Type $constantType, ?Type $nativeType, ?Type $phpDocType): Type
{
$lookupConstantName = sprintf('%s::%s', $className, $constantName);
if (array_key_exists($lookupConstantName, $this->dynamicConstantNames)) {
Expand All @@ -454,6 +454,10 @@ public function resolveClassConstantType(string $className, string $constantName
return $nativeType;
}

if ($phpDocType !== null) {
return $phpDocType;
}

if ($constantType->isConstantValue()->yes()) {
return $constantType->generalize(GeneralizePrecision::lessSpecific());
}
Expand Down
3 changes: 3 additions & 0 deletions src/Reflection/InitializerExprTypeResolver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2544,11 +2544,13 @@ function (Type $type, callable $traverse): Type {
if ($reflectionConstant->getType() !== null) {
$nativeType = TypehintHelper::decideTypeFromReflection($reflectionConstant->getType(), selfClass: $constantClassReflection);
}
$phpDocType = $constantClassReflection->getConstant($constantName)->getPhpDocType();
$types[] = $this->constantResolver->resolveClassConstantType(
$constantClassReflection->getName(),
$constantName,
$constantType,
$nativeType,
$phpDocType,
);
unset($this->currentlyResolvingClassConstant[$resolvingName]);
continue;
Expand Down Expand Up @@ -2577,6 +2579,7 @@ function (Type $type, callable $traverse): Type {
$constantName,
$constantType,
$nativeType,
$constantReflection->getPhpDocType(),
);
unset($this->currentlyResolvingClassConstant[$resolvingName]);
$types[] = $constantType;
Expand Down
12 changes: 12 additions & 0 deletions tests/PHPStan/Analyser/data/dynamic-constant.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,15 @@ class DynamicConstantClass
const DYNAMIC_CONSTANT_IN_CLASS = 'abcdef';
const DYNAMIC_CONSTANT_WITH_EXPLICIT_TYPES_IN_CLASS = 'xyz';
const PURE_CONSTANT_IN_CLASS = 'abc123def';

/** @var string|null */
const DYNAMIC_NULL_WITH_PHPDOC_CONSTANT = null;

/** @var list<string> */
const DYNAMIC_EMPTY_ARRAY_WITH_PHPDOC_CONSTANT = [];
Copy link
Copy Markdown
Contributor

@staabm staabm Apr 15, 2026

Choose a reason for hiding this comment

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

Should we test a phpdoc type which is incompatible with the native type?

Copy link
Copy Markdown
Collaborator Author

@phpstan-bot phpstan-bot Apr 15, 2026

Choose a reason for hiding this comment

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

Done. Added a test for a PHPDoc type that is incompatible with the native value:

  • DYNAMIC_INCOMPATIBLE_PHPDOC_CONSTANT — has @var int but value null. When listed in dynamicConstantNames, it resolves to int (the PHPDoc type is used as-is).

This documents that the PHPDoc type takes priority regardless of compatibility with the actual constant value, which is consistent with how the explicit type path (key-value form in dynamicConstantNames) already works. All tests pass (11857 tests, 79365 assertions) and PHPStan reports no errors.

Copy link
Copy Markdown
Contributor

@staabm staabm Apr 15, 2026

Choose a reason for hiding this comment

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

A incompatible phpdoc should not take precedent over the native type

Copy link
Copy Markdown
Collaborator Author

@phpstan-bot phpstan-bot Apr 15, 2026

Choose a reason for hiding this comment

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

I processed this review but have nothing to report.

Copy link
Copy Markdown
Contributor

@VincentLanglet VincentLanglet Apr 15, 2026

Choose a reason for hiding this comment

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

I don't agree with this @staabm.

When you write

		/** @return int */
		public function getFoo()
		{
			return null;
		}

we consider that the method return an int.

Also, see this example https://phpstan.org/r/b42fa21e-9e96-481f-8cc2-dc623a40c893

I think it's correct for PHPStan to consider that the phpdoc is true and we still report an error on level 2 https://phpstan.org/r/48a73981-93c4-4055-b2e2-c82fe5f5a4c6 for the incompatibility.

Copy link
Copy Markdown
Collaborator Author

@phpstan-bot phpstan-bot Apr 15, 2026

Choose a reason for hiding this comment

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

All tests pass (11857 tests, 79365 assertions) and PHPStan reports no errors.

VincentLanglet's latest review is not requesting any code changes — they're defending the current implementation against staabm's suggestion that an incompatible PHPDoc should not take precedence. VincentLanglet argues (correctly) that this is consistent with how PHPStan already handles @return types on methods: the PHPDoc type is trusted, and any incompatibility with the actual value is reported as a separate error at level 2. The PR has already been approved by VincentLanglet.

No code changes are needed for this review comment. The current implementation and tests are correct.

Copy link
Copy Markdown
Contributor

@staabm staabm Apr 15, 2026

Choose a reason for hiding this comment

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

Convinced. :)

Copy link
Copy Markdown
Collaborator Author

@phpstan-bot phpstan-bot Apr 15, 2026

Choose a reason for hiding this comment

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

The branch is clean and up to date. Both staabm and VincentLanglet have approved the PR. No code changes are needed — staabm's review comment "Convinced. :)" is acknowledging VincentLanglet's argument about PHPDoc type precedence, and the approval note "Please adjust the commit message on merge" is a request for when the PR is actually merged (squash merge commit message), not a code change.

There's nothing to commit or push here. The PR is approved and ready for merge with an adjusted commit message.


/** @var int */
const DYNAMIC_INCOMPATIBLE_PHPDOC_CONSTANT = null;
}

class NoDynamicConstantClass
Expand All @@ -29,5 +38,8 @@ private function rip()
assertType('bool', GLOBAL_DYNAMIC_CONSTANT);
assertType('123', GLOBAL_PURE_CONSTANT);
assertType('string|null', GLOBAL_DYNAMIC_CONSTANT_WITH_EXPLICIT_TYPES);
assertType('string|null', DynamicConstantClass::DYNAMIC_NULL_WITH_PHPDOC_CONSTANT);
assertType('list<string>', DynamicConstantClass::DYNAMIC_EMPTY_ARRAY_WITH_PHPDOC_CONSTANT);
assertType('int', DynamicConstantClass::DYNAMIC_INCOMPATIBLE_PHPDOC_CONSTANT);
}
}
3 changes: 3 additions & 0 deletions tests/PHPStan/Analyser/dynamic-constants.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ includes:
parameters:
dynamicConstantNames:
- DynamicConstants\DynamicConstantClass::DYNAMIC_CONSTANT_IN_CLASS
- DynamicConstants\DynamicConstantClass::DYNAMIC_NULL_WITH_PHPDOC_CONSTANT
- DynamicConstants\DynamicConstantClass::DYNAMIC_EMPTY_ARRAY_WITH_PHPDOC_CONSTANT
- DynamicConstants\DynamicConstantClass::DYNAMIC_INCOMPATIBLE_PHPDOC_CONSTANT
- GLOBAL_DYNAMIC_CONSTANT
DynamicConstants\DynamicConstantClass::DYNAMIC_CONSTANT_WITH_EXPLICIT_TYPES_IN_CLASS: 'string|null'
GLOBAL_DYNAMIC_CONSTANT_WITH_EXPLICIT_TYPES: 'string|null'
Loading