diff --git a/src/Analyser/ArgumentsNormalizer.php b/src/Analyser/ArgumentsNormalizer.php index 5b3e35e3e96..8aca7cabe32 100644 --- a/src/Analyser/ArgumentsNormalizer.php +++ b/src/Analyser/ArgumentsNormalizer.php @@ -3,6 +3,7 @@ namespace PHPStan\Analyser; use PhpParser\Node\Arg; +use PhpParser\Node\Expr\Array_; use PhpParser\Node\Expr\FuncCall; use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Expr\New_; @@ -89,6 +90,88 @@ public static function reorderCallUserFuncArguments( ), $acceptsNamedArguments]; } + /** + * @return array{ParametersAcceptor, FuncCall, TrinaryLogic}|null + */ + public static function reorderCallUserFuncArrayArguments( + FuncCall $callUserFuncArrayCall, + Scope $scope, + ): ?array + { + $args = $callUserFuncArrayCall->getArgs(); + if (count($args) < 2) { + return null; + } + + $callbackArg = null; + $argsArrayArg = null; + foreach ($args as $i => $arg) { + if ($callbackArg === null) { + if ($arg->name === null && $i === 0) { + $callbackArg = $arg; + continue; + } + if ($arg->name !== null && ($arg->name->toString() === 'callback' || $arg->name->toString() === 'function')) { + $callbackArg = $arg; + continue; + } + } + + if ($argsArrayArg === null) { + if ($arg->name === null && $i === 1) { + $argsArrayArg = $arg; + continue; + } + if ($arg->name !== null && ($arg->name->toString() === 'args' || $arg->name->toString() === 'parameters')) { + $argsArrayArg = $arg; + continue; + } + } + } + + if ($callbackArg === null || $argsArrayArg === null) { + return null; + } + + if (!$argsArrayArg->value instanceof Array_) { + return null; + } + + $calledOnType = $scope->getType($callbackArg->value); + if (!$calledOnType->isCallable()->yes()) { + return null; + } + + $passThruArgs = []; + foreach ($argsArrayArg->value->items as $item) { + $passThruArgs[] = new Arg( + $item->value, + $item->byRef, + $item->unpack, + $item->getAttributes(), + ); + } + + $callableParametersAcceptors = $calledOnType->getCallableParametersAcceptors($scope); + $parametersAcceptor = ParametersAcceptorSelector::selectFromArgs( + $scope, + $passThruArgs, + $callableParametersAcceptors, + null, + ); + + $acceptsNamedArguments = TrinaryLogic::createYes(); + foreach ($callableParametersAcceptors as $callableParametersAcceptor) { + $acceptsNamedArguments = $acceptsNamedArguments->and($callableParametersAcceptor->acceptsNamedArguments()); + } + + return [$parametersAcceptor, new FuncCall( + $callbackArg->value, + $passThruArgs, + $callUserFuncArrayCall->getAttributes(), + ), $acceptsNamedArguments]; + } + public static function reorderFuncArguments( ParametersAcceptor $parametersAcceptor, FuncCall $functionCall, diff --git a/src/Analyser/ExprHandler/FuncCallHandler.php b/src/Analyser/ExprHandler/FuncCallHandler.php index 96cf7fa0f14..e345e823659 100644 --- a/src/Analyser/ExprHandler/FuncCallHandler.php +++ b/src/Analyser/ExprHandler/FuncCallHandler.php @@ -770,6 +770,15 @@ public function resolveType(MutatingScope $scope, Expr $expr): Type } } + if ($functionReflection->getName() === 'call_user_func_array') { + $result = ArgumentsNormalizer::reorderCallUserFuncArrayArguments($expr, $scope); + if ($result !== null) { + [, $innerFuncCall] = $result; + + return $scope->getType($innerFuncCall); + } + } + $parametersAcceptor = ParametersAcceptorSelector::selectFromArgs( $scope, $expr->getArgs(), diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 8935ade8e2c..0fe914a50c8 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -3535,10 +3535,107 @@ public function processArgs( } } + // For call_user_func_array with a resolvable callback, use the callback's + // parameter types for by-reference variables instead of blindly using mixed + if ( + $calleeReflection instanceof FunctionReflection + && $calleeReflection->getName() === 'call_user_func_array' + && $callLike instanceof FuncCall + ) { + $rewriteResult = ArgumentsNormalizer::reorderCallUserFuncArrayArguments($callLike, $scope); + if ($rewriteResult !== null) { + [$callbackParametersAcceptor] = $rewriteResult; + $callbackParameters = $callbackParametersAcceptor->getParameters(); + $callArgs = $callLike->getArgs(); + if (isset($callArgs[1]) && $callArgs[1]->value instanceof Array_) { + foreach ($callArgs[1]->value->items as $i => $arrayItem) { + // Handle nested by-ref items in sub-arrays + if ($arrayItem->value instanceof Array_) { + $scope = $this->invalidateByRefVariablesInArrayArg($scope, $arrayItem->value); + } + + if (!$arrayItem->byRef) { + continue; + } + + if (!$arrayItem->value instanceof Variable || !is_string($arrayItem->value->name)) { + continue; + } + + $matchedParam = null; + if (isset($callbackParameters[$i])) { + $matchedParam = $callbackParameters[$i]; + } elseif (count($callbackParameters) > 0 && $callbackParametersAcceptor->isVariadic()) { + $matchedParam = $callbackParameters[count($callbackParameters) - 1]; + } + + if ($matchedParam !== null && $matchedParam->passedByReference()->createsNewVariable()) { + $byRefType = $matchedParam->getType(); + if ( + $matchedParam instanceof ExtendedParameterReflection + && $matchedParam->getOutType() !== null + ) { + $byRefType = $matchedParam->getOutType(); + } + + $scope = $this->processVirtualAssign( + $scope, + $storage, + $stmt, + $arrayItem->value, + new TypeExpr($byRefType), + $nodeCallback, + )->getScope(); + } else { + // Callback parameter is not by-ref or unknown - invalidate to mixed + // since we can't determine what happens with the reference + $scope = $scope->assignVariable($arrayItem->value->name, new MixedType(), new MixedType(), TrinaryLogic::createYes()); + } + } + } + } else { + // Could not resolve the callback - fall back to generic invalidation + foreach ($args as $arg) { + $scope = $this->invalidateByRefVariablesInArrayArg($scope, $arg->value); + } + } + } else { + // Invalidate variables passed by reference inside array arguments + // e.g. some_function([&$var, ...]) - $var might be modified + foreach ($args as $arg) { + $scope = $this->invalidateByRefVariablesInArrayArg($scope, $arg->value); + } + } + // not storing this, it's scope after processing all args return new ExpressionResult($scope, $hasYield, $isAlwaysTerminating, $throwPoints, $impurePoints); } + private function invalidateByRefVariablesInArrayArg(MutatingScope $scope, Expr $expr): MutatingScope + { + if (!$expr instanceof Array_) { + return $scope; + } + + foreach ($expr->items as $arrayItem) { + if ($arrayItem->value instanceof Array_) { + $scope = $this->invalidateByRefVariablesInArrayArg($scope, $arrayItem->value); + } + + if (!$arrayItem->byRef) { + continue; + } + + if (!$arrayItem->value instanceof Variable || !is_string($arrayItem->value->name)) { + continue; + } + + $scope = $scope->assignVariable($arrayItem->value->name, new MixedType(), new MixedType(), TrinaryLogic::createYes()); + } + + return $scope; + } + /** * @param MethodReflection|FunctionReflection|null $calleeReflection */ diff --git a/tests/PHPStan/Analyser/nsrt/bug-6799.php b/tests/PHPStan/Analyser/nsrt/bug-6799.php new file mode 100644 index 00000000000..6d0b112278b --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-6799.php @@ -0,0 +1,78 @@ + $value) { + call_user_func_array(array($this, 'addFilter'), array(&$whereFilter, $filters[$type], $value)); + } + assertType('array', $whereFilter); + } + } +} + +function testSimple(): void +{ + $arr = []; + some_function(1, [&$arr, 'foo']); + assertType('mixed', $arr); +} + +function testDirectFunction(): void +{ + $result = []; + call_user_func_array('Bug6799\modify_by_ref', [&$result, 'value']); + assertType('array', $result); +} + +/** @param callable $callback */ +function testUnresolvableCallback($callback): void +{ + $arr = []; + call_user_func_array($callback, [&$arr, 'foo']); + assertType('mixed', $arr); +} + +function testCallbackNotByRef(): void +{ + $arr = []; + call_user_func_array('Bug6799\some_function', [1, [&$arr, 'foo']]); + assertType('mixed', $arr); +} + +/** + * @param string[] $arr + */ +function modify_by_ref(array &$arr, string $value): void +{ + $arr[] = $value; +} + +/** + * @param mixed $x + */ +function some_function(int $a, $x): void +{ +}