Skip to content

Commit 08758f4

Browse files
TomasVotrubaTomasVotruba
authored
[CodeQuality] Add SimplerWithEqualToRector to collapse assertSame callback in with() to equalTo() matcher (#714)
* [CodeQuality] Handle return throw and return null in RemoveReturnFromVoidMethodMockCallbackRector void callbacks * [CodeQuality] Add SimplerWithEqualToRector to collapse assertSame callback in with() to equalTo() matcher * [CodeQuality] Loop all with() args in SimplerWithIsInstanceOfRector * [CodeQuality] Rename SimplerWithEqualToRector to CallbackSingleAssertToSimplerRector
1 parent 749dc8b commit 08758f4

10 files changed

Lines changed: 407 additions & 23 deletions

File tree

‎config/sets/phpunit-code-quality.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
use Rector\PHPUnit\CodeQuality\Rector\MethodCall\AssertSameBoolNullToSpecificMethodRector;
4444
use Rector\PHPUnit\CodeQuality\Rector\MethodCall\AssertSameTrueFalseToAssertTrueFalseRector;
4545
use Rector\PHPUnit\CodeQuality\Rector\MethodCall\AssertTrueFalseToSpecificMethodRector;
46+
use Rector\PHPUnit\CodeQuality\Rector\MethodCall\CallbackSingleAssertToSimplerRector;
4647
use Rector\PHPUnit\CodeQuality\Rector\MethodCall\FlipAssertRector;
4748
use Rector\PHPUnit\CodeQuality\Rector\MethodCall\MatchAssertSameExpectedTypeRector;
4849
use Rector\PHPUnit\CodeQuality\Rector\MethodCall\MergeWithCallableAndWillReturnRector;
@@ -137,6 +138,7 @@
137138
*/
138139
RemoveExpectAnyFromMockRector::class,
139140
SingleMockPropertyTypeRector::class,
141+
CallbackSingleAssertToSimplerRector::class,
140142
SimplerWithIsInstanceOfRector::class,
141143
DirectInstanceOverMockArgRector::class,
142144

‎rules-tests/CodeQuality/Rector/MethodCall/CallbackSingleAssertToSimplerRector/CallbackSingleAssertToSimplerRectorTest.php‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\CallbackSingleAssertToSimplerRector;
6+
7+
use Iterator;
8+
use PHPUnit\Framework\Attributes\DataProvider;
9+
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
10+
11+
final class CallbackSingleAssertToSimplerRectorTest extends AbstractRectorTestCase
12+
{
13+
#[DataProvider('provideData')]
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+
}

‎rules-tests/CodeQuality/Rector/MethodCall/CallbackSingleAssertToSimplerRector/Fixture/multiple_with_args.php.inc‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php
2+
3+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\CallbackSingleAssertToSimplerRector\Fixture;
4+
5+
use PHPUnit\Framework\TestCase;
6+
7+
final class MultipleWithArgs extends TestCase
8+
{
9+
public function test()
10+
{
11+
$builder = $this->getMockBuilder('AnyType')->getMock();
12+
13+
$builder->expects($this->exactly(2))
14+
->method('add')
15+
->with($this->callback(function ($type): bool {
16+
$this->assertSame(TextType::class, $type);
17+
18+
return true;
19+
}), $this->callback(function ($options): bool {
20+
$this->assertSame([], $options);
21+
22+
return true;
23+
}));
24+
}
25+
}
26+
27+
?>
28+
-----
29+
<?php
30+
31+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\CallbackSingleAssertToSimplerRector\Fixture;
32+
33+
use PHPUnit\Framework\TestCase;
34+
35+
final class MultipleWithArgs extends TestCase
36+
{
37+
public function test()
38+
{
39+
$builder = $this->getMockBuilder('AnyType')->getMock();
40+
41+
$builder->expects($this->exactly(2))
42+
->method('add')
43+
->with($this->equalTo(TextType::class), $this->equalTo([]));
44+
}
45+
}
46+
47+
?>

‎rules-tests/CodeQuality/Rector/MethodCall/CallbackSingleAssertToSimplerRector/Fixture/skip_different_assert.php.inc‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php
2+
3+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\CallbackSingleAssertToSimplerRector\Fixture;
4+
5+
use PHPUnit\Framework\TestCase;
6+
7+
final class SkipDifferentAssert extends TestCase
8+
{
9+
public function test()
10+
{
11+
$builder = $this->getMockBuilder('AnyType')->getMock();
12+
13+
$builder->expects($this->exactly(2))
14+
->method('add')
15+
->with($this->callback(function ($type): bool {
16+
$this->assertInstanceOf(TextType::class, $type);
17+
18+
return true;
19+
}));
20+
}
21+
}

‎rules-tests/CodeQuality/Rector/MethodCall/CallbackSingleAssertToSimplerRector/Fixture/skip_multiple_stmts.php.inc‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
3+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\CallbackSingleAssertToSimplerRector\Fixture;
4+
5+
use PHPUnit\Framework\TestCase;
6+
7+
final class SkipMultipleStmts extends TestCase
8+
{
9+
public function test()
10+
{
11+
$builder = $this->getMockBuilder('AnyType')->getMock();
12+
13+
$builder->expects($this->exactly(2))
14+
->method('add')
15+
->with($this->callback(function ($type): bool {
16+
$this->assertSame(TextType::class, $type);
17+
$this->assertNotNull($type);
18+
19+
return true;
20+
}));
21+
}
22+
}

‎rules-tests/CodeQuality/Rector/MethodCall/CallbackSingleAssertToSimplerRector/Fixture/some_file.php.inc‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<?php
2+
3+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\CallbackSingleAssertToSimplerRector\Fixture;
4+
5+
use PHPUnit\Framework\TestCase;
6+
7+
final class SomeFile extends TestCase
8+
{
9+
public function test()
10+
{
11+
$builder = $this->getMockBuilder('AnyType')->getMock();
12+
13+
$builder->expects($this->exactly(2))
14+
->method('add')
15+
->with($this->callback(function ($type): bool {
16+
$this->assertSame(TextType::class, $type);
17+
18+
return true;
19+
}));
20+
}
21+
}
22+
23+
?>
24+
-----
25+
<?php
26+
27+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\CallbackSingleAssertToSimplerRector\Fixture;
28+
29+
use PHPUnit\Framework\TestCase;
30+
31+
final class SomeFile extends TestCase
32+
{
33+
public function test()
34+
{
35+
$builder = $this->getMockBuilder('AnyType')->getMock();
36+
37+
$builder->expects($this->exactly(2))
38+
->method('add')
39+
->with($this->equalTo(TextType::class));
40+
}
41+
}
42+
43+
?>

‎rules-tests/CodeQuality/Rector/MethodCall/CallbackSingleAssertToSimplerRector/config/configured_rule.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
use Rector\Config\RectorConfig;
6+
use Rector\PHPUnit\CodeQuality\Rector\MethodCall\CallbackSingleAssertToSimplerRector;
7+
8+
return static function (RectorConfig $rectorConfig): void {
9+
$rectorConfig->rule(CallbackSingleAssertToSimplerRector::class);
10+
};

‎rules-tests/CodeQuality/Rector/MethodCall/SimplerWithIsInstanceOfRector/Fixture/multiple_with_args.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 Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\SimplerWithIsInstanceOfRector\Fixture;
4+
5+
use PHPUnit\Framework\TestCase;
6+
7+
final class MultipleWithArgs extends TestCase
8+
{
9+
public function test()
10+
{
11+
$someMock = $this->getMockBuilder('AnyType')->getMock();
12+
13+
$someMock->expects($this->any())
14+
->method('dispatch')
15+
->with($this->callback(function ($event): bool {
16+
$this->assertInstanceOf(\stdClass::class, $event);
17+
return true;
18+
}), $this->callback(function ($name): bool {
19+
$this->assertInstanceOf(\Stringable::class, $name);
20+
return true;
21+
}));
22+
}
23+
}
24+
25+
?>
26+
-----
27+
<?php
28+
29+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\SimplerWithIsInstanceOfRector\Fixture;
30+
31+
use PHPUnit\Framework\TestCase;
32+
33+
final class MultipleWithArgs extends TestCase
34+
{
35+
public function test()
36+
{
37+
$someMock = $this->getMockBuilder('AnyType')->getMock();
38+
39+
$someMock->expects($this->any())
40+
->method('dispatch')
41+
->with($this->isInstanceOf(\stdClass::class), $this->isInstanceOf(\Stringable::class));
42+
}
43+
}
44+
45+
?>

‎rules/CodeQuality/Rector/MethodCall/CallbackSingleAssertToSimplerRector.php‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\PHPUnit\CodeQuality\Rector\MethodCall;
6+
7+
use PhpParser\Node;
8+
use PhpParser\Node\Expr;
9+
use PhpParser\Node\Expr\Closure;
10+
use PhpParser\Node\Expr\ConstFetch;
11+
use PhpParser\Node\Expr\MethodCall;
12+
use PhpParser\Node\Stmt\Expression;
13+
use PhpParser\Node\Stmt\Return_;
14+
use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer;
15+
use Rector\Rector\AbstractRector;
16+
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
17+
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
18+
19+
/**
20+
* @see \Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\CallbackSingleAssertToSimplerRector\CallbackSingleAssertToSimplerRectorTest
21+
*/
22+
final class CallbackSingleAssertToSimplerRector extends AbstractRector
23+
{
24+
public function __construct(
25+
private readonly TestsNodeAnalyzer $testsNodeAnalyzer
26+
) {
27+
}
28+
29+
public function getRuleDefinition(): RuleDefinition
30+
{
31+
return new RuleDefinition(
32+
'Replaces use of with, callback and sole assertSame() to simple equalTo() matcher',
33+
[
34+
new CodeSample(
35+
<<<'CODE_SAMPLE'
36+
use PHPUnit\Framework\TestCase;
37+
38+
final class SomeClass extends TestCase
39+
{
40+
public function test()
41+
{
42+
$builder->expects($this->exactly(2))
43+
->method('add')
44+
->with($this->callback(function ($type): bool {
45+
$this->assertSame(TextType::class, $type);
46+
47+
return true;
48+
}));
49+
}
50+
}
51+
CODE_SAMPLE
52+
,
53+
<<<'CODE_SAMPLE'
54+
use PHPUnit\Framework\TestCase;
55+
56+
final class SomeClass extends TestCase
57+
{
58+
public function test()
59+
{
60+
$builder->expects($this->exactly(2))
61+
->method('add')
62+
->with($this->equalTo(TextType::class));
63+
}
64+
}
65+
CODE_SAMPLE
66+
),
67+
]
68+
);
69+
}
70+
71+
public function getNodeTypes(): array
72+
{
73+
return [MethodCall::class];
74+
}
75+
76+
/**
77+
* @param MethodCall $node
78+
*/
79+
public function refactor(Node $node): MethodCall|null
80+
{
81+
if (! $this->testsNodeAnalyzer->isInTestClass($node)) {
82+
return null;
83+
}
84+
85+
if ($node->isFirstClassCallable()) {
86+
return null;
87+
}
88+
89+
if (! $this->isName($node->name, 'with')) {
90+
return null;
91+
}
92+
93+
$hasChanged = false;
94+
95+
foreach ($node->getArgs() as $arg) {
96+
$expectedExpr = $this->matchCallbackSoleAssertSameExpected($arg->value);
97+
if (! $expectedExpr instanceof Expr) {
98+
continue;
99+
}
100+
101+
$arg->value = $this->nodeFactory->createMethodCall('this', 'equalTo', [$expectedExpr]);
102+
$hasChanged = true;
103+
}
104+
105+
if (! $hasChanged) {
106+
return null;
107+
}
108+
109+
return $node;
110+
}
111+
112+
private function matchCallbackSoleAssertSameExpected(Expr $expr): ?Expr
113+
{
114+
if (! $expr instanceof MethodCall || ! $this->isName($expr->name, 'callback')) {
115+
return null;
116+
}
117+
118+
$callbackArgs = $expr->getArgs();
119+
if ($callbackArgs === []) {
120+
return null;
121+
}
122+
123+
$innerClosure = $callbackArgs[0]->value;
124+
if (! $innerClosure instanceof Closure) {
125+
return null;
126+
}
127+
128+
// exactly assertSame() expression + "return true;"
129+
$closureStmts = $innerClosure->getStmts();
130+
if (count($closureStmts) !== 2) {
131+
return null;
132+
}
133+
134+
[$firstStmt, $secondStmt] = $closureStmts;
135+
136+
if (! $firstStmt instanceof Expression) {
137+
return null;
138+
}
139+
140+
$firstStmtExpr = $firstStmt->expr;
141+
if (! $firstStmtExpr instanceof MethodCall || ! $this->isName($firstStmtExpr->name, 'assertSame')) {
142+
return null;
143+
}
144+
145+
if (! $secondStmt instanceof Return_ || ! $this->isTrueReturn($secondStmt)) {
146+
return null;
147+
}
148+
149+
return $firstStmtExpr->getArgs()[0]
150+
->value;
151+
}
152+
153+
private function isTrueReturn(Return_ $return): bool
154+
{
155+
return $return->expr instanceof ConstFetch && $this->isName($return->expr, 'true');
156+
}
157+
}

0 commit comments

Comments
 (0)