Skip to content

Commit 89d0610

Browse files
staabmstaabm
authored
Fix missing detection of dead code in expressions
1 parent 0082df6 commit 89d0610

File tree

9 files changed

+588
-12
lines changed

9 files changed

+588
-12
lines changed

‎src/Analyser/ExpressionResult.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ final class ExpressionResult
2424
public function __construct(
2525
private MutatingScope $scope,
2626
private bool $hasYield,
27+
private bool $isAlwaysTerminating,
2728
private array $throwPoints,
2829
private array $impurePoints,
2930
?callable $truthyScopeCallback = null,
@@ -90,4 +91,9 @@ public function getFalseyScope(): MutatingScope
9091
return $this->falseyScope;
9192
}
9293

94+
public function isAlwaysTerminating(): bool
95+
{
96+
return $this->isAlwaysTerminating;
97+
}
98+
9399
}

‎src/Analyser/NodeScopeResolver.php

Lines changed: 119 additions & 12 deletions
Large diffs are not rendered by default.

‎tests/PHPStan/Analyser/ExpressionResultTest.php

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Analyser;
4+
5+
use PhpParser\Node\Stmt;
6+
use PHPStan\Parser\Parser;
7+
use PHPStan\ShouldNotHappenException;
8+
use PHPStan\Testing\PHPStanTestCase;
9+
use PHPStan\TrinaryLogic;
10+
use PHPStan\Type\IntegerType;
11+
use PHPUnit\Framework\Attributes\DataProvider;
12+
use function count;
13+
use function get_class;
14+
use function sprintf;
15+
16+
class ExpressionResultTest extends PHPStanTestCase
17+
{
18+
19+
public static function dataIsAlwaysTerminating(): array
20+
{
21+
return [
22+
[
23+
'sprintf("hello %s", "abc");',
24+
false,
25+
],
26+
[
27+
'isset($x);',
28+
false,
29+
],
30+
[
31+
'$x ? "def" : "abc";',
32+
false,
33+
],
34+
[
35+
'(string) $x;',
36+
false,
37+
],
38+
[
39+
'$x || exit();',
40+
false,
41+
],
42+
[
43+
'$x ?? exit();',
44+
false,
45+
],
46+
[
47+
'sprintf("hello %s", exit());',
48+
true,
49+
],
50+
[
51+
'(string) exit();',
52+
true,
53+
],
54+
[
55+
'!exit();',
56+
true,
57+
],
58+
[
59+
'eval(exit());',
60+
true,
61+
],
62+
[
63+
'empty(exit());',
64+
true,
65+
],
66+
[
67+
'isset(exit());',
68+
true,
69+
],
70+
[
71+
'$x ? "abc" : exit();',
72+
false,
73+
],
74+
[
75+
'$x ? exit() : "abc";',
76+
false,
77+
],
78+
[
79+
'fn() => yield (exit());',
80+
false,
81+
],
82+
[
83+
'@exit();',
84+
true,
85+
],
86+
[
87+
'$x && exit();',
88+
false,
89+
],
90+
[
91+
'exit() && $x;',
92+
true,
93+
],
94+
[
95+
'exit() || $x;',
96+
true,
97+
],
98+
[
99+
'exit() ?? $x;',
100+
true,
101+
],
102+
[
103+
'var_dump(1+exit());',
104+
true,
105+
],
106+
[
107+
'var_dump(1-exit());',
108+
true,
109+
],
110+
[
111+
'var_dump(1*exit());',
112+
true,
113+
],
114+
[
115+
'var_dump(1**exit());',
116+
true,
117+
],
118+
[
119+
'var_dump(1/exit());',
120+
true,
121+
],
122+
[
123+
'var_dump("a".exit());',
124+
true,
125+
],
126+
[
127+
'var_dump(exit()."a");',
128+
true,
129+
],
130+
];
131+
}
132+
133+
#[DataProvider('dataIsAlwaysTerminating')]
134+
public function testIsAlwaysTerminating(
135+
string $code,
136+
bool $expectedIsAlwaysTerminating,
137+
): void
138+
{
139+
/** @var Parser $parser */
140+
$parser = self::getContainer()->getService('currentPhpVersionRichParser');
141+
142+
/** @var Stmt[] $stmts */
143+
$stmts = $parser->parseString(sprintf('<?php %s', $code));
144+
if (count($stmts) !== 1) {
145+
throw new ShouldNotHappenException('Expecting code which evaluates to a single statement, got: ' . count($stmts));
146+
}
147+
if (!$stmts[0] instanceof Stmt\Expression) {
148+
throw new ShouldNotHappenException('Expecting code contains a single statement expression, got: ' . get_class($stmts[0]));
149+
}
150+
$stmt = $stmts[0];
151+
$expr = $stmt->expr;
152+
153+
/** @var NodeScopeResolver $nodeScopeResolver */
154+
$nodeScopeResolver = self::getContainer()->getByType(NodeScopeResolver::class);
155+
/** @var ScopeFactory $scopeFactory */
156+
$scopeFactory = self::getContainer()->getByType(ScopeFactory::class);
157+
$scope = $scopeFactory->create(ScopeContext::create('test.php'))
158+
->assignVariable('x', new IntegerType(), new IntegerType(), TrinaryLogic::createYes());
159+
160+
$result = $nodeScopeResolver->processExprNode(
161+
$stmt,
162+
$expr,
163+
$scope,
164+
static function (): void {
165+
},
166+
ExpressionContext::createTopLevel(),
167+
);
168+
$this->assertSame($expectedIsAlwaysTerminating, $result->isAlwaysTerminating());
169+
}
170+
171+
}

‎tests/PHPStan/Rules/DeadCode/UnreachableStatementRuleTest.php

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use PHPStan\Rules\Rule;
66
use PHPStan\Testing\RuleTestCase;
77
use PHPUnit\Framework\Attributes\DataProvider;
8+
use PHPUnit\Framework\Attributes\RequiresPhp;
89

910
/**
1011
* @extends RuleTestCase<UnreachableStatementRule>
@@ -251,4 +252,100 @@ public function testMultipleUnreachable(): void
251252
]);
252253
}
253254

255+
#[RequiresPhp('>= 8.1')]
256+
public function testBug11909(): void
257+
{
258+
$this->treatPhpDocTypesAsCertain = false;
259+
$this->analyse([__DIR__ . '/data/bug-11909.php'], [
260+
[
261+
'Unreachable statement - code above always terminates.',
262+
10,
263+
],
264+
]);
265+
}
266+
267+
#[RequiresPhp('>= 8.1')]
268+
public function testBug13232a(): void
269+
{
270+
$this->treatPhpDocTypesAsCertain = false;
271+
$this->analyse([__DIR__ . '/data/bug-13232a.php'], [
272+
[
273+
'Unreachable statement - code above always terminates.',
274+
10,
275+
],
276+
[
277+
'Unreachable statement - code above always terminates.',
278+
17,
279+
],
280+
[
281+
'Unreachable statement - code above always terminates.',
282+
23,
283+
],
284+
[
285+
'Unreachable statement - code above always terminates.',
286+
32,
287+
],
288+
[
289+
'Unreachable statement - code above always terminates.',
290+
38,
291+
],
292+
[
293+
'Unreachable statement - code above always terminates.',
294+
44,
295+
],
296+
[
297+
'Unreachable statement - code above always terminates.',
298+
52,
299+
],
300+
[
301+
'Unreachable statement - code above always terminates.',
302+
61,
303+
],
304+
[
305+
'Unreachable statement - code above always terminates.',
306+
70,
307+
],
308+
]);
309+
}
310+
311+
#[RequiresPhp('>= 8.1')]
312+
public function testBug13232b(): void
313+
{
314+
$this->treatPhpDocTypesAsCertain = false;
315+
$this->analyse([__DIR__ . '/data/bug-13232b.php'], [
316+
[
317+
'Unreachable statement - code above always terminates.',
318+
19,
319+
],
320+
]);
321+
}
322+
323+
#[RequiresPhp('>= 8.1')]
324+
public function testBug13232c(): void
325+
{
326+
$this->treatPhpDocTypesAsCertain = false;
327+
$this->analyse([__DIR__ . '/data/bug-13232c.php'], [
328+
[
329+
'Unreachable statement - code above always terminates.',
330+
12,
331+
],
332+
[
333+
'Unreachable statement - code above always terminates.',
334+
20,
335+
],
336+
]);
337+
}
338+
339+
#[RequiresPhp('>= 8.1')]
340+
public function testBug13232d(): void
341+
{
342+
$this->treatPhpDocTypesAsCertain = false;
343+
$this->analyse([__DIR__ . '/data/bug-13232d.php'], [
344+
[
345+
'Unreachable statement - code above always terminates.',
346+
11,
347+
],
348+
]);
349+
}
350+
254351
}

‎tests/PHPStan/Rules/DeadCode/data/bug-11909.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug11909;
4+
5+
function doFoo(): never {
6+
throw new LogicException("throws");
7+
}
8+
9+
echo doFoo();
10+
echo "hello";

‎tests/PHPStan/Rules/DeadCode/data/bug-13232a.php

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
<?php
2+
3+
namespace Bug13232a;
4+
5+
final class HelloWorld
6+
{
7+
public function sayHa(): void
8+
{
9+
echo sprintf("Hello, %s no way", $this->neverReturnsMethod());
10+
echo 'this will never happen';
11+
}
12+
13+
public function sayHi(): void
14+
{
15+
echo 'Hello, ' . neverReturns()
16+
. ' no way';
17+
echo 'this will never happen';
18+
}
19+
20+
public function sayHo(): void
21+
{
22+
echo "Hello, {$this->neverReturnsMethod()} no way";
23+
echo 'this will never happen';
24+
}
25+
26+
public function sayHe(): void
27+
{
28+
$callable = function (): never {
29+
exit();
30+
};
31+
echo sprintf("Hello, %s no way", $callable());
32+
echo 'this will never happen';
33+
}
34+
35+
public function sayHe2(): void
36+
{
37+
$this->doFoo($this->neverReturnsMethod());
38+
echo 'this will never happen';
39+
}
40+
41+
public function sayHe3(): void
42+
{
43+
self::doStaticFoo($this->neverReturnsMethod());
44+
echo 'this will never happen';
45+
}
46+
47+
public function sayHuu(): void
48+
{
49+
$x = [
50+
$this->neverReturnsMethod()
51+
];
52+
echo 'this will never happen';
53+
}
54+
55+
public function sayClosure(): void
56+
{
57+
$callable = function (): never {
58+
exit();
59+
};
60+
$callable();
61+
echo 'this will never happen';
62+
}
63+
64+
public function sayIIFE(): void
65+
{
66+
(function (): never {
67+
exit();
68+
})();
69+
70+
echo 'this will never happen';
71+
}
72+
73+
function neverReturnsMethod(): never {
74+
exit();
75+
}
76+
77+
public function doFoo() {}
78+
79+
static public function doStaticFoo() {}
80+
}
81+
function neverReturns(): never {
82+
exit();
83+
}
84+

0 commit comments

Comments
 (0)