Skip to content

Commit 8c6144f

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 5679c36 commit 8c6144f

5 files changed

Lines changed: 212 additions & 60 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: 42 additions & 39 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,24 +2023,11 @@ 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);
2023-
if ($callableParameters !== null) {
2024-
if (isset($callableParameters[$i])) {
2025-
$parameterType = self::intersectButNotNever($parameterType, $callableParameters[$i]->getType());
2026-
} elseif (count($callableParameters) > 0) {
2027-
$lastParameter = array_last($callableParameters);
2028-
if ($lastParameter->isVariadic()) {
2029-
$parameterType = self::intersectButNotNever($parameterType, $lastParameter->getType());
2030-
} else {
2031-
$parameterType = self::intersectButNotNever($parameterType, new MixedType());
2032-
}
2033-
} else {
2034-
$parameterType = self::intersectButNotNever($parameterType, new MixedType());
2035-
}
2036-
}
2037-
$holder = ExpressionTypeHolder::createYes($parameter->var, $parameterType);
2038-
$expressionTypes[$paramExprString] = $holder;
2039-
$nativeTypes[$paramExprString] = $holder;
2026+
$declaredParameterType = $this->getFunctionType($parameter->type, $isNullable, $parameter->variadic);
2027+
$parameterType = self::resolveCallableParameterType($declaredParameterType, $callableParameters, $i);
2028+
$nativeParameterType = self::resolveCallableParameterType($declaredParameterType, $nativeCallableParameters, $i);
2029+
$expressionTypes[$paramExprString] = ExpressionTypeHolder::createYes($parameter->var, $parameterType);
2030+
$nativeTypes[$paramExprString] = ExpressionTypeHolder::createYes($parameter->var, $nativeParameterType);
20402031
}
20412032

20422033
$nonRefVariableNames = [];
@@ -2192,15 +2183,16 @@ private function invalidateStaticExpressions(array $expressionTypes): array
21922183
/**
21932184
* @api
21942185
* @param ParameterReflection[]|null $callableParameters
2186+
* @param ParameterReflection[]|null $nativeCallableParameters
21952187
*/
2196-
public function enterArrowFunction(Expr\ArrowFunction $arrowFunction, ?array $callableParameters): self
2188+
public function enterArrowFunction(Expr\ArrowFunction $arrowFunction, ?array $callableParameters, ?array $nativeCallableParameters = null): self
21972189
{
21982190
$anonymousFunctionReflection = $this->resolveType('__phpStanArrowFn', $arrowFunction);
21992191
if (!$anonymousFunctionReflection instanceof ClosureType) {
22002192
throw new ShouldNotHappenException();
22012193
}
22022194

2203-
$scope = $this->enterArrowFunctionWithoutReflection($arrowFunction, $callableParameters);
2195+
$scope = $this->enterArrowFunctionWithoutReflection($arrowFunction, $callableParameters, $nativeCallableParameters);
22042196

22052197
return $this->scopeFactory->create(
22062198
$scope->context,
@@ -2224,33 +2216,21 @@ public function enterArrowFunction(Expr\ArrowFunction $arrowFunction, ?array $ca
22242216

22252217
/**
22262218
* @param ParameterReflection[]|null $callableParameters
2219+
* @param ParameterReflection[]|null $nativeCallableParameters
22272220
*/
2228-
public function enterArrowFunctionWithoutReflection(Expr\ArrowFunction $arrowFunction, ?array $callableParameters): self
2221+
public function enterArrowFunctionWithoutReflection(Expr\ArrowFunction $arrowFunction, ?array $callableParameters, ?array $nativeCallableParameters): self
22292222
{
22302223
$arrowFunctionScope = $this;
22312224
foreach ($arrowFunction->params as $i => $parameter) {
22322225
$isNullable = $this->isParameterValueNullable($parameter);
2233-
$parameterType = $this->getFunctionType($parameter->type, $isNullable, $parameter->variadic);
2234-
2235-
if ($callableParameters !== null) {
2236-
if (isset($callableParameters[$i])) {
2237-
$parameterType = self::intersectButNotNever($parameterType, $callableParameters[$i]->getType());
2238-
} elseif (count($callableParameters) > 0) {
2239-
$lastParameter = array_last($callableParameters);
2240-
if ($lastParameter->isVariadic()) {
2241-
$parameterType = self::intersectButNotNever($parameterType, $lastParameter->getType());
2242-
} else {
2243-
$parameterType = self::intersectButNotNever($parameterType, new MixedType());
2244-
}
2245-
} else {
2246-
$parameterType = self::intersectButNotNever($parameterType, new MixedType());
2247-
}
2248-
}
2226+
$declaredParameterType = $this->getFunctionType($parameter->type, $isNullable, $parameter->variadic);
2227+
$parameterType = self::resolveCallableParameterType($declaredParameterType, $callableParameters, $i);
2228+
$nativeParameterType = self::resolveCallableParameterType($declaredParameterType, $nativeCallableParameters, $i);
22492229

22502230
if (!$parameter->var instanceof Variable || !is_string($parameter->var->name)) {
22512231
throw new ShouldNotHappenException();
22522232
}
2253-
$arrowFunctionScope = $arrowFunctionScope->assignVariable($parameter->var->name, $parameterType, $parameterType, TrinaryLogic::createYes());
2233+
$arrowFunctionScope = $arrowFunctionScope->assignVariable($parameter->var->name, $parameterType, $nativeParameterType, TrinaryLogic::createYes());
22542234
}
22552235

22562236
if ($arrowFunction->static) {
@@ -2310,6 +2290,29 @@ public function getFunctionType($type, bool $isNullable, bool $isVariadic): Type
23102290
return $this->initializerExprTypeResolver->getFunctionType($type, $isNullable, false, InitializerExprContext::fromScope($this));
23112291
}
23122292

2293+
/**
2294+
* @param ParameterReflection[]|null $callableParameters
2295+
*/
2296+
private static function resolveCallableParameterType(Type $declaredType, ?array $callableParameters, int $i): Type
2297+
{
2298+
if ($callableParameters === null) {
2299+
return $declaredType;
2300+
}
2301+
2302+
if (isset($callableParameters[$i])) {
2303+
return self::intersectButNotNever($declaredType, $callableParameters[$i]->getType());
2304+
}
2305+
2306+
if (count($callableParameters) > 0) {
2307+
$lastParameter = array_last($callableParameters);
2308+
if ($lastParameter->isVariadic()) {
2309+
return self::intersectButNotNever($declaredType, $lastParameter->getType());
2310+
}
2311+
}
2312+
2313+
return self::intersectButNotNever($declaredType, new MixedType());
2314+
}
2315+
23132316
public static function intersectButNotNever(Type $nativeType, Type $inferredType): Type
23142317
{
23152318
if ($nativeType->isSuperTypeOf($inferredType)->no()) {

‎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)