From d3e7b9cd30d1fe0a323ec42d75dc74821a0def84 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes <ondrej@mirtes.cz> Date: Sat, 24 Oct 2020 14:07:54 +0200 Subject: [PATCH] Scope - any variable after extract() call might exist --- src/Analyser/DirectScopeFactory.php | 3 ++ src/Analyser/LazyScopeFactory.php | 3 ++ src/Analyser/MutatingScope.php | 53 +++++++++++++++++-- src/Analyser/NodeScopeResolver.php | 4 ++ src/Analyser/ScopeFactory.php | 2 + .../Analyser/NodeScopeResolverTest.php | 6 +++ tests/PHPStan/Analyser/data/bug-3990.php | 21 ++++++++ 7 files changed, 88 insertions(+), 4 deletions(-) create mode 100644 tests/PHPStan/Analyser/data/bug-3990.php diff --git a/src/Analyser/DirectScopeFactory.php b/src/Analyser/DirectScopeFactory.php index 7f915f6c02..556abff86c 100644 --- a/src/Analyser/DirectScopeFactory.php +++ b/src/Analyser/DirectScopeFactory.php @@ -76,6 +76,7 @@ public function __construct( * @param array<string, true> $currentlyAssignedExpressions * @param array<string, Type> $nativeExpressionTypes * @param array<\PHPStan\Reflection\FunctionReflection|\PHPStan\Reflection\MethodReflection> $inFunctionCallsStack + * @param bool $afterExtractCall * @param Scope|null $parentScope * * @return MutatingScope @@ -94,6 +95,7 @@ public function create( array $currentlyAssignedExpressions = [], array $nativeExpressionTypes = [], array $inFunctionCallsStack = [], + bool $afterExtractCall = false, ?Scope $parentScope = null ): MutatingScope { @@ -126,6 +128,7 @@ public function create( $inFunctionCallsStack, $this->dynamicConstantNames, $this->treatPhpDocTypesAsCertain, + $afterExtractCall, $parentScope ); } diff --git a/src/Analyser/LazyScopeFactory.php b/src/Analyser/LazyScopeFactory.php index a06dfb019c..8ae7592fdc 100644 --- a/src/Analyser/LazyScopeFactory.php +++ b/src/Analyser/LazyScopeFactory.php @@ -48,6 +48,7 @@ public function __construct( * @param array<string, true> $currentlyAssignedExpressions * @param array<string, Type> $nativeExpressionTypes * @param array<\PHPStan\Reflection\FunctionReflection|\PHPStan\Reflection\MethodReflection> $inFunctionCallsStack + * @param bool $afterExtractCall * @param Scope|null $parentScope * * @return MutatingScope @@ -66,6 +67,7 @@ public function create( array $currentlyAssignedExpressions = [], array $nativeExpressionTypes = [], array $inFunctionCallsStack = [], + bool $afterExtractCall = false, ?Scope $parentScope = null ): MutatingScope { @@ -98,6 +100,7 @@ public function create( $inFunctionCallsStack, $this->dynamicConstantNames, $this->treatPhpDocTypesAsCertain, + $afterExtractCall, $parentScope ); } diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index c0fd91af84..a3f827fed3 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -155,6 +155,8 @@ class MutatingScope implements Scope private bool $treatPhpDocTypesAsCertain; + private bool $afterExtractCall; + private ?Scope $parentScope; /** @@ -181,6 +183,7 @@ class MutatingScope implements Scope * @param array<MethodReflection|FunctionReflection> $inFunctionCallsStack * @param string[] $dynamicConstantNames * @param bool $treatPhpDocTypesAsCertain + * @param bool $afterExtractCall * @param Scope|null $parentScope */ public function __construct( @@ -207,6 +210,7 @@ public function __construct( array $inFunctionCallsStack = [], array $dynamicConstantNames = [], bool $treatPhpDocTypesAsCertain = true, + bool $afterExtractCall = false, ?Scope $parentScope = null ) { @@ -237,6 +241,7 @@ public function __construct( $this->inFunctionCallsStack = $inFunctionCallsStack; $this->dynamicConstantNames = $dynamicConstantNames; $this->treatPhpDocTypesAsCertain = $treatPhpDocTypesAsCertain; + $this->afterExtractCall = $afterExtractCall; $this->parentScope = $parentScope; } @@ -335,9 +340,30 @@ private function getVariableTypes(): array return $this->variableTypes; } - private function isRootScope(): bool + private function canAnyVariableExist(): bool + { + return ($this->function === null && !$this->isInAnonymousFunction()) || $this->afterExtractCall; + } + + public function afterExtractCall(): self { - return $this->function === null && !$this->isInAnonymousFunction(); + return $this->scopeFactory->create( + $this->context, + $this->isDeclareStrictTypes(), + $this->constantTypes, + $this->getFunction(), + $this->getNamespace(), + $this->getVariableTypes(), + $this->moreSpecificTypes, + $this->inClosureBindScopeClass, + $this->anonymousFunctionReflection, + $this->isInFirstLevelStatement(), + $this->currentlyAssignedExpressions, + $this->nativeExpressionTypes, + $this->inFunctionCallsStack, + true, + $this->parentScope + ); } public function hasVariableType(string $variableName): TrinaryLogic @@ -347,7 +373,7 @@ public function hasVariableType(string $variableName): TrinaryLogic } if (!isset($this->variableTypes[$variableName])) { - if ($this->isRootScope()) { + if ($this->canAnyVariableExist()) { return TrinaryLogic::createMaybe(); } @@ -1957,6 +1983,7 @@ public function doNotTreatPhpDocTypesAsCertain(): Scope $this->inFunctionCallsStack, $this->dynamicConstantNames, false, + $this->afterExtractCall, $this->parentScope ); } @@ -2213,6 +2240,7 @@ public function pushInFunctionCall($reflection): self $this->currentlyAssignedExpressions, $this->nativeExpressionTypes, $stack, + $this->afterExtractCall, $this->parentScope ); } @@ -2236,6 +2264,7 @@ public function popInFunctionCall(): self $this->currentlyAssignedExpressions, $this->nativeExpressionTypes, $stack, + $this->afterExtractCall, $this->parentScope ); } @@ -2622,6 +2651,7 @@ public function enterAnonymousFunction( [], $nativeTypes, [], + false, $this ); } @@ -2668,6 +2698,7 @@ public function enterArrowFunction(Expr\ArrowFunction $arrowFunction): self [], [], [], + $this->afterExtractCall, $this->parentScope ); } @@ -2782,6 +2813,7 @@ public function enterExpressionAssign(Expr $expr): self $currentlyAssignedExpressions, $this->nativeExpressionTypes, [], + $this->afterExtractCall, $this->parentScope ); } @@ -2806,6 +2838,7 @@ public function exitExpressionAssign(Expr $expr): self $currentlyAssignedExpressions, $this->nativeExpressionTypes, [], + $this->afterExtractCall, $this->parentScope ); } @@ -2861,6 +2894,7 @@ public function assignVariable(string $variableName, Type $type, ?TrinaryLogic $ $this->currentlyAssignedExpressions, $nativeTypes, $this->inFunctionCallsStack, + $this->afterExtractCall, $this->parentScope ); } @@ -2890,6 +2924,7 @@ public function unsetExpression(Expr $expr): self [], $nativeTypes, [], + $this->afterExtractCall, $this->parentScope ); } elseif ($expr instanceof Expr\ArrayDimFetch && $expr->dim !== null) { @@ -2948,6 +2983,7 @@ public function specifyExpressionType(Expr $expr, Type $type): self $this->currentlyAssignedExpressions, $this->nativeExpressionTypes, $this->inFunctionCallsStack, + $this->afterExtractCall, $this->parentScope ); } @@ -2979,6 +3015,7 @@ public function specifyExpressionType(Expr $expr, Type $type): self $this->currentlyAssignedExpressions, $nativeTypes, $this->inFunctionCallsStack, + $this->afterExtractCall, $this->parentScope ); } elseif ($expr instanceof Expr\ArrayDimFetch && $expr->dim !== null) { @@ -3055,6 +3092,7 @@ public function invalidateExpression(Expr $expressionToInvalidate, bool $require $this->currentlyAssignedExpressions, [], [], + $this->afterExtractCall, $this->parentScope ); } @@ -3167,6 +3205,7 @@ public function exitFirstLevelStatements(): self $this->currentlyAssignedExpressions, $this->nativeExpressionTypes, $this->inFunctionCallsStack, + $this->afterExtractCall, $this->parentScope ); } @@ -3202,6 +3241,7 @@ private function addMoreSpecificTypes(array $types): self $this->currentlyAssignedExpressions, $this->nativeExpressionTypes, [], + $this->afterExtractCall, $this->parentScope ); } @@ -3221,7 +3261,7 @@ public function mergeWith(?self $otherScope): self $ourVariableTypes = $this->getVariableTypes(); $theirVariableTypes = $otherScope->getVariableTypes(); - if ($this->isRootScope()) { + if ($this->canAnyVariableExist()) { foreach (array_keys($theirVariableTypes) as $name) { if (array_key_exists($name, $ourVariableTypes)) { continue; @@ -3253,6 +3293,7 @@ public function mergeWith(?self $otherScope): self return $holder->getCertainty()->yes(); })), [], + $this->afterExtractCall && $otherScope->afterExtractCall, $this->parentScope ); } @@ -3323,6 +3364,7 @@ public function processFinallyScope(self $finallyScope, self $originalFinallySco array_map($typeToVariableHolder, $originalFinallyScope->nativeExpressionTypes) )), [], + $this->afterExtractCall, $this->parentScope ); } @@ -3414,6 +3456,7 @@ public function processClosureScope( [], $this->nativeExpressionTypes, $this->inFunctionCallsStack, + $this->afterExtractCall, $this->parentScope ); } @@ -3462,6 +3505,7 @@ public function processAlwaysIterableForeachScopeWithoutPollute(self $finalScope [], $nativeTypes, [], + $this->afterExtractCall, $this->parentScope ); } @@ -3506,6 +3550,7 @@ public function generalizeWith(self $otherScope): self [], $nativeTypes, [], + $this->afterExtractCall, $this->parentScope ); } diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index adca703de1..a651c4e6e0 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -1562,6 +1562,10 @@ function (MutatingScope $scope) use ($expr, $nodeCallback, $context): Expression ) { $scope = $scope->assignVariable('http_response_header', new ArrayType(new IntegerType(), new StringType())); } + + if (isset($functionReflection) && $functionReflection->getName() === 'extract') { + $scope = $scope->afterExtractCall(); + } } elseif ($expr instanceof MethodCall) { $originalScope = $scope; if ( diff --git a/src/Analyser/ScopeFactory.php b/src/Analyser/ScopeFactory.php index 17ea66ba31..cc3d71cd02 100644 --- a/src/Analyser/ScopeFactory.php +++ b/src/Analyser/ScopeFactory.php @@ -24,6 +24,7 @@ interface ScopeFactory * @param array<string, true> $currentlyAssignedExpressions * @param array<string, Type> $nativeExpressionTypes * @param array<MethodReflection|FunctionReflection> $inFunctionCallsStack + * @param bool $afterExtractCall * @param Scope|null $parentScope * * @return MutatingScope @@ -42,6 +43,7 @@ public function create( array $currentlyAssignedExpressions = [], array $nativeExpressionTypes = [], array $inFunctionCallsStack = [], + bool $afterExtractCall = false, ?Scope $parentScope = null ): MutatingScope; diff --git a/tests/PHPStan/Analyser/NodeScopeResolverTest.php b/tests/PHPStan/Analyser/NodeScopeResolverTest.php index ac24755809..b9d31c92c8 100644 --- a/tests/PHPStan/Analyser/NodeScopeResolverTest.php +++ b/tests/PHPStan/Analyser/NodeScopeResolverTest.php @@ -10230,6 +10230,11 @@ public function dataArraySliceNonEmpty(): array return $this->gatherAssertTypes(__DIR__ . '/data/array-slice-non-empty.php'); } + public function dataBug3990(): array + { + return $this->gatherAssertTypes(__DIR__ . '/data/bug-3990.php'); + } + /** * @dataProvider dataBug2574 * @dataProvider dataBug2577 @@ -10318,6 +10323,7 @@ public function dataArraySliceNonEmpty(): array * @dataProvider dataBug2816Two * @dataProvider dataBug3985 * @dataProvider dataArraySliceNonEmpty + * @dataProvider dataBug3990 * @param string $assertType * @param string $file * @param mixed ...$args diff --git a/tests/PHPStan/Analyser/data/bug-3990.php b/tests/PHPStan/Analyser/data/bug-3990.php new file mode 100644 index 0000000000..8661fcda37 --- /dev/null +++ b/tests/PHPStan/Analyser/data/bug-3990.php @@ -0,0 +1,21 @@ +<?php + +namespace Bug3990; + +use PHPStan\TrinaryLogic; +use function PHPStan\Analyser\assertVariableCertainty; + +function doFoo(array $config): void +{ + extract($config); + + assertVariableCertainty(TrinaryLogic::createMaybe(), $a); + + if (isset($a)) { + assertVariableCertainty(TrinaryLogic::createYes(), $a); + } else { + assertVariableCertainty(TrinaryLogic::createMaybe(), $a); + } + + assertVariableCertainty(TrinaryLogic::createMaybe(), $a); +}