Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions config/rector/sets/cakephp53.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use Cake\Upgrade\Rector\Rector\MethodCall\EntityPatchRector;
use Cake\Upgrade\Rector\Rector\MethodCall\NewExprToFuncRector;
use Cake\Upgrade\Rector\Rector\MethodCall\QueryParamAccessRector;
use Cake\Upgrade\Rector\Rector\MethodCall\RouteBuilderCleanupRector;
use Rector\Config\RectorConfig;
use Rector\Renaming\Rector\MethodCall\RenameMethodRector;
use Rector\Renaming\Rector\Name\RenameClassRector;
Expand All @@ -15,6 +16,10 @@
return static function (RectorConfig $rectorConfig): void {
// Apply newExpr()->count() -> func()->count('*') transformation before general newExpr rename
$rectorConfig->rule(NewExprToFuncRector::class);
$rectorConfig->rule(EntityIsEmptyRector::class);
$rectorConfig->rule(EntityPatchRector::class);
$rectorConfig->rule(FormExecuteToProcessRector::class);
$rectorConfig->rule(QueryParamAccessRector::class);

$rectorConfig->ruleWithConfiguration(RenameMethodRector::class, [
new MethodCallRename('Cake\Database\Query', 'newExpr', 'expr'),
Expand All @@ -24,8 +29,12 @@
'Cake\TestSuite\Fixture\TransactionFixtureStrategy' => 'Cake\TestSuite\Fixture\TransactionStrategy',
'Cake\TestSuite\Fixture\TruncateFixtureStrategy' => 'Cake\TestSuite\Fixture\TruncateStrategy',
]);
$rectorConfig->rule(EntityIsEmptyRector::class);
$rectorConfig->rule(EntityPatchRector::class);
$rectorConfig->rule(FormExecuteToProcessRector::class);
$rectorConfig->rule(QueryParamAccessRector::class);
$rectorConfig->ruleWithConfiguration(RouteBuilderCleanupRector::class, [
'methods' => [
'scope' => ['path', 'params', 'callback'],
'resources' => ['name', 'options', 'callback'],
'prefix' => ['name', 'params', 'callback'],
'plugin' => ['name', 'options', 'callback'],
],
]);
};
147 changes: 147 additions & 0 deletions src/Rector/Rector/MethodCall/RouteBuilderCleanupRector.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
<?php
declare(strict_types=1);

namespace Cake\Upgrade\Rector\Rector\MethodCall;

use Cake\Routing\RouteBuilder;
use PhpParser\Node;
use PhpParser\Node\Arg;
use PhpParser\Node\Expr\ArrowFunction;
use PhpParser\Node\Expr\Closure;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Identifier;
use PHPStan\Type\ObjectType;
use Rector\Contract\Rector\ConfigurableRectorInterface;
use Rector\Rector\AbstractRector;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

final class RouteBuilderCleanupRector extends AbstractRector implements ConfigurableRectorInterface
{
/**
* @var array<string, string[]>
* e.g. ['scope' => ['path', 'options', 'callback']]
*/
private array $methods = [];

public function configure(array $configuration): void
{
$this->methods = $configuration['methods'] ?? [];
}

public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition('Normalize RouteBuilder calls to always use named arguments based on configuration', [
new CodeSample(
<<<'CODE_SAMPLE'
$routes->scope('/api', function (RouteBuilder $routes): void {});
CODE_SAMPLE,
<<<'CODE_SAMPLE'
$routes->scope(path: '/api', options: [], callback: function (RouteBuilder $routes): void {});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the reaming of $params to $options, we use the term "route params".

We need to first update the method and make $params argument default to empty array and make $callable not nullable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll leave those changes to the the core to you @ADmad

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems only resources() should be $options.

CODE_SAMPLE,
),
]);
}

public function getNodeTypes(): array
{
return [MethodCall::class];
}

public function refactor(Node $node): ?Node
{
if (! $node instanceof MethodCall) {
return null;
}

$methodName = $this->getName($node->name);
if (! isset($this->methods[$methodName])) {
return null;
}

// Must be called on a Cake\Routing\RouteBuilder
$callerType = $this->getType($node->var);
if (! (new ObjectType(RouteBuilder::class))->isSuperTypeOf($callerType)->yes()) {
return null;
}

$argNames = $this->methods[$methodName];
$args = $node->args;

$pathValue = $args[0]->value ?? null;
$optionsValue = null;
$callbackValue = null;

// Handle case where 2nd param is callable or array
if (isset($args[1])) {
if ($this->isCallableNode($args[1]->value)) {
// Case: scope('/api', fn() => null)
$callbackValue = $args[1]->value;
} else {
// Case: scope('/api', [], fn() => null)
$optionsValue = $args[1]->value;
$callbackValue = $args[2]->value ?? null;
}
}

if ($callbackValue === null) {
// no callable = no change
return null;
}

$newArgs = [];

// always add first argument (path/name) without a named argument
if (isset($argNames[0]) && $pathValue !== null) {
$newArgs[] = new Arg(
$pathValue,
false,
false,
[],
);
}

// only add options if it existed in original call
if (isset($argNames[1]) && $optionsValue !== null) {
$newArgs[] = new Arg(
$optionsValue,
false,
false,
[],
new Identifier($argNames[1]),
);
} elseif ($methodName === 'scope') {
// scope() must always have options
$newArgs[] = new Arg(
$optionsValue ?? $this->nodeFactory->createArray([]),
false,
false,
[],
new Identifier($argNames[1]),
);
}

// always add callback if present
if (isset($argNames[2])) {
$newArgs[] = new Arg(
$callbackValue,
false,
false,
[],
new Identifier($argNames[2]),
);
}

$node->args = $newArgs;

return $node;
}

private function isCallableNode(Node $node): bool
{
return $node instanceof Closure
|| $node instanceof ArrowFunction
|| $node instanceof FuncCall;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
use Cake\ORM\Entity;
use Cake\ORM\Locator\LocatorAwareTrait;
use Cake\ORM\Query;
use Cake\Routing\RouteBuilder;
use Cake\Routing\RouteCollection;

class SomeTest
{
Expand All @@ -27,4 +29,21 @@ public function testRenames(): void
public function findSomething(Query $query, array $options): Query {
return $query;
}

public function routes(): void
{
$routes = new RouteBuilder(new RouteCollection(), '/');

$routes->scope('/api', function ($routes): void {});
$routes->scope('/api', ['something'], function ($routes): void {});

$routes->resources('/api', function ($routes): void {});
$routes->resources('/api', ['something'], function ($routes): void {});

$routes->prefix('/api', function ($routes): void {});
$routes->prefix('/api', ['something'], function ($routes): void {});

$routes->plugin('/api', function ($routes): void {});
$routes->plugin('/api', ['something'], function ($routes): void {});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
use Cake\ORM\Entity;
use Cake\ORM\Locator\LocatorAwareTrait;
use Cake\ORM\Query;
use Cake\Routing\RouteBuilder;
use Cake\Routing\RouteCollection;

class SomeTest
{
Expand All @@ -27,4 +29,21 @@ public function testRenames(): void
public function findSomething(\Cake\ORM\Query\SelectQuery $query, array $options): \Cake\ORM\Query\SelectQuery {
return $query;
}

public function routes(): void
{
$routes = new RouteBuilder(new RouteCollection(), '/');

$routes->scope('/api', params: [], callback: function ($routes): void {});
$routes->scope('/api', params: ['something'], callback: function ($routes): void {});

$routes->resources('/api', callback: function ($routes): void {});
$routes->resources('/api', options: ['something'], callback: function ($routes): void {});

$routes->prefix('/api', callback: function ($routes): void {});
$routes->prefix('/api', params: ['something'], callback: function ($routes): void {});

$routes->plugin('/api', callback: function ($routes): void {});
$routes->plugin('/api', options: ['something'], callback: function ($routes): void {});
}
}