Skip to content

Allow mixed type dynamic constants #3583

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
merged 5 commits into from
Aug 30, 2025
Merged

Allow mixed type dynamic constants #3583

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9767538
Allow mixed type dynamic constants.
ssigwart Oct 26, 2024
45e3dd5
PHP 7.4 fix
ssigwart Nov 8, 2024
7e803ee
Fix rebase
ssigwart Dec 19, 2024
ed0d74a
Update src/Analyser/ConstantResolver.php
ondrejmirtes Aug 30, 2025
a46679d
Update src/Analyser/ConstantResolver.php
ondrejmirtes Aug 30, 2025
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
30 changes: 28 additions & 2 deletions src/Analyser/ConstantResolver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@

use PhpParser\Node\Name;
use PHPStan\DependencyInjection\AutowiredService;
use PHPStan\DependencyInjection\Container;
use PHPStan\Php\ComposerPhpVersionFactory;
use PHPStan\Php\PhpVersion;
use PHPStan\PhpDoc\TypeStringResolver;
use PHPStan\Reflection\NamespaceAnswerer;
use PHPStan\Reflection\ReflectionProvider;
use PHPStan\Reflection\ReflectionProvider\ReflectionProviderProvider;
Expand Down Expand Up @@ -50,6 +52,7 @@ public function __construct(
private array $dynamicConstantNames,
private int|array|null $phpVersion,
private ComposerPhpVersionFactory $composerPhpVersionFactory,
private ?Container $container,
Copy link
Contributor

@staabm staabm Nov 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

make this new param the TypeStringResolver itself.

It will be injected by the DIC automatically

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I initially thought that might be the way to do it. However, when I tried it, I wound up getting an error about circular dependencies. So that's why I switched to the entire container. Do you have any ideas on how to get around the circular dependency error? It's also possible I just did it wrong. I assumed I needed to update the places that called the constructor, so I was using the container there to get the TypeStringResolver.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah I can see it now.

TypeStringResolver has a TypeNodeResolver contruction dependency which in turn has a ConstantResolver dependency.

so when you add a TypeStringResolver dependency on ConstantResolver you build a cycle.

I will think about it and report back

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess one way we could break the cycle is to remove the ConstantResolver construction-time dependency of TypeNodeResolver and turn it into a setter injection (similar how we do it in TypeSpecifierFactory for the TypeSpecifier).

in my local testing I did not yet succeed in doing so :)

Copy link
Contributor

@staabm staabm Nov 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

or maybe better instead remove TypeNodeResolver from TypeStringResolver construction-time deps and move it to setter-injection

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, @staabm. I don't have experience with DI in PHP, so I'm not entirely sure how to do this. I assume you meant to add a setTypeNodeResolver function to TypeStringResolver. I'm not sure where I would call setTypeNodeResolver though. I thought maybe I needed to add a TypeStringResolverFactory. I'm guessing that maybe I need to update conf/config.neon, so I changed it to this:

-
	class: PHPStan\PhpDoc\TypeStringResolver
typeStringResolverFactory:
	class: PHPStan\PhpDoc\TypeStringResolverFactory

In a new TypeStringResolverFactory class, I added this:

public function create(): TypeStringResolver
{
	$typeStringResolver = new TypeStringResolver(
		$this->typeLexer,
		$this->typeParser
	);
	$typeStringResolver->setTypeNodeResolver($this->container->getByType(TypeNodeResolver::class));

	return $typeStringResolver;
}

I still wound up with this when running the test though:

In Container.php line 327:

  Circular reference detected for: reflectionProvider, betterReflectionProvider, 093,
  058, 042, 040.

Am I on the right track? Do you have any suggestions? Thanks.

)
{
}
Expand Down Expand Up @@ -404,8 +407,17 @@ private function getMaxPhpVersion(): ?PhpVersion

public function resolveConstantType(string $constantName, Type $constantType): Type
{
if ($constantType->isConstantValue()->yes() && in_array($constantName, $this->dynamicConstantNames, true)) {
return $constantType->generalize(GeneralizePrecision::lessSpecific());
if ($constantType->isConstantValue()->yes()) {
if (array_key_exists($constantName, $this->dynamicConstantNames)) {
$phpdocTypes = $this->dynamicConstantNames[$constantName];
if ($this->container !== null) {
$typeStringResolver = $this->container->getByType(TypeStringResolver::class);
return $typeStringResolver->resolve($phpdocTypes, new NameScope(null, [], null));
}
}
if (in_array($constantName, $this->dynamicConstantNames, true)) {
return $constantType->generalize(GeneralizePrecision::lessSpecific());
}
}

return $constantType;
Expand All @@ -414,6 +426,20 @@ public function resolveConstantType(string $constantName, Type $constantType): T
public function resolveClassConstantType(string $className, string $constantName, Type $constantType, ?Type $nativeType): Type
{
$lookupConstantName = sprintf('%s::%s', $className, $constantName);
if (array_key_exists($lookupConstantName, $this->dynamicConstantNames)) {
if ($nativeType !== null) {
return $nativeType;
}

if ($constantType->isConstantValue()->yes()) {
$phpdocTypes = $this->dynamicConstantNames[$lookupConstantName];
if ($this->container !== null) {
$typeStringResolver = $this->container->getByType(TypeStringResolver::class);
return $typeStringResolver->resolve($phpdocTypes, new NameScope(null, [], $className));
}
}
}

if (in_array($lookupConstantName, $this->dynamicConstantNames, true)) {
if ($nativeType !== null) {
return $nativeType;
Expand Down
1 change: 1 addition & 0 deletions src/Analyser/ConstantResolverFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ public function create(): ConstantResolver
$this->container->getParameter('dynamicConstantNames'),
$this->container->getParameter('phpVersion'),
$composerFactory,
$this->container,
);
}

Expand Down
5 changes: 5 additions & 0 deletions src/Analyser/MutatingScope.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6191,6 +6191,11 @@ public function getConstantReflection(Type $typeWithConstant, string $constantNa
return $typeWithConstant->getConstant($constantName);
}

public function getConstantExplicitTypeFromConfig(string $constantName, Type $constantType): Type
{
return $this->constantResolver->resolveConstantType($constantName, $constantType);
}

/**
* @return array<string, ExpressionTypeHolder>
*/
Expand Down
2 changes: 2 additions & 0 deletions src/Analyser/Scope.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ public function getMethodReflection(Type $typeWithMethod, string $methodName): ?

public function getConstantReflection(Type $typeWithConstant, string $constantName): ?ClassConstantReflection;

public function getConstantExplicitTypeFromConfig(string $constantName, Type $constantType): Type;

public function getIterableKeyType(Type $iteratee): Type;

public function getIterableValueType(Type $iteratee): Type;
Expand Down
2 changes: 1 addition & 1 deletion src/DependencyInjection/ValidateIgnoredErrorsExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public function loadConfiguration(): void
ReflectionProviderStaticAccessor::registerInstance($reflectionProvider);
PhpVersionStaticAccessor::registerInstance(new PhpVersion(PHP_VERSION_ID));
$composerPhpVersionFactory = new ComposerPhpVersionFactory([]);
$constantResolver = new ConstantResolver($reflectionProviderProvider, [], null, $composerPhpVersionFactory);
$constantResolver = new ConstantResolver($reflectionProviderProvider, [], null, $composerPhpVersionFactory, null);

$phpDocParserConfig = new ParserConfig([]);
$ignoredRegexValidator = new IgnoredRegexValidator(
Expand Down
2 changes: 1 addition & 1 deletion src/Testing/PHPStanTestCase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public static function createScopeFactory(ReflectionProvider $reflectionProvider

$reflectionProviderProvider = new DirectReflectionProviderProvider($reflectionProvider);
$composerPhpVersionFactory = $container->getByType(ComposerPhpVersionFactory::class);
$constantResolver = new ConstantResolver($reflectionProviderProvider, $dynamicConstantNames, null, $composerPhpVersionFactory);
$constantResolver = new ConstantResolver($reflectionProviderProvider, $dynamicConstantNames, null, $composerPhpVersionFactory, $container);

$initializerExprTypeResolver = new InitializerExprTypeResolver(
$constantResolver,
Expand Down
8 changes: 7 additions & 1 deletion src/Type/Php/DefineConstantTypeSpecifyingExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,17 @@ public function specifyTypes(
return new SpecifiedTypes([], []);
}

$valueType = $scope->getType($node->getArgs()[1]->value);
$finalType = $scope->getConstantExplicitTypeFromConfig(
$constantName->getValue(),
$valueType,
);

return $this->typeSpecifier->create(
new Node\Expr\ConstFetch(
new Node\Name\FullyQualified($constantName->getValue()),
),
$scope->getType($node->getArgs()[1]->value),
$finalType,
TypeSpecifierContext::createTruthy(),
$scope,
)->setAlwaysOverwriteTypes();
Expand Down
16 changes: 13 additions & 3 deletions tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8244,6 +8244,10 @@ public static function dataDynamicConstants(): array
'string',
'DynamicConstants\DynamicConstantClass::DYNAMIC_CONSTANT_IN_CLASS',
],
[
'string|null',
'DynamicConstants\DynamicConstantClass::DYNAMIC_CONSTANT_WITH_EXPLICIT_TYPES_IN_CLASS',
],
[
"'abc123def'",
'DynamicConstants\DynamicConstantClass::PURE_CONSTANT_IN_CLASS',
Expand All @@ -8253,13 +8257,17 @@ public static function dataDynamicConstants(): array
'DynamicConstants\NoDynamicConstantClass::DYNAMIC_CONSTANT_IN_CLASS',
],
[
'false',
'bool',
'GLOBAL_DYNAMIC_CONSTANT',
],
[
'123',
'GLOBAL_PURE_CONSTANT',
],
[
'string|null',
'GLOBAL_DYNAMIC_CONSTANT_WITH_EXPLICIT_TYPES',
],
];
}

Expand All @@ -8275,8 +8283,10 @@ public function testDynamicConstants(
$expression,
'die',
[
'DynamicConstants\\DynamicConstantClass::DYNAMIC_CONSTANT_IN_CLASS',
'GLOBAL_DYNAMIC_CONSTANT',
0 => 'DynamicConstants\\DynamicConstantClass::DYNAMIC_CONSTANT_IN_CLASS',
1 => 'GLOBAL_DYNAMIC_CONSTANT',
'DynamicConstants\\DynamicConstantClass::DYNAMIC_CONSTANT_WITH_EXPLICIT_TYPES_IN_CLASS' => 'string|null',
'GLOBAL_DYNAMIC_CONSTANT_WITH_EXPLICIT_TYPES' => 'string|null',
],
);
}
Expand Down
2 changes: 2 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 @@ -4,10 +4,12 @@

define('GLOBAL_PURE_CONSTANT', 123);
define('GLOBAL_DYNAMIC_CONSTANT', false);
define('GLOBAL_DYNAMIC_CONSTANT_WITH_EXPLICIT_TYPES', null);

class DynamicConstantClass
{
const DYNAMIC_CONSTANT_IN_CLASS = 'abcdef';
const DYNAMIC_CONSTANT_WITH_EXPLICIT_TYPES_IN_CLASS = 'xyz';
const PURE_CONSTANT_IN_CLASS = 'abc123def';
}

Expand Down
Loading