Allow forwarding a @pure-unless-callable-is-impure callback

src/Rules/Pure/FunctionPurityCheck.php

@@ -2,6 +2,7 @@
 
 namespace PHPStan\Rules\Pure;
 
+use PhpParser\Node\Arg;
 use PhpParser\Node\Expr\FuncCall;
 use PhpParser\Node\Expr\Variable;
 use PhpParser\Node\Name;
@@ -13,6 +14,8 @@
 use PHPStan\Reflection\ExtendedMethodReflection;
 use PHPStan\Reflection\ExtendedParameterReflection;
 use PHPStan\Reflection\FunctionReflection;
+use PHPStan\Reflection\ParametersAcceptorSelector;
+use PHPStan\Reflection\ReflectionProvider;
 use PHPStan\Rules\Functions\CallToFunctionStatementWithoutSideEffectsRule;
 use PHPStan\Rules\IdentifierRuleError;
 use PHPStan\Rules\RuleErrorBuilder;
@@ -30,6 +33,10 @@
 final class FunctionPurityCheck
 {
 
+	public function __construct(private ReflectionProvider $reflectionProvider)
+	{
+	}
+
 	/**
 	 * @param 'Function'|'Method' $identifier
 	 * @param ExtendedParameterReflection[] $parameters
@@ -117,12 +124,12 @@ public function check(
 				))->identifier(sprintf('pure%s.void', $identifier))->build();
 			}
 
-			$errors = array_merge($errors, $this->reportImpurePoints($impurePoints, $pureUnlessCallableParamNames, $functionDescription));
+			$errors = array_merge($errors, $this->reportImpurePoints($scope, $impurePoints, $pureUnlessCallableParamNames, $functionDescription));
 		} elseif ($pureUnlessCallableParamNames !== []) {
 			// A function declared @pure-unless-callable-is-impure is pure except
 			// for the flagged callables, so its body is checked for purity while
 			// the flagged callables' own invocations are exempt.
-			$errors = array_merge($errors, $this->reportImpurePoints($impurePoints, $pureUnlessCallableParamNames, $functionDescription));
+			$errors = array_merge($errors, $this->reportImpurePoints($scope, $impurePoints, $pureUnlessCallableParamNames, $functionDescription));
 		} elseif ($isPure->no()) {
 			if (
 				count($throwPoints) === 0
@@ -192,11 +199,11 @@ public function check(
 	 * @param array<string, true> $pureUnlessCallableParamNames
 	 * @return list<IdentifierRuleError>
 	 */
-	private function reportImpurePoints(array $impurePoints, array $pureUnlessCallableParamNames, string $functionDescription): array
+	private function reportImpurePoints(Scope $scope, array $impurePoints, array $pureUnlessCallableParamNames, string $functionDescription): array
 	{
 		$errors = [];
 		foreach ($impurePoints as $impurePoint) {
-			if ($this->isPureUnlessCallableInvocation($impurePoint, $pureUnlessCallableParamNames)) {
+			if ($this->isPureUnlessCallableExempt($scope, $impurePoint, $pureUnlessCallableParamNames)) {
 				continue;
 			}
 
@@ -219,26 +226,133 @@ private function reportImpurePoints(array $impurePoints, array $pureUnlessCallab
 	}
 
 	/**
+	 * Decides whether an impure point found inside a @pure-unless-callable-is-impure
+	 * function's body is actually covered by that annotation, and so should not be
+	 * reported. Three shapes are exempt:
+	 *
+	 *  - the flagged callback used as a value, e.g. `$fun` (passed onward as an argument);
+	 *  - `$fun(...)`, a direct invocation of a flagged callback;
+	 *  - `otherFun($fun)`, a delegating call to another @pure-unless-callable-is-impure
+	 *    function that forwards the flagged callback(s) into all of its own flagged
+	 *    callable parameters.
+	 *
 	 * @param array<string, true> $pureUnlessCallableParamNames
 	 */
-	private function isPureUnlessCallableInvocation(ImpurePoint $impurePoint, array $pureUnlessCallableParamNames): bool
+	private function isPureUnlessCallableExempt(Scope $scope, ImpurePoint $impurePoint, array $pureUnlessCallableParamNames): bool
 	{
 		if ($pureUnlessCallableParamNames === []) {
 			return false;
 		}
 
 		$node = $impurePoint->getNode();
+
+		if ($node instanceof Variable) {
+			return is_string($node->name) && array_key_exists($node->name, $pureUnlessCallableParamNames);
+		}
+
 		if (!$node instanceof FuncCall) {
 			return false;
 		}
-		if (!$node->name instanceof Variable) {
+
+		if ($node->name instanceof Variable) {
+			return is_string($node->name->name) && array_key_exists($node->name->name, $pureUnlessCallableParamNames);
+		}
+
+		if ($node->name instanceof Name) {
+			return $this->isPureUnlessCallableDelegation($scope, $node, $pureUnlessCallableParamNames);
+		}
+
+		return false;
+	}
+
+	/**
+	 * Exempts a call like `otherFun($fun)` where otherFun() is itself flagged
+	 * pure-unless-callable-is-impure and every one of its flagged callable
+	 * parameters receives, as its argument, one of the enclosing function's own
+	 * flagged callbacks. Such a call is pure modulo the enclosing function's
+	 * callbacks, which is exactly what the enclosing function already declares.
+	 *
+	 * Conservative by construction: if the callee can't be resolved, has no
+	 * flagged callable parameters, or any flagged slot is unmatched or filled
+	 * with something other than one of the enclosing function's flagged
+	 * callbacks, this returns false and the impure point is still reported.
+	 *
+	 * @param array<string, true> $pureUnlessCallableParamNames
+	 */
+	private function isPureUnlessCallableDelegation(Scope $scope, FuncCall $node, array $pureUnlessCallableParamNames): bool
+	{
+		if (!$node->name instanceof Name) {
+			return false;
+		}
+		if ($node->isFirstClassCallable()) {
+			return false;
+		}
+		if (!$this->reflectionProvider->hasFunction($node->name, $scope)) {
 			return false;
 		}
-		if (!is_string($node->name->name)) {
+
+		$function = $this->reflectionProvider->getFunction($node->name, $scope);
+		$calleeFlaggedParameters = $function->getPureUnlessCallableIsImpureParameters();
+		if ($calleeFlaggedParameters === []) {
 			return false;
 		}
 
-		return array_key_exists($node->name->name, $pureUnlessCallableParamNames);
+		$variant = ParametersAcceptorSelector::selectFromArgs($scope, $node->getArgs(), $function->getVariants());
+
+		$hasFlaggedParameter = false;
+		foreach ($variant->getParameters() as $parameterIndex => $parameter) {
+			if (!array_key_exists($parameter->getName(), $calleeFlaggedParameters)) {
+				continue;
+			}
+
+			$hasFlaggedParameter = true;
+
+			$matchedArg = $this->findMatchedArg($node->getArgs(), $parameterIndex, $parameter->getName());
+			if ($matchedArg === null) {
+				return false;
+			}
+
+			if (!$matchedArg->value instanceof Variable) {
+				return false;
+			}
+			if (!is_string($matchedArg->value->name)) {
+				return false;
+			}
+			if (!array_key_exists($matchedArg->value->name, $pureUnlessCallableParamNames)) {
+				return false;
+			}
+		}
+
+		return $hasFlaggedParameter;
+	}
+
+	/**
+	 * Matches call arguments to a parameter the same way
+	 * SimpleImpurePoint::resolvePureUnlessCallableIsImpureVerdict() does:
+	 * a named argument matching the parameter name wins, otherwise the
+	 * positional argument at the parameter's index.
+	 *
+	 * @param Arg[] $args
+	 */
+	private function findMatchedArg(array $args, int $parameterIndex, string $parameterName): ?Arg
+	{
+		$hasNamedParameter = false;
+		foreach ($args as $i => $arg) {
+			if ($arg->name !== null) {
+				$hasNamedParameter = true;
+				if ($arg->name->name === $parameterName) {
+					return $arg;
+				}
+
+				continue;
+			}
+
+			if (!$hasNamedParameter && $i === $parameterIndex) {
+				return $arg;
+			}
+		}
+
+		return null;
 	}
 
 }

tests/PHPStan/Rules/Pure/PureFunctionRuleTest.php

@@ -14,7 +14,7 @@ class PureFunctionRuleTest extends RuleTestCase
 
 	public function getRule(): Rule
 	{
-		return new PureFunctionRule(new FunctionPurityCheck());
+		return new PureFunctionRule(new FunctionPurityCheck($this->createReflectionProvider()));
 	}
 
 	public function testRule(): void
@@ -293,6 +293,22 @@ public function testPureUnlessCallableIsImpure(): void
 				'Possibly impure call to method PureUnlessCallableIsImpureFunction\InheritedMapperRenamedChild::map() in pure function PureUnlessCallableIsImpureFunction\pureCallingRenamedInheritedMethodWithOpaqueCallback().',
 				399,
 			],
+			[
+				'Impure call to function PureUnlessCallableIsImpureFunction\haveFun() in pure function PureUnlessCallableIsImpureFunction\indirectFunWithDifferentCallback().',
+				432,
+			],
+			[
+				'Impure echo in pure function PureUnlessCallableIsImpureFunction\indirectFunWithDifferentCallback().',
+				433,
+			],
+			[
+				'Impure call to function PureUnlessCallableIsImpureFunction\haveTwoFuns() in pure function PureUnlessCallableIsImpureFunction\indirectFunPartialForward().',
+				458,
+			],
+			[
+				'Impure echo in pure function PureUnlessCallableIsImpureFunction\indirectFunPartialForward().',
+				459,
+			],
 		]);
 	}
 

tests/PHPStan/Rules/Pure/PureMethodRuleTest.php

@@ -17,7 +17,7 @@ class PureMethodRuleTest extends RuleTestCase
 
 	public function getRule(): Rule
 	{
-		return new PureMethodRule(new FunctionPurityCheck());
+		return new PureMethodRule(new FunctionPurityCheck($this->createReflectionProvider()));
 	}
 
 	protected function shouldTreatPhpDocTypesAsCertain(): bool

tests/PHPStan/Rules/Pure/data/pure-unless-callable-is-impure.php

@@ -398,3 +398,64 @@ function pureCallingRenamedInheritedMethodWithOpaqueCallback(InheritedMapperRena
 	// makes the call possibly impure.
 	return $mapper->map($cb, $arr);
 }
+
+/**
+ * @pure-unless-callable-is-impure $fun
+ * @param callable(): void $fun
+ */
+function haveFun(callable $fun): void
+{
+	$fun();
+}
+
+/**
+ * @pure-unless-callable-is-impure $fun
+ * @param callable(): void $fun
+ */
+function indirectFun(callable $fun): void
+{
+	// haveFun() is itself @pure-unless-callable-is-impure and $fun is forwarded
+	// into its only flagged parameter, so this call is pure modulo $fun -
+	// exactly what indirectFun() already declares for itself. No error expected.
+	haveFun($fun);
+}
+
+/**
+ * @pure-unless-callable-is-impure $fun
+ * @param callable(): void $fun
+ */
+function indirectFunWithDifferentCallback(callable $fun): void
+{
+	// haveFun()'s flagged parameter is filled with an inline impure closure,
+	// not with indirectFunWithDifferentCallback()'s own flagged $fun, so the
+	// call is not exempt and must still be reported.
+	haveFun(static function (): void {
+		echo 'side effect';
+	});
+}
+
+/**
+ * @param callable(): void $one
+ * @param callable(): void $two
+ * @pure-unless-callable-is-impure $one
+ * @pure-unless-callable-is-impure $two
+ */
+function haveTwoFuns(callable $one, callable $two): void
+{
+	$one();
+	$two();
+}
+
+/**
+ * @pure-unless-callable-is-impure $fun
+ * @param callable(): void $fun
+ */
+function indirectFunPartialForward(callable $fun): void
+{
+	// haveTwoFuns() has two flagged parameters; only $one receives a forwarded
+	// flagged callback ($fun), while $two receives an opaque impure closure.
+	// Not every flagged slot is covered, so the call must still be reported.
+	haveTwoFuns($fun, static function (): void {
+		echo 'side effect';
+	});
+}