Skip to content
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.

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

Improve expression resolving of superglobals #2012

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions conf/config.neon
Expand Up @@ -605,6 +605,8 @@ services:

-
class: PHPStan\Analyser\ScopeFactory
arguments:
explicitMixedForGlobalVariables: %featureToggles.explicitMixedForGlobalVariables%

-
class: PHPStan\Analyser\NodeScopeResolver
Expand Down
10 changes: 0 additions & 10 deletions src/Analyser/DirectInternalScopeFactory.php
Expand Up @@ -34,20 +34,11 @@ public function __construct(
private bool $treatPhpDocTypesAsCertain,
private PhpVersion $phpVersion,
private bool $explicitMixedInUnknownGenericNew,
private bool $explicitMixedForGlobalVariables,
private ConstantResolver $constantResolver,
)
{
}

/**
* @param array<string, ExpressionTypeHolder> $expressionTypes
* @param array<string, ExpressionTypeHolder> $nativeExpressionTypes
* @param array<string, ConditionalExpressionHolder[]> $conditionalExpressions
* @param array<(FunctionReflection|MethodReflection)> $inFunctionCallsStack
* @param array<string, true> $currentlyAssignedExpressions
* @param array<string, true> $currentlyAllowedUndefinedExpressions
*/
public function create(
ScopeContext $context,
bool $declareStrictTypes = false,
Expand Down Expand Up @@ -102,7 +93,6 @@ public function create(
$parentScope,
$nativeTypesPromoted,
$this->explicitMixedInUnknownGenericNew,
$this->explicitMixedForGlobalVariables,
);
}

Expand Down
2 changes: 1 addition & 1 deletion src/Analyser/InternalScopeFactory.php
Expand Up @@ -15,7 +15,7 @@ interface InternalScopeFactory
* @param array<string, ConditionalExpressionHolder[]> $conditionalExpressions
* @param array<string, true> $currentlyAssignedExpressions
* @param array<string, true> $currentlyAllowedUndefinedExpressions
* @param array<MethodReflection|FunctionReflection> $inFunctionCallsStack
* @param array<FunctionReflection|MethodReflection> $inFunctionCallsStack
*/
public function create(
ScopeContext $context,
Expand Down
13 changes: 0 additions & 13 deletions src/Analyser/LazyInternalScopeFactory.php
Expand Up @@ -22,8 +22,6 @@ class LazyInternalScopeFactory implements InternalScopeFactory

private bool $explicitMixedInUnknownGenericNew;

private bool $explicitMixedForGlobalVariables;

/**
* @param class-string $scopeClass
*/
Expand All @@ -34,18 +32,8 @@ public function __construct(
{
$this->treatPhpDocTypesAsCertain = $container->getParameter('treatPhpDocTypesAsCertain');
$this->explicitMixedInUnknownGenericNew = $this->container->getParameter('featureToggles')['explicitMixedInUnknownGenericNew'];
$this->explicitMixedForGlobalVariables = $this->container->getParameter('featureToggles')['explicitMixedForGlobalVariables'];
}

/**
* @param array<string, ExpressionTypeHolder> $expressionTypes
* @param array<string, ExpressionTypeHolder> $nativeExpressionTypes
* @param array<string, ConditionalExpressionHolder[]> $conditionalExpressions
* @param array<string, true> $currentlyAssignedExpressions
* @param array<string, true> $currentlyAllowedUndefinedExpressions
* @param array<(FunctionReflection|MethodReflection)> $inFunctionCallsStack
*
*/
public function create(
ScopeContext $context,
bool $declareStrictTypes = false,
Expand Down Expand Up @@ -100,7 +88,6 @@ public function create(
$parentScope,
$nativeTypesPromoted,
$this->explicitMixedInUnknownGenericNew,
$this->explicitMixedForGlobalVariables,
);
}

Expand Down
73 changes: 38 additions & 35 deletions src/Analyser/MutatingScope.php
Expand Up @@ -190,7 +190,6 @@ public function __construct(
private ?Scope $parentScope = null,
private bool $nativeTypesPromoted = false,
private bool $explicitMixedInUnknownGenericNew = false,
private bool $explicitMixedForGlobalVariables = false,
)
{
if ($namespace === '') {
Expand Down Expand Up @@ -468,10 +467,6 @@ public function afterOpenSslCall(string $openSslFunctionName): self
/** @api */
public function hasVariableType(string $variableName): TrinaryLogic
{
if ($this->isGlobalVariable($variableName)) {
return TrinaryLogic::createYes();
herndlm marked this conversation as resolved.
Show resolved Hide resolved
}

$varExprString = '$' . $variableName;
if (!isset($this->expressionTypes[$varExprString])) {
if ($this->canAnyVariableExist()) {
Expand Down Expand Up @@ -502,10 +497,6 @@ public function getVariableType(string $variableName): Type
}
}

if ($this->isGlobalVariable($variableName)) {
return new ArrayType(new StringType(), new MixedType($this->explicitMixedForGlobalVariables));
}

if ($this->hasVariableType($variableName)->no()) {
throw new UndefinedVariableException($this, $variableName);
}
Expand Down Expand Up @@ -539,21 +530,6 @@ public function getDefinedVariables(): array
return $variables;
}

private function isGlobalVariable(string $variableName): bool
{
return in_array($variableName, [
'GLOBALS',
'_SERVER',
'_GET',
'_POST',
'_FILES',
'_COOKIE',
'_SESSION',
'_REQUEST',
'_ENV',
], true);
}

/** @api */
public function hasConstant(Name $name): bool
{
Expand Down Expand Up @@ -2173,7 +2149,6 @@ public function doNotTreatPhpDocTypesAsCertain(): Scope
$this->parentScope,
false,
$this->explicitMixedInUnknownGenericNew,
$this->explicitMixedForGlobalVariables,
);
}

Expand Down Expand Up @@ -2470,12 +2445,8 @@ public function enterClass(ClassReflection $classReflection): self
$this->isDeclareStrictTypes(),
null,
$this->getNamespace(),
array_merge($this->getConstantTypes(), [
'$this' => $thisHolder,
]),
array_merge($this->getNativeConstantTypes(), [
'$this' => $thisHolder,
]),
array_merge($this->getSuperglobalTypes(), $this->getConstantTypes(), ['$this' => $thisHolder]),
array_merge($this->getNativeSuperglobalTypes(), $this->getNativeConstantTypes(), ['$this' => $thisHolder]),
[],
null,
null,
Expand Down Expand Up @@ -2711,8 +2682,8 @@ private function enterFunctionLike(
$this->isDeclareStrictTypes(),
$functionReflection,
$this->getNamespace(),
array_merge($this->getConstantTypes(), $expressionTypes),
array_merge($this->getNativeConstantTypes(), $nativeExpressionTypes),
array_merge($this->getSuperglobalTypes(), $this->getConstantTypes(), $expressionTypes),
array_merge($this->getNativeSuperglobalTypes(), $this->getNativeConstantTypes(), $nativeExpressionTypes),
);
}

Expand All @@ -2724,6 +2695,8 @@ public function enterNamespace(string $namespaceName): self
$this->isDeclareStrictTypes(),
null,
$namespaceName,
$this->getSuperglobalTypes(),
$this->getNativeSuperglobalTypes(),
);
}

Expand Down Expand Up @@ -2947,8 +2920,8 @@ private function enterAnonymousFunctionWithoutReflection(
$this->isDeclareStrictTypes(),
$this->getFunction(),
$this->getNamespace(),
array_merge($this->getConstantTypes(), $expressionTypes),
array_merge($this->getNativeConstantTypes(), $nativeTypes),
array_merge($this->getSuperglobalTypes(), $this->getConstantTypes(), $expressionTypes),
array_merge($this->getNativeSuperglobalTypes(), $this->getNativeConstantTypes(), $nativeTypes),
[],
$this->inClosureBindScopeClass,
new TrivialParametersAcceptor(),
Expand Down Expand Up @@ -5021,4 +4994,34 @@ private function getNativeConstantTypes(): array
return $constantTypes;
}

/** @return array<string, ExpressionTypeHolder> */
private function getSuperglobalTypes(): array
{
$superglobalTypes = [];
$exprStrings = ['$GLOBALS', '$_SERVER', '$_GET', '$_POST', '$_FILES', '$_COOKIE', '$_SESSION', '$_REQUEST', '$_ENV'];
foreach ($this->expressionTypes as $exprString => $typeHolder) {
herndlm marked this conversation as resolved.
Show resolved Hide resolved
if (!in_array($exprString, $exprStrings, true)) {
continue;
}

$superglobalTypes[$exprString] = $typeHolder;
}
return $superglobalTypes;
}

/** @return array<string, ExpressionTypeHolder> */
private function getNativeSuperglobalTypes(): array
{
$superglobalTypes = [];
$exprStrings = ['$GLOBALS', '$_SERVER', '$_GET', '$_POST', '$_FILES', '$_COOKIE', '$_SESSION', '$_REQUEST', '$_ENV'];
foreach ($this->nativeExpressionTypes as $exprString => $typeHolder) {
if (!in_array($exprString, $exprStrings, true)) {
continue;
}

$superglobalTypes[$exprString] = $typeHolder;
}
return $superglobalTypes;
}

}
32 changes: 30 additions & 2 deletions src/Analyser/ScopeFactory.php
Expand Up @@ -2,17 +2,45 @@

namespace PHPStan\Analyser;

use PhpParser\Node\Expr\Variable;
use PHPStan\Type\ArrayType;
use PHPStan\Type\MixedType;
use PHPStan\Type\StringType;

/** @api */
class ScopeFactory
{

public function __construct(private InternalScopeFactory $internalScopeFactory)
public function __construct(
private InternalScopeFactory $internalScopeFactory,
private bool $explicitMixedForGlobalVariables,
)
{
}

public function create(ScopeContext $context): MutatingScope
{
return $this->internalScopeFactory->create($context);
$superglobalType = new ArrayType(new StringType(), new MixedType($this->explicitMixedForGlobalVariables));
herndlm marked this conversation as resolved.
Show resolved Hide resolved
$expressionTypes = [
'$GLOBALS' => ExpressionTypeHolder::createYes(new Variable('GLOBALS'), $superglobalType),
'$_SERVER' => ExpressionTypeHolder::createYes(new Variable('_SERVER'), $superglobalType),
'$_GET' => ExpressionTypeHolder::createYes(new Variable('_GET'), $superglobalType),
'$_POST' => ExpressionTypeHolder::createYes(new Variable('_POST'), $superglobalType),
'$_FILES' => ExpressionTypeHolder::createYes(new Variable('_FILES'), $superglobalType),
'$_COOKIE' => ExpressionTypeHolder::createYes(new Variable('_COOKIE'), $superglobalType),
'$_SESSION' => ExpressionTypeHolder::createYes(new Variable('_SESSION'), $superglobalType),
'$_REQUEST' => ExpressionTypeHolder::createYes(new Variable('_REQUEST'), $superglobalType),
'$_ENV' => ExpressionTypeHolder::createYes(new Variable('_ENV'), $superglobalType),
];

return $this->internalScopeFactory->create(
$context,
false,
null,
null,
$expressionTypes,
$expressionTypes,
);
}

}
2 changes: 1 addition & 1 deletion src/Testing/PHPStanTestCase.php
Expand Up @@ -178,9 +178,9 @@ public function createScopeFactory(ReflectionProvider $reflectionProvider, TypeS
$this->shouldTreatPhpDocTypesAsCertain(),
$container->getByType(PhpVersion::class),
$container->getParameter('featureToggles')['explicitMixedInUnknownGenericNew'],
$container->getParameter('featureToggles')['explicitMixedForGlobalVariables'],
$constantResolver,
),
$container->getParameter('featureToggles')['explicitMixedForGlobalVariables'],
);
}

Expand Down
1 change: 1 addition & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
Expand Up @@ -457,6 +457,7 @@ public function dataFileAsserts(): iterable
yield from $this->gatherAssertTypes(__DIR__ . '/data/closure-types.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-5219.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/strval.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/superglobals.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/array-next.php');

yield from $this->gatherAssertTypes(__DIR__ . '/data/non-empty-string.php');
Expand Down
59 changes: 59 additions & 0 deletions tests/PHPStan/Analyser/data/superglobals.php
@@ -0,0 +1,59 @@
<?php declare(strict_types=1);

namespace Superglobals;

use function PHPStan\Testing\assertNativeType;
use function PHPStan\Testing\assertType;

class Superglobals
{

public function originalTypes(): void
{
assertType('array<string, mixed>', $GLOBALS);
assertType('array<string, mixed>', $_SERVER);
assertType('array<string, mixed>', $_GET);
assertType('array<string, mixed>', $_POST);
assertType('array<string, mixed>', $_FILES);
assertType('array<string, mixed>', $_COOKIE);
assertType('array<string, mixed>', $_SESSION);
assertType('array<string, mixed>', $_REQUEST);
assertType('array<string, mixed>', $_ENV);
}

public function canBeOverwritten(): void
{
$GLOBALS = [];
assertType('array{}', $GLOBALS);
assertNativeType('array{}', $GLOBALS);
}

public function canBePartlyOverwritten(): void
{
$GLOBALS['foo'] = 'foo';
assertType("non-empty-array<string, mixed>&hasOffsetValue('foo', 'foo')", $GLOBALS);
assertNativeType("non-empty-array<string, mixed>&hasOffsetValue('foo', 'foo')", $GLOBALS);
}

public function canBeNarrowed(): void
{
if (isset($GLOBALS['foo'])) {
assertType("array<string, mixed>&hasOffsetValue('foo', mixed~null)", $GLOBALS);
assertNativeType("array<string, mixed>&hasOffset('foo')", $GLOBALS); // https://github.com/phpstan/phpstan/issues/8395
} else {
assertType('array<string, mixed>', $GLOBALS);
assertNativeType('array<string, mixed>', $GLOBALS);
}
assertType('array<string, mixed>', $GLOBALS);
assertNativeType('array<string, mixed>', $GLOBALS);
}

}

function functionScope() {
assertType('array<string, mixed>', $GLOBALS);
assertNativeType('array<string, mixed>', $GLOBALS);
}

assertType('array<string, mixed>', $GLOBALS);
assertNativeType('array<string, mixed>', $GLOBALS);