Skip to content

Commit 68db7e9

Browse files
peterfoxpeterfox
authored
Refactor If statements to use helpers with conditionals and provides a set (#194)
* AbortIfRector rule * Adds the report if rule * Updates the docs * If helper set * Add config to rector paths * CS Fixes
1 parent 35a90f3 commit 68db7e9

15 files changed

+525
-1
lines changed

Diff for: ‎config/sets/laravel-if-helpers.php

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
use Rector\Config\RectorConfig;
5+
use RectorLaravel\Rector\If_\AbortIfRector;
6+
use RectorLaravel\Rector\If_\ReportIfRector;
7+
use RectorLaravel\Rector\If_\ThrowIfRector;
8+
9+
return static function (RectorConfig $rectorConfig): void {
10+
$rectorConfig->import(__DIR__ . '/../config.php');
11+
12+
$rectorConfig->rule(AbortIfRector::class);
13+
$rectorConfig->rule(ReportIfRector::class);
14+
$rectorConfig->rule(ThrowIfRector::class);
15+
};

Diff for: ‎docs/rector_rules_overview.md

+39-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,23 @@
1-
# 58 Rules Overview
1+
# 60 Rules Overview
2+
3+
## AbortIfRector
4+
5+
Change if abort to abort_if
6+
7+
- class: [`RectorLaravel\Rector\If_\AbortIfRector`](../src/Rector/If_/AbortIfRector.php)
8+
9+
```diff
10+
-if ($condition) {
11+
- abort(404);
12+
-}
13+
-if (!$condition) {
14+
- abort(404);
15+
-}
16+
+abort_if($condition, 404);
17+
+abort_unless($condition, 404);
18+
```
19+
20+
<br>
221

322
## AddArgumentDefaultValueRector
423

@@ -1069,6 +1088,25 @@ Replace `withoutJobs`, `withoutEvents` and `withoutNotifications` with Facade `f
10691088

10701089
<br>
10711090

1091+
## ReportIfRector
1092+
1093+
Change if report to report_if
1094+
1095+
- class: [`RectorLaravel\Rector\If_\ReportIfRector`](../src/Rector/If_/ReportIfRector.php)
1096+
1097+
```diff
1098+
-if ($condition) {
1099+
- report(new Exception());
1100+
-}
1101+
-if (!$condition) {
1102+
- report(new Exception());
1103+
-}
1104+
+report_if($condition, new Exception());
1105+
+report_unless($condition, new Exception());
1106+
```
1107+
1108+
<br>
1109+
10721110
## RequestStaticValidateToInjectRector
10731111

10741112
Change static `validate()` method to `$request->validate()`

Diff for: ‎rector.php

+1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
->withPaths([
1111
__DIR__ . '/src',
1212
__DIR__ . '/tests',
13+
__DIR__ . '/config',
1314
])
1415
->withSkip([
1516
// for tests

Diff for: ‎src/Rector/If_/AbortIfRector.php

+123
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
<?php
2+
3+
namespace RectorLaravel\Rector\If_;
4+
5+
use PhpParser\Node;
6+
use PhpParser\Node\Arg;
7+
use PhpParser\Node\Expr;
8+
use PhpParser\Node\Expr\Assign;
9+
use PhpParser\Node\Expr\BooleanNot;
10+
use PhpParser\Node\Expr\FuncCall;
11+
use PhpParser\Node\Expr\Variable;
12+
use PhpParser\Node\Name;
13+
use PhpParser\Node\Stmt\Expression;
14+
use PhpParser\Node\Stmt\If_;
15+
use PhpParser\NodeTraverser;
16+
use Rector\Rector\AbstractRector;
17+
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
18+
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
19+
20+
/**
21+
* @see \RectorLaravel\Tests\Rector\If_\AbortIfRector\AbortIfRectorTest
22+
*/
23+
class AbortIfRector extends AbstractRector
24+
{
25+
public function getRuleDefinition(): RuleDefinition
26+
{
27+
return new RuleDefinition('Change if abort to abort_if', [
28+
new CodeSample(
29+
<<<'CODE_SAMPLE'
30+
if ($condition) {
31+
abort(404);
32+
}
33+
if (!$condition) {
34+
abort(404);
35+
}
36+
CODE_SAMPLE
37+
,
38+
<<<'CODE_SAMPLE'
39+
abort_if($condition, 404);
40+
abort_unless($condition, 404);
41+
CODE_SAMPLE
42+
),
43+
]);
44+
}
45+
46+
public function getNodeTypes(): array
47+
{
48+
return [If_::class];
49+
}
50+
51+
public function refactor(Node $node): ?Node
52+
{
53+
if (! $node instanceof If_) {
54+
return null;
55+
}
56+
57+
$ifStmts = $node->stmts;
58+
59+
// Check if there's a single statement inside the if block to call abort()
60+
if (
61+
count($ifStmts) === 1 &&
62+
$ifStmts[0] instanceof Expression &&
63+
$ifStmts[0]->expr instanceof FuncCall &&
64+
$ifStmts[0]->expr->name instanceof Name &&
65+
$this->isName($ifStmts[0]->expr, 'abort')
66+
) {
67+
$condition = $node->cond;
68+
/** @var FuncCall $abortCall */
69+
$abortCall = $ifStmts[0]->expr;
70+
71+
if ($this->exceptionUsesVariablesAssignedByCondition($abortCall, $condition)) {
72+
return null;
73+
}
74+
75+
// Check if the condition is a negation
76+
if ($condition instanceof BooleanNot) {
77+
// Create a new throw_unless function call
78+
return new Expression(new FuncCall(new Name('abort_unless'), [
79+
new Arg($condition->expr),
80+
...$abortCall->args,
81+
]));
82+
} else {
83+
// Create a new throw_if function call
84+
return new Expression(new FuncCall(new Name('abort_if'), [
85+
new Arg($condition),
86+
...$abortCall->args,
87+
]));
88+
}
89+
}
90+
91+
return null;
92+
}
93+
94+
/**
95+
* Make sure the exception doesn't use variables assigned by the condition or this
96+
* will cause broken code to be generated
97+
*/
98+
private function exceptionUsesVariablesAssignedByCondition(Expr $throwExpr, Expr $condition): bool
99+
{
100+
$conditionVariables = [];
101+
$returnValue = false;
102+
103+
$this->traverseNodesWithCallable($condition, function (Node $node) use (&$conditionVariables): null {
104+
if ($node instanceof Assign) {
105+
$conditionVariables[] = $this->getName($node->var);
106+
}
107+
108+
return null;
109+
});
110+
111+
$this->traverseNodesWithCallable($throwExpr, function (Node $node) use ($conditionVariables, &$returnValue): ?int {
112+
if ($node instanceof Variable && in_array($this->getName($node), $conditionVariables, true)) {
113+
$returnValue = true;
114+
115+
return NodeTraverser::STOP_TRAVERSAL;
116+
}
117+
118+
return null;
119+
});
120+
121+
return $returnValue;
122+
}
123+
}

Diff for: ‎src/Rector/If_/ReportIfRector.php

+123
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
<?php
2+
3+
namespace RectorLaravel\Rector\If_;
4+
5+
use PhpParser\Node;
6+
use PhpParser\Node\Arg;
7+
use PhpParser\Node\Expr;
8+
use PhpParser\Node\Expr\Assign;
9+
use PhpParser\Node\Expr\BooleanNot;
10+
use PhpParser\Node\Expr\FuncCall;
11+
use PhpParser\Node\Expr\Variable;
12+
use PhpParser\Node\Name;
13+
use PhpParser\Node\Stmt\Expression;
14+
use PhpParser\Node\Stmt\If_;
15+
use PhpParser\NodeTraverser;
16+
use Rector\Rector\AbstractRector;
17+
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
18+
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
19+
20+
/**
21+
* @see \RectorLaravel\Tests\Rector\If_\ReportIfRector\ReportIfRectorTest
22+
*/
23+
class ReportIfRector extends AbstractRector
24+
{
25+
public function getRuleDefinition(): RuleDefinition
26+
{
27+
return new RuleDefinition('Change if report to report_if', [
28+
new CodeSample(
29+
<<<'CODE_SAMPLE'
30+
if ($condition) {
31+
report(new Exception());
32+
}
33+
if (!$condition) {
34+
report(new Exception());
35+
}
36+
CODE_SAMPLE
37+
,
38+
<<<'CODE_SAMPLE'
39+
report_if($condition, new Exception());
40+
report_unless($condition, new Exception());
41+
CODE_SAMPLE
42+
),
43+
]);
44+
}
45+
46+
public function getNodeTypes(): array
47+
{
48+
return [If_::class];
49+
}
50+
51+
public function refactor(Node $node): ?Node
52+
{
53+
if (! $node instanceof If_) {
54+
return null;
55+
}
56+
57+
$ifStmts = $node->stmts;
58+
59+
// Check if there's a single statement inside the if block to call abort()
60+
if (
61+
count($ifStmts) === 1 &&
62+
$ifStmts[0] instanceof Expression &&
63+
$ifStmts[0]->expr instanceof FuncCall &&
64+
$ifStmts[0]->expr->name instanceof Name &&
65+
$this->isName($ifStmts[0]->expr, 'report')
66+
) {
67+
$condition = $node->cond;
68+
/** @var FuncCall $abortCall */
69+
$abortCall = $ifStmts[0]->expr;
70+
71+
if ($this->exceptionUsesVariablesAssignedByCondition($abortCall, $condition)) {
72+
return null;
73+
}
74+
75+
// Check if the condition is a negation
76+
if ($condition instanceof BooleanNot) {
77+
// Create a new throw_unless function call
78+
return new Expression(new FuncCall(new Name('report_unless'), [
79+
new Arg($condition->expr),
80+
...$abortCall->args,
81+
]));
82+
} else {
83+
// Create a new throw_if function call
84+
return new Expression(new FuncCall(new Name('report_if'), [
85+
new Arg($condition),
86+
...$abortCall->args,
87+
]));
88+
}
89+
}
90+
91+
return null;
92+
}
93+
94+
/**
95+
* Make sure the exception doesn't use variables assigned by the condition or this
96+
* will cause broken code to be generated
97+
*/
98+
private function exceptionUsesVariablesAssignedByCondition(Expr $throwExpr, Expr $condition): bool
99+
{
100+
$conditionVariables = [];
101+
$returnValue = false;
102+
103+
$this->traverseNodesWithCallable($condition, function (Node $node) use (&$conditionVariables): null {
104+
if ($node instanceof Assign) {
105+
$conditionVariables[] = $this->getName($node->var);
106+
}
107+
108+
return null;
109+
});
110+
111+
$this->traverseNodesWithCallable($throwExpr, function (Node $node) use ($conditionVariables, &$returnValue): ?int {
112+
if ($node instanceof Variable && in_array($this->getName($node), $conditionVariables, true)) {
113+
$returnValue = true;
114+
115+
return NodeTraverser::STOP_TRAVERSAL;
116+
}
117+
118+
return null;
119+
});
120+
121+
return $returnValue;
122+
}
123+
}

Diff for: ‎tests/Rector/If_/AbortIfRector/AbortIfRectorTest.php

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace RectorLaravel\Tests\Rector\If_\AbortIfRector;
6+
7+
use Iterator;
8+
use PHPUnit\Framework\Attributes\DataProvider;
9+
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
10+
11+
final class AbortIfRectorTest extends AbstractRectorTestCase
12+
{
13+
public static function provideData(): Iterator
14+
{
15+
return self::yieldFilesFromDirectory(__DIR__ . '/Fixture');
16+
}
17+
18+
/**
19+
* @test
20+
*/
21+
#[DataProvider('provideData')]
22+
public function test(string $filePath): void
23+
{
24+
$this->doTestFile($filePath);
25+
}
26+
27+
public function provideConfigFilePath(): string
28+
{
29+
return __DIR__ . '/config/configured_rule.php';
30+
}
31+
}

Diff for: ‎tests/Rector/If_/AbortIfRector/Fixture/fixture.php.inc

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<?php
2+
3+
namespace RectorLaravel\Tests\Rector\If_\AbortIfRector\Fixture;
4+
5+
class Fixture
6+
{
7+
public function handle($condition)
8+
{
9+
if ($condition) {
10+
abort(404);
11+
}
12+
if (!$condition) {
13+
abort(404);
14+
}
15+
if ($condition = call()) {
16+
abort(404);
17+
}
18+
if ($condition) {
19+
abort(404);
20+
}
21+
}
22+
}
23+
24+
?>
25+
-----
26+
<?php
27+
28+
namespace RectorLaravel\Tests\Rector\If_\AbortIfRector\Fixture;
29+
30+
class Fixture
31+
{
32+
public function handle($condition)
33+
{
34+
abort_if($condition, 404);
35+
abort_unless($condition, 404);
36+
abort_if($condition = call(), 404);
37+
abort_if($condition, 404);
38+
}
39+
}
40+
41+
?>

0 commit comments

Comments
 (0)