Skip to content

Commit dda82d9

Browse files
ondrejmirtesondrejmirtes
committed
Close soundness holes in closure signature inference
2 parents 7bc4365 + 8e3162d commit dda82d9

45 files changed

Lines changed: 2541 additions & 393 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎src/Analyser/ArgumentsHandler.php‎

Lines changed: 68 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -685,11 +685,16 @@ public function processArgs(
685685
// still its bound there - observe the declared parameter type,
686686
// where such a template is uninformative and the receiver's
687687
// class-level arguments are already in place
688-
$scope = $scope->addTemplateArgumentConstraints($this->templateArgumentObserver->collectArgument(
689-
$this->findOriginalParameterType($argMetadataAcceptor, $parameter) ?? $parameter->getType(),
690-
$gatheredArgTypeByIndex[$i],
691-
($calleeReflection instanceof FunctionReflection || $calleeReflection instanceof ExtendedMethodReflection) && $calleeReflection->isPure()->yes(),
692-
));
688+
$isPure = ($calleeReflection instanceof FunctionReflection || $calleeReflection instanceof ExtendedMethodReflection) && $calleeReflection->isPure()->yes();
689+
if ($originalArg->unpack) {
690+
$scope = $this->observeUnpackedArgument($scope, $argMetadataAcceptor, $i, $gatheredArgTypeByIndex[$i], $isPure);
691+
} else {
692+
$scope = $scope->addTemplateArgumentConstraints($this->templateArgumentObserver->collectArgument(
693+
$this->findOriginalParameterType($argMetadataAcceptor, $parameter) ?? $parameter->getType(),
694+
$gatheredArgTypeByIndex[$i],
695+
$isPure,
696+
));
697+
}
693698
}
694699
}
695700

@@ -1274,6 +1279,64 @@ private function resolveClosureThisType(
12741279
return null;
12751280
}
12761281

1282+
/**
1283+
* An unpacked argument passes its values, not itself: each value is observed
1284+
* against the parameter it lands in - by position, or by name for a string
1285+
* key. Values of an array whose keys are not known may land in any parameter
1286+
* from the argument's position on, and with string keys in any at all.
1287+
*/
1288+
private function observeUnpackedArgument(MutatingScope $scope, ParametersAcceptor $acceptor, int $position, Type $unpackedType, bool $isPure): MutatingScope
1289+
{
1290+
$parameters = $acceptor->getParameters();
1291+
$constantArrays = $unpackedType->getConstantArrays();
1292+
if (count($constantArrays) === 0) {
1293+
$valueType = $unpackedType->getIterableValueType();
1294+
$from = $unpackedType->getIterableKeyType()->isString()->no() ? $position : 0;
1295+
for ($k = $from; $k < count($parameters); $k++) {
1296+
$scope = $this->observeArgumentValue($scope, $acceptor, $parameters[$k], $valueType, $isPure);
1297+
}
1298+
1299+
return $scope;
1300+
}
1301+
1302+
foreach ($constantArrays as $constantArray) {
1303+
$valueTypes = $constantArray->getValueTypes();
1304+
foreach ($constantArray->getKeyTypes() as $j => $keyType) {
1305+
$key = $keyType->getValue();
1306+
$parameter = null;
1307+
if (is_string($key)) {
1308+
foreach ($parameters as $candidate) {
1309+
if ($candidate->getName() !== $key) {
1310+
continue;
1311+
}
1312+
$parameter = $candidate;
1313+
break;
1314+
}
1315+
} else {
1316+
$parameter = $parameters[$position + $j] ?? null;
1317+
}
1318+
if ($parameter === null && $acceptor->isVariadic() && count($parameters) > 0) {
1319+
$parameter = $parameters[count($parameters) - 1];
1320+
}
1321+
if ($parameter === null) {
1322+
continue;
1323+
}
1324+
$scope = $this->observeArgumentValue($scope, $acceptor, $parameter, $valueTypes[$j], $isPure);
1325+
}
1326+
}
1327+
1328+
return $scope;
1329+
}
1330+
1331+
private function observeArgumentValue(MutatingScope $scope, ParametersAcceptor $acceptor, ParameterReflection $parameter, Type $valueType, bool $isPure): MutatingScope
1332+
{
1333+
return $scope->addTemplateArgumentConstraints($this->templateArgumentObserver->collectArgument(
1334+
$this->findOriginalParameterType($acceptor, $parameter) ?? $parameter->getType(),
1335+
$valueType,
1336+
$isPure,
1337+
));
1338+
}
1339+
12771340
/**
12781341
* The parameter type an argument is observed against: the declared one with
12791342
* the template types the call already decided substituted - the receiver's

‎src/Analyser/ExprHandler/ArrayHandler.php‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use PHPStan\Analyser\ExpressionResultFactory;
1616
use PHPStan\Analyser\ExpressionResultStorage;
1717
use PHPStan\Analyser\ExprHandler;
18+
use PHPStan\Analyser\Generics\ClosureSignatureInference;
1819
use PHPStan\Analyser\MutatingScope;
1920
use PHPStan\Analyser\NodeScopeResolver;
2021
use PHPStan\Analyser\SpecifiedTypes;
@@ -146,6 +147,9 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
146147
$nodeScopeResolver->callNodeCallback($nodeCallback, $arrayItem, $itemCallbackScope, $storage);
147148
}
148149
$nodeScopeResolver->callNodeCallback($nodeCallback, new LiteralArrayNode($expr, $itemNodes), $scope, $storage);
150+
if ($nodeScopeResolver->observingTemplateArgumentFrame($scope) !== null) {
151+
$scope = $this->collectAbsorbedItems($expr, $itemResults, $scope);
152+
}
149153

150154
return $this->expressionResultFactory->create(
151155
$scope,
@@ -200,6 +204,34 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
200204
);
201205
}
202206

207+
/**
208+
* A literal generalized by an unpacked item absorbs the closures of its items
209+
* into a wider value type - see ClosureSignatureInference::collectAbsorbed().
210+
*
211+
* @param array<int, ExpressionResult> $itemResults
212+
*/
213+
private function collectAbsorbedItems(Array_ $expr, array $itemResults, MutatingScope $scope): MutatingScope
214+
{
215+
$itemTypes = [];
216+
foreach ($expr->items as $arrayItem) {
217+
$itemType = $itemResults[spl_object_id($arrayItem->value)]->getType();
218+
if (!ClosureSignatureInference::hasMarkers($itemType)) {
219+
continue;
220+
}
221+
$itemTypes[] = $itemType;
222+
}
223+
if ($itemTypes === []) {
224+
return $scope;
225+
}
226+
227+
$arrayType = $this->initializerExprTypeResolver->getArrayType($expr, static fn (Expr $inner): Type => $itemResults[spl_object_id($inner)]->getType());
228+
foreach ($itemTypes as $itemType) {
229+
$scope = $scope->addTemplateArgumentConstraints(ClosureSignatureInference::collectAbsorbed($itemType, $arrayType));
230+
}
231+
232+
return $scope;
233+
}
234+
203235
private function getExpectedArrayType(?Type $type): ?Type
204236
{
205237
if ($type === null || $type->isIterable()->no()) {

‎src/Analyser/ExprHandler/AssignHandler.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
use PHPStan\Analyser\ExprHandler\Helper\MethodThrowPointHelper;
3737
use PHPStan\Analyser\ExprHandler\Helper\NonNullabilityHelper;
3838
use PHPStan\Analyser\ExprHandler\Helper\VirtualExprResultHelper;
39+
use PHPStan\Analyser\Generics\ClosureSignatureInference;
3940
use PHPStan\Analyser\Generics\TemplateArgumentConstraints;
4041
use PHPStan\Analyser\Generics\TemplateArgumentObserver;
4142
use PHPStan\Analyser\ImpurePoint;
@@ -1454,6 +1455,10 @@ public function applyWrite(
14541455
TrinaryLogic::createYes(),
14551456
[],
14561457
);
1458+
if ($nodeScopeResolver->observingTemplateArgumentFrame($scope) !== null) {
1459+
// the array's value type may absorb the closures written into it
1460+
$scope = $scope->addTemplateArgumentConstraints(ClosureSignatureInference::collectAbsorbed($writtenValueType, $valueToWrite));
1461+
}
14571462
} else {
14581463
if ($var instanceof PropertyFetch || $var instanceof StaticPropertyFetch) {
14591464
$nodeScopeResolver->callNodeCallback($nodeCallback, new PropertyAssignNode($var, $assignedPropertyExpr, $isAssignOp), $scopeBeforeAssignEval, $storage);

‎src/Analyser/ExprHandler/AssignOpHandler.php‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use PHPStan\Analyser\ExprHandler\Helper\CoalesceCompositionHelper;
1616
use PHPStan\Analyser\ExprHandler\Helper\DefaultNarrowingHelper;
1717
use PHPStan\Analyser\ExprHandler\Helper\ImplicitToStringCallHelper;
18+
use PHPStan\Analyser\Generics\ClosureSignatureInference;
1819
use PHPStan\Analyser\InternalThrowPoint;
1920
use PHPStan\Analyser\MutatingScope;
2021
use PHPStan\Analyser\NodeScopeResolver;
@@ -108,8 +109,12 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
108109
$rhsResult = $valueResult;
109110
if ($expr instanceof Expr\AssignOp\Coalesce) {
110111
$rightResult = $valueResult;
112+
$coalescedScope = $rightResult->getScope()->mergeWith($valueBeforeScope);
113+
if ($nodeScopeResolver->observingTemplateArgumentFrame($coalescedScope) !== null) {
114+
$coalescedScope = $coalescedScope->addTemplateArgumentConstraints(ClosureSignatureInference::collectAbsorbedInUnion([$condResult->getType(), $rightResult->getType()]));
115+
}
111116
$valueResult = $this->expressionResultFactory->create(
112-
$rightResult->getScope()->mergeWith($valueBeforeScope),
117+
$coalescedScope,
113118
$valueBeforeScope,
114119
$expr->expr,
115120
$rightResult->hasYield(),

‎src/Analyser/ExprHandler/CoalesceHandler.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use PHPStan\Analyser\ExprHandler\Helper\CoalesceCompositionHelper;
1414
use PHPStan\Analyser\ExprHandler\Helper\DefaultNarrowingHelper;
1515
use PHPStan\Analyser\ExprHandler\Helper\NonNullabilityHelper;
16+
use PHPStan\Analyser\Generics\ClosureSignatureInference;
1617
use PHPStan\Analyser\MutatingScope;
1718
use PHPStan\Analyser\NodeScopeResolver;
1819
use PHPStan\Analyser\SpecifiedTypes;
@@ -102,6 +103,10 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
102103
$scope = $scope->applySpecifiedTypes($leftIssetTypes)->mergeWith($rightResult->getScope());
103104
}
104105

106+
if ($nodeScopeResolver->observingTemplateArgumentFrame($scope) !== null) {
107+
$scope = $scope->addTemplateArgumentConstraints(ClosureSignatureInference::collectAbsorbedInUnion([$condResult->getType(), $rightExprType]));
108+
}
109+
105110
$nodeScopeResolver->callNodeCallbackWithExpression($nodeCallback, new CoalesceExpressionNode($expr, $condResult, $rightResult, 'on left side of ??'), $beforeScope, $storage, $context);
106111

107112
return $this->expressionResultFactory->create(

‎src/Analyser/ExprHandler/FuncCallHandler.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -560,6 +560,9 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
560560

561561
$byRefWrittenNames = [];
562562
if ($nameType !== null) {
563+
if ($nodeScopeResolver->observingTemplateArgumentFrame($scope) !== null) {
564+
$scope = $scope->addTemplateArgumentConstraints(ClosureSignatureInference::collectInvokedCallee($nameType));
565+
}
563566
[$scope, $byRefThrowPoints, $byRefWrittenNames] = $this->processByRefInvocations($nodeScopeResolver, $normalizedExpr, $nameType, $scope, $storage);
564567
$throwPoints = array_merge($throwPoints, $byRefThrowPoints);
565568
}

‎src/Analyser/ExprHandler/MatchHandler.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use PHPStan\Analyser\ExprHandler;
2222
use PHPStan\Analyser\ExprHandler\Helper\DefaultNarrowingHelper;
2323
use PHPStan\Analyser\ExprHandler\Helper\IdenticalNarrowingHelper;
24+
use PHPStan\Analyser\Generics\ClosureSignatureInference;
2425
use PHPStan\Analyser\InternalThrowPoint;
2526
use PHPStan\Analyser\MutatingScope;
2627
use PHPStan\Analyser\NodeScopeResolver;
@@ -499,6 +500,13 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
499500
}
500501

501502
$scope = $scope->addTemplateArgumentConstraints($scopeForMatchNodeCallback->getTemplateArgumentConstraints());
503+
if ($nodeScopeResolver->observingTemplateArgumentFrame($scope) !== null) {
504+
$armTypes = [];
505+
foreach ($armTypeResults as [$armResult, $bodyScope]) {
506+
$armTypes[] = $armResult->getTypeOnScope($bodyScope, false);
507+
}
508+
$scope = $scope->addTemplateArgumentConstraints(ClosureSignatureInference::collectAbsorbedInUnion($armTypes));
509+
}
502510

503511
ksort($armNodes, SORT_NUMERIC);
504512

‎src/Analyser/ExprHandler/MethodCallHandler.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use PHPStan\Analyser\ExprHandler\Helper\EarlyTerminatingCallHelper;
2222
use PHPStan\Analyser\ExprHandler\Helper\MethodCallReturnTypeHelper;
2323
use PHPStan\Analyser\ExprHandler\Helper\MethodThrowPointHelper;
24+
use PHPStan\Analyser\Generics\ClosureSignatureInference;
2425
use PHPStan\Analyser\Generics\TemplateArgumentFrame;
2526
use PHPStan\Analyser\ImpurePoint;
2627
use PHPStan\Analyser\InternalThrowPoint;
@@ -120,6 +121,11 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
120121
if (isset($closureCallScope)) {
121122
$scope = $scope->restoreOriginalScopeAfterClosureBind($originalScope);
122123
}
124+
if ($nodeScopeResolver->observingTemplateArgumentFrame($scope) !== null) {
125+
// a method of a closure - __invoke(), call(), bindTo() - runs it or
126+
// hands it on where nothing follows its signature
127+
$scope = $scope->addTemplateArgumentConstraints(ClosureSignatureInference::collectEscapes($varResult->getType()));
128+
}
123129
$parametersAcceptor = null;
124130
$variants = [];
125131
$namedArgumentsVariants = null;

‎src/Analyser/ExprHandler/TernaryHandler.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use PHPStan\Analyser\ExprHandler;
1414
use PHPStan\Analyser\ExprHandler\Helper\BooleanNarrowingHelper;
1515
use PHPStan\Analyser\ExprHandler\Helper\DefaultNarrowingHelper;
16+
use PHPStan\Analyser\Generics\ClosureSignatureInference;
1617
use PHPStan\Analyser\MutatingScope;
1718
use PHPStan\Analyser\NodeScopeResolver;
1819
use PHPStan\Analyser\PerFileAnalysisResettable;
@@ -141,6 +142,12 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
141142

142143
$finalScope = $finalScope->addTemplateArgumentConstraints($ifTrueScope->getTemplateArgumentConstraints())
143144
->addTemplateArgumentConstraints($ifFalseScope->getTemplateArgumentConstraints());
145+
if ($nodeScopeResolver->observingTemplateArgumentFrame($finalScope) !== null) {
146+
$finalScope = $finalScope->addTemplateArgumentConstraints(ClosureSignatureInference::collectAbsorbedInUnion([
147+
$ifResult !== null ? $ifResult->getTypeOnScope($ifProcessingScope, false) : $ternaryCondResult->getType(),
148+
$elseResult->getTypeOnScope($elseProcessingScope, false),
149+
]));
150+
}
144151

145152
// lazily memoized merged-falsey scope of the (cond && if) disjunct
146153
$aFalseyScope = null;

‎src/Analyser/ExprHandler/Virtual/FunctionCallableNodeHandler.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use PHPStan\Analyser\ExpressionResultStorage;
1212
use PHPStan\Analyser\ExprHandler;
1313
use PHPStan\Analyser\ExprHandler\Helper\DefaultNarrowingHelper;
14+
use PHPStan\Analyser\Generics\ClosureSignatureInference;
1415
use PHPStan\Analyser\MutatingScope;
1516
use PHPStan\Analyser\NodeScopeResolver;
1617
use PHPStan\Analyser\TypeSpecifierContext;
@@ -55,6 +56,11 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
5556
if ($expr->getName() instanceof Expr) {
5657
$nameResult = $nodeScopeResolver->processExprNode($stmt, $expr->getName(), $scope, $storage, $nodeCallback, ExpressionContext::createDeep($context->shouldResolveTemplateArguments()));
5758
$scope = $nameResult->getScope();
59+
if ($nodeScopeResolver->observingTemplateArgumentFrame($scope) !== null && !self::isClosureObject($nameResult->getType())) {
60+
// the callable built from anything but a closure object runs the
61+
// closures it carries where nothing follows their signature
62+
$scope = $scope->addTemplateArgumentConstraints(ClosureSignatureInference::collectEscapes($nameResult->getType()));
63+
}
5864
$hasYield = $nameResult->hasYield();
5965
$throwPoints = $nameResult->getThrowPoints();
6066
$impurePoints = $nameResult->getImpurePoints();
@@ -85,6 +91,10 @@ private function resolveType(MutatingScope $scope, FunctionCallableNode $expr, ?
8591
throw new ShouldNotHappenException();
8692
}
8793
$callableType = $nameResult->getTypeOnScope($scope, $scope->nativeTypesPromoted);
94+
if (self::isClosureObject($callableType)) {
95+
// the first-class callable of a closure object is the object itself
96+
return $callableType;
97+
}
8898
if (!$callableType->isCallable()->yes()) {
8999
return new ObjectType(Closure::class);
90100
}
@@ -99,4 +109,9 @@ private function resolveType(MutatingScope $scope, FunctionCallableNode $expr, ?
99109
return $this->initializerExprTypeResolver->getFirstClassCallableType($originalNode, InitializerExprContext::fromScope($scope), $scope->nativeTypesPromoted);
100110
}
101111

112+
private static function isClosureObject(Type $type): bool
113+
{
114+
return (new ObjectType(Closure::class))->isSuperTypeOf($type)->yes();
115+
}
116+
102117
}

0 commit comments

Comments
 (0)