Skip to content

Commit 15c788b

Browse files
phpstan-botondrejmirtes
authored andcommitted
Treat declarations with a non-void @throws as not effect-free in the CallTo*StatementWithoutImpurePointsRule family
- `ConstructorWithoutImpurePointsCollector`, `MethodWithoutImpurePointsCollector` and `FunctionWithoutImpurePointsCollector` now bail out (return `null`) when the declaration has a non-null, non-`void` `getThrowType()`. - This fixes false positives where a constructor/function/method declares `@throws` but its only `throw` lives in a branch PHPStan proves dead, so the throw point is discarded and the declaration was wrongly considered pure. - The transitive chain was already covered: a live call to a `@throws` declaration produces an explicit throw point at the call site, which the resolver already treats as impure. - Added `testThrows` regression tests and data files for all four rules (constructor, function, method, static method), each pairing a `@throws` declaration (not reported) with a no-`@throws` sibling (still reported).
1 parent 7fe8b00 commit 15c788b

11 files changed

Lines changed: 195 additions & 0 deletions

src/Rules/DeadCode/ConstructorWithoutImpurePointsCollector.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ public function processNode(Node $node, Scope $scope)
4848
return null;
4949
}
5050

51+
$throwType = $method->getThrowType();
52+
if ($throwType !== null && !$throwType->isVoid()->yes()) {
53+
return null;
54+
}
55+
5156
$dependencies = $this->purityResolver->resolveDependencies(
5257
$node->getImpurePoints(),
5358
$node->getStatementResult()->getThrowPoints(),

src/Rules/DeadCode/FunctionWithoutImpurePointsCollector.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,11 @@ public function processNode(Node $node, Scope $scope)
4747
return null;
4848
}
4949

50+
$throwType = $function->getThrowType();
51+
if ($throwType !== null && !$throwType->isVoid()->yes()) {
52+
return null;
53+
}
54+
5055
$dependencies = $this->purityResolver->resolveDependencies(
5156
$node->getImpurePoints(),
5257
$node->getStatementResult()->getThrowPoints(),

src/Rules/DeadCode/MethodWithoutImpurePointsCollector.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,11 @@ public function processNode(Node $node, Scope $scope)
5151
return null;
5252
}
5353

54+
$throwType = $method->getThrowType();
55+
if ($throwType !== null && !$throwType->isVoid()->yes()) {
56+
return null;
57+
}
58+
5459
$dependencies = $this->purityResolver->resolveDependencies(
5560
$node->getImpurePoints(),
5661
$node->getStatementResult()->getThrowPoints(),

tests/PHPStan/Rules/DeadCode/CallToConstructorStatementWithoutImpurePointsRuleTest.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,16 @@ public function testTransitive(): void
3636
]);
3737
}
3838

39+
public function testThrows(): void
40+
{
41+
$this->analyse([__DIR__ . '/data/call-to-constructor-without-impure-points-throws.php'], [
42+
[
43+
'Call to new CallToConstructorWithoutImpurePointsThrows\NoThrows() on a separate line has no effect.',
44+
39,
45+
],
46+
]);
47+
}
48+
3949
protected function getCollectors(): array
4050
{
4151
$purityResolver = new PossiblyPureCallTransitivePurityResolver(self::createReflectionProvider());

tests/PHPStan/Rules/DeadCode/CallToFunctionStatementWithoutImpurePointsRuleTest.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,16 @@ public function testTransitive(): void
4545
]);
4646
}
4747

48+
public function testThrows(): void
49+
{
50+
$this->analyse([__DIR__ . '/data/call-to-function-without-impure-points-throws.php'], [
51+
[
52+
'Call to function CallToFunctionWithoutImpurePointsThrows\noThrowsFunc() on a separate line has no effect.',
53+
29,
54+
],
55+
]);
56+
}
57+
4858
#[RequiresPhp('>= 8.5.0')]
4959
public function testPipeOperator(): void
5060
{

tests/PHPStan/Rules/DeadCode/CallToMethodStatementWithoutImpurePointsRuleTest.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,16 @@ public function testBug11011(): void
100100
]);
101101
}
102102

103+
public function testThrows(): void
104+
{
105+
$this->analyse([__DIR__ . '/data/call-to-method-without-impure-points-throws.php'], [
106+
[
107+
'Call to method CallToMethodWithoutImpurePointsThrows\Foo::noThrowsMethod() on a separate line has no effect.',
108+
34,
109+
],
110+
]);
111+
}
112+
103113
#[RequiresPhp('>= 8.0.0')]
104114
public function testBug12379(): void
105115
{

tests/PHPStan/Rules/DeadCode/CallToStaticMethodStatementWithoutImpurePointsRuleTest.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,16 @@ public function testRule(): void
7171
]);
7272
}
7373

74+
public function testThrows(): void
75+
{
76+
$this->analyse([__DIR__ . '/data/call-to-static-method-without-impure-points-throws.php'], [
77+
[
78+
'Call to CallToStaticMethodWithoutImpurePointsThrows\Foo::noThrowsStatic() on a separate line has no effect.',
79+
34,
80+
],
81+
]);
82+
}
83+
7484
#[RequiresPhp('>= 8.5.0')]
7585
public function testPipeOperator(): void
7686
{
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<?php
2+
3+
namespace CallToConstructorWithoutImpurePointsThrows;
4+
5+
class InvalidFileInfo extends \Exception
6+
{
7+
8+
}
9+
10+
class FileIteratorSourceLocator
11+
{
12+
13+
/**
14+
* @param array<int> $ints
15+
* @throws InvalidFileInfo
16+
*/
17+
public function __construct(array $ints)
18+
{
19+
foreach ($ints as $int) {
20+
if (!is_int($int)) {
21+
throw new InvalidFileInfo();
22+
}
23+
}
24+
}
25+
26+
}
27+
28+
class NoThrows
29+
{
30+
31+
public function __construct()
32+
{
33+
}
34+
35+
}
36+
37+
function (): void {
38+
new FileIteratorSourceLocator([1, 2, 3]);
39+
new NoThrows();
40+
};
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php
2+
3+
namespace CallToFunctionWithoutImpurePointsThrows;
4+
5+
class InvalidValue extends \Exception
6+
{
7+
8+
}
9+
10+
/**
11+
* @param array<int> $ints
12+
* @throws InvalidValue
13+
*/
14+
function throwingFunc(array $ints)
15+
{
16+
foreach ($ints as $int) {
17+
if (!is_int($int)) {
18+
throw new InvalidValue();
19+
}
20+
}
21+
}
22+
23+
function noThrowsFunc()
24+
{
25+
}
26+
27+
function (): void {
28+
throwingFunc([1, 2, 3]);
29+
noThrowsFunc();
30+
};
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<?php
2+
3+
namespace CallToMethodWithoutImpurePointsThrows;
4+
5+
class InvalidValue extends \Exception
6+
{
7+
8+
}
9+
10+
final class Foo
11+
{
12+
13+
/**
14+
* @param array<int> $ints
15+
* @throws InvalidValue
16+
*/
17+
public function throwingMethod(array $ints)
18+
{
19+
foreach ($ints as $int) {
20+
if (!is_int($int)) {
21+
throw new InvalidValue();
22+
}
23+
}
24+
}
25+
26+
public function noThrowsMethod()
27+
{
28+
}
29+
30+
}
31+
32+
function (Foo $foo): void {
33+
$foo->throwingMethod([1, 2, 3]);
34+
$foo->noThrowsMethod();
35+
};

0 commit comments

Comments
 (0)