Project call-target narrowings through stored booleans

src/Analyser/ExprHandler/AssignHandler.php

@@ -13,7 +13,9 @@
 use PhpParser\Node\Expr\FuncCall;
 use PhpParser\Node\Expr\List_;
 use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Expr\NullsafeMethodCall;
 use PhpParser\Node\Expr\PropertyFetch;
+use PhpParser\Node\Expr\StaticCall;
 use PhpParser\Node\Expr\StaticPropertyFetch;
 use PhpParser\Node\Expr\Ternary;
 use PhpParser\Node\Expr\Variable;
@@ -44,6 +46,7 @@
 use PHPStan\Node\Expr\TypeExpr;
 use PHPStan\Node\PropertyAssignNode;
 use PHPStan\Node\VariableAssignNode;
+use PHPStan\Node\VirtualNode;
 use PHPStan\Php\PhpVersion;
 use PHPStan\ShouldNotHappenException;
 use PHPStan\TrinaryLogic;
@@ -259,23 +262,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, $impurePoints, $assignedExpr);
+						$conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($condScope, $var->name, $conditionalExpressions, $truthySpecifiedTypes, $truthyType, $impurePoints, $assignedExpr);
+						$conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($condScope, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType, $impurePoints, $assignedExpr);
+						$conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($condScope, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType, $impurePoints, $assignedExpr);
 					}
 				}
 
 				$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, $impurePoints, $assignedExpr);
+					$conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $truthySpecifiedTypes, $truthyType, $impurePoints, $assignedExpr);
 
 					$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, $impurePoints, $assignedExpr);
+					$conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType, $impurePoints, $assignedExpr);
 				}
 
 				foreach ([null, false, 0, 0.0, '', '0', []] as $falseyScalar) {
@@ -304,13 +307,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, $impurePoints, $assignedExpr);
+					$conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $notIdenticalSpecifiedTypes, $withoutFalseyType, $impurePoints, $assignedExpr);
 
 					$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, $impurePoints, $assignedExpr);
+					$conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($scope, $var->name, $conditionalExpressions, $identicalSpecifiedTypes, $falseyType, $impurePoints, $assignedExpr);
 				}
 
 				$nodeScopeResolver->callNodeCallback($nodeCallback, new VariableAssignNode($var, $assignedExpr), $scopeBeforeAssignEval, $storage);
@@ -848,24 +851,13 @@ private function unwrapAssign(Expr $expr): Expr
 
 	/**
 	 * @param array<string, ConditionalExpressionHolder[]> $conditionalExpressions
+	 * @param ImpurePoint[] $rhsImpurePoints
 	 * @return array<string, ConditionalExpressionHolder[]>
 	 */
-	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, array $rhsImpurePoints, Expr $assignedExpr): array
 	{
 		foreach ($specifiedTypes->getSureTypes() as $exprString => [$expr, $exprType]) {
-			if ($expr instanceof Variable) {
-				if (!is_string($expr->name)) {
-					continue;
-				}
-
-				if ($expr->name === $variableName) {
-					continue;
-				}
-			} elseif (
-				!$expr instanceof PropertyFetch
-				&& !$expr instanceof ArrayDimFetch
-				&& !$expr instanceof FuncCall
-			) {
+			if (!$this->isExprSafeToProjectThroughVariable($expr, $variableName, $rhsImpurePoints, $assignedExpr)) {
 				continue;
 			}
 
@@ -887,24 +879,13 @@ private function processSureTypesForConditionalExpressionsAfterAssign(Scope $sco
 
 	/**
 	 * @param array<string, ConditionalExpressionHolder[]> $conditionalExpressions
+	 * @param ImpurePoint[] $rhsImpurePoints
 	 * @return array<string, ConditionalExpressionHolder[]>
 	 */
-	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, array $rhsImpurePoints, Expr $assignedExpr): array
 	{
 		foreach ($specifiedTypes->getSureNotTypes() as $exprString => [$expr, $exprType]) {
-			if ($expr instanceof Variable) {
-				if (!is_string($expr->name)) {
-					continue;
-				}
-
-				if ($expr->name === $variableName) {
-					continue;
-				}
-			} elseif (
-				!$expr instanceof PropertyFetch
-				&& !$expr instanceof ArrayDimFetch
-				&& !$expr instanceof FuncCall
-			) {
+			if (!$this->isExprSafeToProjectThroughVariable($expr, $variableName, $rhsImpurePoints, $assignedExpr)) {
 				continue;
 			}
 
@@ -924,6 +905,76 @@ private function processSureNotTypesForConditionalExpressionsAfterAssign(Scope $
 		return $conditionalExpressions;
 	}
 
+	/**
+	 * We're about to remember "when $variableName is truthy/falsy, $expr has a narrower type".
+	 * Whether that's safe to project forward depends on whether re-evaluating $expr later will
+	 * still return the same value as when we observed the narrowing — i.e. whether $expr is
+	 * referentially transparent with respect to the intervening code.
+	 *
+	 * Scalar/const-fetch literals are never narrowing targets, so skip them up front (they also
+	 * happen to stringify to numeric exprStrings which collide with PHP's numeric-string
+	 * array-key autocast).
+	 *
+	 * A plain variable is always safe: reading it doesn't produce side effects, and if it gets
+	 * reassigned the existing conditional-expression-holder machinery invalidates the binding.
+	 * This case matters for e.g. `$ok = preg_match(..., $matches); if ($ok) { use $matches }` —
+	 * `preg_match` itself has impure points, but `$matches` is a plain variable and the
+	 * narrowing attached to it should still survive.
+	 *
+	 * Other common tracked expressions (property/dim fetches, function/method calls) can always
+	 * carry narrowings: PHPStan already memoises their types per exprString, and condition
+	 * checks like `$x = $obj->foo() !== null; if ($x) { $obj->foo(); }` rely on this even when
+	 * the RHS itself has impure points (as a method call without @phpstan-pure always does).
+	 *
+	 * Anything else is accepted only when the right-hand side evaluation recorded zero impure
+	 * points — in that case all sub-expressions it produced sure types for were evaluated
+	 * without side effects and can be re-evaluated later with the same result.
+	 *
+	 * @param ImpurePoint[] $rhsImpurePoints
+	 */
+	private function isExprSafeToProjectThroughVariable(Expr $expr, string $variableName, array $rhsImpurePoints, Expr $assignedExpr): bool
+	{
+		// Scalar/const-fetch literals and PHPStan virtual nodes (e.g. NativeTypeExpr) are never
+		// narrowing targets at a usage site — skip them so they don't collide with PHP's
+		// numeric-string array-key autocast or leak internal virtual expressions into the
+		// conditional-expression map.
+		if ($expr instanceof Node\Scalar || $expr instanceof ConstFetch || $expr instanceof VirtualNode) {
+			return false;
+		}
+
+		if ($expr instanceof Variable) {
+			return is_string($expr->name) && $expr->name !== $variableName;
+		}
+
+		if (
+			$expr instanceof PropertyFetch
+			|| $expr instanceof ArrayDimFetch
+		) {
+			return true;
+		}
+
+		if (
+			$expr instanceof FuncCall
+			|| $expr instanceof MethodCall
+			|| $expr instanceof NullsafeMethodCall
+			|| $expr instanceof StaticCall
+		) {
+			// A call's type can change between evaluations. We're willing to project the
+			// narrowing through a stored boolean only when the sure-type expression is a
+			// *sub*-expression of the assigned RHS — e.g. `$ok = $x->foo() !== null` builds
+			// a sure type for the sub-call `$x->foo()`. In that case the RHS as a whole
+			// carries the comparison result, and later `if ($ok)` usefully re-narrows the
+			// remembered sub-call. When the sure-type expression IS the whole RHS (e.g.
+			// `$device = $this->nullable(); if ($device === null) { … }` with the
+			// falsey-scalar loop producing `$this->nullable() === null` narrowings), the
+			// projection would survive across subsequent reassignments of the target
+			// expression and wrongly re-narrow fresh calls — so skip it.
+			return $expr !== $assignedExpr;
+		}
+
+		return count($rhsImpurePoints) === 0;
+	}
+
 	/**
 	 * @param list<ArrayDimFetch> $dimFetchStack
 	 */

tests/PHPStan/Analyser/nsrt/bug-5207.php

@@ -0,0 +1,24 @@
+<?php
+
+namespace Bug5207;
+
+use function PHPStan\Testing\assertType;
+
+abstract class HelloWorld
+{
+
+	abstract public function getChild(): ?HelloWorld;
+
+	public function sayHello(): HelloWorld
+	{
+		$foo = null !== $this->getChild();
+
+		if ($foo) {
+			assertType(HelloWorld::class, $this->getChild());
+			return $this->getChild();
+		}
+
+		throw new \Exception();
+	}
+
+}

tests/PHPStan/Analyser/nsrt/bug-9455.php

@@ -0,0 +1,51 @@
+<?php
+
+namespace Bug9455;
+
+use function PHPStan\Testing\assertType;
+
+class A {
+	public function __construct(private int $id) {}
+
+	public function getId(): int
+	{
+		return $this->id;
+	}
+}
+
+class B {
+	public function __construct(private int $id, private ?A $a = null) {}
+
+	public function getId(): int
+	{
+		return $this->id;
+	}
+
+	public function getA(): ?A
+	{
+		return $this->a;
+	}
+}
+
+class HelloWorld
+{
+
+	public function testFails(B $b): void
+	{
+		$hasA = $b->getA() !== null;
+
+		if ($hasA) {
+			assertType(A::class, $b->getA());
+			echo $b->getA()->getId();
+		}
+	}
+
+	public function testSucceeds(B $b): void
+	{
+		if ($b->getA() !== null) {
+			assertType(A::class, $b->getA());
+			echo $b->getA()->getId();
+		}
+	}
+
+}

tests/PHPStan/Analyser/nsrt/conditional-expr-narrowing-through-variable.php

@@ -0,0 +1,47 @@
+<?php
+
+namespace ConditionalExprNarrowingThroughVariable;
+
+use function PHPStan\Testing\assertType;
+
+class Holder
+{
+
+	/**
+	 * @return array{int, string}|null
+	 * @phpstan-pure
+	 */
+	public function getPair(): ?array
+	{
+		throw new \Exception();
+	}
+
+}
+
+function pureMethodCallNarrowsThroughVariable(Holder $a, Holder $b): void
+{
+	$bothReady = $a->getPair() !== null && $b->getPair() !== null;
+
+	if ($bothReady) {
+		$aPair = $a->getPair();
+		$bPair = $b->getPair();
+
+		assertType('array{int, string}', $aPair);
+		assertType('array{int, string}', $bPair);
+
+		assertType('int', $aPair[0]);
+		assertType('int', $bPair[0]);
+	}
+}
+
+function pregMatchNarrowsByRefVariable(string $in): void
+{
+	$matches = [];
+	$result = preg_match('~^/xxx/([\w\-]+)/?([\w\-]+)?/?$~', $in, $matches);
+	if ($result) {
+		// preg_match has impure points (it writes $matches by ref), but $matches
+		// is a plain variable so the narrowing attached to it must still survive
+		// through the stored `$result` guard.
+		assertType('array{0: non-falsy-string, 1: non-empty-string, 2?: non-empty-string}', $matches);
+	}
+}

tests/PHPStan/Analyser/nsrt/try-catch-reassign-method-narrowing.php

@@ -0,0 +1,40 @@
+<?php
+
+namespace TryCatchReassignMethodNarrowing;
+
+use function PHPStan\Testing\assertType;
+
+class Foo
+{
+
+	public function doFoo(): void
+	{
+		$device = $this->nullable();
+		if ($device === null) {
+			$device = 1;
+			try {
+				$device = $this->throwsException();
+			} catch (\Exception) {
+				$device = $this->nullable();
+				// After reassignment to a fresh `$this->nullable()` call inside the catch,
+				// the variable's type must be the method's declared return type. Earlier
+				// conditional-expression holders stored at the initial `$device = $this->nullable()`
+				// (narrowing `$this->nullable()` to `null` when `$device` is null) must not leak
+				// through to this re-evaluation: each call may return a different value.
+				assertType('int|null', $device);
+			}
+		}
+	}
+
+	public function nullable(): ?int
+	{
+		throw new \Exception();
+	}
+
+	/** @throws \Exception */
+	private function throwsException(): int
+	{
+		throw new \Exception();
+	}
+
+}