Skip to content

Test the ability to define global types with ExpressionTypeResolverExtension #4233

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

Closed
wants to merge 4 commits into from
Closed

Test the ability to define global types with ExpressionTypeResolverExtension #4233

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c15b9a2
Permit to define global types with `ExpressionTypeResolverExtension`
cedric-anne Aug 10, 2025
9d81092
revert useless change
cedric-anne Aug 10, 2025
8793f30
Add ability to check if a variable is global in scope
cedric-anne Aug 12, 2025
f29758a
use returned value
cedric-anne Aug 12, 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
2 changes: 2 additions & 0 deletions src/Analyser/DirectInternalScopeFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ public function create(
bool $afterExtractCall = false,
?Scope $parentScope = null,
bool $nativeTypesPromoted = false,
array $globalVariables = [],
): MutatingScope
{
return new MutatingScope(
Expand Down Expand Up @@ -90,6 +91,7 @@ public function create(
$afterExtractCall,
$parentScope,
$nativeTypesPromoted,
$globalVariables,
);
}

Expand Down
2 changes: 2 additions & 0 deletions src/Analyser/InternalScopeFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ interface InternalScopeFactory
* @param array<string, true> $currentlyAssignedExpressions
* @param array<string, true> $currentlyAllowedUndefinedExpressions
* @param list<array{FunctionReflection|MethodReflection|null, ParameterReflection|null}> $inFunctionCallsStack
* @param list<string> $globalVariables
*/
public function create(
ScopeContext $context,
Expand All @@ -37,6 +38,7 @@ public function create(
bool $afterExtractCall = false,
?Scope $parentScope = null,
bool $nativeTypesPromoted = false,
array $globalVariables = [],
): MutatingScope;

}
2 changes: 2 additions & 0 deletions src/Analyser/LazyInternalScopeFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public function create(
bool $afterExtractCall = false,
?Scope $parentScope = null,
bool $nativeTypesPromoted = false,
array $globalVariables = [],
): MutatingScope
{
return new MutatingScope(
Expand Down Expand Up @@ -80,6 +81,7 @@ public function create(
$afterExtractCall,
$parentScope,
$nativeTypesPromoted,
$globalVariables,
);
}

Expand Down
52 changes: 48 additions & 4 deletions src/Analyser/MutatingScope.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ final class MutatingScope implements Scope
* @param array<string, true> $currentlyAllowedUndefinedExpressions
* @param array<string, ExpressionTypeHolder> $nativeExpressionTypes
* @param list<array{MethodReflection|FunctionReflection|null, ParameterReflection|null}> $inFunctionCallsStack
* @param list<string> $globalVariables
*/
public function __construct(
private InternalScopeFactory $scopeFactory,
Expand Down Expand Up @@ -235,6 +236,7 @@ public function __construct(
private bool $afterExtractCall = false,
private ?Scope $parentScope = null,
private bool $nativeTypesPromoted = false,
private array $globalVariables = [],
)
{
if ($namespace === '') {
Expand Down Expand Up @@ -363,6 +365,7 @@ public function rememberConstructorScope(): self
$this->afterExtractCall,
$this->parentScope,
$this->nativeTypesPromoted,
$this->globalVariables,
);
}

Expand Down Expand Up @@ -441,6 +444,7 @@ public function afterExtractCall(): self
true,
$this->parentScope,
$this->nativeTypesPromoted,
$this->globalVariables,
);
}

Expand Down Expand Up @@ -497,6 +501,7 @@ public function afterClearstatcacheCall(): self
$this->afterExtractCall,
$this->parentScope,
$this->nativeTypesPromoted,
$this->globalVariables,
);
}

Expand Down Expand Up @@ -579,13 +584,14 @@ public function afterOpenSslCall(string $openSslFunctionName): self
$this->afterExtractCall,
$this->parentScope,
$this->nativeTypesPromoted,
$this->globalVariables,
);
}

/** @api */
public function hasVariableType(string $variableName): TrinaryLogic
{
if ($this->isGlobalVariable($variableName)) {
if ($this->isSuperGlobalVariable($variableName)) {
return TrinaryLogic::createYes();
}

Expand Down Expand Up @@ -626,7 +632,7 @@ public function getVariableType(string $variableName): Type

$varExprString = '$' . $variableName;
if (!array_key_exists($varExprString, $this->expressionTypes)) {
if ($this->isGlobalVariable($variableName)) {
if ($this->isSuperGlobalVariable($variableName)) {
return new ArrayType(new BenevolentUnionType([new IntegerType(), new StringType()]), new MixedType(true));
}
return new MixedType();
Expand All @@ -635,6 +641,18 @@ public function getVariableType(string $variableName): Type
return TypeUtils::resolveLateResolvableTypes($this->expressionTypes[$varExprString]->getType());
}

public function setVariableAsGlobal(string $variableName): self
{
$this->globalVariables[] = $variableName;

return $this;
}

public function isGlobalVariable(string $variableName): bool
{
return in_array($variableName, $this->globalVariables, true);
}

/**
* @api
* @return list<string>
Expand Down Expand Up @@ -677,7 +695,7 @@ public function getMaybeDefinedVariables(): array
return $variables;
}

private function isGlobalVariable(string $variableName): bool
private function isSuperGlobalVariable(string $variableName): bool
{
return in_array($variableName, self::SUPERGLOBAL_VARIABLES, true);
}
Expand Down Expand Up @@ -2762,6 +2780,7 @@ private function promoteNativeTypes(): self
$this->afterExtractCall,
$this->parentScope,
true,
$this->globalVariables,
);
}

Expand Down Expand Up @@ -2956,6 +2975,7 @@ public function pushInFunctionCall($reflection, ?ParameterReflection $parameter)
$this->afterExtractCall,
$this->parentScope,
$this->nativeTypesPromoted,
$this->globalVariables,
);
}

Expand All @@ -2981,6 +3001,7 @@ public function popInFunctionCall(): self
$this->afterExtractCall,
$this->parentScope,
$this->nativeTypesPromoted,
$this->globalVariables,
);
}

Expand Down Expand Up @@ -3572,6 +3593,7 @@ public function restoreThis(self $restoreThisScope): self
$this->afterExtractCall,
$this->parentScope,
$this->nativeTypesPromoted,
$this->globalVariables,
);
}

Expand Down Expand Up @@ -3635,6 +3657,7 @@ public function enterAnonymousFunction(
false,
$this,
$this->nativeTypesPromoted,
[],
);
}

Expand Down Expand Up @@ -3743,6 +3766,7 @@ private function enterAnonymousFunctionWithoutReflection(
false,
$this,
$this->nativeTypesPromoted,
[],
);
}

Expand Down Expand Up @@ -3811,6 +3835,7 @@ public function enterArrowFunction(Expr\ArrowFunction $arrowFunction, ?array $ca
$scope->afterExtractCall,
$scope->parentScope,
$this->nativeTypesPromoted,
[],
);
}

Expand Down Expand Up @@ -3870,6 +3895,7 @@ private function enterArrowFunctionWithoutReflection(Expr\ArrowFunction $arrowFu
$arrowFunctionScope->afterExtractCall,
$arrowFunctionScope->parentScope,
$this->nativeTypesPromoted,
[],
);
}

Expand Down Expand Up @@ -4033,6 +4059,7 @@ public function enterExpressionAssign(Expr $expr): self
$this->afterExtractCall,
$this->parentScope,
$this->nativeTypesPromoted,
$this->globalVariables,
);
$scope->resolvedTypes = $this->resolvedTypes;
$scope->truthyScopes = $this->truthyScopes;
Expand Down Expand Up @@ -4064,6 +4091,7 @@ public function exitExpressionAssign(Expr $expr): self
$this->afterExtractCall,
$this->parentScope,
$this->nativeTypesPromoted,
$this->globalVariables,
);
$scope->resolvedTypes = $this->resolvedTypes;
$scope->truthyScopes = $this->truthyScopes;
Expand Down Expand Up @@ -4106,6 +4134,7 @@ public function setAllowedUndefinedExpression(Expr $expr): self
$this->afterExtractCall,
$this->parentScope,
$this->nativeTypesPromoted,
$this->globalVariables,
);
$scope->resolvedTypes = $this->resolvedTypes;
$scope->truthyScopes = $this->truthyScopes;
Expand Down Expand Up @@ -4137,6 +4166,7 @@ public function unsetAllowedUndefinedExpression(Expr $expr): self
$this->afterExtractCall,
$this->parentScope,
$this->nativeTypesPromoted,
$this->globalVariables,
);
$scope->resolvedTypes = $this->resolvedTypes;
$scope->truthyScopes = $this->truthyScopes;
Expand Down Expand Up @@ -4284,6 +4314,7 @@ public function specifyExpressionType(Expr $expr, Type $type, Type $nativeType,
$this->afterExtractCall,
$this->parentScope,
$this->nativeTypesPromoted,
$this->globalVariables,
);

if ($expr instanceof AlwaysRememberedExpr) {
Expand Down Expand Up @@ -4393,6 +4424,7 @@ public function invalidateExpression(Expr $expressionToInvalidate, bool $require
$this->afterExtractCall,
$this->parentScope,
$this->nativeTypesPromoted,
$this->globalVariables,
);
}

Expand Down Expand Up @@ -4493,6 +4525,7 @@ private function invalidateMethodsOnExpression(Expr $expressionToInvalidate): se
$this->afterExtractCall,
$this->parentScope,
$this->nativeTypesPromoted,
$this->globalVariables,
);
}

Expand Down Expand Up @@ -4705,6 +4738,7 @@ public function filterBySpecifiedTypes(SpecifiedTypes $specifiedTypes): self
$scope->afterExtractCall,
$scope->parentScope,
$scope->nativeTypesPromoted,
$this->globalVariables,
);
}

Expand Down Expand Up @@ -4732,6 +4766,7 @@ public function addConditionalExpressions(string $exprString, array $conditional
$this->afterExtractCall,
$this->parentScope,
$this->nativeTypesPromoted,
$this->globalVariables,
);
}

Expand Down Expand Up @@ -4762,6 +4797,7 @@ public function exitFirstLevelStatements(): self
$this->afterExtractCall,
$this->parentScope,
$this->nativeTypesPromoted,
$this->globalVariables,
);
$scope->resolvedTypes = $this->resolvedTypes;
$scope->truthyScopes = $this->truthyScopes;
Expand Down Expand Up @@ -4799,6 +4835,9 @@ public function mergeWith(?self $otherScope): self
$ourExpressionTypes,
$mergedExpressionTypes,
);

$mergedGlobalVariables = array_merge($this->globalVariables, $otherScope->globalVariables);

return $this->scopeFactory->create(
$this->context,
$this->isDeclareStrictTypes(),
Expand All @@ -4816,6 +4855,7 @@ public function mergeWith(?self $otherScope): self
$this->afterExtractCall && $otherScope->afterExtractCall,
$this->parentScope,
$this->nativeTypesPromoted,
$mergedGlobalVariables,
);
}

Expand Down Expand Up @@ -4929,7 +4969,7 @@ private function createConditionalExpressions(
private function mergeVariableHolders(array $ourVariableTypeHolders, array $theirVariableTypeHolders): array
{
$intersectedVariableTypeHolders = [];
$globalVariableCallback = fn (Node $node) => $node instanceof Variable && is_string($node->name) && $this->isGlobalVariable($node->name);
$globalVariableCallback = fn (Node $node) => $node instanceof Variable && is_string($node->name) && $this->isSuperGlobalVariable($node->name);
$nodeFinder = new NodeFinder();
foreach ($ourVariableTypeHolders as $exprString => $variableTypeHolder) {
if (isset($theirVariableTypeHolders[$exprString])) {
Expand Down Expand Up @@ -5020,6 +5060,7 @@ public function processFinallyScope(self $finallyScope, self $originalFinallySco
$this->afterExtractCall,
$this->parentScope,
$this->nativeTypesPromoted,
$this->globalVariables,
);
}

Expand Down Expand Up @@ -5115,6 +5156,7 @@ public function processClosureScope(
$this->afterExtractCall,
$this->parentScope,
$this->nativeTypesPromoted,
$this->globalVariables,
);
}

Expand Down Expand Up @@ -5164,6 +5206,7 @@ public function processAlwaysIterableForeachScopeWithoutPollute(self $finalScope
$this->afterExtractCall,
$this->parentScope,
$this->nativeTypesPromoted,
$this->globalVariables,
);
}

Expand Down Expand Up @@ -5195,6 +5238,7 @@ public function generalizeWith(self $otherScope): self
$this->afterExtractCall,
$this->parentScope,
$this->nativeTypesPromoted,
$this->globalVariables,
);
}

Expand Down
2 changes: 2 additions & 0 deletions src/Analyser/NodeScopeResolver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1966,6 +1966,8 @@ static function (Node $node, Scope $scope) use ($nodeCallback): void {
}

$scope = $scope->assignVariable($var->name, new MixedType(), new MixedType(), TrinaryLogic::createYes());
Copy link
Contributor

@staabm staabm Aug 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

are you sure this change is necessary?

I think it causes the test errors

Copy link
Contributor

Choose a reason for hiding this comment

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

I assume without this it's always considered as MixedType and do not use the ExpressionTypeResolverExtension

Looking at the failing test, it need to add an extra check to transform ErrorType into MixedType.

But I think the scope::getType is "just" here to apply

foreach ($this->expressionTypeResolverExtensionRegistry->getExtensions() as $extension) {
			$type = $extension->getType($var, $scope);
			if ($type !== null) {
				return $type;
			}
		}

so it seems better to write

$type = null;
foreach ($this->expressionTypeResolverExtensionRegistry->getExtensions() as $extension) {
     $type = $extension->getType($var, $scope);
     if ($type !== null) {
        break;
      }
}

$scope = $scope->assignVariable($var->name, $type ?? new MixedType(), new MixedType(), TrinaryLogic::createYes());

Copy link
Contributor

Choose a reason for hiding this comment

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

the PRs tests do not fail when we delete this change here, therefore it seems either the tests are wonky or phpstan already works like expected before this PR

Copy link
Author

Choose a reason for hiding this comment

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

Indeed. It looks like I missed something. At first I did non found in the documentation how to specify global types, so I did some reverse engeneering to see if it was possible. I understood the modified line as a force MixedType that cannot be altered by any extension, but it seems that I was wrong. I will do some tests on my own extension, to see if I can achieve what I want to do.

Also, I did not found any documentation related to the usage of ExpressionTypeResolverExtension. Maybe it should be documented, as it can be really usefull for extension developers.

Copy link
Author

Choose a reason for hiding this comment

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

The ExpressionTypeResolverExtension works as expected, as it permits to force any type during a variable declaration, but it does not completely fits my needs as I am not able to differenciate a global var statement from a function parameter.

Indeed both global $MY_VAR; and function foo ($MY_VAR) {} are similar from the extension point of view. However, it is probably out of scope of the current PR and I will use some hacks to achieve my goals.

I removed the modified line. Feel free to close this PR if the new test is considered useless.

Copy link
Contributor

@staabm staabm Aug 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@cedric-anne please add the example (and failling assertions) which does not work as expected into your test-case of this PR, so we can look into what needs to be done to make it pass

Copy link
Author

Choose a reason for hiding this comment

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

The failing test case would be the following:

function bar() {
    global $MY_FRAMEWORK_GLOBAL;
    // This one works as expected.
    assertType('mixed', $MY_FRAMEWORK_GLOBAL);
}

function foo($MY_FRAMEWORK_GLOBAL) {
    // I have no idea how I could distinguish the `$MY_FRAMEWORK_GLOBAL` parameter definition
    // from a `global $MY_FRAMEWORK_GLOBAL` statement in my `GlobalExpressionTypeResolverExtension::getType()` method.
    //
    // The following assert with fail, as the extension currently defines it as a `bool`.
    assertType('mixed', $MY_FRAMEWORK_GLOBAL);
}

As far as I understand, it is not possible to access the parent PhpParser\Node\Stmt\Global_ statement of the PhpParser\Node\Expr\Variable expression. Without this, there is no way to distinguish a global statement like the one in the bar() function from a parameter definition like the one in the foo() function.

IMHO, we can consider that the extension works as expected, from the PHPStan point of view. There is no bug here, just a limitation.

Copy link
Contributor

@staabm staabm Aug 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think it might be possible to turn $scope->isGlobalVariable() public, after we added support for recognizing the global keyword.

one idea which comes to mind is, to mark variables which are preceeded by Global_ AST node with a attribute and in MutatingScope->isGlobalVariable look whether the given attribute is set.
see e.g. src/Parser/ArrayFilterArgVisitor.php for inspiration on how this can be achieved.

beware: ondrej might have a different idea of the problem and might not be happy with the above suggestion :)

Copy link
Author

@cedric-anne cedric-anne Aug 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I tried to set an attribute in a visitor, but I was not able to retrieve it from the scope object.

I tried another solution, byt saving the global variables names in the scope and add a public function isGlobalVariable(string $variableName): bool function, that works as expected, but I cannot tell if this is the right way to do. I am not sure to have properly defined the correct value in every InternalScopeFactory::create() call.

If the solution seems correct, I can try to add some tests.

Maybe it would be preferable to add a new bool $isGlobal parameter to the MutatingScope::assignVariable() rather than adding a new MutatingScope::setVariableAsGlobal(string $variableName) method.

Copy link
Author

Choose a reason for hiding this comment

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

I proposed another solution in #4245. It seems simplier.

$scope = $scope->setVariableAsGlobal($var->name);

$vars[] = $var->name;
}
$scope = $this->processVarAnnotation($scope, $vars, $stmt);
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 @@ -61,6 +61,8 @@ public function hasVariableType(string $variableName): TrinaryLogic;

public function getVariableType(string $variableName): Type;

public function isGlobalVariable(string $variableName): bool;

public function canAnyVariableExist(): bool;

/**
Expand Down
3 changes: 2 additions & 1 deletion tests/PHPStan/Analyser/ExpressionTypeResolverExtensionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ class ExpressionTypeResolverExtensionTest extends TypeInferenceTestCase

public static function dataFileAsserts(): iterable
{
yield from self::gatherAssertTypes(__DIR__ . '/data/expression-type-resolver-extension.php');
yield from self::gatherAssertTypes(__DIR__ . '/data/expression-type-resolver-extension-method-call-returns-bool.php');
yield from self::gatherAssertTypes(__DIR__ . '/data/expression-type-resolver-extension-global-statement.php');
}

/**
Expand Down
Loading
Loading