diff --git a/src/Analyser/ExprHandler/AssignHandler.php b/src/Analyser/ExprHandler/AssignHandler.php index 3ea0196e4aa..f6d161265d1 100644 --- a/src/Analyser/ExprHandler/AssignHandler.php +++ b/src/Analyser/ExprHandler/AssignHandler.php @@ -71,6 +71,7 @@ use function array_slice; use function count; use function in_array; +use function is_array; use function is_int; use function is_string; @@ -242,6 +243,7 @@ public function processAssignVar( $type = $scopeBeforeAssignEval->getType($assignedExpr); $conditionalExpressions = []; + $assignedExprContainsNullsafe = $this->exprContainsNullsafe($assignedExpr); if ($assignedExpr instanceof Ternary) { $if = $assignedExpr->if; if ($if === null) { @@ -259,23 +261,23 @@ public function processAssignVar( $truthyType->isSuperTypeOf($falseyType)->no() && $falseyType->isSuperTypeOf($truthyType)->no() ) { - $conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($condScope, $var->name, $conditionalExpressions, $truthySpecifiedTypes, $truthyType); - $conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($condScope, $var->name, $conditionalExpressions, $truthySpecifiedTypes, $truthyType); - $conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($condScope, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType); - $conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($condScope, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType); + $conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($condScope, $var->name, $conditionalExpressions, $truthySpecifiedTypes, $truthyType, $assignedExprContainsNullsafe); + $conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($condScope, $var->name, $conditionalExpressions, $truthySpecifiedTypes, $truthyType, $assignedExprContainsNullsafe); + $conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($condScope, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType, $assignedExprContainsNullsafe); + $conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($condScope, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType, $assignedExprContainsNullsafe); } } $truthyType = TypeCombinator::removeFalsey($type); if ($truthyType !== $type) { $truthySpecifiedTypes = $this->typeSpecifier->specifyTypesInCondition($scope, $assignedExpr, TypeSpecifierContext::createTruthy()); - $conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $truthySpecifiedTypes, $truthyType); - $conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $truthySpecifiedTypes, $truthyType); + $conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $truthySpecifiedTypes, $truthyType, $assignedExprContainsNullsafe); + $conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $truthySpecifiedTypes, $truthyType, $assignedExprContainsNullsafe); $falseyType = TypeCombinator::intersect($type, StaticTypeFactory::falsey()); $falseySpecifiedTypes = $this->typeSpecifier->specifyTypesInCondition($scope, $assignedExpr, TypeSpecifierContext::createFalsey()); - $conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType); - $conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType); + $conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType, $assignedExprContainsNullsafe); + $conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType, $assignedExprContainsNullsafe); } foreach ([null, false, 0, 0.0, '', '0', []] as $falseyScalar) { @@ -304,13 +306,13 @@ public function processAssignVar( $notIdenticalConditionExpr = new Expr\BinaryOp\NotIdentical($assignedExpr, $astNode); $notIdenticalSpecifiedTypes = $this->typeSpecifier->specifyTypesInCondition($scope, $notIdenticalConditionExpr, TypeSpecifierContext::createTrue()); - $conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $notIdenticalSpecifiedTypes, $withoutFalseyType); - $conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $notIdenticalSpecifiedTypes, $withoutFalseyType); + $conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $notIdenticalSpecifiedTypes, $withoutFalseyType, $assignedExprContainsNullsafe); + $conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $notIdenticalSpecifiedTypes, $withoutFalseyType, $assignedExprContainsNullsafe); $identicalConditionExpr = new Expr\BinaryOp\Identical($assignedExpr, $astNode); $identicalSpecifiedTypes = $this->typeSpecifier->specifyTypesInCondition($scope, $identicalConditionExpr, TypeSpecifierContext::createTrue()); - $conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $identicalSpecifiedTypes, $falseyType); - $conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $identicalSpecifiedTypes, $falseyType); + $conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $identicalSpecifiedTypes, $falseyType, $assignedExprContainsNullsafe); + $conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $identicalSpecifiedTypes, $falseyType, $assignedExprContainsNullsafe); } $nodeScopeResolver->callNodeCallback($nodeCallback, new VariableAssignNode($var, $assignedExpr), $scopeBeforeAssignEval, $storage); @@ -850,7 +852,7 @@ private function unwrapAssign(Expr $expr): Expr * @param array $conditionalExpressions * @return array */ - private function processSureTypesForConditionalExpressionsAfterAssign(Scope $scope, string $variableName, array $conditionalExpressions, SpecifiedTypes $specifiedTypes, Type $variableType): array + private function processSureTypesForConditionalExpressionsAfterAssign(Scope $scope, string $variableName, array $conditionalExpressions, SpecifiedTypes $specifiedTypes, Type $variableType, bool $assignedExprContainsNullsafe): array { foreach ($specifiedTypes->getSureTypes() as $exprString => [$expr, $exprType]) { if ($expr instanceof Variable) { @@ -871,6 +873,10 @@ private function processSureTypesForConditionalExpressionsAfterAssign(Scope $sco continue; } + if ($assignedExprContainsNullsafe && ($expr instanceof MethodCall || $expr instanceof Expr\StaticCall)) { + continue; + } + if (!isset($conditionalExpressions[$exprString])) { $conditionalExpressions[$exprString] = []; } @@ -891,7 +897,7 @@ private function processSureTypesForConditionalExpressionsAfterAssign(Scope $sco * @param array $conditionalExpressions * @return array */ - private function processSureNotTypesForConditionalExpressionsAfterAssign(Scope $scope, string $variableName, array $conditionalExpressions, SpecifiedTypes $specifiedTypes, Type $variableType): array + private function processSureNotTypesForConditionalExpressionsAfterAssign(Scope $scope, string $variableName, array $conditionalExpressions, SpecifiedTypes $specifiedTypes, Type $variableType, bool $assignedExprContainsNullsafe): array { foreach ($specifiedTypes->getSureNotTypes() as $exprString => [$expr, $exprType]) { if ($expr instanceof Variable) { @@ -912,6 +918,10 @@ private function processSureNotTypesForConditionalExpressionsAfterAssign(Scope $ continue; } + if ($assignedExprContainsNullsafe && ($expr instanceof MethodCall || $expr instanceof Expr\StaticCall)) { + continue; + } + if (!isset($conditionalExpressions[$exprString])) { $conditionalExpressions[$exprString] = []; } @@ -928,6 +938,30 @@ private function processSureNotTypesForConditionalExpressionsAfterAssign(Scope $ return $conditionalExpressions; } + private function exprContainsNullsafe(Expr $expr): bool + { + if ($expr instanceof Expr\NullsafeMethodCall || $expr instanceof Expr\NullsafePropertyFetch) { + return true; + } + + foreach ($expr->getSubNodeNames() as $name) { + $subNode = $expr->{$name}; + if ($subNode instanceof Expr) { + if ($this->exprContainsNullsafe($subNode)) { + return true; + } + } elseif (is_array($subNode)) { + foreach ($subNode as $item) { + if ($item instanceof Expr && $this->exprContainsNullsafe($item)) { + return true; + } + } + } + } + + return false; + } + /** * @param list $dimFetchStack */ diff --git a/tests/PHPStan/Rules/Methods/NullsafeMethodCallRuleTest.php b/tests/PHPStan/Rules/Methods/NullsafeMethodCallRuleTest.php index 0d1bc3e1b6e..2941692d9ce 100644 --- a/tests/PHPStan/Rules/Methods/NullsafeMethodCallRuleTest.php +++ b/tests/PHPStan/Rules/Methods/NullsafeMethodCallRuleTest.php @@ -80,4 +80,10 @@ public function testBug12222(): void $this->analyse([__DIR__ . '/data/bug-12222.php'], []); } + #[RequiresPhp('>= 8.0.0')] + public function testBug14493(): void + { + $this->analyse([__DIR__ . '/data/bug-14493.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Methods/data/bug-14493.php b/tests/PHPStan/Rules/Methods/data/bug-14493.php new file mode 100644 index 00000000000..07b7e8d07d7 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/bug-14493.php @@ -0,0 +1,63 @@ += 8.0 + +declare(strict_types = 1); + +namespace Bug14493; + +class OrderEntity { + static public function getStaticOrderCustomer(): ?OrderCustomerEntity + { + return new OrderCustomerEntity(); + } + + public function getOrderCustomer(): ?OrderCustomerEntity + { + return new OrderCustomerEntity(); + } +} + +class OrderCustomerEntity { + public function getCustomer(): ?CustomerEntity { return null; } + /** @return array|null */ + public function getVatIds(): ?array { return null; } +} + +class CustomerEntity { + public const ACCOUNT_TYPE_BUSINESS = 'business'; + + public function getAccountType(): string { return ''; } +} + + +abstract class AbstractDocumentRenderer +{ + protected function doFoo(OrderEntity $order): bool + { + $customerType = $order->getOrderCustomer()?->getCustomer()?->getAccountType(); + if ($customerType !== CustomerEntity::ACCOUNT_TYPE_BUSINESS) { + return false; + } + + $vatIds = $order->getOrderCustomer()?->getVatIds(); + if (!is_array($vatIds)) { + return false; + } + + return true; + } + + protected function doBar(OrderEntity $order): bool + { + $customerType = $order::getStaticOrderCustomer()?->getCustomer()?->getAccountType(); + if ($customerType !== CustomerEntity::ACCOUNT_TYPE_BUSINESS) { + return false; + } + + $vatIds = $order::getStaticOrderCustomer()?->getVatIds(); + if (!is_array($vatIds)) { + return false; + } + + return true; + } +} diff --git a/tests/PHPStan/Rules/Properties/NullsafePropertyFetchRuleTest.php b/tests/PHPStan/Rules/Properties/NullsafePropertyFetchRuleTest.php index e4c37b08ca0..4dfd535fb16 100644 --- a/tests/PHPStan/Rules/Properties/NullsafePropertyFetchRuleTest.php +++ b/tests/PHPStan/Rules/Properties/NullsafePropertyFetchRuleTest.php @@ -81,4 +81,10 @@ public function testBug6922(): void $this->analyse([__DIR__ . '/data/bug-6922.php'], []); } + #[RequiresPhp('>= 8.0.0')] + public function testBug14493(): void + { + $this->analyse([__DIR__ . '/data/bug-14493.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Properties/data/bug-14493.php b/tests/PHPStan/Rules/Properties/data/bug-14493.php new file mode 100644 index 00000000000..37f7668d415 --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/bug-14493.php @@ -0,0 +1,43 @@ += 8.0 + +declare(strict_types = 1); + +namespace Bug14493NullsafeProperty; + +class OrderEntity { + public function getOrderCustomer(): ?OrderCustomerEntity + { + return new OrderCustomerEntity(); + } +} + +class OrderCustomerEntity { + public ?CustomerEntity $customer = null; + /** @var array|null */ + public ?array $vatIds = null; +} + +class CustomerEntity { + public const ACCOUNT_TYPE_BUSINESS = 'business'; + + public string $accountType = ''; +} + + +abstract class AbstractDocumentRenderer +{ + protected function doFoo(OrderEntity $order): bool + { + $customerType = $order->getOrderCustomer()?->customer?->accountType; + if ($customerType !== CustomerEntity::ACCOUNT_TYPE_BUSINESS) { + return false; + } + + $vatIds = $order->getOrderCustomer()?->vatIds; + if (!is_array($vatIds)) { + return false; + } + + return true; + } +}