From a1b52327507104d1f9a7bf0bffc7cbad425eb500 Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Fri, 14 Aug 2026 12:39:10 +0900 Subject: [PATCH 1/2] Allow forwarding a @pure-unless-callable-is-impure callback A @pure-unless-callable-is-impure function is pure as long as its flagged callback is pure. Forwarding that callback to another @pure-unless-callable-is-impure function was still reported as impure; such forwarded calls are now exempted. --- src/Rules/Pure/FunctionPurityCheck.php | 128 ++++++++++++++++-- .../Rules/Pure/PureFunctionRuleTest.php | 18 ++- .../PHPStan/Rules/Pure/PureMethodRuleTest.php | 2 +- .../data/pure-unless-callable-is-impure.php | 61 +++++++++ 4 files changed, 199 insertions(+), 10 deletions(-) diff --git a/src/Rules/Pure/FunctionPurityCheck.php b/src/Rules/Pure/FunctionPurityCheck.php index bbe0c7bfddb..1b70f1a5072 100644 --- a/src/Rules/Pure/FunctionPurityCheck.php +++ b/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 $pureUnlessCallableParamNames * @return list */ - 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,131 @@ 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 $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 $pureUnlessCallableParamNames + */ + private function isPureUnlessCallableDelegation(Scope $scope, FuncCall $node, array $pureUnlessCallableParamNames): bool + { + if (!$node->name instanceof Name) { return false; } - if (!is_string($node->name->name)) { + if ($node->isFirstClassCallable()) { return false; } + if (!$this->reflectionProvider->hasFunction($node->name, $scope)) { + return false; + } + + $function = $this->reflectionProvider->getFunction($node->name, $scope); + $variant = ParametersAcceptorSelector::selectFromArgs($scope, $node->getArgs(), $function->getVariants()); + + $hasFlaggedParameter = false; + foreach ($variant->getParameters() as $parameterIndex => $parameter) { + if (!$parameter instanceof ExtendedParameterReflection) { + continue; + } + if ($parameter->isPureUnlessCallableIsImpureParameter()->no()) { + 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 array_key_exists($node->name->name, $pureUnlessCallableParamNames); + return null; } } diff --git a/tests/PHPStan/Rules/Pure/PureFunctionRuleTest.php b/tests/PHPStan/Rules/Pure/PureFunctionRuleTest.php index 1a4d3c3fbba..68ddabf8833 100644 --- a/tests/PHPStan/Rules/Pure/PureFunctionRuleTest.php +++ b/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, + ], ]); } diff --git a/tests/PHPStan/Rules/Pure/PureMethodRuleTest.php b/tests/PHPStan/Rules/Pure/PureMethodRuleTest.php index a31dcfe0bd0..0649813fa46 100644 --- a/tests/PHPStan/Rules/Pure/PureMethodRuleTest.php +++ b/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 diff --git a/tests/PHPStan/Rules/Pure/data/pure-unless-callable-is-impure.php b/tests/PHPStan/Rules/Pure/data/pure-unless-callable-is-impure.php index 87e6f0315f4..95478ffc068 100644 --- a/tests/PHPStan/Rules/Pure/data/pure-unless-callable-is-impure.php +++ b/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'; + }); +} From da37c6bdff8f9d8879726b9ed039239166d365d6 Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Fri, 14 Aug 2026 13:45:11 +0900 Subject: [PATCH 2/2] Look up the callee's flagged parameters by name Reading the per-parameter TrinaryLogic flag was an equivalent mutation here: a Maybe flag only comes from combining several acceptors, which cannot happen for the resolved function of a plain function call. Use the function-level map with array_key_exists instead, like the check above. --- src/Rules/Pure/FunctionPurityCheck.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Rules/Pure/FunctionPurityCheck.php b/src/Rules/Pure/FunctionPurityCheck.php index 1b70f1a5072..8b98507f439 100644 --- a/src/Rules/Pure/FunctionPurityCheck.php +++ b/src/Rules/Pure/FunctionPurityCheck.php @@ -292,14 +292,16 @@ private function isPureUnlessCallableDelegation(Scope $scope, FuncCall $node, ar } $function = $this->reflectionProvider->getFunction($node->name, $scope); + $calleeFlaggedParameters = $function->getPureUnlessCallableIsImpureParameters(); + if ($calleeFlaggedParameters === []) { + return false; + } + $variant = ParametersAcceptorSelector::selectFromArgs($scope, $node->getArgs(), $function->getVariants()); $hasFlaggedParameter = false; foreach ($variant->getParameters() as $parameterIndex => $parameter) { - if (!$parameter instanceof ExtendedParameterReflection) { - continue; - } - if ($parameter->isPureUnlessCallableIsImpureParameter()->no()) { + if (!array_key_exists($parameter->getName(), $calleeFlaggedParameters)) { continue; }