Skip to content

Commit

Permalink
Avoid false positive useless catch (#141)
Browse files Browse the repository at this point in the history
* Add failing test

* Try

* Fix lint

* Fix phpstan
  • Loading branch information
VincentLanglet authored Jan 18, 2021
1 parent 5c9012d commit c590c05
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 12 deletions.
24 changes: 17 additions & 7 deletions src/Rules/ThrowsPhpDocRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,15 @@ private function processFunction(Node\FunctionLike $node, Scope $scope): array
throw new ShouldNotHappenException();
}

if ($classReflection->getNativeReflection()->getMethod($methodReflection->getName())->isAbstract()) {
try {
$nativeMethodReflection = $classReflection->getNativeReflection()->getMethod(
$methodReflection->getName()
);
} catch (ReflectionException $exception) {
throw new ShouldNotHappenException();
}

if ($nativeMethodReflection->isAbstract()) {
return [];
}
}
Expand Down Expand Up @@ -551,8 +559,14 @@ private function filterUnusedExceptions(array $declaredThrows, array $usedThrows

if (!$this->reportUnusedCheckedThrowsInSubtypes && $functionReflection instanceof MethodReflection) {
$declaringClass = $functionReflection->getDeclaringClass();
$nativeClassReflection = $declaringClass->getNativeReflection();
$nativeMethodReflection = $nativeClassReflection->getMethod($functionReflection->getName());

try {
$nativeMethodReflection = $declaringClass->getNativeReflection()->getMethod(
$functionReflection->getName()
);
} catch (ReflectionException $exception) {
throw new ShouldNotHappenException();
}

if ($this->isImplementation($nativeMethodReflection)) {
return [];
Expand Down Expand Up @@ -634,10 +648,6 @@ private function processCatch(Catch_ $node): array
continue;
}

if (!$this->reportUnusedCatchesOfUncheckedExceptions) {
$caughtExceptions = $this->checkedExceptionService->filterCheckedExceptions($caughtExceptions);
}

if (count($caughtExceptions) > 0) {
continue;
}
Expand Down
45 changes: 40 additions & 5 deletions tests/src/Rules/ThrowsPhpDocRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Pepakriz\PHPStanExceptionRules\Rules;

use LogicException;
use Pepakriz\PHPStanExceptionRules\CheckedExceptionService;
use Pepakriz\PHPStanExceptionRules\DefaultThrowTypeExtension;
use Pepakriz\PHPStanExceptionRules\DefaultThrowTypeService;
Expand Down Expand Up @@ -38,6 +39,20 @@ class ThrowsPhpDocRuleTest extends RuleTestCase
*/
private $reportCheckedThrowsInGlobalScope = false;

/**
* @var string[]
*/
private $checkedExceptions = [
RuntimeException::class,
CheckedException::class,
ReflectionException::class,
];

/**
* @var string[]
*/
private $uncheckedExceptions = [];

/**
* @var array<string, string>
*/
Expand Down Expand Up @@ -67,11 +82,8 @@ protected function getRule(): Rule

return new ThrowsPhpDocRule(
new CheckedExceptionService(
[
RuntimeException::class,
CheckedException::class,
ReflectionException::class,
]
$this->checkedExceptions,
$this->uncheckedExceptions
),
new DynamicThrowTypeService(
$extensions,
Expand Down Expand Up @@ -122,6 +134,29 @@ public function testUnusedCatches(): void
$this->analyseFile(__DIR__ . '/data/unused-catches.php');
}

public function testUnusedCatchesUncheckedExceptions(): void
{
$this->methodThrowTypes = [
Phar::class => [
'extractTo' => [
RuntimeException::class,
],
],
UnusedCatches::class => [
'methodWithDefaultThrowType' => [
FooException::class,
],
],
];

$this->checkedExceptions = [];
$this->uncheckedExceptions = [
LogicException::class,
];

$this->analyseFile(__DIR__ . '/data/unused-catches.php');
}

public function testAllUnusedCatches(): void
{
$this->reportUnusedCatchesOfUncheckedExceptions = true;
Expand Down
9 changes: 9 additions & 0 deletions tests/src/Rules/data/unused-catches.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,15 @@ public function correctCatchMethodCallWithThrows(): void
}
}

public function correctCatchMethodCallWithThrows2(): void
{
try {
$this->throwLogic();
} catch (\Throwable $e) {

}
}

private function someVoidMethod(): void
{
}
Expand Down

0 comments on commit c590c05

Please sign in to comment.