Skip to content

Commit dc67915

Browse files
ondrejmirtesondrejmirtes
committed
Some exceptions should always be rethrown
1 parent ed77cc1 commit dc67915

File tree

6 files changed

+311
-1
lines changed

6 files changed

+311
-1
lines changed

‎README.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,10 @@ This extension provides following features:
1919
* Event listeners through the `on*` properties
2020
* Defines early terminating method calls for Presenter methods to prevent `Undefined variable` errors
2121

22-
It also contains this framework-specific rule (can be enabled separately):
22+
It also contains these framework-specific rules (can be enabled separately):
2323

2424
* Do not extend Nette\Object, use Nette\SmartObject trait instead
25+
* Rethrow exceptions that are always meant to be rethrown (like `AbortException`)
2526

2627
## Usage
2728

‎phpstan.neon‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,7 @@ includes:
33
- vendor/phpstan/phpstan-phpunit/extension.neon
44
- vendor/phpstan/phpstan-phpunit/rules.neon
55
- vendor/phpstan/phpstan-strict-rules/rules.neon
6+
7+
parameters:
8+
excludes_analyse:
9+
- %rootDir%/../../../tests/*/data/*

‎rules.neon‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,23 @@
1+
parameters:
2+
methodsThrowingExceptions:
3+
Nette\Application\UI\Presenter:
4+
redirectUrl: Nette\Application\AbortException
5+
sendJson: Nette\Application\AbortException
6+
sendResponse: Nette\Application\AbortException
7+
terminate: Nette\Application\AbortException
8+
forward: Nette\Application\AbortException
9+
Nette\Application\UI\Component:
10+
redirect: Nette\Application\AbortException
11+
redirectPermanent: Nette\Application\AbortException
12+
error: Nette\Application\BadRequestException
13+
114
rules:
215
- PHPStan\Rule\Nette\DoNotExtendNetteObjectRule
16+
17+
services:
18+
-
19+
class: PHPStan\Rule\Nette\RethrowAbortExceptionRule
20+
arguments:
21+
methods: %methodsThrowingExceptions%
22+
tags:
23+
- phpstan.rules.rule

‎src/Rule/Nette/RethrowExceptionRule.php‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rule\Nette;
4+
5+
use PhpParser\Node;
6+
use PhpParser\Node\Expr\Variable;
7+
use PhpParser\Node\Stmt\TryCatch;
8+
use PHPStan\Analyser\Scope;
9+
use PHPStan\Type\ObjectType;
10+
use PHPStan\Type\TypeCombinator;
11+
12+
class RethrowExceptionRule implements \PHPStan\Rules\Rule
13+
{
14+
15+
/** @var array<string, string[]> */
16+
private $methods;
17+
18+
/**
19+
* @param string[][] $methods
20+
*/
21+
public function __construct(array $methods)
22+
{
23+
$this->methods = $methods;
24+
}
25+
26+
public function getNodeType(): string
27+
{
28+
return TryCatch::class;
29+
}
30+
31+
/**
32+
* @param \PhpParser\Node\Stmt\TryCatch $node
33+
* @param \PHPStan\Analyser\Scope $scope
34+
* @return string[] errors
35+
*/
36+
public function processNode(Node $node, Scope $scope): array
37+
{
38+
$hasGeneralCatch = false;
39+
foreach ($node->catches as $catch) {
40+
foreach ($catch->types as $type) {
41+
$typeClass = (string) $type;
42+
if ($typeClass === 'Exception' || $typeClass === \Throwable::class) {
43+
$hasGeneralCatch = true;
44+
break 2;
45+
}
46+
}
47+
}
48+
if (!$hasGeneralCatch) {
49+
return [];
50+
}
51+
52+
$exceptions = $this->getExceptionTypes($scope, $node->stmts);
53+
if (count($exceptions) === 0) {
54+
return [];
55+
}
56+
57+
$messages = [];
58+
foreach ($exceptions as $exceptionName) {
59+
$exceptionType = new ObjectType($exceptionName);
60+
foreach ($node->catches as $catch) {
61+
$caughtType = TypeCombinator::union(...array_map(function (string $class): ObjectType {
62+
return new ObjectType($class);
63+
}, $catch->types));
64+
if (!$caughtType->isSuperTypeOf($exceptionType)->yes()) {
65+
continue;
66+
}
67+
if (
68+
count($catch->stmts) === 1
69+
&& $catch->stmts[0] instanceof Node\Stmt\Throw_
70+
&& $catch->stmts[0]->expr instanceof Variable
71+
&& is_string($catch->var->name)
72+
&& is_string($catch->stmts[0]->expr->name)
73+
&& $catch->var->name === $catch->stmts[0]->expr->name
74+
) {
75+
continue 2;
76+
}
77+
}
78+
79+
$messages[] = sprintf('Exception %s needs to be rethrown.', $exceptionName);
80+
}
81+
82+
return $messages;
83+
}
84+
85+
/**
86+
* @param \PHPStan\Analyser\Scope $scope
87+
* @param \PhpParser\Node|\PhpParser\Node[] $node
88+
* @return string[]
89+
*/
90+
private function getExceptionTypes(Scope $scope, $node): array
91+
{
92+
$exceptions = [];
93+
if ($node instanceof Node) {
94+
foreach ($node->getSubNodeNames() as $subNodeName) {
95+
$subNode = $node->{$subNodeName};
96+
$exceptions = array_merge($exceptions, $this->getExceptionTypes($scope, $subNode));
97+
}
98+
if ($node instanceof Node\Expr\MethodCall) {
99+
$methodCalledOn = $scope->getType($node->var);
100+
foreach ($this->methods as $type => $methods) {
101+
if (!$node->name instanceof Node\Identifier) {
102+
continue;
103+
}
104+
if (!(new ObjectType($type))->isSuperTypeOf($methodCalledOn)->yes()) {
105+
continue;
106+
}
107+
108+
$methodName = strtolower((string) $node->name);
109+
foreach ($methods as $throwingMethodName => $exception) {
110+
if (strtolower($throwingMethodName) !== $methodName) {
111+
continue;
112+
}
113+
$exceptions[] = $exception;
114+
}
115+
}
116+
}
117+
} elseif (is_array($node)) {
118+
foreach ($node as $subNode) {
119+
$exceptions = array_merge($exceptions, $this->getExceptionTypes($scope, $subNode));
120+
}
121+
}
122+
123+
return array_unique($exceptions);
124+
}
125+
126+
}

‎tests/Rule/Nette/RethrowExceptionRuleTest.php‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rule\Nette;
4+
5+
use Nette\Application\AbortException;
6+
use Nette\Application\UI\Presenter;
7+
use PHPStan\Rules\Rule;
8+
use PHPStan\Testing\RuleTestCase;
9+
10+
class RethrowExceptionRuleTest extends RuleTestCase
11+
{
12+
13+
protected function getRule(): Rule
14+
{
15+
return new RethrowExceptionRule(
16+
[Presenter::class => ['redirect' => AbortException::class]]
17+
);
18+
}
19+
20+
public function testRule(): void
21+
{
22+
$this->analyse([__DIR__ . '/data/rethrow-abort.php'], [
23+
[
24+
'Exception Nette\Application\AbortException needs to be rethrown.',
25+
8,
26+
],
27+
[
28+
'Exception Nette\Application\AbortException needs to be rethrown.',
29+
14,
30+
],
31+
[
32+
'Exception Nette\Application\AbortException needs to be rethrown.',
33+
76,
34+
],
35+
[
36+
'Exception Nette\Application\AbortException needs to be rethrown.',
37+
95,
38+
],
39+
]);
40+
}
41+
42+
}

‎tests/Rule/Nette/data/rethrow-abort.php‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
<?php
2+
3+
class FooPresenter extends \Nette\Application\UI\Presenter
4+
{
5+
6+
public function doFoo()
7+
{
8+
try {
9+
$this->redirect('this');
10+
} catch (\Throwable $e) {
11+
12+
}
13+
14+
try {
15+
$this->redirect('this');
16+
} catch (\Exception $e) {
17+
18+
}
19+
}
20+
21+
public function doBar()
22+
{
23+
try {
24+
$this->redirect('this'); // OK
25+
} catch (\InvalidArgumentException $e) {
26+
27+
}
28+
}
29+
30+
public function doIpsum()
31+
{
32+
try {
33+
$this->redirect('this'); // OK
34+
} catch (\Nette\Application\AbortException $e) {
35+
throw $e;
36+
}
37+
}
38+
39+
public function doBaz()
40+
{
41+
try {
42+
$this->redirect('this'); // OK
43+
} catch (\Nette\Application\AbortException | \InvalidArgumentException $e) {
44+
throw $e;
45+
}
46+
}
47+
48+
public function doLorem()
49+
{
50+
try {
51+
$this->redirect('this'); // OK
52+
} catch (\Nette\Application\AbortException $e) {
53+
throw $e;
54+
} catch (\Throwable $e) {
55+
56+
}
57+
58+
try {
59+
$this->redirect('this'); // OK
60+
} catch (\Nette\Application\AbortException $e) {
61+
throw $e;
62+
} catch (\Exception $e) {
63+
64+
}
65+
66+
try {
67+
$this->redirect('this'); // OK
68+
} catch (\InvalidArgumentException $e) {
69+
70+
} catch (\Nette\Application\AbortException $e) {
71+
throw $e;
72+
} catch (\Exception $e) {
73+
74+
}
75+
76+
try {
77+
$this->redirect('this');
78+
} catch (\InvalidArgumentException $e) {
79+
80+
} catch (\Exception $e) {
81+
82+
}
83+
84+
$this->redirect('this'); // OK, outside of try
85+
}
86+
87+
public function doDolor()
88+
{
89+
try {
90+
$this->getAction();
91+
} catch (\Exception $e) {
92+
93+
}
94+
95+
try {
96+
$this->redirect('this');
97+
} catch (\Nette\Application\AbortException $e) {
98+
// does not rethrow
99+
} catch (\Throwable $e) {
100+
101+
}
102+
103+
try {
104+
$this->redirect('this');
105+
} catch (\Throwable $e) {
106+
throw $e;
107+
}
108+
109+
try {
110+
$this->redirect('this');
111+
} catch (\Exception $e) {
112+
throw $e;
113+
}
114+
}
115+
116+
}

0 commit comments

Comments
 (0)