diff --git a/src/Rules/Functions/CallToFunctionStatementWithNoDiscardRule.php b/src/Rules/Functions/CallToFunctionStatementWithNoDiscardRule.php index 9e38f44bb12..e7f3802bbb9 100644 --- a/src/Rules/Functions/CallToFunctionStatementWithNoDiscardRule.php +++ b/src/Rules/Functions/CallToFunctionStatementWithNoDiscardRule.php @@ -9,8 +9,12 @@ use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use PHPStan\TrinaryLogic; +use PHPStan\Type\Type; use PHPStan\Type\VerbosityLevel; +use function in_array; +use function ltrim; use function sprintf; +use function strtolower; /** * @implements Rule @@ -19,6 +23,13 @@ final class CallToFunctionStatementWithNoDiscardRule implements Rule { + private const CALL_USER_FUNC_FUNCTIONS = [ + 'call_user_func', + 'call_user_func_array', + 'forward_static_call', + 'forward_static_call_array', + ]; + public function __construct( private ReflectionProvider $reflectionProvider, ) @@ -59,6 +70,8 @@ public function processNode(Node $node, Scope $scope): array return []; } + $funcCall = $this->unwrapCallUserFunc($funcCall, $scope); + if ($funcCall->name instanceof Node\Name) { if (!$this->reflectionProvider->hasFunction($funcCall->name, $scope)) { return []; @@ -92,15 +105,11 @@ public function processNode(Node $node, Scope $scope): array } $callableType = $scope->getType($funcCall->name); - if (!$callableType->isCallable()->yes()) { + $mustUseReturnValue = $this->getCallableMustUseReturnValue($callableType, $funcCall, $scope); + if ($mustUseReturnValue === null) { return []; } - $mustUseReturnValue = TrinaryLogic::createNo(); - foreach ($callableType->getCallableParametersAcceptors($scope) as $callableParametersAcceptor) { - $mustUseReturnValue = $mustUseReturnValue->or($callableParametersAcceptor->mustUseReturnValue()); - } - if ($isInVoidCast) { if ($mustUseReturnValue->no()) { return [ @@ -126,4 +135,104 @@ public function processNode(Node $node, Scope $scope): array ]; } + /** + * Replaces call_user_func('foo') and similar with a call of the callback, + * so that the discarded return value of the callback is reported. + */ + private function unwrapCallUserFunc(Node\Expr\FuncCall $funcCall, Scope $scope): Node\Expr\FuncCall + { + while ($funcCall->name instanceof Node\Name) { + if (!$this->reflectionProvider->hasFunction($funcCall->name, $scope)) { + break; + } + + $functionName = strtolower($this->reflectionProvider->getFunction($funcCall->name, $scope)->getName()); + $innerFuncCall = $this->createCallbackFuncCall($functionName, $funcCall); + if ($innerFuncCall === null) { + break; + } + + $funcCall = $innerFuncCall; + } + + return $funcCall; + } + + private function createCallbackFuncCall(string $functionName, Node\Expr\FuncCall $funcCall): ?Node\Expr\FuncCall + { + if (!in_array($functionName, self::CALL_USER_FUNC_FUNCTIONS, true)) { + return null; + } + + $callbackArg = null; + $passThruArgs = []; + foreach ($funcCall->getArgs() as $i => $arg) { + if ($callbackArg === null) { + if ($arg->name === null && $i === 0) { + $callbackArg = $arg; + continue; + } + if ($arg->name !== null && $arg->name->toString() === 'callback') { + $callbackArg = $arg; + continue; + } + } + + $passThruArgs[] = $arg; + } + + if ($callbackArg === null) { + return null; + } + + if (in_array($functionName, ['call_user_func_array', 'forward_static_call_array'], true)) { + $passThruArgs = []; + } + + return new Node\Expr\FuncCall($callbackArg->value, $passThruArgs, $funcCall->getAttributes()); + } + + private function getCallableMustUseReturnValue(Type $callableType, Node\Expr\FuncCall $funcCall, Scope $scope): ?TrinaryLogic + { + $constantStrings = $callableType->getConstantStrings(); + $callsCallUserFunc = false; + foreach ($constantStrings as $constantString) { + if (in_array(strtolower(ltrim($constantString->getValue(), '\\')), self::CALL_USER_FUNC_FUNCTIONS, true)) { + $callsCallUserFunc = true; + break; + } + } + + if (!$callsCallUserFunc) { + if (!$callableType->isCallable()->yes()) { + return null; + } + + $mustUseReturnValue = TrinaryLogic::createNo(); + foreach ($callableType->getCallableParametersAcceptors($scope) as $callableParametersAcceptor) { + $mustUseReturnValue = $mustUseReturnValue->or($callableParametersAcceptor->mustUseReturnValue()); + } + + return $mustUseReturnValue; + } + + $mustUseReturnValue = null; + foreach ($constantStrings as $constantString) { + $innerFuncCall = $this->createCallbackFuncCall(strtolower(ltrim($constantString->getValue(), '\\')), $funcCall); + if ($innerFuncCall !== null && $innerFuncCall->name instanceof Node\Expr) { + $innerMustUseReturnValue = $this->getCallableMustUseReturnValue($scope->getType($innerFuncCall->name), $innerFuncCall, $scope); + } else { + $innerMustUseReturnValue = $this->getCallableMustUseReturnValue($constantString, $funcCall, $scope); + } + + if ($innerMustUseReturnValue === null) { + continue; + } + + $mustUseReturnValue = $mustUseReturnValue === null ? $innerMustUseReturnValue : $mustUseReturnValue->or($innerMustUseReturnValue); + } + + return $mustUseReturnValue; + } + } diff --git a/tests/PHPStan/Rules/Functions/CallToFunctionStatementWithNoDiscardRuleTest.php b/tests/PHPStan/Rules/Functions/CallToFunctionStatementWithNoDiscardRuleTest.php index 0475ab9d394..8046cefcc29 100644 --- a/tests/PHPStan/Rules/Functions/CallToFunctionStatementWithNoDiscardRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToFunctionStatementWithNoDiscardRuleTest.php @@ -46,11 +46,60 @@ public function testRule(): void 'Call to callable Closure(): 1 on a separate line discards return value.', 43, ], + [ + 'Call to callable \'call_user_func\' on a separate line discards return value.', + 64, + ], + [ + 'Call to callable \'FunctionCallStateme…\' on a separate line discards return value.', + 67, + ], + [ + 'Call to callable \'FunctionCallStateme…\' on a separate line discards return value.', + 68, + ], + [ + 'Call to callable \'FunctionCallStateme…\' on a separate line discards return value.', + 69, + ], + [ + 'Call to callable \'FunctionCallStateme…\' on a separate line discards return value.', + 70, + ], + [ + 'Call to callable \'FunctionCallStateme…\' on a separate line discards return value.', + 71, + ], + [ + 'Call to callable \'call_user_func\' on a separate line discards return value.', + 72, + ], + [ + 'Call to callable Closure(int): array on a separate line discards return value.', + 73, + ], + [ + 'Call to callable array{FunctionCallStatementResultDiscarded\\Foo, \'doFoo\'} on a separate line discards return value.', + 99, + ], + [ + 'Call to callable array{\'FunctionCallStatementResultDiscarded\\\\Foo\', \'doBar\'} on a separate line discards return value.', + 100, + ], + [ + 'Call to callable \'FunctionCallStateme…\' on a separate line discards return value.', + 101, + ], + [ + 'Call to callable \'call_user_func\'|\'nope\' on a separate line discards return value.', + 106, + ], ]); } // the (void) cast and the pipe operator are PHP 8.5 syntax #[RequiresPhp('>= 8.5.0')] + public function testRulePhp85(): void { $this->analyse([__DIR__ . '/data/function-call-statement-result-discarded-php85.php'], [ @@ -78,6 +127,10 @@ public function testRulePhp85(): void 'Call to function FunctionCallStatementResultDiscardedPhp85\canDiscard() in (void) cast but function allows discarding return value.', 30, ], + [ + 'Call to callable \'FunctionCallStateme…\' in (void) cast but callable allows discarding return value.', + 33, + ], ]); } diff --git a/tests/PHPStan/Rules/Functions/data/function-call-statement-result-discarded-php85.php b/tests/PHPStan/Rules/Functions/data/function-call-statement-result-discarded-php85.php index a01feea8fe1..1576c35418c 100644 --- a/tests/PHPStan/Rules/Functions/data/function-call-statement-result-discarded-php85.php +++ b/tests/PHPStan/Rules/Functions/data/function-call-statement-result-discarded-php85.php @@ -28,3 +28,6 @@ function canDiscard(int $i): int 5 |> (fn ($x) => canDiscard($x)); (void) 5 |> (fn ($x) => withSideEffects($x)); (void) 5 |> (fn ($x) => canDiscard($x)); + +(void) call_user_func('FunctionCallStatementResultDiscardedPhp85\\withSideEffects', 5); +(void) call_user_func('FunctionCallStatementResultDiscardedPhp85\\canDiscard', 5); diff --git a/tests/PHPStan/Rules/Functions/data/function-call-statement-result-discarded.php b/tests/PHPStan/Rules/Functions/data/function-call-statement-result-discarded.php index ca6504b267c..0544063b9a4 100644 --- a/tests/PHPStan/Rules/Functions/data/function-call-statement-result-discarded.php +++ b/tests/PHPStan/Rules/Functions/data/function-call-statement-result-discarded.php @@ -53,3 +53,58 @@ function canDiscard(int $i): int $canDiscardCb = 'FunctionCallStatementResultDiscarded\\canDiscard'; $canDiscardCb(); + +function (): void { + if (random_int(1, 1)) { + $fn = 'call_user_func'; + } else { + $fn = 'nope'; + } + + $fn('FunctionCallStatementResultDiscarded\\withSideEffects', 5); +}; + +call_user_func('FunctionCallStatementResultDiscarded\\withSideEffects', 5); +call_user_func_array('FunctionCallStatementResultDiscarded\\withSideEffects', [5]); +forward_static_call('FunctionCallStatementResultDiscarded\\withSideEffects', 5); +forward_static_call_array('FunctionCallStatementResultDiscarded\\withSideEffects', [5]); +call_user_func(callback: 'FunctionCallStatementResultDiscarded\\withSideEffects', i: 5); +call_user_func('call_user_func', 'FunctionCallStatementResultDiscarded\\withSideEffects', 5); +call_user_func($firstClassCallable, 5); +call_user_func('FunctionCallStatementResultDiscarded\\canDiscard', 5); +$result = call_user_func('FunctionCallStatementResultDiscarded\\withSideEffects', 5); + +class Foo +{ + + #[\NoDiscard] + public function doFoo(): int + { + return 1; + } + + #[\NoDiscard] + public static function doBar(): int + { + return 1; + } + + public function canDiscard(): int + { + return 1; + } + +} + +call_user_func([new Foo(), 'doFoo']); +call_user_func([Foo::class, 'doBar']); +call_user_func('FunctionCallStatementResultDiscarded\\Foo::doBar'); +call_user_func([new Foo(), 'canDiscard']); + +function (bool $b): void { + $fn = $b ? 'call_user_func' : 'nope'; + $fn('FunctionCallStatementResultDiscarded\\withSideEffects', 5); + + $fn = $b ? 'call_user_func' : 'FunctionCallStatementResultDiscarded\\canDiscard'; + $fn('FunctionCallStatementResultDiscarded\\canDiscard', 5); +};