Type-specified nullsafe call also removes null from the chain

phpstan · Apr 25, 2021 · 6622401 · 6622401
1 parent 2ec878b
commit 6622401
Show file tree

Hide file tree

Showing 5 changed files with 300 additions and 4 deletions.
diff --git 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
@@ -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
@@ -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
@@ -0,0 +1,243 @@
+<?php // lint >= 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<min, -1>|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
@@ -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);