Skip to content

Commit 29216a3

Browse files
VincentLangletVincentLanglet
committed
Do not use callable parameter types as native types for closure and arrow function parameters
- In `enterAnonymousFunctionWithoutReflection` and `enterArrowFunctionWithoutReflection`, the callable parameter types (derived from calling context, often via PHPDoc/generics) were used as both expression types AND native types for closure parameters - Now only the declared PHP type hint is used for native types, while the intersected callable parameter type is used for expression types (PHPDoc-aware) - This matches the behavior of `foreach` which correctly distinguishes native vs PHPDoc iterable value types - Fixes false positives from `function.alreadyNarrowedType` when `treatPhpDocTypesAsCertain` is false and callbacks are passed to array_map, array_filter, usort, etc.
1 parent 745b400 commit 29216a3

5 files changed

Lines changed: 192 additions & 31 deletions

File tree

‎src/Analyser/ExprHandler/Helper/ClosureTypeResolver.php‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
use PHPStan\Node\PropertyAssignNode;
2323
use PHPStan\Parser\ArrayMapArgVisitor;
2424
use PHPStan\Parser\ImmediatelyInvokedClosureVisitor;
25+
use PHPStan\Reflection\ExtendedParameterReflection;
2526
use PHPStan\Reflection\Callables\SimpleImpurePoint;
2627
use PHPStan\Reflection\Callables\SimpleThrowPoint;
2728
use PHPStan\Reflection\Native\NativeParameterReflection;
@@ -97,29 +98,35 @@ public function getClosureType(
9798
}
9899

99100
$callableParameters = null;
101+
$nativeCallableParameters = null;
100102
$arrayMapArgs = $expr->getAttribute(ArrayMapArgVisitor::ATTRIBUTE_NAME);
101103
$immediatelyInvokedArgs = $expr->getAttribute(ImmediatelyInvokedClosureVisitor::ARGS_ATTRIBUTE_NAME);
102104
if ($arrayMapArgs !== null) {
103105
$callableParameters = [];
106+
$nativeCallableParameters = [];
104107
foreach ($arrayMapArgs as $funcCallArg) {
105108
$callableParameters[] = new DummyParameter('item', $scope->getType($funcCallArg->value)->getIterableValueType(), optional: false, passedByReference: PassedByReference::createNo(), variadic: false, defaultValue: null);
109+
$nativeCallableParameters[] = new DummyParameter('item', $scope->getNativeType($funcCallArg->value)->getIterableValueType(), optional: false, passedByReference: PassedByReference::createNo(), variadic: false, defaultValue: null);
106110
}
107111
} elseif ($immediatelyInvokedArgs !== null) {
108112
foreach ($immediatelyInvokedArgs as $immediatelyInvokedArg) {
109113
$callableParameters[] = new DummyParameter('item', $scope->getType($immediatelyInvokedArg->value), optional: false, passedByReference: PassedByReference::createNo(), variadic: false, defaultValue: null);
114+
$nativeCallableParameters[] = new DummyParameter('item', $scope->getNativeType($immediatelyInvokedArg->value), optional: false, passedByReference: PassedByReference::createNo(), variadic: false, defaultValue: null);
110115
}
111116
} else {
112117
$inFunctionCallsStackCount = count($scope->inFunctionCallsStack);
113118
if ($inFunctionCallsStackCount > 0) {
114119
[, $inParameter] = $scope->inFunctionCallsStack[$inFunctionCallsStackCount - 1];
115120
if ($inParameter !== null) {
116121
$callableParameters = $this->nodeScopeResolver->createCallableParameters($scope, $expr, null, $inParameter->getType());
122+
$nativeType = $inParameter instanceof ExtendedParameterReflection ? $inParameter->getNativeType() : $inParameter->getType();
123+
$nativeCallableParameters = $this->nodeScopeResolver->createNativeCallableParameters($scope, $expr, null, $nativeType);
117124
}
118125
}
119126
}
120127

121128
if ($expr instanceof ArrowFunction) {
122-
$arrowScope = $scope->enterArrowFunctionWithoutReflection($expr, $callableParameters);
129+
$arrowScope = $scope->enterArrowFunctionWithoutReflection($expr, $callableParameters, $nativeCallableParameters);
123130

124131
if ($expr->expr instanceof Yield_ || $expr->expr instanceof YieldFrom) {
125132
$yieldNode = $expr->expr;
@@ -232,7 +239,7 @@ static function (Node $node, Scope $scope) use ($arrowScope, &$arrowFunctionImpu
232239

233240
self::$resolveClosureTypeDepth++;
234241

235-
$closureScope = $scope->enterAnonymousFunctionWithoutReflection($expr, $callableParameters);
242+
$closureScope = $scope->enterAnonymousFunctionWithoutReflection($expr, $callableParameters, $nativeCallableParameters);
236243
$closureReturnStatements = [];
237244
$closureYieldStatements = [];
238245
$onlyNeverExecutionEnds = null;

‎src/Analyser/MutatingScope.php‎

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1970,18 +1970,20 @@ public function isInClosureBind(): bool
19701970
/**
19711971
* @api
19721972
* @param ParameterReflection[]|null $callableParameters
1973+
* @param ParameterReflection[]|null $nativeCallableParameters
19731974
*/
19741975
public function enterAnonymousFunction(
19751976
Expr\Closure $closure,
19761977
?array $callableParameters,
1978+
?array $nativeCallableParameters = null,
19771979
): self
19781980
{
19791981
$anonymousFunctionReflection = $this->resolveType('__phpstanClosure', $closure);
19801982
if (!$anonymousFunctionReflection instanceof ClosureType) {
19811983
throw new ShouldNotHappenException();
19821984
}
19831985

1984-
$scope = $this->enterAnonymousFunctionWithoutReflection($closure, $callableParameters);
1986+
$scope = $this->enterAnonymousFunctionWithoutReflection($closure, $callableParameters, $nativeCallableParameters);
19851987

19861988
return $this->scopeFactory->create(
19871989
$scope->context,
@@ -2005,10 +2007,12 @@ public function enterAnonymousFunction(
20052007

20062008
/**
20072009
* @param ParameterReflection[]|null $callableParameters
2010+
* @param ParameterReflection[]|null $nativeCallableParameters
20082011
*/
20092012
public function enterAnonymousFunctionWithoutReflection(
20102013
Expr\Closure $closure,
20112014
?array $callableParameters,
2015+
?array $nativeCallableParameters,
20122016
): self
20132017
{
20142018
$expressionTypes = [];
@@ -2019,13 +2023,15 @@ public function enterAnonymousFunctionWithoutReflection(
20192023
}
20202024
$paramExprString = sprintf('$%s', $parameter->var->name);
20212025
$isNullable = $this->isParameterValueNullable($parameter);
2022-
$parameterType = $this->getFunctionType($parameter->type, $isNullable, $parameter->variadic);
2026+
$nativeParameterType = $parameterType = $this->getFunctionType($parameter->type, $isNullable, $parameter->variadic);
20232027
if ($callableParameters !== null) {
20242028
$parameterType = self::intersectButNotNever($parameterType, $this->getCallableParameterType($callableParameters, $i));
20252029
}
2026-
$holder = ExpressionTypeHolder::createYes($parameter->var, $parameterType);
2027-
$expressionTypes[$paramExprString] = $holder;
2028-
$nativeTypes[$paramExprString] = $holder;
2030+
if ($nativeCallableParameters !== null) {
2031+
$nativeParameterType = self::intersectButNotNever($nativeParameterType, $this->getCallableParameterType($nativeCallableParameters, $i));
2032+
}
2033+
$expressionTypes[$paramExprString] = ExpressionTypeHolder::createYes($parameter->var, $parameterType);
2034+
$nativeTypes[$paramExprString] = ExpressionTypeHolder::createYes($parameter->var, $nativeParameterType);
20292035
}
20302036

20312037
$nonRefVariableNames = [];
@@ -2181,15 +2187,16 @@ private function invalidateStaticExpressions(array $expressionTypes): array
21812187
/**
21822188
* @api
21832189
* @param ParameterReflection[]|null $callableParameters
2190+
* @param ParameterReflection[]|null $nativeCallableParameters
21842191
*/
2185-
public function enterArrowFunction(Expr\ArrowFunction $arrowFunction, ?array $callableParameters): self
2192+
public function enterArrowFunction(Expr\ArrowFunction $arrowFunction, ?array $callableParameters, ?array $nativeCallableParameters = null): self
21862193
{
21872194
$anonymousFunctionReflection = $this->resolveType('__phpStanArrowFn', $arrowFunction);
21882195
if (!$anonymousFunctionReflection instanceof ClosureType) {
21892196
throw new ShouldNotHappenException();
21902197
}
21912198

2192-
$scope = $this->enterArrowFunctionWithoutReflection($arrowFunction, $callableParameters);
2199+
$scope = $this->enterArrowFunctionWithoutReflection($arrowFunction, $callableParameters, $nativeCallableParameters);
21932200

21942201
return $this->scopeFactory->create(
21952202
$scope->context,
@@ -2213,21 +2220,25 @@ public function enterArrowFunction(Expr\ArrowFunction $arrowFunction, ?array $ca
22132220

22142221
/**
22152222
* @param ParameterReflection[]|null $callableParameters
2223+
* @param ParameterReflection[]|null $nativeCallableParameters
22162224
*/
2217-
public function enterArrowFunctionWithoutReflection(Expr\ArrowFunction $arrowFunction, ?array $callableParameters): self
2225+
public function enterArrowFunctionWithoutReflection(Expr\ArrowFunction $arrowFunction, ?array $callableParameters, ?array $nativeCallableParameters): self
22182226
{
22192227
$arrowFunctionScope = $this;
22202228
foreach ($arrowFunction->params as $i => $parameter) {
22212229
$isNullable = $this->isParameterValueNullable($parameter);
2222-
$parameterType = $this->getFunctionType($parameter->type, $isNullable, $parameter->variadic);
2230+
$nativeParameterType = $parameterType = $this->getFunctionType($parameter->type, $isNullable, $parameter->variadic);
22232231
if ($callableParameters !== null) {
22242232
$parameterType = self::intersectButNotNever($parameterType, $this->getCallableParameterType($callableParameters, $i));
22252233
}
2234+
if ($nativeCallableParameters !== null) {
2235+
$nativeParameterType = self::intersectButNotNever($nativeParameterType, $this->getCallableParameterType($nativeCallableParameters, $i));
2236+
}
22262237

22272238
if (!$parameter->var instanceof Variable || !is_string($parameter->var->name)) {
22282239
throw new ShouldNotHappenException();
22292240
}
2230-
$arrowFunctionScope = $arrowFunctionScope->assignVariable($parameter->var->name, $parameterType, $parameterType, TrinaryLogic::createYes());
2241+
$arrowFunctionScope = $arrowFunctionScope->assignVariable($parameter->var->name, $parameterType, $nativeParameterType, TrinaryLogic::createYes());
22312242
}
22322243

22332244
if ($arrowFunction->static) {

‎src/Analyser/NodeScopeResolver.php‎

Lines changed: 35 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,8 @@
131131
use PHPStan\Reflection\ParameterReflection;
132132
use PHPStan\Reflection\ParametersAcceptor;
133133
use PHPStan\Reflection\ParametersAcceptorSelector;
134+
use PHPStan\Reflection\PassedByReference;
135+
use PHPStan\Reflection\Php\DummyParameter;
134136
use PHPStan\Reflection\Php\PhpFunctionFromParserNodeReflection;
135137
use PHPStan\Reflection\Php\PhpMethodFromParserNodeReflection;
136138
use PHPStan\Reflection\Php\PhpMethodReflection;
@@ -2704,6 +2706,7 @@ public function processClosureNode(
27042706
callable $nodeCallback,
27052707
ExpressionContext $context,
27062708
?Type $passedToType,
2709+
?Type $nativePassedToType = null,
27072710
): ProcessClosureResult
27082711
{
27092712
foreach ($expr->params as $param) {
@@ -2713,12 +2716,8 @@ public function processClosureNode(
27132716
$byRefUses = [];
27142717

27152718
$closureCallArgs = $expr->getAttribute(ClosureArgVisitor::ATTRIBUTE_NAME);
2716-
$callableParameters = $this->createCallableParameters(
2717-
$scope,
2718-
$expr,
2719-
$closureCallArgs,
2720-
$passedToType,
2721-
);
2719+
$callableParameters = $this->createCallableParameters($scope, $expr, $closureCallArgs, $passedToType);
2720+
$nativeCallableParameters = $this->createNativeCallableParameters($scope, $expr, $closureCallArgs, $nativePassedToType);
27222721

27232722
$useScope = $scope;
27242723
foreach ($expr->uses as $use) {
@@ -2769,7 +2768,7 @@ public function processClosureNode(
27692768
$this->callNodeCallback($nodeCallback, $expr->returnType, $scope, $storage);
27702769
}
27712770

2772-
$closureScope = $scope->enterAnonymousFunction($expr, $callableParameters);
2771+
$closureScope = $scope->enterAnonymousFunction($expr, $callableParameters, $nativeCallableParameters);
27732772
$closureScope = $closureScope->processClosureScope($scope, null, $byRefUses);
27742773
$closureType = $closureScope->getAnonymousFunctionReflection();
27752774
if (!$closureType instanceof ClosureType) {
@@ -2851,7 +2850,7 @@ public function processClosureNode(
28512850
break;
28522851
}
28532852

2854-
$closureScope = $scope->enterAnonymousFunction($expr, $callableParameters);
2853+
$closureScope = $scope->enterAnonymousFunction($expr, $callableParameters, $nativeCallableParameters);
28552854
$closureScope = $closureScope->processClosureScope($intermediaryClosureScope, $prevScope, $byRefUses);
28562855

28572856
if ($closureScope->equals($prevScope)) {
@@ -2916,6 +2915,7 @@ public function processArrowFunctionNode(
29162915
ExpressionResultStorage $storage,
29172916
callable $nodeCallback,
29182917
?Type $passedToType,
2918+
?Type $nativePassedToType = null,
29192919
): ExpressionResult
29202920
{
29212921
foreach ($expr->params as $param) {
@@ -2926,12 +2926,9 @@ public function processArrowFunctionNode(
29262926
}
29272927

29282928
$arrowFunctionCallArgs = $expr->getAttribute(ArrowFunctionArgVisitor::ATTRIBUTE_NAME);
2929-
$arrowFunctionScope = $scope->enterArrowFunction($expr, $this->createCallableParameters(
2930-
$scope,
2931-
$expr,
2932-
$arrowFunctionCallArgs,
2933-
$passedToType,
2934-
));
2929+
$callableParameters = $this->createCallableParameters($scope, $expr, $arrowFunctionCallArgs, $passedToType);
2930+
$nativeCallableParameters = $this->createNativeCallableParameters($scope, $expr, $arrowFunctionCallArgs, $nativePassedToType);
2931+
$arrowFunctionScope = $scope->enterArrowFunction($expr, $callableParameters, $nativeCallableParameters);
29352932
$arrowFunctionType = $arrowFunctionScope->getAnonymousFunctionReflection();
29362933
if ($arrowFunctionType === null) {
29372934
throw new ShouldNotHappenException();
@@ -2943,14 +2940,33 @@ public function processArrowFunctionNode(
29432940
}
29442941

29452942
/**
2946-
* @param Node\Arg[] $args
2943+
* @param Node\Arg[]|null $args
29472944
* @return ParameterReflection[]|null
29482945
*/
29492946
public function createCallableParameters(Scope $scope, Expr $closureExpr, ?array $args, ?Type $passedToType): ?array
2947+
{
2948+
return $this->doCreateCallableParameters($scope, $closureExpr, $args, $passedToType, static fn (Scope $s, Expr $e) => $s->getType($e));
2949+
}
2950+
2951+
/**
2952+
* @param Node\Arg[]|null $args
2953+
* @return ParameterReflection[]|null
2954+
*/
2955+
public function createNativeCallableParameters(Scope $scope, Expr $closureExpr, ?array $args, ?Type $nativePassedToType): ?array
2956+
{
2957+
return $this->doCreateCallableParameters($scope, $closureExpr, $args, $nativePassedToType, static fn (Scope $s, Expr $e) => $s->getNativeType($e));
2958+
}
2959+
2960+
/**
2961+
* @param Node\Arg[]|null $args
2962+
* @param Closure(Scope, Expr): Type $typeGetter
2963+
* @return ParameterReflection[]|null
2964+
*/
2965+
private function doCreateCallableParameters(Scope $scope, Expr $closureExpr, ?array $args, ?Type $passedToType, Closure $typeGetter): ?array
29502966
{
29512967
$callableParameters = null;
29522968
if ($args !== null) {
2953-
$closureType = $scope->getType($closureExpr);
2969+
$closureType = $typeGetter($scope, $closureExpr);
29542970

29552971
if ($closureType->isCallable()->no()) {
29562972
return null;
@@ -2965,7 +2981,7 @@ public function createCallableParameters(Scope $scope, Expr $closureExpr, ?array
29652981
continue;
29662982
}
29672983

2968-
$type = $scope->getType($args[$index]->value);
2984+
$type = $typeGetter($scope, $args[$index]->value);
29692985
$callableParameters[$index] = new NativeParameterReflection(
29702986
$callableParameter->getName(),
29712987
$callableParameter->isOptional(),
@@ -3383,7 +3399,7 @@ public function processArgs(
33833399
}
33843400

33853401
$this->callNodeCallbackWithExpression($nodeCallback, $arg->value, $scopeToPass, $storage, $context);
3386-
$closureResult = $this->processClosureNode($stmt, $arg->value, $scopeToPass, $storage, $nodeCallback, $context, $parameterType ?? null);
3402+
$closureResult = $this->processClosureNode($stmt, $arg->value, $scopeToPass, $storage, $nodeCallback, $context, $parameterType ?? null, $parameterNativeType);
33873403
if ($this->callCallbackImmediately($parameter, $parameterType, $calleeReflection)) {
33883404
$throwPoints = array_merge($throwPoints, array_map(static fn (InternalThrowPoint $throwPoint) => $throwPoint->isExplicit() ? InternalThrowPoint::createExplicit($scope, $throwPoint->getType(), $arg->value, $throwPoint->canContainAnyThrowable()) : InternalThrowPoint::createImplicit($scope, $arg->value), $closureResult->getThrowPoints()));
33893405
$impurePoints = array_merge($impurePoints, $closureResult->getImpurePoints());
@@ -3442,7 +3458,7 @@ public function processArgs(
34423458
}
34433459

34443460
$this->callNodeCallbackWithExpression($nodeCallback, $arg->value, $scopeToPass, $storage, $context);
3445-
$arrowFunctionResult = $this->processArrowFunctionNode($stmt, $arg->value, $scopeToPass, $storage, $nodeCallback, $parameterType ?? null);
3461+
$arrowFunctionResult = $this->processArrowFunctionNode($stmt, $arg->value, $scopeToPass, $storage, $nodeCallback, $parameterType ?? null, $parameterNativeType);
34463462
if ($this->callCallbackImmediately($parameter, $parameterType, $calleeReflection)) {
34473463
$throwPoints = array_merge($throwPoints, array_map(static fn (InternalThrowPoint $throwPoint) => $throwPoint->isExplicit() ? InternalThrowPoint::createExplicit($scope, $throwPoint->getType(), $arg->value, $throwPoint->canContainAnyThrowable()) : InternalThrowPoint::createImplicit($scope, $arg->value), $arrowFunctionResult->getThrowPoints()));
34483464
$impurePoints = array_merge($impurePoints, $arrowFunctionResult->getImpurePoints());

0 commit comments

Comments
 (0)