Skip to content

Commit fc3c4d6

Browse files
peterfoxpeterfox
authored
Upgrade RouteActionCallableRector rule to handle groups (#284)
1 parent 478d0ed commit fc3c4d6

File tree

8 files changed

+208
-3
lines changed

8 files changed

+208
-3
lines changed

‎docs/rector_rules_overview.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1296,6 +1296,11 @@ Use PHP callable syntax instead of string syntax for controller route declaratio
12961296
```diff
12971297
-Route::get('/users', 'UserController@index');
12981298
+Route::get('/users', [\App\Http\Controllers\UserController::class, 'index']);
1299+
1300+
Route::group(['namespace' => 'Admin'], function () {
1301+
- Route::get('/users', 'UserController@index');
1302+
+ Route::get('/users', [\App\Http\Controllers\Admin\UserController::class, 'index']);
1303+
})
12991304
```
13001305

13011306
<br>

‎src/NodeFactory/RouterRegisterNodeAnalyzer.php‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,14 @@
44

55
namespace RectorLaravel\NodeFactory;
66

7+
use PhpParser\Node\Arg;
78
use PhpParser\Node\Expr;
9+
use PhpParser\Node\Expr\Array_;
10+
use PhpParser\Node\Expr\ArrayItem;
811
use PhpParser\Node\Expr\MethodCall;
912
use PhpParser\Node\Expr\StaticCall;
1013
use PhpParser\Node\Identifier;
14+
use PhpParser\Node\Scalar\String_;
1115
use PHPStan\Type\ObjectType;
1216
use Rector\NodeNameResolver\NodeNameResolver;
1317
use Rector\NodeTypeResolver\NodeTypeResolver;
@@ -74,4 +78,40 @@ public function isRegisterFallback(Identifier|Expr $name): bool
7478
{
7579
return $this->nodeNameResolver->isName($name, 'fallback');
7680
}
81+
82+
public function isGroup(Identifier|Expr $name): bool
83+
{
84+
return $this->nodeNameResolver->isName($name, 'group');
85+
}
86+
87+
public function getGroupNamespace(MethodCall|StaticCall $call): string|null|false
88+
{
89+
if (! isset($call->args[0]) || ! $call->args[0] instanceof Arg) {
90+
return null;
91+
}
92+
93+
$firstArg = $call->args[0]->value;
94+
if (! $firstArg instanceof Array_) {
95+
return null;
96+
}
97+
98+
foreach ($firstArg->items as $item) {
99+
if (! $item instanceof ArrayItem) {
100+
continue;
101+
}
102+
103+
if ($item->key instanceof String_ && $item->key->value === 'namespace') {
104+
105+
if ($item->value instanceof String_) {
106+
return $item->value->value;
107+
}
108+
109+
// if we can't find the namespace value we specify it exists but is
110+
// unreadable with false
111+
return false;
112+
}
113+
}
114+
115+
return null;
116+
}
77117
}

‎src/Rector/StaticCall/RouteActionCallableRector.php‎

Lines changed: 63 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
namespace RectorLaravel\Rector\StaticCall;
66

7+
use Illuminate\Support\Facades\Route;
78
use PhpParser\Node;
89
use PhpParser\Node\Arg;
910
use PhpParser\Node\ArrayItem;
@@ -42,6 +43,11 @@ final class RouteActionCallableRector extends AbstractRector implements Configur
4243
*/
4344
final public const NAMESPACE = 'namespace';
4445

46+
/**
47+
* @var string
48+
*/
49+
final public const NAMESPACE_ATTRIBUTE = 'laravel_route_group_namespace';
50+
4551
/**
4652
* @var string
4753
*/
@@ -66,11 +72,19 @@ public function getRuleDefinition(): RuleDefinition
6672
new ConfiguredCodeSample(
6773
<<<'CODE_SAMPLE'
6874
Route::get('/users', 'UserController@index');
75+
76+
Route::group(['namespace' => 'Admin'], function () {
77+
Route::get('/users', 'UserController@index');
78+
})
6979
CODE_SAMPLE
7080

7181
,
7282
<<<'CODE_SAMPLE'
7383
Route::get('/users', [\App\Http\Controllers\UserController::class, 'index']);
84+
85+
Route::group(['namespace' => 'Admin'], function () {
86+
Route::get('/users', [\App\Http\Controllers\Admin\UserController::class, 'index']);
87+
})
7488
CODE_SAMPLE
7589
,
7690
[
@@ -91,12 +105,55 @@ public function getNodeTypes(): array
91105
/**
92106
* @param MethodCall|StaticCall $node
93107
*/
94-
public function refactor(Node $node): ?Node
108+
public function refactor(Node $node): MethodCall|StaticCall|null
95109
{
110+
if ($this->routerRegisterNodeAnalyzer->isGroup($node->name)) {
111+
if (! isset($node->args[1]) || ! $node->args[1] instanceof Arg) {
112+
return null;
113+
}
114+
115+
$namespace = $this->routerRegisterNodeAnalyzer->getGroupNamespace($node);
116+
117+
$groupNamespace = $node->getAttribute(self::NAMESPACE_ATTRIBUTE);
118+
119+
// if the route is in a namespace but can't be resolved to a value, don't continue
120+
if (! is_string($groupNamespace) && ! is_null($groupNamespace)) {
121+
return null;
122+
}
123+
124+
if (is_string($groupNamespace)) {
125+
$namespace = $groupNamespace . '\\' . $namespace;
126+
}
127+
128+
$this->traverseNodesWithCallable($node->args[1]->value, function (Node $node) use ($namespace): Node|int|null {
129+
if (! $node instanceof MethodCall && ! $node instanceof StaticCall) {
130+
return null;
131+
}
132+
133+
if (
134+
$this->routerRegisterNodeAnalyzer->isRegisterMethodStaticCall($node) ||
135+
$this->routerRegisterNodeAnalyzer->isGroup($node->name)
136+
) {
137+
$node->setAttribute(self::NAMESPACE_ATTRIBUTE, $namespace);
138+
}
139+
140+
return null;
141+
});
142+
143+
return null;
144+
}
145+
96146
if (! $this->routerRegisterNodeAnalyzer->isRegisterMethodStaticCall($node)) {
97147
return null;
98148
}
99149

150+
$groupNamespace = $node->getAttribute(self::NAMESPACE_ATTRIBUTE);
151+
152+
// if the route is in a namespace but can't be resolved to a value, don't continue
153+
if (! is_string($groupNamespace) && ! is_null($groupNamespace)) {
154+
return null;
155+
}
156+
100157
$position = $this->getActionPosition($node->name);
101158

102159
if (! isset($node->args[$position])) {
@@ -110,7 +167,7 @@ public function refactor(Node $node): ?Node
110167
$arg = $node->args[$position];
111168

112169
$argValue = $this->valueResolver->getValue($arg->value);
113-
$segments = $this->resolveControllerFromAction($argValue);
170+
$segments = $this->resolveControllerFromAction($argValue, $groupNamespace);
114171
if ($segments === null) {
115172
return null;
116173
}
@@ -182,7 +239,7 @@ public function configure(array $configuration): void
182239
/**
183240
* @return array{string, string}|null
184241
*/
185-
private function resolveControllerFromAction(mixed $action): ?array
242+
private function resolveControllerFromAction(mixed $action, ?string $groupNamespace = null): ?array
186243
{
187244
if (! $this->isActionString($action)) {
188245
return null;
@@ -199,6 +256,9 @@ private function resolveControllerFromAction(mixed $action): ?array
199256

200257
[$controller, $method] = $segments;
201258
$namespace = $this->getNamespace($this->file->getFilePath());
259+
if ($groupNamespace !== null) {
260+
$namespace .= '\\' . $groupNamespace;
261+
}
202262
if (! str_starts_with($controller, '\\')) {
203263
$controller = $namespace . '\\' . $controller;
204264
}

‎tests/Rector/StaticCall/RouteActionCallableRector/Fixture/fixture.php.inc‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ Route::fallback('SomeController@index');
1616
Route::options('/users', 'SomeController@index');
1717
Route::middleware([])->options('/users', 'SomeController@index');
1818

19+
Route::group(['namespace' => 'SomeNamespace'], function () {
20+
Route::get('/users', 'SomeController@index');
21+
})
22+
1923
?>
2024
-----
2125
<?php
@@ -36,4 +40,8 @@ Route::fallback([\RectorLaravel\Tests\Rector\StaticCall\RouteActionCallableRecto
3640
Route::options('/users', [\RectorLaravel\Tests\Rector\StaticCall\RouteActionCallableRector\Source\SomeController::class, 'index']);
3741
Route::middleware([])->options('/users', [\RectorLaravel\Tests\Rector\StaticCall\RouteActionCallableRector\Source\SomeController::class, 'index']);
3842

43+
Route::group(['namespace' => 'SomeNamespace'], function () {
44+
Route::get('/users', [\RectorLaravel\Tests\Rector\StaticCall\RouteActionCallableRector\Source\SomeNamespace\SomeController::class, 'index']);
45+
})
46+
3947
?>

‎tests/Rector/StaticCall/RouteActionCallableRector/Fixture/nested_groups.php.inc‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php
2+
3+
namespace RectorLaravel\Tests\Rector\StaticCall\RouteActionCallableRector\Fixture;
4+
5+
use Illuminate\Support\Facades\Route;
6+
7+
Route::group(['namespace' => 'SomeNamespace'], function () {
8+
Route::group(['namespace' => 'SomeOtherNamespace'], function () {
9+
Route::get('/users', 'SomeController@index');
10+
});
11+
12+
Route::get('/users', 'SomeController@index');
13+
});
14+
15+
?>
16+
-----
17+
<?php
18+
19+
namespace RectorLaravel\Tests\Rector\StaticCall\RouteActionCallableRector\Fixture;
20+
21+
use Illuminate\Support\Facades\Route;
22+
23+
Route::group(['namespace' => 'SomeNamespace'], function () {
24+
Route::group(['namespace' => 'SomeOtherNamespace'], function () {
25+
Route::get('/users', [\RectorLaravel\Tests\Rector\StaticCall\RouteActionCallableRector\Source\SomeNamespace\SomeOtherNamespace\SomeController::class, 'index']);
26+
});
27+
28+
Route::get('/users', [\RectorLaravel\Tests\Rector\StaticCall\RouteActionCallableRector\Source\SomeNamespace\SomeController::class, 'index']);
29+
});
30+
31+
?>

‎tests/Rector/StaticCall/RouteActionCallableRector/Fixture/skip_invalid_group_namespace.php.inc‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
<?php
2+
3+
namespace RectorLaravel\Tests\Rector\StaticCall\RouteActionCallableRector\Fixture;
4+
5+
use Illuminate\Support\Facades\Route;
6+
7+
// the concat should make the rule ignore the group and any routes within it
8+
Route::group(['namespace' => 'Some' . 'Namespace'], function () {
9+
Route::get('/users', 'SomeController@index');
10+
});
11+
12+
Route::group(['namespace' => 'SomeNamespace'], function () {
13+
Route::get('/users', 'SomeController@index');
14+
15+
Route::group(['namespace' => 'SomeOther' . 'Namespace'], function () {
16+
Route::get('/users', 'SomeController@index');
17+
});
18+
});
19+
20+
Route::get('/users', 'SomeController@index');
21+
22+
?>
23+
-----
24+
<?php
25+
26+
namespace RectorLaravel\Tests\Rector\StaticCall\RouteActionCallableRector\Fixture;
27+
28+
use Illuminate\Support\Facades\Route;
29+
30+
// the concat should make the rule ignore the group and any routes within it
31+
Route::group(['namespace' => 'Some' . 'Namespace'], function () {
32+
Route::get('/users', 'SomeController@index');
33+
});
34+
35+
Route::group(['namespace' => 'SomeNamespace'], function () {
36+
Route::get('/users', [\RectorLaravel\Tests\Rector\StaticCall\RouteActionCallableRector\Source\SomeNamespace\SomeController::class, 'index']);
37+
38+
Route::group(['namespace' => 'SomeOther' . 'Namespace'], function () {
39+
Route::get('/users', 'SomeController@index');
40+
});
41+
});
42+
43+
Route::get('/users', [\RectorLaravel\Tests\Rector\StaticCall\RouteActionCallableRector\Source\SomeController::class, 'index']);
44+
45+
?>

‎tests/Rector/StaticCall/RouteActionCallableRector/Source/SomeNamespace/SomeController.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
3+
namespace RectorLaravel\Tests\Rector\StaticCall\RouteActionCallableRector\Source\SomeNamespace;
4+
5+
class SomeController
6+
{
7+
public function index() {}
8+
}

‎tests/Rector/StaticCall/RouteActionCallableRector/Source/SomeNamespace/SomeOtherNamespace/SomeController.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
3+
namespace RectorLaravel\Tests\Rector\StaticCall\RouteActionCallableRector\Source\SomeNamespace\SomeOtherNamespace;
4+
5+
class SomeController
6+
{
7+
public function index() {}
8+
}

0 commit comments

Comments
 (0)