Skip to content

Commit 853cc0d

Browse files
VincentLangletphpstan-bot
authored andcommitted
Do not propagate isAlwaysTerminating from immediately-invoked callable arguments to the outer call
- Remove `isAlwaysTerminating` propagation from closure, arrow function, and other callable arguments in `NodeScopeResolver::processArgs()` when the parameter is marked as immediately-invoked - Remove now-unused `ProcessClosureResult::isAlwaysTerminating()` method and its backing field - Update `ExpressionResultTest` expectations for `call_user_func` with never-returning callbacks (minor false negative trade-off) - "Immediately invoked" means the callback runs during the function's execution, not that it is unconditionally called on every code path - Throw points and impure points still propagate correctly
1 parent 0beee48 commit 853cc0d

6 files changed

Lines changed: 107 additions & 17 deletions

File tree

src/Analyser/NodeScopeResolver.php

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2788,9 +2788,6 @@ public function processClosureNode(
27882788
throw new ShouldNotHappenException();
27892789
}
27902790

2791-
$returnType = $closureType->getReturnType();
2792-
$isAlwaysTerminating = ($returnType instanceof NeverType && $returnType->isExplicit());
2793-
27942791
$this->callNodeCallback($nodeCallback, new InClosureNode($closureType, $expr), $closureScope, $storage);
27952792

27962793
$executionEnds = [];
@@ -2844,7 +2841,7 @@ public function processClosureNode(
28442841
array_merge($publicStatementResult->getImpurePoints(), $closureImpurePoints),
28452842
), $closureScope, $storage);
28462843

2847-
return new ProcessClosureResult($scope, $statementResult->getThrowPoints(), $statementResult->getImpurePoints(), $invalidateExpressions, $isAlwaysTerminating);
2844+
return new ProcessClosureResult($scope, $statementResult->getThrowPoints(), $statementResult->getImpurePoints(), $invalidateExpressions);
28482845
}
28492846

28502847
$originalStorage = $storage;
@@ -2894,7 +2891,7 @@ public function processClosureNode(
28942891
array_merge($publicStatementResult->getImpurePoints(), $closureImpurePoints),
28952892
), $closureScope, $storage);
28962893

2897-
return new ProcessClosureResult($scope, $statementResult->getThrowPoints(), $statementResult->getImpurePoints(), $invalidateExpressions, $isAlwaysTerminating, $closureResultScope, $byRefUses);
2894+
return new ProcessClosureResult($scope, $statementResult->getThrowPoints(), $statementResult->getImpurePoints(), $invalidateExpressions, $closureResultScope, $byRefUses);
28982895
}
28992896

29002897
/**
@@ -3402,7 +3399,6 @@ public function processArgs(
34023399
if ($this->callCallbackImmediately($parameter, $parameterType, $calleeReflection)) {
34033400
$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()));
34043401
$impurePoints = array_merge($impurePoints, $closureResult->getImpurePoints());
3405-
$isAlwaysTerminating = $isAlwaysTerminating || $closureResult->isAlwaysTerminating();
34063402
}
34073403

34083404
$this->storeBeforeScope($storage, $arg->value, $scopeToPass);
@@ -3462,7 +3458,6 @@ public function processArgs(
34623458
if ($this->callCallbackImmediately($parameter, $parameterType, $calleeReflection)) {
34633459
$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()));
34643460
$impurePoints = array_merge($impurePoints, $arrowFunctionResult->getImpurePoints());
3465-
$isAlwaysTerminating = $isAlwaysTerminating || $arrowFunctionResult->isAlwaysTerminating();
34663461
}
34673462
$this->storeBeforeScope($storage, $arg->value, $scopeToPass);
34683463
} else {
@@ -3492,8 +3487,6 @@ public function processArgs(
34923487
}
34933488
$throwPoints = array_merge($throwPoints, $callableThrowPoints);
34943489
$impurePoints = array_merge($impurePoints, array_map(static fn (SimpleImpurePoint $impurePoint) => new ImpurePoint($scope, $arg->value, $impurePoint->getIdentifier(), $impurePoint->getDescription(), $impurePoint->isCertain()), $acceptors[0]->getImpurePoints()));
3495-
$returnType = $acceptors[0]->getReturnType();
3496-
$isAlwaysTerminating = $isAlwaysTerminating || ($returnType instanceof NeverType && $returnType->isExplicit());
34973490
}
34983491
}
34993492
}

src/Analyser/ProcessClosureResult.php

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ public function __construct(
1919
private array $throwPoints,
2020
private array $impurePoints,
2121
private array $invalidateExpressions,
22-
private bool $isAlwaysTerminating,
2322
private ?MutatingScope $byRefClosureResultScope = null,
2423
private array $byRefUses = [],
2524
)
@@ -64,9 +63,4 @@ public function getInvalidateExpressions(): array
6463
return $this->invalidateExpressions;
6564
}
6665

67-
public function isAlwaysTerminating(): bool
68-
{
69-
return $this->isAlwaysTerminating;
70-
}
71-
7266
}

tests/PHPStan/Analyser/ExpressionResultTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ public static function dataIsAlwaysTerminating(): array
111111
],
112112
[
113113
'call_user_func(fn() => exit());',
114-
true,
114+
false,
115115
],
116116
[
117117
'(function() { exit(); })();',
@@ -123,7 +123,7 @@ public static function dataIsAlwaysTerminating(): array
123123
],
124124
[
125125
'call_user_func(function() { exit(); });',
126-
true,
126+
false,
127127
],
128128
[
129129
'usort($arr, static function($a, $b):int { return $a <=> $b; });',
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?php // lint >= 8.0
2+
3+
declare(strict_types = 1);
4+
5+
namespace Bug14582;
6+
7+
use function PHPStan\Testing\assertType;
8+
9+
function testArrayFilter(): void
10+
{
11+
$b = array_filter([], fn() => throw new \Error());
12+
assertType('array{}', $b);
13+
}
14+
15+
function testArrayMap(): void
16+
{
17+
$result = array_map(fn() => throw new \Error(), []);
18+
assertType('array{}', $result);
19+
}

tests/PHPStan/Rules/DeadCode/UnreachableStatementRuleTest.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,4 +412,11 @@ public function testBug14369(): void
412412
]);
413413
}
414414

415+
#[RequiresPhp('>= 8.1.0')]
416+
public function testBug14582(): void
417+
{
418+
$this->treatPhpDocTypesAsCertain = true;
419+
$this->analyse([__DIR__ . '/data/bug-14582.php'], []);
420+
}
421+
415422
}
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
<?php // lint >= 8.1
2+
3+
declare(strict_types=1);
4+
5+
namespace Bug14582;
6+
7+
use Closure;
8+
use RuntimeException;
9+
10+
/**
11+
* @template TLeft
12+
* @template TRight
13+
*/
14+
interface Choice
15+
{
16+
/**
17+
* @template TResult
18+
*
19+
* @param (Closure(TRight): TResult) $right
20+
* @param-immediately-invoked-callable $right
21+
* @param (Closure(TLeft): TResult) $left
22+
* @param-immediately-invoked-callable $left
23+
*
24+
* @return TResult
25+
*/
26+
public function proceed(Closure $right, Closure $left): mixed;
27+
}
28+
29+
/** @return Choice<string, null> */
30+
function getChoice(bool $right): Choice
31+
{
32+
throw new RuntimeException('stub');
33+
}
34+
35+
function noop(): void
36+
{
37+
}
38+
39+
function doSomethingAfterwards(): void
40+
{
41+
}
42+
43+
function test(bool $right): void
44+
{
45+
getChoice($right)->proceed(
46+
right: noop(...),
47+
left: fn (string $message) => throw new RuntimeException($message),
48+
);
49+
doSomethingAfterwards();
50+
}
51+
52+
function testArrayFilter(): void
53+
{
54+
$b = array_filter([], fn() => throw new \Error());
55+
echo $b;
56+
}
57+
58+
function testArrayMap(): void
59+
{
60+
array_map(fn() => throw new \Error(), []);
61+
echo 'reachable';
62+
}
63+
64+
function testUsort(): void
65+
{
66+
$a = [1, 2, 3];
67+
usort($a, fn($a, $b) => throw new \Error());
68+
echo 'reachable';
69+
}
70+
71+
function testClosureArgument(): void
72+
{
73+
array_filter([], function () {
74+
throw new \Error();
75+
});
76+
echo 'reachable';
77+
}

0 commit comments

Comments
 (0)