Do not remember a void equality assertion statement as true when its arguments contain an impure call

src/Analyser/StmtHandler/ExpressionHandler.php

@@ -8,6 +8,7 @@
 use PhpParser\Node\Stmt\Expression;
 use PHPStan\Analyser\ExpressionContext;
 use PHPStan\Analyser\ExpressionResultStorage;
+use PHPStan\Analyser\ImpurePoint;
 use PHPStan\Analyser\InternalStatementExitPoint;
 use PHPStan\Analyser\InternalStatementResult;
 use PHPStan\Analyser\MutatingScope;
@@ -17,6 +18,7 @@
 use PHPStan\Analyser\StatementsHandler;
 use PHPStan\Analyser\StmtHandler;
 use PHPStan\Analyser\TypeSpecifierContext;
+use PHPStan\DependencyInjection\AutowiredParameter;
 use PHPStan\DependencyInjection\AutowiredService;
 use PHPStan\Node\NoopExpressionNode;
 use PHPStan\Node\PropertyAssignNode;
@@ -37,6 +39,8 @@ final class ExpressionHandler implements StmtHandler
 
 	public function __construct(
 		private StatementsHandler $statementsHandler,
+		#[AutowiredParameter]
+		private bool $rememberPossiblyImpureFunctionValues,
 	)
 	{
 	}
@@ -102,7 +106,7 @@ public function processStmt(
 		$specifiedTypes = $result->getSpecifiedTypesForScope($scope, TypeSpecifierContext::createNull());
 		$scope = $scope->applySpecifiedTypes($specifiedTypes);
 
-		if ($specifiedTypes->isEquality()) {
+		if ($specifiedTypes->isEquality() && $this->isCallRememberedDespiteItsOwnImpurity($result->getImpurePoints(), $stmt->expr)) {
 			// Statement counterpart of ExpressionResult's equality handling:
 			// store the call's true result so a duplicate void assertion statement is
 			// reported as always-true. We assign directly because void calls have no
@@ -127,4 +131,38 @@ public function processStmt(
 		return new InternalStatementResult($scope, hasYield: $hasYield, isAlwaysTerminating: $isAlwaysTerminating, exitPoints: [], throwPoints: $throwPoints, impurePoints: $impurePoints, variableFlow: $result->getVariableFlow());
 	}
 
+	/**
+	 * A void assertion is impure by itself, but repeating it only repeats the
+	 * same check when nothing else in it has side effects - an impure call in
+	 * its arguments (`assertSame($a, $counter->next())`) yields a new value
+	 * every time.
+	 *
+	 * @param ImpurePoint[] $impurePoints
+	 */
+	private function isCallRememberedDespiteItsOwnImpurity(array $impurePoints, Expr $call): bool
+	{
+		foreach ($impurePoints as $impurePoint) {
+			$node = $impurePoint->getNode();
+			if ($node === $call) {
+				continue;
+			}
+			if (
+				!$node instanceof Expr\FuncCall
+				&& !$node instanceof Expr\MethodCall
+				&& !$node instanceof Expr\StaticCall
+				&& !$node instanceof Expr\NullsafeMethodCall
+			) {
+				continue;
+			}
+
+			if (!$impurePoint->isCertain() && $this->rememberPossiblyImpureFunctionValues) {
+				continue;
+			}
+
+			return false;
+		}
+
+		return true;
+	}
+
 }

tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php

@@ -537,6 +537,17 @@ public function testBug14705Php8(): void
 		]);
 	}
 
+	#[RequiresPhp('>= 8.0.0')]
+	public function testBug15328(): void
+	{
+		$this->analyse([__DIR__ . '/data/bug-15328.php'], [
+			[
+				'Call to function assertSame() with int and int will always evaluate to true.',
+				59,
+			],
+		]);
+	}
+
 	public function testBug2755(): void
 	{
 		$this->analyse([__DIR__ . '/data/bug-2755.php'], []);

tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeMethodCallRuleTest.php

@@ -320,6 +320,18 @@ public function testBug10337(): void
 		$this->analyse([__DIR__ . '/data/bug-10337.php'], []);
 	}
 
+	#[RequiresPhp('>= 8.0.0')]
+	public function testBug15328(): void
+	{
+		$this->treatPhpDocTypesAsCertain = true;
+		$this->analyse([__DIR__ . '/data/bug-15328-method.php'], [
+			[
+				'Call to method Bug15328Method\\Assert::assertSame() with int and int will always evaluate to true.',
+				69,
+			],
+		]);
+	}
+
 	public function testBug14705(): void
 	{
 		$this->treatPhpDocTypesAsCertain = true;

tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeStaticMethodCallRuleTest.php

@@ -126,6 +126,18 @@ public function testBug15223TreatPhpDocTypesAsCertain(): void
 		$this->analyse([__DIR__ . '/data/bug-15223-static.php'], []);
 	}
 
+	#[RequiresPhp('>= 8.0.0')]
+	public function testBug15328(): void
+	{
+		$this->treatPhpDocTypesAsCertain = true;
+		$this->analyse([__DIR__ . '/data/bug-15328-method.php'], [
+			[
+				'Call to static method Bug15328Method\\Assert::assertSame() with int and int will always evaluate to true.',
+				75,
+			],
+		]);
+	}
+
 	public function testAssertUnresolvedGeneric(): void
 	{
 		$this->treatPhpDocTypesAsCertain = true;

tests/PHPStan/Rules/Comparison/data/bug-15328-method.php

@@ -0,0 +1,78 @@
+<?php // lint >= 8.0
+
+declare(strict_types = 1);
+
+namespace Bug15328Method;
+
+use Exception;
+
+class Assert
+{
+
+	/**
+	 * @template ExpectedType
+	 *
+	 * @param ExpectedType $expected
+	 *
+	 * @phpstan-assert =ExpectedType $actual
+	 */
+	final public static function assertSame(mixed $expected, mixed $actual): void
+	{
+		if ($expected !== $actual) {
+			throw new Exception;
+		}
+	}
+
+}
+
+final class Counter
+{
+	private int $count = 0;
+
+	/** @phpstan-impure */
+	public function next(): int
+	{
+		return ++$this->count;
+	}
+}
+
+final class ContextTest extends Assert
+{
+
+	public function testMethodCall(int $key): void
+	{
+		$counter = new Counter;
+
+		$this->assertSame($key, $counter->next());
+		$this->assertSame($key, $counter->next());
+	}
+
+	public function testStaticCall(int $key): void
+	{
+		$counter = new Counter;
+
+		self::assertSame($key, $counter->next());
+		self::assertSame($key, $counter->next());
+	}
+
+	public function testImpureFunction(int $key): void
+	{
+		$this->assertSame($key, random_int(1, 10));
+		$this->assertSame($key, random_int(1, 10));
+		self::assertSame($key, random_int(1, 10));
+		self::assertSame($key, random_int(1, 10));
+	}
+
+	public function testPureMethodCall(int $key, int $actual): void
+	{
+		$this->assertSame($key, $actual);
+		$this->assertSame($key, $actual);
+	}
+
+	public function testPureStaticCall(int $key, int $actual): void
+	{
+		self::assertSame($key, $actual);
+		self::assertSame($key, $actual);
+	}
+
+}

tests/PHPStan/Rules/Comparison/data/bug-15328.php

@@ -0,0 +1,60 @@
+<?php // lint >= 8.0
+
+declare(strict_types = 1);
+
+namespace Bug15328;
+
+use Exception;
+
+/**
+ * Same signature and PHPDoc as PHPUnit\Framework\Assert::assertSame()
+ *
+ * @template ExpectedType
+ *
+ * @param ExpectedType $expected
+ *
+ * @phpstan-assert =ExpectedType $actual
+ */
+function assertSame(mixed $expected, mixed $actual): void
+{
+	if ($expected !== $actual) {
+		throw new Exception;
+	}
+}
+
+final class Counter
+{
+	private int $count = 0;
+
+	/** @phpstan-impure */
+	public function next(): int
+	{
+		return ++$this->count;
+	}
+}
+
+function impureMethod(int $key): void
+{
+	$counter = new Counter;
+
+	assertSame($key, $counter->next());
+	assertSame($key, $counter->next());
+}
+
+function impureFunction(int $key): void
+{
+	assertSame($key, random_int(1, 10));
+	assertSame($key, random_int(1, 10));
+}
+
+function impureExpected(int $actual): void
+{
+	assertSame(random_int(1, 10), $actual);
+	assertSame(random_int(1, 10), $actual);
+}
+
+function pure(int $key, int $actual): void
+{
+	assertSame($key, $actual);
+	assertSame($key, $actual);
+}

turbo-ext/src/ExpressionHandler.cpp

@@ -68,10 +68,12 @@ class ExpressionHandler
 public:
 	explicit ExpressionHandler(zend_object *self) : self(self) {}
 
-	/* the constructor body: the promoted property */
-	void construct(zval *statementsHandler)
+	/* the constructor body: the promoted properties */
+	void construct(zval *statementsHandler, bool rememberPossiblyImpureFunctionValues)
 	{
-		zv::ObjRef(self).propAtWrite(slots::statementsHandler, zv::Val::copyOf(zv::Ref(statementsHandler)));
+		zv::ObjRef object(self);
+		object.propAtWrite(slots::statementsHandler, zv::Val::copyOf(zv::Ref(statementsHandler)));
+		object.propAtWrite(slots::rememberPossiblyImpureFunctionValues, zv::Val::boolean(rememberPossiblyImpureFunctionValues));
 	}
 
 	/* Mirrors supports(); false = pending exception */
@@ -166,13 +168,20 @@ class ExpressionHandler
 
 		bool isEquality;
 		if (UNEXPECTED(!pt_specified_types_is_equality(specifiedTypes.raw(), isEquality))) return zv::Val();
+		zval *statementExpression = NULL;
+		bool remembered = false;
 		if (isEquality) {
+			statementExpression = statementExpr(stmt);
+			if (UNEXPECTED(statementExpression == NULL)) return zv::Val();
+			impurePoints = pt_expression_result_impure_points(resultValue, impurePointsHold);
+			if (UNEXPECTED(impurePoints == NULL)) return zv::Val();
+			if (UNEXPECTED(!isCallRememberedDespiteItsOwnImpurity(impurePoints, statementExpression, remembered))) return zv::Val();
+		}
+		if (remembered) {
 			// Statement counterpart of ExpressionResult's equality handling:
 			// store the call's true result so a duplicate void assertion statement is
 			// reported as always-true. Assigned directly because void calls have no
 			// return value to protect, and intersecting true with void would produce never.
-			zval *statementExpression = statementExpr(stmt);
-			if (UNEXPECTED(statementExpression == NULL)) return zv::Val();
 			zval trueType, trueNativeType;
 			if (UNEXPECTED(!pt_constant_boolean_type_new(&trueType, true))) return zv::Val();
 			zv::Val trueTypeHold = zv::Val::adopt(trueType);
@@ -223,6 +232,51 @@ class ExpressionHandler
 private:
 	zend_object *self;
 
+	/* Mirrors isCallRememberedDespiteItsOwnImpurity(); false = pending exception */
+	[[nodiscard]] bool isCallRememberedDespiteItsOwnImpurity(zval *impurePoints, zval *call, bool &out) const
+	{
+		if (UNEXPECTED(Z_TYPE_P(impurePoints) != IS_ARRAY)) {
+			zend_type_error("foreach() argument must be of type array|object, %s given", zend_zval_value_name(impurePoints));
+			return false;
+		}
+		/* the slot is borrowed: keep the array alive across the calls */
+		zv::Val impurePointsCopy = zv::Val::copyOf(zv::Ref(impurePoints));
+		for (zv::ArrayEntry entry : zv::ArrRef(impurePointsCopy.raw())) {
+			zval *impurePoint = entry.value().deref().raw();
+			if (UNEXPECTED(Z_TYPE_P(impurePoint) != IS_OBJECT)) {
+				memberCallOnNonObject("getNode", impurePoint);
+				return false;
+			}
+			zv::Val nodeHold;
+			zval *node = pt_impure_point_node(impurePoint, nodeHold);
+			if (UNEXPECTED(node == NULL)) return false;
+			if (Z_TYPE_P(node) == IS_OBJECT && Z_TYPE_P(call) == IS_OBJECT && Z_OBJ_P(node) == Z_OBJ_P(call)) {
+				continue;
+			}
+			bool error = false;
+			bool isCall = ptsh::isInstanceOf(node, PT_CLASS_FUNC_CALL, error)
+				|| ptsh::isInstanceOf(node, PT_CLASS_METHOD_CALL, error)
+				|| ptsh::isInstanceOf(node, PT_CLASS_STATIC_CALL, error)
+				|| ptsh::isInstanceOf(node, PT_CLASS_NULLSAFE_METHOD_CALL, error);
+			if (UNEXPECTED(error)) return false;
+			if (!isCall) {
+				continue;
+			}
+
+			bool certain = false;
+			if (UNEXPECTED(!pt_impure_point_is_certain(impurePoint, certain))) return false;
+			if (!certain && Z_TYPE_P(OBJ_PROP_NUM(self, slots::rememberPossiblyImpureFunctionValues)) == IS_TRUE) {
+				continue;
+			}
+
+			out = false;
+			return true;
+		}
+
+		out = true;
+		return true;
+	}
+
 	/* the twin's try block: the expression walked, a thrown expression's
 	 * @var-changed-type node emitted */
 	static zv::Val processExpression(zval *nodeScopeResolver, zval *stmt, zval *scope, zval *storage, zval *nodeCallback, zval *context, zval *statementsHandler, zval *preAnnotationScope)
@@ -332,8 +386,9 @@ PT_MINIT_REGISTRATION(pt_register_expression_handler)
 	 * service by reflecting the constructor */
 	cls.method(sigs::__construct, [](INTERNAL_FUNCTION_PARAMETERS) {
 		zval *statementsHandler;
-		if (!zp::parse<zp::Obj>(execute_data, statementsHandler)) RETURN_THROWS();
-		ExpressionHandler(Z_OBJ_P(ZEND_THIS)).construct(statementsHandler);
+		bool rememberPossiblyImpureFunctionValues;
+		if (!zp::parse<zp::Obj, zp::Bool>(execute_data, statementsHandler, rememberPossiblyImpureFunctionValues)) RETURN_THROWS();
+		ExpressionHandler(Z_OBJ_P(ZEND_THIS)).construct(statementsHandler, rememberPossiblyImpureFunctionValues);
 	});
 
 	cls.method<&ExpressionHandler::supports, zp::Obj>(sigs::supports);