Apply the impure-call purity gate to a call's own key when it also carries asserts, and to nullsafe chain keys

- MethodCallHandler::specifyTypes(): the assert branch unioned the asserts with
  an ungated specifyDefaultTypes(), so an @phpstan-impure method carrying
  @phpstan-assert-if-true keyed its own truthiness. It now unions with
  defaultMethodCallNarrowing(), which applies the same purity gate (and nullsafe
  receiver fan) as the assert-less path.
- StaticCallHandler::specifyTypes(): same fix through defaultStaticCallNarrowing().
- DefaultNarrowingHelper::specifyDefaultTypesWithPlainTwin(): the `?->` node's own
  key and the deeper chain's fully plain form are rebuilt nodes, so the identity-
  based impure-point gate in createSubjectTypesFromResultState() never matched
  them and an impure `$a?->b()` was remembered as true. The gate is now read once
  off the walked twin node and applied to the whole key family.
- Extracted the impure-point gate into DefaultNarrowingHelper::isSubjectValueRemembered()
  and reused it from createSubjectTypesFromResultState().
- Probed and found already correct: FuncCallHandler (fixed by 3b57b6b),
  NewHandler and the callable-call assert path (neither unions a default key,
  matching 2.2.x), conditional return types, type-specifying extensions,
  @phpstan-assert / @phpstan-assert-if-false, static::/parent::/self:: calls,
  interface methods, loops, match, empty(), ternary, ?:, first-class callables
  and closures - with rememberPossiblyImpureFunctionValues both true and false.

Author: phpstan-bot

src/Analyser/ExprHandler/Helper/DefaultNarrowingHelper.php

@@ -121,9 +121,8 @@ public function specifyDefaultTypes(Expr $expr, TypeSpecifierContext $context):
 	 * specifyDefaultTypes() for a chain containing a nullsafe: a truthy chain
 	 * did not short-circuit, so its fully plain twin ($a?->b()?->c() ->
 	 * $a->b()->c()) holds the same value and the source reads it that way once
-	 * every link is known non-null. The twin is keyed through the create path,
-	 * so the impure gate applies to it exactly as to the chain itself - an
-	 * impure call's value is never remembered, plain or nullsafe.
+	 * every link is known non-null. The impure gate applies to the whole family
+	 * of keys - an impure call's value is never remembered, plain or nullsafe.
 	 */
 	public function specifyDefaultTypesWithPlainTwin(Expr $expr, ?ExpressionResult $exprResult, TypeSpecifierContext $context, MutatingScope $s): SpecifiedTypes
 	{
@@ -134,11 +133,18 @@ public function specifyDefaultTypesWithPlainTwin(Expr $expr, ?ExpressionResult $
 
 		// The result is the one-level twin's ($a?->b() walked as $a->b()), and
 		// its impure point is keyed to that very node - so the twin node it
-		// carries is the subject, not a rebuilt one (the impure gate compares
-		// node identity). For a deeper chain the fully plain form differs from
-		// the twin only in its receiver; it is keyed too, through the same gate.
+		// carries is the subject, not a rebuilt one (the gate compares node
+		// identity). The `?->` node and the deeper chain's fully plain form are
+		// rebuilt, so they cannot be gated by identity - they name the same call
+		// and share the twin's verdict.
 		$twinNode = $exprResult->getExpr();
 		$twin = $this->createSubjectTypesFromResultState($s, $twinNode, $exprResult, StaticTypeFactory::falsey(), TypeSpecifierContext::createFalse());
+		if (!$this->isSubjectValueRemembered($exprResult, $twinNode)) {
+			// only the receiver-not-null fan createSubjectTypesFromResultState()
+			// produced for the impure call survives
+			return $twin->setRootExpr($expr);
+		}
+
 		$plainChain = NullsafeOperatorHelper::getNullsafeShortcircuitedExpr($expr);
 		if ($plainChain !== $expr && $this->exprPrinter->printExpr($plainChain) !== $this->exprPrinter->printExpr($twinNode)) {
 			$twin = $twin->unionWith($this->createSubjectTypesFromResultState($s, $plainChain, $exprResult, StaticTypeFactory::falsey(), TypeSpecifierContext::createFalse()));
@@ -147,6 +153,34 @@ public function specifyDefaultTypesWithPlainTwin(Expr $expr, ?ExpressionResult $
 		return $default->unionWith($twin)->setRootExpr($expr);
 	}
 
+	/**
+	 * The impure gate read off an already-processed result: a call whose own
+	 * execution is (possibly) impure must not get a remembered type. The
+	 * result's impure point is keyed to the very node that was walked, so the
+	 * subject has to be that node and not a rebuilt one.
+	 */
+	private function isSubjectValueRemembered(ExpressionResult $subjectResult, Expr $subject): bool
+	{
+		if (
+			!$subject instanceof Expr\FuncCall
+			&& !$subject instanceof Expr\MethodCall
+			&& !$subject instanceof Expr\StaticCall
+			&& !$subject instanceof Expr\NullsafeMethodCall
+		) {
+			return true;
+		}
+
+		foreach ($subjectResult->getImpurePoints() as $impurePoint) {
+			if ($impurePoint->getNode() !== $subject) {
+				continue;
+			}
+
+			return !$impurePoint->isCertain() && $this->rememberPossiblyImpureFunctionValues;
+		}
+
+		return true;
+	}
+
 	/**
 	 * Converts sure-not entries to sure form against the given evaluation
 	 * scope (position-fixed, captured at compose time) - for the decided
@@ -216,31 +250,17 @@ public function createSubjectTypesFromResultState(MutatingScope $s, Expr $subjec
 			// a call whose own execution is (possibly) impure must not get a
 			// remembered type - the gate reads the result's own impure point
 			// instead of re-asking reflection like the old create() did
-			if (
-				$subject instanceof Expr\FuncCall
-				|| $subject instanceof Expr\MethodCall
-				|| $subject instanceof Expr\StaticCall
-				|| $subject instanceof Expr\NullsafeMethodCall
-			) {
-				foreach ($subjectResult->getImpurePoints() as $impurePoint) {
-					if ($impurePoint->getNode() !== $subject) {
-						continue;
-					}
-					if ($impurePoint->isCertain() || !$this->rememberPossiblyImpureFunctionValues) {
-						// the call's value is not remembered, but a nullsafe
-						// receiver chain still narrows not-null: the chain must
-						// have evaluated for the (impure) call to produce any
-						// non-null value at all (mirrors the old createForExpr()
-						// returning createNullsafeTypes() from its impure branch)
-						if ($subjectResult->containsNullsafe() && $this->nullsafeShortCircuitRuledOut($s, $subjectResult, $type, $context)) {
-							return $this->createFirstNullsafeReceiverTypes($s, $subject) ?? new SpecifiedTypes([], []);
-						}
-
-						return new SpecifiedTypes([], []);
-					}
-
-					break;
+			if (!$this->isSubjectValueRemembered($subjectResult, $subject)) {
+				// the call's value is not remembered, but a nullsafe receiver
+				// chain still narrows not-null: the chain must have evaluated
+				// for the (impure) call to produce any non-null value at all
+				// (mirrors the old createForExpr() returning createNullsafeTypes()
+				// from its impure branch)
+				if ($subjectResult->containsNullsafe() && $this->nullsafeShortCircuitRuledOut($s, $subjectResult, $type, $context)) {
+					return $this->createFirstNullsafeReceiverTypes($s, $subject) ?? new SpecifiedTypes([], []);
 				}
+
+				return new SpecifiedTypes([], []);
 			}
 
 			// a chain containing a nullsafe narrows its short-circuited plain

src/Analyser/ExprHandler/MethodCallHandler.php

@@ -536,8 +536,12 @@ private function specifyTypes(NodeScopeResolver $nodeScopeResolver, MutatingScop
 				));
 				$specifiedTypes = $this->defaultNarrowingHelper->specifyTypesFromAsserts($context, $expr, $asserts, $resolvedParametersAcceptor, $scope);
 				if ($specifiedTypes !== null) {
+					// the asserts narrow their subjects regardless; the call's OWN
+					// key goes through the same purity gate as the assert-less
+					// default narrowing - an impure method evaluated a second time
+					// must not read the first call's truthiness
 					return $specifiedTypes
-						->unionWith($this->defaultNarrowingHelper->specifyDefaultTypes($expr, $context))
+						->unionWith($this->defaultMethodCallNarrowing($scope, $expr, $varResult, $context))
 						->setRootExpr($specifiedTypes->getRootExpr());
 				}
 			}

src/Analyser/ExprHandler/StaticCallHandler.php

@@ -604,8 +604,10 @@ private function specifyTypes(NodeScopeResolver $nodeScopeResolver, MutatingScop
 				));
 				$specifiedTypes = $this->defaultNarrowingHelper->specifyTypesFromAsserts($context, $expr, $asserts, $resolvedParametersAcceptor, $scope);
 				if ($specifiedTypes !== null) {
+					// see MethodCallHandler::specifyTypes() - the call's own key gets
+					// the purity gate, the asserts narrow their subjects regardless
 					return $specifiedTypes
-						->unionWith($this->defaultNarrowingHelper->specifyDefaultTypes($expr, $context))
+						->unionWith($this->defaultStaticCallNarrowing($scope, $expr, $classResult, $nodeScopeResolver, $context))
 						->setRootExpr($specifiedTypes->getRootExpr());
 				}
 			}

tests/PHPStan/Analyser/nsrt/impure-call-asserts-not-remembered.php

@@ -16,3 +16,56 @@ function doFoo(string $path): void
 		assertType('non-empty-string|false', realpath($path));
 	}
 }
+
+class Foo
+{
+
+	/** @var resource|null */
+	private $process = null;
+
+	/**
+	 * @phpstan-impure
+	 * @phpstan-assert-if-true !null $this->process
+	 */
+	private function impureAssert(): bool
+	{
+		return $this->process !== null && (bool) microtime(true);
+	}
+
+	/**
+	 * @phpstan-impure
+	 * @phpstan-assert-if-true !null $foo->process
+	 */
+	private static function impureStaticAssert(self $foo): bool
+	{
+		return $foo->process !== null && (bool) microtime(true);
+	}
+
+	public function doMethod(): void
+	{
+		// the assert narrows $this->process, the impure call's own value is
+		// never remembered
+		if ($this->impureAssert()) {
+			assertType('resource', $this->process);
+			assertType('bool', $this->impureAssert());
+		}
+	}
+
+	public function doStatic(self $foo): void
+	{
+		if (self::impureStaticAssert($foo)) {
+			assertType('resource', $foo->process);
+			assertType('bool', self::impureStaticAssert($foo));
+		}
+	}
+
+	public function doNullsafe(?self $foo): void
+	{
+		if ($foo?->impureAssert()) {
+			assertType('resource', $foo->process);
+			assertType('bool', $foo?->impureAssert());
+			assertType('bool', $foo->impureAssert());
+		}
+	}
+
+}

tests/PHPStan/Analyser/nsrt/impure-nullsafe-call-not-remembered.php

@@ -0,0 +1,63 @@
+<?php declare(strict_types = 1);
+
+namespace ImpureNullsafeCallNotRemembered;
+
+use function PHPStan\Testing\assertType;
+
+class Foo
+{
+
+	/** @phpstan-impure */
+	public function impure(): bool
+	{
+		return (bool) microtime(true);
+	}
+
+	/** @phpstan-pure */
+	public function pure(): bool
+	{
+		return true;
+	}
+
+	/** @phpstan-pure */
+	public function getSelf(): ?self
+	{
+		return $this;
+	}
+
+	public function doImpure(?self $foo): void
+	{
+		// neither the `?->` key nor its plain twin remembers an impure value
+		if ($foo?->impure()) {
+			assertType('ImpureNullsafeCallNotRemembered\Foo', $foo);
+			assertType('bool', $foo?->impure());
+			assertType('bool', $foo->impure());
+		}
+	}
+
+	public function doImpureChain(?self $foo): void
+	{
+		if ($foo?->getSelf()?->impure()) {
+			assertType('ImpureNullsafeCallNotRemembered\Foo', $foo);
+			assertType('bool', $foo?->getSelf()?->impure());
+			assertType('bool', $foo->getSelf()->impure());
+		}
+	}
+
+	public function doPure(?self $foo): void
+	{
+		if ($foo?->pure()) {
+			assertType('true', $foo?->pure());
+			assertType('true', $foo->pure());
+		}
+	}
+
+	public function doPureChain(?self $foo): void
+	{
+		if ($foo?->getSelf()?->pure()) {
+			assertType('true', $foo?->getSelf()?->pure());
+			assertType('true', $foo->getSelf()->pure());
+		}
+	}
+
+}

tests/PHPStan/Rules/Comparison/BooleanAndConstantConditionRuleTest.php

@@ -540,4 +540,10 @@ public function testInTrait(): void
 		]);
 	}
 
+	public function testBug15170(): void
+	{
+		$this->treatPhpDocTypesAsCertain = true;
+		$this->analyse([__DIR__ . '/data/bug-15170.php'], []);
+	}
+
 }

tests/PHPStan/Rules/Comparison/IfConstantConditionRuleTest.php

@@ -423,4 +423,10 @@ public function testMarkerFromAnotherFileDoesNotSuppress(): void
 		]);
 	}
 
+	public function testBug15170(): void
+	{
+		$this->treatPhpDocTypesAsCertain = true;
+		$this->analyse([__DIR__ . '/data/bug-15170.php'], []);
+	}
+
 }