Skip to content

Commit c7301f6

Browse files
ondrejmirtesclaude
ondrejmirtes
and
claude
committed
Do not treat possible Error throws as an effect of an expression statement
Errors (ValueError, TypeError, DivisionByZeroError, ...) signal programmer mistakes, nobody calls an otherwise pure expression just to have one thrown. Explicit throw points whose type is entirely a subtype of Error therefore no longer prevent the NoopExpressionNode, so sprintf(), strpos(), intdiv(), $a / $b and similar on a separate line are reported again after PhpStorm stubs added @throws \ValueError to them. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W2hMwFhyMX7JHn6oVKrTPD
1 parent e8c314c commit c7301f6

7 files changed

Lines changed: 101 additions & 48 deletions

‎src/Analyser/StmtHandler/ExpressionHandler.php‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace PHPStan\Analyser\StmtHandler;
44

5+
use Error;
56
use PhpParser\Node;
67
use PhpParser\Node\Expr;
78
use PhpParser\Node\Stmt;
@@ -22,6 +23,7 @@
2223
use PHPStan\Node\PropertyAssignNode;
2324
use PHPStan\Node\VariableAssignNode;
2425
use PHPStan\Type\NeverType;
26+
use PHPStan\Type\ObjectType;
2527
use function array_filter;
2628
use function count;
2729

@@ -73,7 +75,11 @@ public function processStmt(
7375
}
7476

7577
$nodeScopeResolver->callNodeCallback($nodeCallback, $stmt, $entryScope, $storage);
76-
$throwPoints = array_filter($result->getThrowPoints(), static fn ($throwPoint) => $throwPoint->isExplicit());
78+
// Errors signal programmer mistakes (ValueError, TypeError, DivisionByZeroError...),
79+
// nobody calls an otherwise pure expression just to have them thrown, so they
80+
// do not make the expression statement meaningful.
81+
$errorType = new ObjectType(Error::class);
82+
$throwPoints = array_filter($result->getThrowPoints(), static fn ($throwPoint) => $throwPoint->isExplicit() && !$errorType->isSuperTypeOf($throwPoint->getType())->yes());
7783
if (
7884
count($result->getImpurePoints()) === 0
7985
&& count($throwPoints) === 0

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,25 @@ public function testRule(): void
140140
]);
141141
}
142142

143+
#[RequiresPhp('>= 8.0.0')]
144+
public function testErrorThrows(): void
145+
{
146+
$this->analyse([__DIR__ . '/data/noop-error-throws.php'], [
147+
[
148+
'Expression "$a / $b" on a separate line does not do anything.',
149+
6,
150+
],
151+
[
152+
'Expression "$a % $b" on a separate line does not do anything.',
153+
7,
154+
],
155+
[
156+
'Expression "match ($a) {…" on a separate line does not do anything.',
157+
8,
158+
],
159+
]);
160+
}
161+
143162
public function testNullsafe(): void
144163
{
145164
$this->analyse([__DIR__ . '/data/nullsafe-property-fetch-noop.php'], [

‎tests/PHPStan/Rules/DeadCode/data/noop-error-throws.php‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?php // lint >= 8.0
2+
3+
namespace DeadCodeNoopErrorThrows;
4+
5+
function (int $a, int $b, string $s) {
6+
$a / $b;
7+
$a % $b;
8+
match ($a) {
9+
1 => 'a',
10+
};
11+
new \DateTimeImmutable($s);
12+
};

‎tests/PHPStan/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRuleTest.php‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,32 @@ public function testBug11101(): void
113113
]);
114114
}
115115

116+
public function testErrorThrows(): void
117+
{
118+
$this->analyse([__DIR__ . '/data/function-call-statement-no-side-effects-error-throws.php'], [
119+
[
120+
'Call to function sprintf() on a separate line has no effect.',
121+
22,
122+
],
123+
[
124+
'Call to function strpos() on a separate line has no effect.',
125+
23,
126+
],
127+
[
128+
'Call to function intdiv() on a separate line has no effect.',
129+
24,
130+
],
131+
[
132+
'Call to function array_combine() on a separate line has no effect.',
133+
25,
134+
],
135+
[
136+
'Call to function FunctionCallStatementNoSideEffectsErrorThrows\\pureAndThrowsError() on a separate line has no effect.',
137+
27,
138+
],
139+
]);
140+
}
141+
116142
public function testBug4455(): void
117143
{
118144
require_once __DIR__ . '/data/bug-4455.php';

‎tests/PHPStan/Rules/Functions/data/function-call-statement-no-side-effects-error-throws.php‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php
2+
3+
namespace FunctionCallStatementNoSideEffectsErrorThrows;
4+
5+
/**
6+
* @phpstan-pure
7+
* @throws \TypeError
8+
*/
9+
function pureAndThrowsError(): string { return 'aaa'; }
10+
11+
/**
12+
* @phpstan-pure
13+
* @throws \Exception
14+
*/
15+
function pureAndThrowsException(): string { return 'aaa'; }
16+
17+
class Foo
18+
{
19+
20+
public function doFoo(string $format, string $haystack, string $needle, int $offset, int $a, int $b, string $json): void
21+
{
22+
sprintf($format, $haystack);
23+
strpos($haystack, $needle, $offset);
24+
intdiv($a, $b);
25+
array_combine([$haystack], [$haystack, $needle]);
26+
json_decode($json, true, 512, JSON_THROW_ON_ERROR);
27+
pureAndThrowsError();
28+
pureAndThrowsException();
29+
}
30+
31+
}

‎tests/PHPStan/Rules/Methods/CallToMethodStatementWithoutSideEffectsRuleTest.php‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -31,31 +31,7 @@ protected function getRule(): Rule
3131
);
3232
}
3333

34-
#[RequiresPhp('>= 8.0.0')]
3534
public function testRule(): void
36-
{
37-
$this->analyse([__DIR__ . '/data/method-call-statement-no-side-effects.php'], [
38-
[
39-
'Call to method DateTimeImmutable::modify() on a separate line has no effect.',
40-
15,
41-
],
42-
[
43-
'Call to method Exception::getCode() on a separate line has no effect.',
44-
21,
45-
],
46-
[
47-
'Call to method MethodCallStatementNoSideEffects\Bar::doPure() on a separate line has no effect.',
48-
63,
49-
],
50-
[
51-
'Call to method MethodCallStatementNoSideEffects\Bar::doPureWithThrowsVoid() on a separate line has no effect.',
52-
64,
53-
],
54-
]);
55-
}
56-
57-
#[RequiresPhp('< 8.0.0')]
58-
public function testRulePhp7(): void
5935
{
6036
$this->analyse([__DIR__ . '/data/method-call-statement-no-side-effects.php'], [
6137
[

‎tests/PHPStan/Rules/Methods/CallToStaticMethodStatementWithoutSideEffectsRuleTest.php‎

Lines changed: 6 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
use PHPStan\Rules\RuleLevelHelper;
77
use PHPStan\Testing\RuleTestCase;
88
use PHPUnit\Framework\Attributes\RequiresPhp;
9-
use const PHP_VERSION_ID;
109

1110
/**
1211
* @extends RuleTestCase<CallToStaticMethodStatementWithoutSideEffectsRule>
@@ -32,19 +31,7 @@ protected function getRule(): Rule
3231
);
3332
}
3433

35-
#[RequiresPhp('>= 8.0.0')]
3634
public function testRule(): void
37-
{
38-
$this->analyse([__DIR__ . '/data/static-method-call-statement-no-side-effects.php'], [
39-
[
40-
'Call to method DateTime::format() on a separate line has no effect.',
41-
23,
42-
],
43-
]);
44-
}
45-
46-
#[RequiresPhp('< 8.0.0')]
47-
public function testRulePhp7(): void
4835
{
4936
$this->analyse([__DIR__ . '/data/static-method-call-statement-no-side-effects.php'], [
5037
[
@@ -122,16 +109,12 @@ public function testFirstClassCallables(): void
122109

123110
public function testBug10819(): void
124111
{
125-
$errors = [];
126-
if (PHP_VERSION_ID < 80000) {
127-
$errors = [
128-
[
129-
'Call to static method DateTime::createFromFormat() on a separate line has no effect.',
130-
13,
131-
],
132-
];
133-
}
134-
$this->analyse([__DIR__ . '/data/bug-10819.php'], $errors);
112+
$this->analyse([__DIR__ . '/data/bug-10819.php'], [
113+
[
114+
'Call to static method DateTime::createFromFormat() on a separate line has no effect.',
115+
13,
116+
],
117+
]);
135118
}
136119

137120
public function testDynamicStaticCall(): void

0 commit comments

Comments
 (0)