Combine the callable and parameter-passed purity verdicts

A function can carry both @pure-unless-callable-is-impure and
@pure-unless-parameter-passed at once - preg_replace_callback() is pure
unless its callback is impure or its $count is passed. The two verdicts
were checked in sequence, so a pure callback short-circuited to "pure"
and never looked at $count. Combine them with TrinaryLogic::and() in both
SimpleImpurePoint::createFromVariant() and NewHandler, and flag
preg_replace_callback()'s $count.

Author: zonuexe

bin/functionMetadata_original.php

@@ -5,7 +5,7 @@
  * keyed by lowercase function name or "Class::method". resources/functionMetadata.php
  * is generated from this file by bin/generate-function-metadata.php.
  *
- * Each entry is exactly one of these shapes:
+ * Each entry has one of these shapes:
  *
  *   - ['hasSideEffects' => bool]
  *         false: the call is pure. true: the call has side effects.
@@ -17,9 +17,12 @@
  *         the call is pure unless one of the listed (by-ref out) parameters
  *         (keyed by parameter name) receives an argument, e.g. str_replace()
  *         whose only side effect is writing to its optional 'count' argument.
+ *
+ * The last two can be combined for a call that is pure unless either happens,
+ * e.g. preg_replace_callback() (impure callback or a passed 'count').
  */
 
-/** @var array<string, array{hasSideEffects: bool}|array{pureUnlessCallableIsImpureParameters: array<string, bool>}|array{pureUnlessParameterPassedParameters: array<string, bool>}> */
+/** @var array<string, array{hasSideEffects: bool}|array{pureUnlessCallableIsImpureParameters: array<string, bool>}|array{pureUnlessParameterPassedParameters: array<string, bool>}|array{pureUnlessCallableIsImpureParameters: array<string, bool>, pureUnlessParameterPassedParameters: array<string, bool>}> */
 return [
 	'abs' => ['hasSideEffects' => false],
 	'acos' => ['hasSideEffects' => false],
@@ -274,7 +277,7 @@
 	'preg_match' => ['pureUnlessParameterPassedParameters' => ['matches' => true, 'subpatterns' => true]],
 	'preg_match_all' => ['pureUnlessParameterPassedParameters' => ['matches' => true, 'subpatterns' => true]],
 	'preg_replace' => ['pureUnlessParameterPassedParameters' => ['count' => true]],
-	'preg_replace_callback' => ['pureUnlessCallableIsImpureParameters' => ['callback' => true]],
+	'preg_replace_callback' => ['pureUnlessCallableIsImpureParameters' => ['callback' => true], 'pureUnlessParameterPassedParameters' => ['count' => true]],
 	'similar_text' => ['pureUnlessParameterPassedParameters' => ['percent' => true]],
 	'readfile' => ['hasSideEffects' => true],
 	'rename' => ['hasSideEffects' => true],

resources/functionMetadata.php

@@ -24,7 +24,7 @@
  *     an argument, e.g. str_replace()'s 'count'.
  */
 
-/** @var array<string, array{hasSideEffects: bool}|array{pureUnlessCallableIsImpureParameters: array<string, bool>}|array{pureUnlessParameterPassedParameters: array<string, bool>}> */
+/** @var array<string, array{hasSideEffects: bool}|array{pureUnlessCallableIsImpureParameters: array<string, bool>}|array{pureUnlessParameterPassedParameters: array<string, bool>}|array{pureUnlessCallableIsImpureParameters: array<string, bool>, pureUnlessParameterPassedParameters: array<string, bool>}> */
 return [
 	'BackedEnum::from' => ['hasSideEffects' => false],
 	'BackedEnum::tryFrom' => ['hasSideEffects' => false],
@@ -1642,7 +1642,7 @@
 	'preg_match_all' => ['pureUnlessParameterPassedParameters' => ['matches' => true, 'subpatterns' => true]],
 	'preg_quote' => ['hasSideEffects' => false],
 	'preg_replace' => ['pureUnlessParameterPassedParameters' => ['count' => true]],
-	'preg_replace_callback' => ['pureUnlessCallableIsImpureParameters' => ['callback' => true]],
+	'preg_replace_callback' => ['pureUnlessCallableIsImpureParameters' => ['callback' => true], 'pureUnlessParameterPassedParameters' => ['count' => true]],
 	'preg_split' => ['hasSideEffects' => false],
 	'property_exists' => ['hasSideEffects' => false],
 	'quoted_printable_decode' => ['hasSideEffects' => false],

src/Analyser/ExprHandler/NewHandler.php

@@ -259,24 +259,22 @@ private function processConstructorReflection(string $className, New_ $expr, Mut
 		if ($constructorReflection !== null) {
 			if (!$constructorReflection->hasSideEffects()->no()) {
 				$certain = $constructorReflection->isPure()->no();
+				// A constructor can carry both flags at once, so combine the verdicts
+				// the same way SimpleImpurePoint::createFromVariant() does for calls:
+				// Yes = pure, No = impure, Maybe = possibly impure.
 				$verdict = SimpleImpurePoint::resolvePureUnlessCallableIsImpureVerdict($parametersAcceptor, $scope, $expr->getArgs());
+				$passedVerdict = SimpleImpurePoint::resolvePureUnlessParameterPassedVerdict($parametersAcceptor, $expr->getArgs());
+				if ($passedVerdict !== null) {
+					$verdict = $verdict === null ? $passedVerdict : $verdict->and($passedVerdict);
+				}
+
 				if ($verdict !== null && $verdict->yes()) {
 					return [$constructorReflection, $classReflection, $parametersAcceptor, $impurePoints];
 				}
 				if ($verdict !== null && $verdict->no()) {
 					$certain = true;
 				}
 
-				if (!$certain) {
-					$passedVerdict = SimpleImpurePoint::resolvePureUnlessParameterPassedVerdict($parametersAcceptor, $expr->getArgs());
-					if ($passedVerdict !== null && $passedVerdict->yes()) {
-						return [$constructorReflection, $classReflection, $parametersAcceptor, $impurePoints];
-					}
-					if ($passedVerdict !== null && $passedVerdict->no()) {
-						$certain = true;
-					}
-				}
-
 				$impurePoints[] = new ImpurePoint(
 					$scope,
 					$expr,

src/Reflection/Callables/SimpleImpurePoint.php

@@ -63,7 +63,16 @@ public static function createFromVariant(FunctionReflection|ExtendedMethodReflec
 			}
 
 			if (!$certain && $scope !== null && $variant !== null) {
+				// A function can carry both flags at once (e.g. preg_replace_callback,
+				// which is pure unless its callback is impure or its $count is passed).
+				// It stays pure only when both verdicts agree it is pure, so combine
+				// them: Yes = pure, No = impure, Maybe = possibly impure.
 				$verdict = self::resolvePureUnlessCallableIsImpureVerdict($variant, $scope, $args);
+				$passedVerdict = self::resolvePureUnlessParameterPassedVerdict($variant, $args);
+				if ($passedVerdict !== null) {
+					$verdict = $verdict === null ? $passedVerdict : $verdict->and($passedVerdict);
+				}
+
 				if ($verdict !== null) {
 					if ($verdict->yes()) {
 						return null;
@@ -74,20 +83,6 @@ public static function createFromVariant(FunctionReflection|ExtendedMethodReflec
 				}
 			}
 
-			if (!$certain && $scope !== null && $variant !== null) {
-				$passedVerdict = self::resolvePureUnlessParameterPassedVerdict($variant, $args);
-				if ($passedVerdict !== null) {
-					if ($passedVerdict->yes()) {
-						// None of the @pure-unless-parameter-passed by-ref parameters
-						// received an argument, so the call is pure.
-						return null;
-					}
-					if ($passedVerdict->no()) {
-						$certain = true;
-					}
-				}
-			}
-
 			if ($function instanceof FunctionReflection) {
 				if (isset(self::SIDE_EFFECT_FLIP_PARAMETERS[$function->getName()]) && $scope !== null) {
 					[

tests/PHPStan/Rules/Pure/PureFunctionRuleTest.php

@@ -460,6 +460,18 @@ public function testPureUnlessParameterPassedBuiltin(): void
 				'Impure call to function preg_filter() in pure function PureUnlessParameterPassedBuiltin\purePregFilterWithCount().',
 				60,
 			],
+			[
+				'Impure call to function preg_replace_callback() in pure function PureUnlessParameterPassedBuiltin\purePregReplaceCallbackWithCount().',
+				80,
+			],
+			[
+				'Impure call to function preg_replace_callback() in pure function PureUnlessParameterPassedBuiltin\purePregReplaceCallbackImpureCallback().',
+				89,
+			],
+			[
+				'Impure echo in pure function PureUnlessParameterPassedBuiltin\purePregReplaceCallbackImpureCallback().',
+				90,
+			],
 		]);
 	}
 

tests/PHPStan/Rules/Pure/data/pure-unless-parameter-passed-builtin.php

@@ -59,3 +59,36 @@ function purePregFilterWithCount(string $s): ?string
 
 	return preg_filter('/a/', 'b', $s, -1, $count);
 }
+
+/**
+ * @phpstan-pure
+ */
+function purePregReplaceCallbackPureCallback(string $s): ?string
+{
+	// preg_replace_callback() carries both flags: the callback is pure and the by-ref
+	// $count is omitted, so the call stays pure.
+	return preg_replace_callback('/a/', static fn (array $m): string => $m[0], $s);
+}
+
+/**
+ * @phpstan-pure
+ */
+function purePregReplaceCallbackWithCount(string $s): ?string
+{
+	$count = 0;
+	// The callback is pure but the by-ref $count is passed, so the call is impure.
+	return preg_replace_callback('/a/', static fn (array $m): string => $m[0], $s, -1, $count);
+}
+
+/**
+ * @phpstan-pure
+ */
+function purePregReplaceCallbackImpureCallback(string $s): ?string
+{
+	// The callback itself is impure, so the call is impure regardless of $count.
+	return preg_replace_callback('/a/', static function (array $m): string {
+		echo $m[0];
+
+		return $m[0];
+	}, $s);
+}