Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 87 additions & 0 deletions src/Analyser/ExprHandler/FuncCallHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
use PHPStan\Reflection\Callables\SimpleImpurePoint;
use PHPStan\Reflection\Callables\SimpleThrowPoint;
use PHPStan\Reflection\FunctionReflection;
use PHPStan\Reflection\MethodReflection;
use PHPStan\Reflection\ParametersAcceptor;
use PHPStan\Reflection\ParametersAcceptorSelector;
use PHPStan\Reflection\ReflectionProvider;
Expand Down Expand Up @@ -208,6 +209,52 @@
$impurePoints = array_merge($impurePoints, $argsResult->getImpurePoints());
$isAlwaysTerminating = $isAlwaysTerminating || $argsResult->isAlwaysTerminating();

if (
$functionReflection !== null
&& in_array($functionReflection->getName(), ['call_user_func_array', 'call_user_func'], true)
) {
$innerResult = $functionReflection->getName() === 'call_user_func_array'
? ArgumentsNormalizer::reorderCallUserFuncArrayArguments($expr, $scope)
: ArgumentsNormalizer::reorderCallUserFuncArguments($expr, $scope);

if ($innerResult !== null) {
[$innerParametersAcceptor, $innerFuncCall] = $innerResult;
$innerParameters = $innerParametersAcceptor->getParameters();
$innerArgs = $innerFuncCall->getArgs();

$innerCalleeReflection = $this->resolveCallUserFuncCalleeReflection($innerFuncCall, $scope);
Comment thread
staabm marked this conversation as resolved.
Outdated

foreach ($innerArgs as $i => $innerArg) {
$innerParameter = null;
if (isset($innerParameters[$i])) {
$innerParameter = $innerParameters[$i];
} elseif (count($innerParameters) > 0 && $innerParametersAcceptor->isVariadic()) {
$innerParameter = $innerParameters[count($innerParameters) - 1];
}

if ($innerParameter === null || !$innerParameter->passedByReference()->createsNewVariable()) {
continue;
}

$argValue = $innerArg->value;
if ($argValue instanceof Variable && $argValue->name === 'this') {
continue;
}
Comment thread
staabm marked this conversation as resolved.
Outdated

$byRefType = $nodeScopeResolver->resolveByRefParameterType($innerFuncCall, $innerCalleeReflection, $innerParameter, $scope);
$scope = $nodeScopeResolver->processVirtualAssign(
$scope,
$storage,
$stmt,
$argValue,
new TypeExpr($byRefType),
$nodeCallback,
)->getScope();
$scope = $nodeScopeResolver->lookForUnsetAllowedUndefinedExpressions($scope, $argValue);
}
}
}

if ($normalizedExpr->name instanceof Expr) {
$nameType = $scope->getType($normalizedExpr->name);
if (
Expand Down Expand Up @@ -820,6 +867,46 @@
return VoidToNullTypeTransformer::transform($parametersAcceptor->getReturnType(), $expr);
}

/**
* @return FunctionReflection|MethodReflection|null
Comment thread
staabm marked this conversation as resolved.
Outdated
*/
private function resolveCallUserFuncCalleeReflection(FuncCall $innerFuncCall, MutatingScope $scope)
{
if ($innerFuncCall->name instanceof Name && $this->reflectionProvider->hasFunction($innerFuncCall->name, $scope)) {
return $this->reflectionProvider->getFunction($innerFuncCall->name, $scope);
}

if (!$innerFuncCall->name instanceof Expr) {
return null;
}

$callbackType = $scope->getType($innerFuncCall->name);

foreach ($callbackType->getConstantStrings() as $constantString) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those two foreach loops are not correct since they return the first result meaning that a call to 'foo'|'bar' will be considered as a call to 'foo'.

Let's just simplify and support only when there is 1 value.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. The change simplifies resolveCallUserFuncCalleeReflection to only resolve the callee when there's exactly one constant string or one constant array callback value, instead of iterating with foreach loops that incorrectly returned the first match from union types like 'foo'|'bar'.

All tests pass (11656 tests, 78894 assertions) and make phpstan reports no errors.

if ($constantString->getValue() === '') {
continue;
}
$funcName = new Name($constantString->getValue());
if ($this->reflectionProvider->hasFunction($funcName, $scope)) {
return $this->reflectionProvider->getFunction($funcName, $scope);
}
}
Comment thread
staabm marked this conversation as resolved.
Outdated

foreach ($callbackType->getConstantArrays() as $constantArray) {
foreach ($constantArray->findTypeAndMethodNames() as $typeAndMethod) {
if ($typeAndMethod->isUnknown() || !$typeAndMethod->getCertainty()->yes()) {

Check warning on line 897 in src/Analyser/ExprHandler/FuncCallHandler.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ foreach ($callbackType->getConstantArrays() as $constantArray) { foreach ($constantArray->findTypeAndMethodNames() as $typeAndMethod) { - if ($typeAndMethod->isUnknown() || !$typeAndMethod->getCertainty()->yes()) { + if ($typeAndMethod->isUnknown() || $typeAndMethod->getCertainty()->no()) { continue; } $methodType = $typeAndMethod->getType();

Check warning on line 897 in src/Analyser/ExprHandler/FuncCallHandler.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ foreach ($callbackType->getConstantArrays() as $constantArray) { foreach ($constantArray->findTypeAndMethodNames() as $typeAndMethod) { - if ($typeAndMethod->isUnknown() || !$typeAndMethod->getCertainty()->yes()) { + if ($typeAndMethod->isUnknown() || $typeAndMethod->getCertainty()->no()) { continue; } $methodType = $typeAndMethod->getType();
continue;
}
$methodType = $typeAndMethod->getType();
if ($methodType->hasMethod($typeAndMethod->getMethod())->yes()) {

Check warning on line 901 in src/Analyser/ExprHandler/FuncCallHandler.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ continue; } $methodType = $typeAndMethod->getType(); - if ($methodType->hasMethod($typeAndMethod->getMethod())->yes()) { + if (!$methodType->hasMethod($typeAndMethod->getMethod())->no()) { return $methodType->getMethod($typeAndMethod->getMethod(), $scope); } }

Check warning on line 901 in src/Analyser/ExprHandler/FuncCallHandler.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ continue; } $methodType = $typeAndMethod->getType(); - if ($methodType->hasMethod($typeAndMethod->getMethod())->yes()) { + if (!$methodType->hasMethod($typeAndMethod->getMethod())->no()) { return $methodType->getMethod($typeAndMethod->getMethod(), $scope); } }
return $methodType->getMethod($typeAndMethod->getMethod(), $scope);
}
}
}

return null;
}

private function getDynamicFunctionReturnType(MutatingScope $scope, FuncCall $normalizedNode, FunctionReflection $functionReflection): ?Type
{
foreach ($this->dynamicReturnTypeExtensionRegistryProvider->getRegistry()->getDynamicFunctionReturnTypeExtensions($functionReflection) as $dynamicFunctionReturnTypeExtension) {
Expand Down
56 changes: 35 additions & 21 deletions src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -3475,27 +3475,7 @@ public function processArgs(

$argValue = $arg->value;
if (!$argValue instanceof Variable || $argValue->name !== 'this') {
$paramOutType = $this->getParameterOutExtensionsType($callLike, $calleeReflection, $currentParameter, $scope);
if ($paramOutType !== null) {
$byRefType = $paramOutType;
} elseif (
$currentParameter instanceof ExtendedParameterReflection
&& $currentParameter->getOutType() !== null
) {
$byRefType = $currentParameter->getOutType();
} elseif (
$calleeReflection instanceof MethodReflection
&& !$calleeReflection->getDeclaringClass()->isBuiltin()
) {
$byRefType = $currentParameter->getType();
} elseif (
$calleeReflection instanceof FunctionReflection
&& !$calleeReflection->isBuiltin()
) {
$byRefType = $currentParameter->getType();
} else {
$byRefType = new MixedType();
}
$byRefType = $this->resolveByRefParameterType($callLike, $calleeReflection, $currentParameter, $scope);

$scope = $this->processVirtualAssign(
$scope,
Expand Down Expand Up @@ -3605,6 +3585,40 @@ private function getParameterTypeFromParameterClosureTypeExtension(CallLike $cal
return null;
}

/**
* @param MethodReflection|FunctionReflection|null $calleeReflection
Comment thread
staabm marked this conversation as resolved.
Outdated
*/
public function resolveByRefParameterType(CallLike $callLike, $calleeReflection, ParameterReflection $currentParameter, MutatingScope $scope): Type
{
$paramOutType = $this->getParameterOutExtensionsType($callLike, $calleeReflection, $currentParameter, $scope);
if ($paramOutType !== null) {
return $paramOutType;
}

if (
$currentParameter instanceof ExtendedParameterReflection
&& $currentParameter->getOutType() !== null
) {
return $currentParameter->getOutType();
}

if (
$calleeReflection instanceof MethodReflection
&& !$calleeReflection->getDeclaringClass()->isBuiltin()
) {
return $currentParameter->getType();
}

if (
$calleeReflection instanceof FunctionReflection
&& !$calleeReflection->isBuiltin()
) {
return $currentParameter->getType();
}

return new MixedType();
}

/**
* @param MethodReflection|FunctionReflection|null $calleeReflection
*/
Expand Down
54 changes: 54 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-6799.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<?php declare(strict_types = 1);

namespace Bug6799;
Comment thread
staabm marked this conversation as resolved.

use function PHPStan\Testing\assertType;

class HelloWorld
{
/**
* @param string[] $where
* @param string $sqlTableName
* @param mixed[] $filter
* @param string $value
*/
protected function listingAddWhereFilterAtableDefault(array &$where, string $sqlTableName, array $filter, string $value): void
{
if ($value != "" && !empty($filter) && !empty($filter['sql']) && is_string($filter['sql'])) {
$where[] = "`" . $sqlTableName . "`.`" . (string)$filter['sql'] . "` = '" . $value . "'";
}
}

/**
* @param string[] $filterValues
* @param string[] $where
* @param string[] $tables
* @param mixed[] $filters
*/
protected function listingAddWhereFilterAtable(array $filterValues, array &$where, array &$tables, array $filters): void
{
if (!empty($filterValues) && !empty($filters)) {
$whereFilter = array();
foreach ($filterValues as $type => $value) {
call_user_func_array(array($this, 'listingAddWhereFilterAtableDefault'), array(&$whereFilter, 'xxxx', $filters[$type], $value));
}
assertType('array<string>', $whereFilter);
}
}

/**
* @param array<mixed> $items
* @param-out list<int> $items
*/
protected function processWithParamOut(array &$items): void
{
$items = [1, 2, 3];
}

protected function testParamOut(): void
{
$items = [];
call_user_func_array([$this, 'processWithParamOut'], [&$items]);
assertType('list<int>', $items);
}
}
Loading