Skip to content

Commit 79d8d8a

Browse files
committed
add custom rector to cleanup command usages (#348)
* add custom rector to cleanup command usages * test with disabled custom rector * add unit tests * adjust CI * adjust composer.json * Revert "adjust composer.json" This reverts commit d68f12b.
1 parent efcfd0a commit 79d8d8a

File tree

8 files changed

+300
-41
lines changed

8 files changed

+300
-41
lines changed

.github/workflows/ci.yml

Lines changed: 43 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,9 @@ name: CI
22

33
on:
44
push:
5-
branches:
6-
- 3.x
7-
- 4.x
8-
- 5.x
5+
branches: [3.x, 4.x, 5.x]
96
pull_request:
10-
branches:
11-
- '*'
7+
branches: ['*']
128
workflow_dispatch:
139

1410
jobs:
@@ -17,7 +13,7 @@ jobs:
1713
strategy:
1814
fail-fast: false
1915
matrix:
20-
php-version: ['8.1']
16+
php-version: ['8.1', '8.2', '8.3', '8.4']
2117
prefer-lowest: ['']
2218
include:
2319
- php-version: '8.1'
@@ -62,39 +58,49 @@ jobs:
6258
run: echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json"
6359

6460
- name: Run PHPUnit
65-
run: vendor/bin/phpunit --display-incomplete --display-skipped
61+
run: |
62+
if [[ "${{ matrix.php-version }}" == "8.1" && "${{ matrix.prefer-lowest }}" != "prefer-lowest" ]]; then
63+
export CODECOVERAGE=1
64+
vendor/bin/phpunit --display-incomplete --display-skipped --coverage-clover=coverage.xml
65+
else
66+
vendor/bin/phpunit
67+
fi
68+
69+
- name: Submit code coverage
70+
if: matrix.php-version == '8.1' && matrix.prefer-lowest != 'prefer-lowest'
71+
uses: codecov/codecov-action@v5
6672

6773
cs-stan:
6874
name: Coding Standard & Static Analysis
6975
runs-on: ubuntu-22.04
7076

7177
steps:
72-
- uses: actions/checkout@v6
73-
74-
- name: Setup PHP
75-
uses: shivammathur/setup-php@v2
76-
with:
77-
php-version: '8.1'
78-
extensions: mbstring, intl
79-
tools: cs2pr
80-
coverage: none
81-
82-
- name: Get composer cache directory
83-
id: composer-cache
84-
run: echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT
85-
86-
- name: Get date part for cache key
87-
id: key-date
88-
run: echo "date=$(date +'%Y-%m')" >> $GITHUB_OUTPUT
89-
90-
- name: Cache composer dependencies
91-
uses: actions/cache@v5
92-
with:
93-
path: ${{ steps.composer-cache.outputs.dir }}
94-
key: ${{ runner.os }}-composer-${{ steps.key-date.outputs.date }}-${{ hashFiles('composer.json') }}-${{ matrix.prefer-lowest }}
95-
96-
- name: Composer install
97-
run: make install-dev
98-
99-
- name: Run PHP CodeSniffer
100-
run: vendor/bin/phpcs --report=checkstyle | cs2pr
78+
- uses: actions/checkout@v6
79+
80+
- name: Setup PHP
81+
uses: shivammathur/setup-php@v2
82+
with:
83+
php-version: '8.1'
84+
extensions: mbstring, intl
85+
tools: cs2pr
86+
coverage: none
87+
88+
- name: Get composer cache directory
89+
id: composer-cache
90+
run: echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT
91+
92+
- name: Get date part for cache key
93+
id: key-date
94+
run: echo "date=$(date +'%Y-%m')" >> $GITHUB_OUTPUT
95+
96+
- name: Cache composer dependencies
97+
uses: actions/cache@v5
98+
with:
99+
path: ${{ steps.composer-cache.outputs.dir }}
100+
key: ${{ runner.os }}-composer-${{ steps.key-date.outputs.date }}-${{ hashFiles('composer.json') }}-${{ matrix.prefer-lowest }}
101+
102+
- name: Composer install
103+
run: make install-dev
104+
105+
- name: Run PHP CodeSniffer
106+
run: vendor/bin/phpcs --report=checkstyle | cs2pr

config/rector/sets/cakephp60.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
declare(strict_types=1);
33

44
use Cake\Upgrade\Rector\Rector\MethodCall\EventManagerOnRector;
5+
use Cake\Upgrade\Rector\Rector\MethodCall\ReplaceCommandArgsIoWithPropertiesRector;
56
use PHPStan\Type\ObjectType;
67
use Rector\Config\RectorConfig;
78
use Rector\Renaming\Rector\MethodCall\RenameMethodRector;
@@ -20,6 +21,8 @@
2021
// EventManager::on() signature change
2122
$rectorConfig->rule(EventManagerOnRector::class);
2223

24+
$rectorConfig->rule(ReplaceCommandArgsIoWithPropertiesRector::class);
25+
2326
// Changes related to the accessible => patchable rename
2427
$rectorConfig->ruleWithConfiguration(RenameMethodRector::class, [
2528
new MethodCallRename('Cake\ORM\Entity', 'setAccess', 'setPatchable'),
Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
namespace Cake\Upgrade\Rector\Rector\MethodCall;
5+
6+
use Cake\Command\Command;
7+
use PhpParser\Node;
8+
use PhpParser\Node\Expr\MethodCall;
9+
use PhpParser\Node\Expr\PropertyFetch;
10+
use PhpParser\Node\Expr\Variable;
11+
use PhpParser\Node\Param;
12+
use PhpParser\Node\Stmt\ClassMethod;
13+
use PHPStan\Reflection\ReflectionProvider;
14+
use Rector\PhpParser\Node\BetterNodeFinder;
15+
use Rector\PHPStan\ScopeFetcher;
16+
use Rector\Rector\AbstractRector;
17+
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
18+
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
19+
20+
final class ReplaceCommandArgsIoWithPropertiesRector extends AbstractRector
21+
{
22+
public function __construct(
23+
protected BetterNodeFinder $betterNodeFinder,
24+
protected ReflectionProvider $reflectionProvider,
25+
) {
26+
}
27+
28+
public function getRuleDefinition(): RuleDefinition
29+
{
30+
return new RuleDefinition(
31+
'Replace `$args` and `$io` parameters in Command classes with `$this->args` and `$this->io`',
32+
[
33+
new CodeSample(
34+
<<<'CODE_SAMPLE'
35+
class TestCommand extends Command
36+
{
37+
public function execute(Arguments $args, ConsoleIo $io)
38+
{
39+
$io->out('Hello');
40+
$this->someMethod($args, $io);
41+
}
42+
43+
protected function someMethod(Arguments $args, ConsoleIo $io): void
44+
{
45+
$someArg = $args->getArgument('some');
46+
$io->warning('Warn');
47+
}
48+
}
49+
CODE_SAMPLE,
50+
<<<'CODE_SAMPLE'
51+
class TestCommand extends Command
52+
{
53+
public function execute()
54+
{
55+
$this->io->out('Hello');
56+
$this->someMethod();
57+
}
58+
59+
protected function someMethod(): void
60+
{
61+
$someArg = $this->args->getArgument('some');
62+
$this->io->warning('Warn');
63+
}
64+
}
65+
CODE_SAMPLE,
66+
),
67+
],
68+
);
69+
}
70+
71+
public function getNodeTypes(): array
72+
{
73+
return [ClassMethod::class];
74+
}
75+
76+
public function refactor(Node $node): ?Node
77+
{
78+
if (! $node instanceof ClassMethod) {
79+
return null;
80+
}
81+
82+
// Make sure we are in a class
83+
$scope = ScopeFetcher::fetch($node);
84+
if (!$scope->isInClass()) {
85+
return null;
86+
}
87+
$class = $scope->getClassReflection();
88+
89+
// Skip if class doesn't extend Command (you can expand to check parent name)
90+
$baseCommandClass = $this->reflectionProvider->getClass(Command::class);
91+
if ($class->getName() === Command::class || $class->isSubclassOfClass($baseCommandClass) === false) {
92+
return null;
93+
}
94+
95+
// Find if params are $args and/or $io
96+
$argsParam = $this->findParam($node, 'args');
97+
$ioParam = $this->findParam($node, 'io');
98+
99+
if (! $argsParam && ! $ioParam) {
100+
return null;
101+
}
102+
103+
// Replace all `$args` and `$io` usages inside the method body
104+
$this->traverseNodesWithCallable($node->stmts ?? [], function (Node $innerNode) use ($argsParam, $ioParam) {
105+
// Replace `$args` and `$io` variables
106+
if ($innerNode instanceof Variable) {
107+
if ($argsParam && $innerNode->name === 'args') {
108+
return new PropertyFetch(new Variable('this'), 'args');
109+
}
110+
111+
if ($ioParam && $innerNode->name === 'io') {
112+
return new PropertyFetch(new Variable('this'), 'io');
113+
}
114+
}
115+
116+
// Remove `$args` / `$io` from method calls on `$this`
117+
if (
118+
$innerNode instanceof MethodCall
119+
&& $innerNode->var instanceof Variable
120+
&& $innerNode->var->name === 'this'
121+
) {
122+
$innerNode->args = array_values(array_filter(
123+
$innerNode->args,
124+
fn(Node\Arg $arg) => !($arg->value instanceof Variable &&
125+
in_array($arg->value->name, ['args', 'io'], true)),
126+
));
127+
128+
return $innerNode;
129+
}
130+
131+
return null;
132+
});
133+
134+
// Remove the parameters themselves
135+
$node->params = array_filter($node->params, function (Param $param) {
136+
return !in_array($this->getName($param), ['args', 'io'], true);
137+
});
138+
139+
return $node;
140+
}
141+
142+
private function findParam(ClassMethod $method, string $name): ?Param
143+
{
144+
foreach ($method->params as $param) {
145+
if ($this->getName($param) === $name) {
146+
return $param;
147+
}
148+
}
149+
150+
return null;
151+
}
152+
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
<?php
2+
3+
namespace Cake\Upgrade\Test\TestCase\Rector\MethodCall\AddMethodCallArgsRectorTest\Fixture;
4+
5+
use Cake\Command\Command;
6+
use Cake\Console\Arguments;
7+
use Cake\Console\ConsoleIo;
8+
9+
class TestCommand extends Command
10+
{
11+
public function execute(Arguments $args, ConsoleIo $io)
12+
{
13+
$io->out('Hello World');
14+
$this->someMethod($args, $io);
15+
return static::CODE_SUCCESS;
16+
}
17+
18+
protected function someMethod(Arguments $args, ConsoleIo $io): void
19+
{
20+
$someArg = $args->getArgument('some');
21+
$io->warning('Warning');
22+
}
23+
}
24+
?>
25+
-----
26+
<?php
27+
28+
namespace Cake\Upgrade\Test\TestCase\Rector\MethodCall\AddMethodCallArgsRectorTest\Fixture;
29+
30+
use Cake\Command\Command;
31+
use Cake\Console\Arguments;
32+
use Cake\Console\ConsoleIo;
33+
34+
class TestCommand extends Command
35+
{
36+
public function execute()
37+
{
38+
$this->io->out('Hello World');
39+
$this->someMethod();
40+
return static::CODE_SUCCESS;
41+
}
42+
43+
protected function someMethod(): void
44+
{
45+
$someArg = $this->args->getArgument('some');
46+
$this->io->warning('Warning');
47+
}
48+
}
49+
?>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
namespace Cake\Upgrade\Test\TestCase\Rector\MethodCall\AddMethodCallArgsRector;
5+
6+
use Iterator;
7+
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
8+
9+
final class ReplaceCommandArgsIoWithPropertiesRectorTest extends AbstractRectorTestCase
10+
{
11+
/**
12+
* @dataProvider provideData()
13+
*/
14+
public function test(string $filePath): void
15+
{
16+
$this->doTestFile($filePath);
17+
}
18+
19+
public static function provideData(): Iterator
20+
{
21+
return self::yieldFilesFromDirectory(__DIR__ . '/Fixture');
22+
}
23+
24+
public function provideConfigFilePath(): string
25+
{
26+
return __DIR__ . '/config/configured_rule.php';
27+
}
28+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
use Cake\Upgrade\Rector\Rector\MethodCall\ReplaceCommandArgsIoWithPropertiesRector;
5+
use Rector\Config\RectorConfig;
6+
7+
return static function (RectorConfig $rectorConfig): void {
8+
$rectorConfig->rule(ReplaceCommandArgsIoWithPropertiesRector::class);
9+
};

tests/test_apps/original/RectorCommand-testApply60/src/Command/TestCommand.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,13 @@ class TestCommand extends Command
1212
public function execute(Arguments $args, ConsoleIo $io)
1313
{
1414
$io->out('Hello World');
15-
15+
$this->someMethod($args, $io);
1616
return static::CODE_SUCCESS;
1717
}
18+
19+
protected function someMethod(Arguments $args, ConsoleIo $io): void
20+
{
21+
$someArg = $args->getArgument('some');
22+
$io->warning('Warning');
23+
}
1824
}

tests/test_apps/upgraded/RectorCommand-testApply60/src/Command/TestCommand.php

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,16 @@
99

1010
class TestCommand extends Command
1111
{
12-
public function execute(Arguments $args, \Cake\Console\ConsoleIoInterface $io)
12+
public function execute()
1313
{
14-
$io->out('Hello World');
15-
14+
$this->io->out('Hello World');
15+
$this->someMethod();
1616
return static::CODE_SUCCESS;
1717
}
18+
19+
protected function someMethod(): void
20+
{
21+
$someArg = $this->args->getArgument('some');
22+
$this->io->warning('Warning');
23+
}
1824
}

0 commit comments

Comments
 (0)