Skip to content

Commit c590c05

Browse files
Avoid false positive useless catch (#141)
* Add failing test * Try * Fix lint * Fix phpstan
1 parent 5c9012d commit c590c05

File tree

3 files changed

+66
-12
lines changed

3 files changed

+66
-12
lines changed

src/Rules/ThrowsPhpDocRule.php

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,15 @@ private function processFunction(Node\FunctionLike $node, Scope $scope): array
485485
throw new ShouldNotHappenException();
486486
}
487487

488-
if ($classReflection->getNativeReflection()->getMethod($methodReflection->getName())->isAbstract()) {
488+
try {
489+
$nativeMethodReflection = $classReflection->getNativeReflection()->getMethod(
490+
$methodReflection->getName()
491+
);
492+
} catch (ReflectionException $exception) {
493+
throw new ShouldNotHappenException();
494+
}
495+
496+
if ($nativeMethodReflection->isAbstract()) {
489497
return [];
490498
}
491499
}
@@ -551,8 +559,14 @@ private function filterUnusedExceptions(array $declaredThrows, array $usedThrows
551559

552560
if (!$this->reportUnusedCheckedThrowsInSubtypes && $functionReflection instanceof MethodReflection) {
553561
$declaringClass = $functionReflection->getDeclaringClass();
554-
$nativeClassReflection = $declaringClass->getNativeReflection();
555-
$nativeMethodReflection = $nativeClassReflection->getMethod($functionReflection->getName());
562+
563+
try {
564+
$nativeMethodReflection = $declaringClass->getNativeReflection()->getMethod(
565+
$functionReflection->getName()
566+
);
567+
} catch (ReflectionException $exception) {
568+
throw new ShouldNotHappenException();
569+
}
556570

557571
if ($this->isImplementation($nativeMethodReflection)) {
558572
return [];
@@ -634,10 +648,6 @@ private function processCatch(Catch_ $node): array
634648
continue;
635649
}
636650

637-
if (!$this->reportUnusedCatchesOfUncheckedExceptions) {
638-
$caughtExceptions = $this->checkedExceptionService->filterCheckedExceptions($caughtExceptions);
639-
}
640-
641651
if (count($caughtExceptions) > 0) {
642652
continue;
643653
}

tests/src/Rules/ThrowsPhpDocRuleTest.php

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace Pepakriz\PHPStanExceptionRules\Rules;
44

5+
use LogicException;
56
use Pepakriz\PHPStanExceptionRules\CheckedExceptionService;
67
use Pepakriz\PHPStanExceptionRules\DefaultThrowTypeExtension;
78
use Pepakriz\PHPStanExceptionRules\DefaultThrowTypeService;
@@ -38,6 +39,20 @@ class ThrowsPhpDocRuleTest extends RuleTestCase
3839
*/
3940
private $reportCheckedThrowsInGlobalScope = false;
4041

42+
/**
43+
* @var string[]
44+
*/
45+
private $checkedExceptions = [
46+
RuntimeException::class,
47+
CheckedException::class,
48+
ReflectionException::class,
49+
];
50+
51+
/**
52+
* @var string[]
53+
*/
54+
private $uncheckedExceptions = [];
55+
4156
/**
4257
* @var array<string, string>
4358
*/
@@ -67,11 +82,8 @@ protected function getRule(): Rule
6782

6883
return new ThrowsPhpDocRule(
6984
new CheckedExceptionService(
70-
[
71-
RuntimeException::class,
72-
CheckedException::class,
73-
ReflectionException::class,
74-
]
85+
$this->checkedExceptions,
86+
$this->uncheckedExceptions
7587
),
7688
new DynamicThrowTypeService(
7789
$extensions,
@@ -122,6 +134,29 @@ public function testUnusedCatches(): void
122134
$this->analyseFile(__DIR__ . '/data/unused-catches.php');
123135
}
124136

137+
public function testUnusedCatchesUncheckedExceptions(): void
138+
{
139+
$this->methodThrowTypes = [
140+
Phar::class => [
141+
'extractTo' => [
142+
RuntimeException::class,
143+
],
144+
],
145+
UnusedCatches::class => [
146+
'methodWithDefaultThrowType' => [
147+
FooException::class,
148+
],
149+
],
150+
];
151+
152+
$this->checkedExceptions = [];
153+
$this->uncheckedExceptions = [
154+
LogicException::class,
155+
];
156+
157+
$this->analyseFile(__DIR__ . '/data/unused-catches.php');
158+
}
159+
125160
public function testAllUnusedCatches(): void
126161
{
127162
$this->reportUnusedCatchesOfUncheckedExceptions = true;

tests/src/Rules/data/unused-catches.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,15 @@ public function correctCatchMethodCallWithThrows(): void
9494
}
9595
}
9696

97+
public function correctCatchMethodCallWithThrows2(): void
98+
{
99+
try {
100+
$this->throwLogic();
101+
} catch (\Throwable $e) {
102+
103+
}
104+
}
105+
97106
private function someVoidMethod(): void
98107
{
99108
}

0 commit comments

Comments
 (0)