From 2c42ef18ae4c2da04e10ba9519b09d16abe147aa Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Mon, 22 Mar 2021 21:36:40 +0100 Subject: [PATCH] Dependent types - understand truthy BooleanOr and falsey BooleanAnd scope --- src/Analyser/MutatingScope.php | 30 +++++++++- src/Analyser/SpecifiedTypes.php | 16 +++++- src/Analyser/TypeSpecifier.php | 55 ++++++++++++++++++- .../Analyser/NodeScopeResolverTest.php | 6 ++ tests/PHPStan/Analyser/data/bug-4733.php | 38 +++++++++++++ 5 files changed, 139 insertions(+), 6 deletions(-) create mode 100644 tests/PHPStan/Analyser/data/bug-4733.php diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index e785aa7238..46594e74e0 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -3560,7 +3560,7 @@ public function filterBySpecifiedTypes(SpecifiedTypes $specifiedTypes): self $typeGuards['$' . $variableName] = $typeGuard; } - $newConditionalExpressions = []; + $newConditionalExpressions = $specifiedTypes->getNewConditionalExpressionHolders(); foreach ($this->conditionalExpressions as $variableExprString => $conditionalExpressions) { if (array_key_exists($variableExprString, $typeGuards)) { continue; @@ -3614,9 +3614,33 @@ public function filterBySpecifiedTypes(SpecifiedTypes $specifiedTypes): self } } - $scope->conditionalExpressions = $newConditionalExpressions; + return $scope->changeConditionalExpressions($newConditionalExpressions); + } - return $scope; + /** + * @param array $newConditionalExpressionHolders + * @return self + */ + public function changeConditionalExpressions(array $newConditionalExpressionHolders): self + { + return $this->scopeFactory->create( + $this->context, + $this->isDeclareStrictTypes(), + $this->constantTypes, + $this->getFunction(), + $this->getNamespace(), + $this->variableTypes, + $this->moreSpecificTypes, + $newConditionalExpressionHolders, + $this->inClosureBindScopeClass, + $this->anonymousFunctionReflection, + $this->inFirstLevelStatement, + $this->currentlyAssignedExpressions, + $this->nativeExpressionTypes, + $this->inFunctionCallsStack, + $this->afterExtractCall, + $this->parentScope + ); } /** diff --git a/src/Analyser/SpecifiedTypes.php b/src/Analyser/SpecifiedTypes.php index 501dc82afc..0b90bd5f1f 100644 --- a/src/Analyser/SpecifiedTypes.php +++ b/src/Analyser/SpecifiedTypes.php @@ -15,20 +15,26 @@ class SpecifiedTypes private bool $overwrite; + /** @var array */ + private array $newConditionalExpressionHolders; + /** * @param mixed[] $sureTypes * @param mixed[] $sureNotTypes * @param bool $overwrite + * @param array $newConditionalExpressionHolders */ public function __construct( array $sureTypes = [], array $sureNotTypes = [], - bool $overwrite = false + bool $overwrite = false, + array $newConditionalExpressionHolders = [] ) { $this->sureTypes = $sureTypes; $this->sureNotTypes = $sureNotTypes; $this->overwrite = $overwrite; + $this->newConditionalExpressionHolders = $newConditionalExpressionHolders; } /** @@ -52,6 +58,14 @@ public function shouldOverwrite(): bool return $this->overwrite; } + /** + * @return array + */ + public function getNewConditionalExpressionHolders(): array + { + return $this->newConditionalExpressionHolders; + } + public function intersectWith(SpecifiedTypes $other): self { $sureTypeUnion = []; diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php index 5b165dc931..b8e9ae29c7 100644 --- a/src/Analyser/TypeSpecifier.php +++ b/src/Analyser/TypeSpecifier.php @@ -19,6 +19,7 @@ use PhpParser\Node\Expr\StaticPropertyFetch; use PhpParser\Node\Name; use PHPStan\Reflection\ReflectionProvider; +use PHPStan\TrinaryLogic; use PHPStan\Type\Accessory\HasOffsetType; use PHPStan\Type\Accessory\HasPropertyType; use PHPStan\Type\Accessory\NonEmptyArrayType; @@ -578,11 +579,21 @@ public function specifyTypesInCondition( } elseif ($expr instanceof BooleanAnd || $expr instanceof LogicalAnd) { $leftTypes = $this->specifyTypesInCondition($scope, $expr->left, $context); $rightTypes = $this->specifyTypesInCondition($scope, $expr->right, $context); - return $context->true() ? $leftTypes->unionWith($rightTypes) : $leftTypes->intersectWith($rightTypes); + $types = $context->true() ? $leftTypes->unionWith($rightTypes) : $leftTypes->intersectWith($rightTypes); + if ($context->false()) { + return $this->processBooleanConditionalTypes($scope, $types, $leftTypes, $rightTypes); + } + + return $types; } elseif ($expr instanceof BooleanOr || $expr instanceof LogicalOr) { $leftTypes = $this->specifyTypesInCondition($scope, $expr->left, $context); $rightTypes = $this->specifyTypesInCondition($scope, $expr->right, $context); - return $context->true() ? $leftTypes->intersectWith($rightTypes) : $leftTypes->unionWith($rightTypes); + $types = $context->true() ? $leftTypes->intersectWith($rightTypes) : $leftTypes->unionWith($rightTypes); + if ($context->true()) { + return $this->processBooleanConditionalTypes($scope, $types, $leftTypes, $rightTypes); + } + + return $types; } elseif ($expr instanceof Node\Expr\BooleanNot && !$context->null()) { return $this->specifyTypesInCondition($scope, $expr->expr, $context->negate()); } elseif ($expr instanceof Node\Expr\Assign) { @@ -744,6 +755,46 @@ private function handleDefaultTruthyOrFalseyContext(TypeSpecifierContext $contex return new SpecifiedTypes(); } + private function processBooleanConditionalTypes(Scope $scope, SpecifiedTypes $types, SpecifiedTypes $leftTypes, SpecifiedTypes $rightTypes): SpecifiedTypes + { + $conditionExpressionTypes = []; + foreach ($leftTypes->getSureNotTypes() as $exprString => [$expr, $type]) { + if (!$expr instanceof Expr\Variable) { + continue; + } + if (!is_string($expr->name)) { + continue; + } + + $conditionExpressionTypes[$exprString] = TypeCombinator::intersect($scope->getType($expr), $type); + } + + if (count($conditionExpressionTypes) > 0) { + $holders = []; + foreach ($rightTypes->getSureNotTypes() as $exprString => [$expr, $type]) { + if (!$expr instanceof Expr\Variable) { + continue; + } + if (!is_string($expr->name)) { + continue; + } + + if (!isset($holders[$exprString])) { + $holders[$exprString] = []; + } + + $holders[$exprString][] = new ConditionalExpressionHolder( + $conditionExpressionTypes, + new VariableTypeHolder(TypeCombinator::remove($scope->getType($expr), $type), TrinaryLogic::createYes()) // todo yes is wrong + ); + } + + return new SpecifiedTypes($types->getSureTypes(), $types->getSureNotTypes(), false, $holders); + } + + return $types; + } + /** * @param \PHPStan\Analyser\Scope $scope * @param \PhpParser\Node\Expr\BinaryOp $binaryOperation diff --git a/tests/PHPStan/Analyser/NodeScopeResolverTest.php b/tests/PHPStan/Analyser/NodeScopeResolverTest.php index f82634bddc..ee15a9a2bc 100644 --- a/tests/PHPStan/Analyser/NodeScopeResolverTest.php +++ b/tests/PHPStan/Analyser/NodeScopeResolverTest.php @@ -5710,6 +5710,11 @@ public function dataBug4725(): array return $this->gatherAssertTypes(__DIR__ . '/data/bug-4725.php'); } + public function dataBug4733(): array + { + return $this->gatherAssertTypes(__DIR__ . '/data/bug-4733.php'); + } + /** * @dataProvider dataArrayFunctions * @param string $description @@ -11335,6 +11340,7 @@ private function gatherAssertTypes(string $file): array * @dataProvider dataBug4545 * @dataProvider dataBug4714 * @dataProvider dataBug4725 + * @dataProvider dataBug4733 * @param string $assertType * @param string $file * @param mixed ...$args diff --git a/tests/PHPStan/Analyser/data/bug-4733.php b/tests/PHPStan/Analyser/data/bug-4733.php new file mode 100644 index 0000000000..3e8b6b2a95 --- /dev/null +++ b/tests/PHPStan/Analyser/data/bug-4733.php @@ -0,0 +1,38 @@ +