From 662240117e38bdf3fa6628f767e8e3010e28dd9b Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 25 Apr 2021 21:33:45 +0200 Subject: [PATCH] Type-specified nullsafe call also removes null from the chain --- src/Analyser/NodeScopeResolver.php | 12 +- src/Analyser/TypeSpecifier.php | 40 ++- .../Analyser/NodeScopeResolverTest.php | 5 + tests/PHPStan/Analyser/data/bug-4757.php | 243 ++++++++++++++++++ tests/PHPStan/Analyser/data/nullsafe.php | 4 +- 5 files changed, 300 insertions(+), 4 deletions(-) create mode 100644 tests/PHPStan/Analyser/data/bug-4757.php diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index ecc2753fd8..f60dae84bf 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -1981,7 +1981,17 @@ function (MutatingScope $scope) use ($expr, $nodeCallback, $context): Expression $exprResult = $this->processExprNode(new MethodCall($expr->var, $expr->name, $expr->args, $expr->getAttributes()), $nonNullabilityResult->getScope(), $nodeCallback, $context); $scope = $this->revertNonNullability($exprResult->getScope(), $nonNullabilityResult->getSpecifiedExpressions()); - return new ExpressionResult($scope, $exprResult->hasYield(), $exprResult->getThrowPoints()); + return new ExpressionResult( + $scope, + $exprResult->hasYield(), + $exprResult->getThrowPoints(), + static function () use ($scope, $expr): MutatingScope { + return $scope->filterByTruthyValue($expr); + }, + static function () use ($scope, $expr): MutatingScope { + return $scope->filterByFalseyValue($expr); + } + ); } elseif ($expr instanceof StaticCall) { $hasYield = false; $throwPoints = []; diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php index 3c5dce2efc..2763dfb76b 100644 --- a/src/Analyser/TypeSpecifier.php +++ b/src/Analyser/TypeSpecifier.php @@ -793,6 +793,18 @@ public function specifyTypesInCondition( $context ); + $nullSafeTypes = $this->handleDefaultTruthyOrFalseyContext($context, $expr, $scope); + return $context->true() ? $types->unionWith($nullSafeTypes) : $types->intersectWith($nullSafeTypes); + } elseif ($expr instanceof Expr\NullsafeMethodCall && !$context->null()) { + $types = $this->specifyTypesInCondition( + $scope, + new BooleanAnd( + new Expr\BinaryOp\NotIdentical($expr->var, new ConstFetch(new Name('null'))), + new MethodCall($expr->var, $expr->name, $expr->args) + ), + $context + ); + $nullSafeTypes = $this->handleDefaultTruthyOrFalseyContext($context, $expr, $scope); return $context->true() ? $types->unionWith($nullSafeTypes) : $types->intersectWith($nullSafeTypes); } elseif (!$context->null()) { @@ -955,12 +967,38 @@ public function create( $propertyFetchTypes = $propertyFetchTypes->unionWith( $this->create($expr->var, new NullType(), TypeSpecifierContext::createFalse(), false, $scope) ); + return $types->unionWith($propertyFetchTypes); } elseif ($context->false() && TypeCombinator::containsNull($type)) { $propertyFetchTypes = $propertyFetchTypes->unionWith( $this->create($expr->var, new NullType(), TypeSpecifierContext::createFalse(), false, $scope) ); + return $types->unionWith($propertyFetchTypes); + } + } + + if ($expr instanceof Expr\NullsafeMethodCall && !$context->null()) { + $methodCallTypes = $this->create(new MethodCall($expr->var, $expr->name, $expr->args), $type, $context, false, $scope); + if ($context->true() && $scope !== null) { + $resultType = TypeCombinator::intersect($scope->getType($expr), $type); + if (!TypeCombinator::containsNull($resultType)) { + $methodCallTypes = $methodCallTypes->unionWith( + $this->create($expr->var, new NullType(), TypeSpecifierContext::createFalse(), false, $scope) + ); + return $types->unionWith($methodCallTypes); + } + + return new SpecifiedTypes(); + } elseif ($context->false() && $scope !== null) { + $resultType = TypeCombinator::remove($scope->getType($expr), $type); + if (!TypeCombinator::containsNull($resultType)) { + $methodCallTypes = $methodCallTypes->unionWith( + $this->create($expr->var, new NullType(), TypeSpecifierContext::createFalse(), false, $scope) + ); + return $types->unionWith($methodCallTypes); + } + + return new SpecifiedTypes(); } - return $types->unionWith($propertyFetchTypes); } return $types; diff --git a/tests/PHPStan/Analyser/NodeScopeResolverTest.php b/tests/PHPStan/Analyser/NodeScopeResolverTest.php index c06793dc78..943d2c70d3 100644 --- a/tests/PHPStan/Analyser/NodeScopeResolverTest.php +++ b/tests/PHPStan/Analyser/NodeScopeResolverTest.php @@ -389,6 +389,11 @@ public function dataFileAsserts(): iterable yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-4820.php'); yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-4822.php'); yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-4816.php'); + + if (self::$useStaticReflectionProvider || PHP_VERSION_ID >= 80000) { + yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-4757.php'); + } + yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-4814.php'); } diff --git a/tests/PHPStan/Analyser/data/bug-4757.php b/tests/PHPStan/Analyser/data/bug-4757.php new file mode 100644 index 0000000000..72e5ccad5f --- /dev/null +++ b/tests/PHPStan/Analyser/data/bug-4757.php @@ -0,0 +1,243 @@ += 8.0 + +namespace Bug4757; + +use function PHPStan\Testing\assertType; + +class HelloWorld +{ + public function sayHello(?Reservation $oldReservation): void + { + if ($oldReservation?->isFoo()) { + assertType(Reservation::class, $oldReservation); + assertType('true', $oldReservation->isFoo()); + return; + } + + assertType(Reservation::class . '|null', $oldReservation); + } + + public function sayHello2(?Reservation $oldReservation): void + { + if (!$oldReservation?->isFoo()) { + assertType(Reservation::class . '|null', $oldReservation); + assertType('bool', $oldReservation->isFoo()); + return; + } + + assertType(Reservation::class, $oldReservation); + assertType('true', $oldReservation->isFoo()); + } + + public function sayHello3(?Reservation $oldReservation): void + { + if ($oldReservation?->isFoo() === true) { + assertType(Reservation::class, $oldReservation); + assertType('true', $oldReservation->isFoo()); + return; + } + + assertType(Reservation::class . '|null', $oldReservation); + assertType('bool', $oldReservation->isFoo()); + } + + public function sayHello4(?Reservation $oldReservation): void + { + if ($oldReservation?->isFoo() === false) { + assertType(Reservation::class , $oldReservation); + assertType('false', $oldReservation->isFoo()); + return; + } + + //assertType(Reservation::class . '|null', $oldReservation); + assertType('bool', $oldReservation->isFoo()); + } + + public function sayHello5(?Reservation $oldReservation): void + { + if ($oldReservation?->isFoo() === null) { + assertType(Reservation::class . '|null', $oldReservation); + return; + } + + assertType(Reservation::class, $oldReservation); + } + + public function sayHello6(?Reservation $oldReservation): void + { + if ($oldReservation?->isFoo() !== null) { + assertType(Reservation::class, $oldReservation); + assertType('bool', $oldReservation->isFoo()); + return; + } + + assertType(Reservation::class . '|null', $oldReservation); + assertType('bool', $oldReservation->isFoo()); + } + + public function sayHelloImpure(?Reservation $oldReservation): void + { + if ($oldReservation?->isFooImpure()) { + assertType(Reservation::class, $oldReservation); + assertType('bool', $oldReservation->isFooImpure()); + return; + } + + assertType(Reservation::class . '|null', $oldReservation); + } + + public function sayHello2Impure(?Reservation $oldReservation): void + { + if (!$oldReservation?->isFooImpure()) { + assertType(Reservation::class . '|null', $oldReservation); + return; + } + + assertType(Reservation::class, $oldReservation); + } + + public function sayHello3Impure(?Reservation $oldReservation): void + { + if ($oldReservation?->isFooImpure() === true) { + assertType(Reservation::class, $oldReservation); + return; + } + + assertType(Reservation::class . '|null', $oldReservation); + } + + public function sayHello4Impure(?Reservation $oldReservation): void + { + if ($oldReservation?->isFooImpure() === false) { + assertType(Reservation::class , $oldReservation); + return; + } + + //assertType(Reservation::class . '|null', $oldReservation); + } + + public function sayHello5Impure(?Reservation $oldReservation): void + { + if ($oldReservation?->isFooImpure() === null) { + assertType(Reservation::class . '|null', $oldReservation); + return; + } + + assertType(Reservation::class, $oldReservation); + } + + public function sayHello6Impure(?Reservation $oldReservation): void + { + if ($oldReservation?->isFooImpure() !== null) { + assertType(Reservation::class, $oldReservation); + return; + } + + assertType(Reservation::class . '|null', $oldReservation); + } +} + +interface Reservation { + public function isFoo(): bool; + + /** @phpstan-impure */ + public function isFooImpure(): bool; +} + +interface Bar +{ + public function get(): ?int; + + /** @phpstan-impure */ + public function getImpure(): ?int; +} + +class Foo +{ + + public function getBarOrNull(): ?Bar + { + return null; + } + + public function doFoo(Bar $b): void + { + $barOrNull = $this->getBarOrNull(); + if ($barOrNull?->get() === null) { + assertType(Bar::class . '|null', $barOrNull); + assertType('int|null', $barOrNull->get()); + //assertType('null', $barOrNull?->get()); + return; + } + + assertType(Bar::class, $barOrNull); + assertType('int', $barOrNull->get()); + } + + public function doFooImpire(Bar $b): void + { + $barOrNull = $this->getBarOrNull(); + if ($barOrNull?->getImpure() === null) { + assertType(Bar::class . '|null', $barOrNull); + assertType('int|null', $barOrNull->getImpure()); + assertType('int|null', $barOrNull?->getImpure()); + return; + } + + assertType(Bar::class, $barOrNull); + assertType('int|null', $barOrNull->getImpure()); + } + + public function doFoo2(Bar $b): void + { + $barOrNull = $this->getBarOrNull(); + if ($barOrNull?->get() !== null) { + assertType(Bar::class, $barOrNull); + assertType('int', $barOrNull->get()); + return; + } + + assertType(Bar::class . '|null', $barOrNull); + assertType('int|null', $barOrNull->get()); + } + + public function doFoo2Impure(Bar $b): void + { + $barOrNull = $this->getBarOrNull(); + if ($barOrNull?->getImpure() !== null) { + assertType(Bar::class, $barOrNull); + assertType('int|null', $barOrNull->getImpure()); + return; + } + + assertType(Bar::class . '|null', $barOrNull); + assertType('int|null', $barOrNull->getImpure()); + } + + public function doFoo3(Bar $b): void + { + $barOrNull = $this->getBarOrNull(); + if ($barOrNull?->get()) { + assertType(Bar::class, $barOrNull); + assertType('int|int<1, max>', $barOrNull->get()); + return; + } + + assertType(Bar::class . '|null', $barOrNull); + assertType('int|null', $barOrNull->get()); + } + + public function doFoo3Impure(Bar $b): void + { + $barOrNull = $this->getBarOrNull(); + if ($barOrNull?->getImpure()) { + assertType(Bar::class, $barOrNull); + assertType('int|null', $barOrNull->getImpure()); + return; + } + + assertType(Bar::class . '|null', $barOrNull); + assertType('int|null', $barOrNull->getImpure()); + } + +} diff --git a/tests/PHPStan/Analyser/data/nullsafe.php b/tests/PHPStan/Analyser/data/nullsafe.php index 8a81d615bf..fcb27c2ebd 100644 --- a/tests/PHPStan/Analyser/data/nullsafe.php +++ b/tests/PHPStan/Analyser/data/nullsafe.php @@ -56,7 +56,7 @@ public function doLorem(?self $self) assertType('Nullsafe\Foo', $self?->nullableSelf); } else { assertType('Nullsafe\Foo|null', $self); - assertType('null', $self->nullableSelf); + assertType('Nullsafe\Foo|null', $self->nullableSelf); assertType('null', $self?->nullableSelf); } @@ -69,7 +69,7 @@ public function doIpsum(?self $self) { if ($self?->nullableSelf === null) { assertType('Nullsafe\Foo|null', $self); - assertType('null', $self->nullableSelf); + assertType('Nullsafe\Foo|null', $self); assertType('null', $self?->nullableSelf); } else { assertType('Nullsafe\Foo', $self);