From b9066bdd99e40940574e76e45f13409e3048e363 Mon Sep 17 00:00:00 2001 From: Martin Herndl Date: Sat, 19 Nov 2022 20:42:18 +0100 Subject: [PATCH 1/6] Improve expression resolving of superglobals --- src/Analyser/MutatingScope.php | 8 ++--- .../Analyser/NodeScopeResolverTest.php | 1 + tests/PHPStan/Analyser/data/superglobals.php | 35 +++++++++++++++++++ 3 files changed, 40 insertions(+), 4 deletions(-) create mode 100644 tests/PHPStan/Analyser/data/superglobals.php diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 2e930bb7ef..1288ea3499 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -502,16 +502,16 @@ 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); } $varExprString = '$' . $variableName; if (!array_key_exists($varExprString, $this->expressionTypes)) { + if ($this->isGlobalVariable($variableName)) { + return new ArrayType(new StringType(), new MixedType($this->explicitMixedForGlobalVariables)); + } + return new MixedType(); } diff --git a/tests/PHPStan/Analyser/NodeScopeResolverTest.php b/tests/PHPStan/Analyser/NodeScopeResolverTest.php index e0a8eaa2cc..a4da25eff9 100644 --- a/tests/PHPStan/Analyser/NodeScopeResolverTest.php +++ b/tests/PHPStan/Analyser/NodeScopeResolverTest.php @@ -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'); diff --git a/tests/PHPStan/Analyser/data/superglobals.php b/tests/PHPStan/Analyser/data/superglobals.php new file mode 100644 index 0000000000..d70d1a30ad --- /dev/null +++ b/tests/PHPStan/Analyser/data/superglobals.php @@ -0,0 +1,35 @@ +', $GLOBALS); + assertType('array', $_SERVER); + assertType('array', $_GET); + assertType('array', $_POST); + assertType('array', $_FILES); + assertType('array', $_COOKIE); + assertType('array', $_SESSION); + assertType('array', $_REQUEST); + assertType('array', $_ENV); + } + + public function canBeOverwritten(): void + { + $_SESSION = []; + assertType('array{}', $_SESSION); + } + + public function canBePartlyOverwritten(): void + { + $_SESSION['foo'] = 'foo'; + assertType("non-empty-array&hasOffsetValue('foo', 'foo')", $_SESSION); + } + +} From 993581bae549e1116f7522d6a3936623b82cba1b Mon Sep 17 00:00:00 2001 From: Martin Herndl Date: Sun, 20 Nov 2022 01:27:25 +0100 Subject: [PATCH 2/6] Add superglobal variable expressions into scope by default --- conf/config.neon | 2 + src/Analyser/MutatingScope.php | 56 ++++++++------------ src/Analyser/ScopeFactory.php | 32 ++++++++++- src/Testing/PHPStanTestCase.php | 1 + tests/PHPStan/Analyser/data/superglobals.php | 32 +++++++++-- 5 files changed, 84 insertions(+), 39 deletions(-) diff --git a/conf/config.neon b/conf/config.neon index bd50802659..4403d57dcb 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -605,6 +605,8 @@ services: - class: PHPStan\Analyser\ScopeFactory + arguments: + explicitMixedForGlobalVariables: %featureToggles.explicitMixedForGlobalVariables% - class: PHPStan\Analyser\NodeScopeResolver diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 1288ea3499..6727382600 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -468,10 +468,6 @@ public function afterOpenSslCall(string $openSslFunctionName): self /** @api */ public function hasVariableType(string $variableName): TrinaryLogic { - if ($this->isGlobalVariable($variableName)) { - return TrinaryLogic::createYes(); - } - $varExprString = '$' . $variableName; if (!isset($this->expressionTypes[$varExprString])) { if ($this->canAnyVariableExist()) { @@ -508,10 +504,6 @@ public function getVariableType(string $variableName): Type $varExprString = '$' . $variableName; if (!array_key_exists($varExprString, $this->expressionTypes)) { - if ($this->isGlobalVariable($variableName)) { - return new ArrayType(new StringType(), new MixedType($this->explicitMixedForGlobalVariables)); - } - return new MixedType(); } @@ -539,21 +531,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 { @@ -2470,12 +2447,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->getSuperglobalExpressionTypes(), $this->getConstantTypes(), ['$this' => $thisHolder]), + array_merge($this->getSuperglobalExpressionTypes(), $this->getNativeConstantTypes(), ['$this' => $thisHolder]), [], null, null, @@ -2711,8 +2684,8 @@ private function enterFunctionLike( $this->isDeclareStrictTypes(), $functionReflection, $this->getNamespace(), - array_merge($this->getConstantTypes(), $expressionTypes), - array_merge($this->getNativeConstantTypes(), $nativeExpressionTypes), + array_merge($this->getSuperglobalExpressionTypes(), $this->getConstantTypes(), $expressionTypes), + array_merge($this->getSuperglobalExpressionTypes(), $this->getNativeConstantTypes(), $nativeExpressionTypes), ); } @@ -2724,6 +2697,8 @@ public function enterNamespace(string $namespaceName): self $this->isDeclareStrictTypes(), null, $namespaceName, + $this->getSuperglobalExpressionTypes(), + $this->getSuperglobalExpressionTypes(), ); } @@ -2947,8 +2922,8 @@ private function enterAnonymousFunctionWithoutReflection( $this->isDeclareStrictTypes(), $this->getFunction(), $this->getNamespace(), - array_merge($this->getConstantTypes(), $expressionTypes), - array_merge($this->getNativeConstantTypes(), $nativeTypes), + array_merge($this->getSuperglobalExpressionTypes(), $this->getConstantTypes(), $expressionTypes), + array_merge($this->getSuperglobalExpressionTypes(), $this->getNativeConstantTypes(), $nativeTypes), [], $this->inClosureBindScopeClass, new TrivialParametersAcceptor(), @@ -5021,4 +4996,19 @@ private function getNativeConstantTypes(): array return $constantTypes; } + /** @return array */ + private function getSuperglobalExpressionTypes(): array + { + $superglobalExpressionTypes = []; + $exprStrings = ['$GLOBALS', '$_SERVER', '$_GET', '$_POST', '$_FILES', '$_COOKIE', '$_SESSION', '$_REQUEST', '$_ENV']; + foreach ($this->expressionTypes as $exprString => $typeHolder) { + if (!in_array($exprString, $exprStrings, true)) { + continue; + } + + $superglobalExpressionTypes[$exprString] = $typeHolder; + } + return $superglobalExpressionTypes; + } + } diff --git a/src/Analyser/ScopeFactory.php b/src/Analyser/ScopeFactory.php index c2401d375c..ea4c761e32 100644 --- a/src/Analyser/ScopeFactory.php +++ b/src/Analyser/ScopeFactory.php @@ -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)); + $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, + ); } } diff --git a/src/Testing/PHPStanTestCase.php b/src/Testing/PHPStanTestCase.php index 3177fef05e..d35a589ff0 100644 --- a/src/Testing/PHPStanTestCase.php +++ b/src/Testing/PHPStanTestCase.php @@ -181,6 +181,7 @@ public function createScopeFactory(ReflectionProvider $reflectionProvider, TypeS $container->getParameter('featureToggles')['explicitMixedForGlobalVariables'], $constantResolver, ), + $container->getParameter('featureToggles')['explicitMixedForGlobalVariables'], ); } diff --git a/tests/PHPStan/Analyser/data/superglobals.php b/tests/PHPStan/Analyser/data/superglobals.php index d70d1a30ad..2885d59f04 100644 --- a/tests/PHPStan/Analyser/data/superglobals.php +++ b/tests/PHPStan/Analyser/data/superglobals.php @@ -2,6 +2,7 @@ namespace Superglobals; +use function PHPStan\Testing\assertNativeType; use function PHPStan\Testing\assertType; class Superglobals @@ -22,14 +23,37 @@ public function originalTypes(): void public function canBeOverwritten(): void { - $_SESSION = []; - assertType('array{}', $_SESSION); + $GLOBALS = []; + assertType('array{}', $GLOBALS); + assertNativeType('array{}', $GLOBALS); } public function canBePartlyOverwritten(): void { - $_SESSION['foo'] = 'foo'; - assertType("non-empty-array&hasOffsetValue('foo', 'foo')", $_SESSION); + $GLOBALS['foo'] = 'foo'; + assertType("non-empty-array&hasOffsetValue('foo', 'foo')", $GLOBALS); + assertNativeType("non-empty-array&hasOffsetValue('foo', 'foo')", $GLOBALS); + } + + public function canBeNarrowed(): void + { + if (isset($GLOBALS['foo'])) { + assertType("array&hasOffsetValue('foo', mixed~null)", $GLOBALS); + assertNativeType("array&hasOffset('foo')", $GLOBALS); // https://github.com/phpstan/phpstan/issues/8395 + } else { + assertType('array', $GLOBALS); + assertNativeType('array', $GLOBALS); + } + assertType('array', $GLOBALS); + assertNativeType('array', $GLOBALS); } } + +function functionScope() { + assertType('array', $GLOBALS); + assertNativeType('array', $GLOBALS); +} + +assertType('array', $GLOBALS); +assertNativeType('array', $GLOBALS); From 7636d8480dd414e4cd24dce9f78fb5425d31fe15 Mon Sep 17 00:00:00 2001 From: Martin Herndl Date: Sun, 20 Nov 2022 21:39:05 +0100 Subject: [PATCH 3/6] Remove unused `$explicitMixedForGlobalVariables` arg from `MutatingScope` --- src/Analyser/DirectInternalScopeFactory.php | 2 -- src/Analyser/LazyInternalScopeFactory.php | 4 ---- src/Analyser/MutatingScope.php | 2 -- src/Testing/PHPStanTestCase.php | 1 - 4 files changed, 9 deletions(-) diff --git a/src/Analyser/DirectInternalScopeFactory.php b/src/Analyser/DirectInternalScopeFactory.php index e2b88a406d..8a0f72cdf0 100644 --- a/src/Analyser/DirectInternalScopeFactory.php +++ b/src/Analyser/DirectInternalScopeFactory.php @@ -34,7 +34,6 @@ public function __construct( private bool $treatPhpDocTypesAsCertain, private PhpVersion $phpVersion, private bool $explicitMixedInUnknownGenericNew, - private bool $explicitMixedForGlobalVariables, private ConstantResolver $constantResolver, ) { @@ -102,7 +101,6 @@ public function create( $parentScope, $nativeTypesPromoted, $this->explicitMixedInUnknownGenericNew, - $this->explicitMixedForGlobalVariables, ); } diff --git a/src/Analyser/LazyInternalScopeFactory.php b/src/Analyser/LazyInternalScopeFactory.php index bd8d6d92fd..f838208d60 100644 --- a/src/Analyser/LazyInternalScopeFactory.php +++ b/src/Analyser/LazyInternalScopeFactory.php @@ -22,8 +22,6 @@ class LazyInternalScopeFactory implements InternalScopeFactory private bool $explicitMixedInUnknownGenericNew; - private bool $explicitMixedForGlobalVariables; - /** * @param class-string $scopeClass */ @@ -34,7 +32,6 @@ public function __construct( { $this->treatPhpDocTypesAsCertain = $container->getParameter('treatPhpDocTypesAsCertain'); $this->explicitMixedInUnknownGenericNew = $this->container->getParameter('featureToggles')['explicitMixedInUnknownGenericNew']; - $this->explicitMixedForGlobalVariables = $this->container->getParameter('featureToggles')['explicitMixedForGlobalVariables']; } /** @@ -100,7 +97,6 @@ public function create( $parentScope, $nativeTypesPromoted, $this->explicitMixedInUnknownGenericNew, - $this->explicitMixedForGlobalVariables, ); } diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 6727382600..69d47990e0 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -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 === '') { @@ -2150,7 +2149,6 @@ public function doNotTreatPhpDocTypesAsCertain(): Scope $this->parentScope, false, $this->explicitMixedInUnknownGenericNew, - $this->explicitMixedForGlobalVariables, ); } diff --git a/src/Testing/PHPStanTestCase.php b/src/Testing/PHPStanTestCase.php index d35a589ff0..8fe8581f8e 100644 --- a/src/Testing/PHPStanTestCase.php +++ b/src/Testing/PHPStanTestCase.php @@ -178,7 +178,6 @@ 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'], From f1817d186afe46d10c98f0c2c0407b59ce8c0021 Mon Sep 17 00:00:00 2001 From: Martin Herndl Date: Thu, 24 Nov 2022 17:35:56 +0100 Subject: [PATCH 4/6] Handle native superglobals separately --- src/Analyser/MutatingScope.php | 39 +++++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 69d47990e0..95234cc2b2 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -2445,8 +2445,8 @@ public function enterClass(ClassReflection $classReflection): self $this->isDeclareStrictTypes(), null, $this->getNamespace(), - array_merge($this->getSuperglobalExpressionTypes(), $this->getConstantTypes(), ['$this' => $thisHolder]), - array_merge($this->getSuperglobalExpressionTypes(), $this->getNativeConstantTypes(), ['$this' => $thisHolder]), + array_merge($this->getSuperglobalTypes(), $this->getConstantTypes(), ['$this' => $thisHolder]), + array_merge($this->getNativeSuperglobalTypes(), $this->getNativeConstantTypes(), ['$this' => $thisHolder]), [], null, null, @@ -2682,8 +2682,8 @@ private function enterFunctionLike( $this->isDeclareStrictTypes(), $functionReflection, $this->getNamespace(), - array_merge($this->getSuperglobalExpressionTypes(), $this->getConstantTypes(), $expressionTypes), - array_merge($this->getSuperglobalExpressionTypes(), $this->getNativeConstantTypes(), $nativeExpressionTypes), + array_merge($this->getSuperglobalTypes(), $this->getConstantTypes(), $expressionTypes), + array_merge($this->getNativeSuperglobalTypes(), $this->getNativeConstantTypes(), $nativeExpressionTypes), ); } @@ -2695,8 +2695,8 @@ public function enterNamespace(string $namespaceName): self $this->isDeclareStrictTypes(), null, $namespaceName, - $this->getSuperglobalExpressionTypes(), - $this->getSuperglobalExpressionTypes(), + $this->getSuperglobalTypes(), + $this->getNativeSuperglobalTypes(), ); } @@ -2920,8 +2920,8 @@ private function enterAnonymousFunctionWithoutReflection( $this->isDeclareStrictTypes(), $this->getFunction(), $this->getNamespace(), - array_merge($this->getSuperglobalExpressionTypes(), $this->getConstantTypes(), $expressionTypes), - array_merge($this->getSuperglobalExpressionTypes(), $this->getNativeConstantTypes(), $nativeTypes), + array_merge($this->getSuperglobalTypes(), $this->getConstantTypes(), $expressionTypes), + array_merge($this->getNativeSuperglobalTypes(), $this->getNativeConstantTypes(), $nativeTypes), [], $this->inClosureBindScopeClass, new TrivialParametersAcceptor(), @@ -4995,18 +4995,33 @@ private function getNativeConstantTypes(): array } /** @return array */ - private function getSuperglobalExpressionTypes(): array + private function getSuperglobalTypes(): array { - $superglobalExpressionTypes = []; + $superglobalTypes = []; $exprStrings = ['$GLOBALS', '$_SERVER', '$_GET', '$_POST', '$_FILES', '$_COOKIE', '$_SESSION', '$_REQUEST', '$_ENV']; foreach ($this->expressionTypes as $exprString => $typeHolder) { if (!in_array($exprString, $exprStrings, true)) { continue; } - $superglobalExpressionTypes[$exprString] = $typeHolder; + $superglobalTypes[$exprString] = $typeHolder; } - return $superglobalExpressionTypes; + return $superglobalTypes; + } + + /** @return array */ + 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; } } From bb1958acd518b02531397b7aec28ca574a54e143 Mon Sep 17 00:00:00 2001 From: Martin Herndl Date: Fri, 2 Dec 2022 20:08:33 +0100 Subject: [PATCH 5/6] Remove empty phpdoc line who knows, maybe this changes something for rector.. --- src/Analyser/LazyInternalScopeFactory.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Analyser/LazyInternalScopeFactory.php b/src/Analyser/LazyInternalScopeFactory.php index f838208d60..c4307ec820 100644 --- a/src/Analyser/LazyInternalScopeFactory.php +++ b/src/Analyser/LazyInternalScopeFactory.php @@ -41,7 +41,6 @@ public function __construct( * @param array $currentlyAssignedExpressions * @param array $currentlyAllowedUndefinedExpressions * @param array<(FunctionReflection|MethodReflection)> $inFunctionCallsStack - * */ public function create( ScopeContext $context, From e5418079f7c60dfa0d8ce35181e47e76b4c2f787 Mon Sep 17 00:00:00 2001 From: Martin Herndl Date: Fri, 2 Dec 2022 20:16:01 +0100 Subject: [PATCH 6/6] Remove unnecessary PHPDoc from internal scope factories --- src/Analyser/DirectInternalScopeFactory.php | 8 -------- src/Analyser/InternalScopeFactory.php | 2 +- src/Analyser/LazyInternalScopeFactory.php | 8 -------- 3 files changed, 1 insertion(+), 17 deletions(-) diff --git a/src/Analyser/DirectInternalScopeFactory.php b/src/Analyser/DirectInternalScopeFactory.php index 8a0f72cdf0..701b06dbbf 100644 --- a/src/Analyser/DirectInternalScopeFactory.php +++ b/src/Analyser/DirectInternalScopeFactory.php @@ -39,14 +39,6 @@ public function __construct( { } - /** - * @param array $expressionTypes - * @param array $nativeExpressionTypes - * @param array $conditionalExpressions - * @param array<(FunctionReflection|MethodReflection)> $inFunctionCallsStack - * @param array $currentlyAssignedExpressions - * @param array $currentlyAllowedUndefinedExpressions - */ public function create( ScopeContext $context, bool $declareStrictTypes = false, diff --git a/src/Analyser/InternalScopeFactory.php b/src/Analyser/InternalScopeFactory.php index 13ed62fe68..6a19d4cc25 100644 --- a/src/Analyser/InternalScopeFactory.php +++ b/src/Analyser/InternalScopeFactory.php @@ -15,7 +15,7 @@ interface InternalScopeFactory * @param array $conditionalExpressions * @param array $currentlyAssignedExpressions * @param array $currentlyAllowedUndefinedExpressions - * @param array $inFunctionCallsStack + * @param array $inFunctionCallsStack */ public function create( ScopeContext $context, diff --git a/src/Analyser/LazyInternalScopeFactory.php b/src/Analyser/LazyInternalScopeFactory.php index c4307ec820..9bceec4ba3 100644 --- a/src/Analyser/LazyInternalScopeFactory.php +++ b/src/Analyser/LazyInternalScopeFactory.php @@ -34,14 +34,6 @@ public function __construct( $this->explicitMixedInUnknownGenericNew = $this->container->getParameter('featureToggles')['explicitMixedInUnknownGenericNew']; } - /** - * @param array $expressionTypes - * @param array $nativeExpressionTypes - * @param array $conditionalExpressions - * @param array $currentlyAssignedExpressions - * @param array $currentlyAllowedUndefinedExpressions - * @param array<(FunctionReflection|MethodReflection)> $inFunctionCallsStack - */ public function create( ScopeContext $context, bool $declareStrictTypes = false,