Skip to content

Commit 32b6175

Browse files
LordSimalLordSimal
authored
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 b6202ee commit 32b6175

File tree

8 files changed

+324
-87
lines changed

8 files changed

+324
-87
lines changed

‎.github/workflows/ci.yml‎

Lines changed: 67 additions & 83 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,92 +13,80 @@ 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'
2420
prefer-lowest: 'prefer-lowest'
2521

2622
steps:
27-
- uses: actions/checkout@v5
28-
29-
- name: Setup PHP
30-
uses: shivammathur/setup-php@v2
31-
with:
32-
php-version: ${{ matrix.php-version }}
33-
extensions: mbstring, intl
34-
coverage: pcov
35-
36-
- name: Get composer cache directory
37-
id: composer-cache
38-
run: echo "::set-output name=dir::$(composer config cache-files-dir)"
39-
40-
- name: Get date part for cache key
41-
id: key-date
42-
run: echo "::set-output name=date::$(date +'%Y-%m')"
43-
44-
- name: Cache composer dependencies
45-
uses: actions/cache@v4
46-
with:
47-
path: ${{ steps.composer-cache.outputs.dir }}
48-
key: ${{ runner.os }}-composer-${{ steps.key-date.outputs.date }}-${{ hashFiles('composer.json') }}-${{ matrix.prefer-lowest }}
49-
50-
- name: Composer Install
51-
run: |
52-
if ${{ matrix.prefer-lowest == 'prefer-lowest' }}; then
53-
make install-dev-lowest
54-
elif ${{ matrix.php-version == '8.1' }}; then
55-
make install-dev-ignore-reqs
56-
else
57-
make install-dev
58-
fi
59-
60-
- name: Setup problem matchers for PHPUnit
61-
if: matrix.php-version == '8.1'
62-
run: echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json"
63-
64-
- name: Run PHPUnit
65-
run: |
66-
if [[ ${{ matrix.php-version }} == '8.1' ]]; then
67-
export CODECOVERAGE=1 && vendor/bin/phpunit --display-incomplete --display-skipped --coverage-clover=coverage.xml
68-
else
69-
vendor/bin/phpunit
70-
fi
71-
72-
- name: Submit code coverage
73-
if: matrix.php-version == '8.1'
74-
uses: codecov/codecov-action@v5
23+
- uses: actions/checkout@v5
24+
25+
- name: Setup PHP
26+
uses: shivammathur/setup-php@v2
27+
with:
28+
php-version: ${{ matrix.php-version }}
29+
extensions: mbstring, intl
30+
coverage: pcov
31+
32+
- name: Cache composer dependencies
33+
id: composer-cache
34+
uses: actions/cache@v4
35+
with:
36+
path: ~/.composer/cache
37+
key: ${{ runner.os }}-composer-${{ matrix.php-version }}-${{ matrix.prefer-lowest }}-${{ hashFiles('**/composer.lock') }}
38+
restore-keys: |
39+
${{ runner.os }}-composer-${{ matrix.php-version }}-${{ matrix.prefer-lowest }}-
40+
41+
- name: Composer install
42+
run: |
43+
if [[ "${{ matrix.prefer-lowest }}" == "prefer-lowest" ]]; then
44+
make install-dev-lowest
45+
elif [[ "${{ matrix.php-version }}" == "8.1" ]]; then
46+
make install-dev-ignore-reqs
47+
else
48+
make install-dev
49+
fi
50+
51+
- name: Setup problem matchers for PHPUnit
52+
if: matrix.php-version == '8.1'
53+
run: echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json"
54+
55+
- name: Run PHPUnit
56+
run: |
57+
if [[ "${{ matrix.php-version }}" == "8.1" && "${{ matrix.prefer-lowest }}" != "prefer-lowest" ]]; then
58+
export CODECOVERAGE=1
59+
vendor/bin/phpunit --display-incomplete --display-skipped --coverage-clover=coverage.xml
60+
else
61+
vendor/bin/phpunit
62+
fi
63+
64+
- name: Submit code coverage
65+
if: matrix.php-version == '8.1' && matrix.prefer-lowest != 'prefer-lowest'
66+
uses: codecov/codecov-action@v5
7567

7668
cs-stan:
7769
name: Coding Standard & Static Analysis
7870
runs-on: ubuntu-22.04
7971

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

‎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'),

‎src/Rector/Rector/MethodCall/ReplaceCommandArgsIoWithPropertiesRector.php‎

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+
}

‎tests/TestCase/Rector/MethodCall/ReplaceCommandArgsIoWithPorpertiesRector/Fixture/fixture.php.inc‎

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+
?>

0 commit comments

Comments
 (0)