Skip to content

Commit f450d2c

Browse files
TomasVotrubaTomasVotruba
authored
[CodeQuality] Move AssertClassToThisAssertRector hook from Class_ to ClassMethod (#728)
* [CodeQuality] Move AssertClassToThisAssertRector hook from Class_ to ClassMethod The rule only ever rewrote Assert::assert*() calls inside class methods, so hooking on ClassMethod is more precise and avoids re-traversing the whole class. Static methods are now skipped with an early return instead of a DONT_TRAVERSE_CURRENT_AND_CHILDREN check inside the traversal callback; static closures and arrow functions are still skipped during traversal. * Add fixture for Assert::assert*() inside foreach
1 parent b12526b commit f450d2c

10 files changed

Lines changed: 84 additions & 17 deletions

File tree

rules-tests/CodeQuality/Rector/Class_/AssertClassToThisAssertRector/AssertClassToThisAssertRectorTest.php renamed to rules-tests/CodeQuality/Rector/ClassMethod/AssertClassToThisAssertRector/AssertClassToThisAssertRectorTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
declare(strict_types=1);
44

5-
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\AssertClassToThisAssertRector;
5+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\ClassMethod\AssertClassToThisAssertRector;
66

77
use Iterator;
88
use PHPUnit\Framework\Attributes\DataProvider;

rules-tests/CodeQuality/Rector/Class_/AssertClassToThisAssertRector/Fixture/basic.php.inc renamed to rules-tests/CodeQuality/Rector/ClassMethod/AssertClassToThisAssertRector/Fixture/basic.php.inc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<?php
22

3-
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\AssertClassToThisAssertRector\Fixture;
3+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\ClassMethod\AssertClassToThisAssertRector\Fixture;
44

55
use PHPUnit\Framework\Assert;
66
use PHPUnit\Framework\TestCase;
@@ -17,7 +17,7 @@ final class BasicTest extends TestCase
1717
-----
1818
<?php
1919

20-
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\AssertClassToThisAssertRector\Fixture;
20+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\ClassMethod\AssertClassToThisAssertRector\Fixture;
2121

2222
use PHPUnit\Framework\Assert;
2323
use PHPUnit\Framework\TestCase;

rules-tests/CodeQuality/Rector/Class_/AssertClassToThisAssertRector/Fixture/fully_qualified.php.inc renamed to rules-tests/CodeQuality/Rector/ClassMethod/AssertClassToThisAssertRector/Fixture/fully_qualified.php.inc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<?php
22

3-
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\AssertClassToThisAssertRector\Fixture;
3+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\ClassMethod\AssertClassToThisAssertRector\Fixture;
44

55
use PHPUnit\Framework\TestCase;
66

@@ -16,7 +16,7 @@ final class FullyQualifiedTest extends TestCase
1616
-----
1717
<?php
1818

19-
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\AssertClassToThisAssertRector\Fixture;
19+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\ClassMethod\AssertClassToThisAssertRector\Fixture;
2020

2121
use PHPUnit\Framework\TestCase;
2222

‎rules-tests/CodeQuality/Rector/ClassMethod/AssertClassToThisAssertRector/Fixture/inside_foreach.php.inc‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
<?php
2+
3+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\ClassMethod\AssertClassToThisAssertRector\Fixture;
4+
5+
use PHPUnit\Framework\Assert;
6+
use PHPUnit\Framework\TestCase;
7+
8+
final class InsideForeach extends TestCase
9+
{
10+
public function testOriginalKeyOrderingForFullAssociativeArray(): void
11+
{
12+
$entityMock = $this->createMock(SomeEntity::class);
13+
$entityMock->method('getId')
14+
->willReturn(1);
15+
$entities = [$entityMock];
16+
17+
$parameters = [
18+
['id' => 1, 'foo' => 'bar'],
19+
];
20+
21+
$helper = new SomeHelper($parameters);
22+
$orderedEntities = $helper->orderByOriginalKey($entities);
23+
24+
$this->assertSame([0], array_keys($orderedEntities));
25+
26+
foreach ($parameters as $key => $contact) {
27+
Assert::assertEquals($orderedEntities[$key]->getId(), $entities[$key]->getId());
28+
}
29+
}
30+
}
31+
32+
?>
33+
-----
34+
<?php
35+
36+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\ClassMethod\AssertClassToThisAssertRector\Fixture;
37+
38+
use PHPUnit\Framework\Assert;
39+
use PHPUnit\Framework\TestCase;
40+
41+
final class InsideForeach extends TestCase
42+
{
43+
public function testOriginalKeyOrderingForFullAssociativeArray(): void
44+
{
45+
$entityMock = $this->createMock(SomeEntity::class);
46+
$entityMock->method('getId')
47+
->willReturn(1);
48+
$entities = [$entityMock];
49+
50+
$parameters = [
51+
['id' => 1, 'foo' => 'bar'],
52+
];
53+
54+
$helper = new SomeHelper($parameters);
55+
$orderedEntities = $helper->orderByOriginalKey($entities);
56+
57+
$this->assertSame([0], array_keys($orderedEntities));
58+
59+
foreach ($parameters as $key => $contact) {
60+
$this->assertEquals($orderedEntities[$key]->getId(), $entities[$key]->getId());
61+
}
62+
}
63+
}
64+
65+
?>

rules-tests/CodeQuality/Rector/Class_/AssertClassToThisAssertRector/Fixture/skip_non_test_class.php.inc renamed to rules-tests/CodeQuality/Rector/ClassMethod/AssertClassToThisAssertRector/Fixture/skip_non_test_class.php.inc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<?php
22

3-
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\AssertClassToThisAssertRector\Fixture;
3+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\ClassMethod\AssertClassToThisAssertRector\Fixture;
44

55
use PHPUnit\Framework\Assert;
66

rules-tests/CodeQuality/Rector/Class_/AssertClassToThisAssertRector/Fixture/skip_self_call.php.inc renamed to rules-tests/CodeQuality/Rector/ClassMethod/AssertClassToThisAssertRector/Fixture/skip_self_call.php.inc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<?php
22

3-
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\AssertClassToThisAssertRector\Fixture;
3+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\ClassMethod\AssertClassToThisAssertRector\Fixture;
44

55
use PHPUnit\Framework\TestCase;
66

rules-tests/CodeQuality/Rector/Class_/AssertClassToThisAssertRector/Fixture/skip_static_closure.php.inc renamed to rules-tests/CodeQuality/Rector/ClassMethod/AssertClassToThisAssertRector/Fixture/skip_static_closure.php.inc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<?php
22

3-
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\AssertClassToThisAssertRector\Fixture;
3+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\ClassMethod\AssertClassToThisAssertRector\Fixture;
44

55
use PHPUnit\Framework\Assert;
66
use PHPUnit\Framework\TestCase;

rules-tests/CodeQuality/Rector/Class_/AssertClassToThisAssertRector/Fixture/skip_static_method.php.inc renamed to rules-tests/CodeQuality/Rector/ClassMethod/AssertClassToThisAssertRector/Fixture/skip_static_method.php.inc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<?php
22

3-
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\AssertClassToThisAssertRector\Fixture;
3+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\ClassMethod\AssertClassToThisAssertRector\Fixture;
44

55
use PHPUnit\Framework\Assert;
66
use PHPUnit\Framework\TestCase;

rules-tests/CodeQuality/Rector/Class_/AssertClassToThisAssertRector/config/configured_rule.php renamed to rules-tests/CodeQuality/Rector/ClassMethod/AssertClassToThisAssertRector/config/configured_rule.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
declare(strict_types=1);
44

55
use Rector\Config\RectorConfig;
6-
use Rector\PHPUnit\CodeQuality\Rector\Class_\AssertClassToThisAssertRector;
6+
use Rector\PHPUnit\CodeQuality\Rector\ClassMethod\AssertClassToThisAssertRector;
77

88
return static function (RectorConfig $rectorConfig): void {
99
$rectorConfig->rule(AssertClassToThisAssertRector::class);

rules/CodeQuality/Rector/Class_/AssertClassToThisAssertRector.php renamed to rules/CodeQuality/Rector/ClassMethod/AssertClassToThisAssertRector.php

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,13 @@
22

33
declare(strict_types=1);
44

5-
namespace Rector\PHPUnit\CodeQuality\Rector\Class_;
5+
namespace Rector\PHPUnit\CodeQuality\Rector\ClassMethod;
66

77
use PhpParser\Node;
88
use PhpParser\Node\Expr\ArrowFunction;
99
use PhpParser\Node\Expr\Closure;
1010
use PhpParser\Node\Expr\MethodCall;
1111
use PhpParser\Node\Expr\StaticCall;
12-
use PhpParser\Node\Stmt\Class_;
1312
use PhpParser\Node\Stmt\ClassMethod;
1413
use PhpParser\NodeVisitor;
1514
use Rector\PHPUnit\Enum\PHPUnitClassName;
@@ -19,7 +18,7 @@
1918
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
2019

2120
/**
22-
* @see \Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\AssertClassToThisAssertRector\AssertClassToThisAssertRectorTest
21+
* @see \Rector\PHPUnit\Tests\CodeQuality\Rector\ClassMethod\AssertClassToThisAssertRector\AssertClassToThisAssertRectorTest
2322
*/
2423
final class AssertClassToThisAssertRector extends AbstractRector
2524
{
@@ -66,23 +65,26 @@ public function run()
6665
*/
6766
public function getNodeTypes(): array
6867
{
69-
return [Class_::class];
68+
return [ClassMethod::class];
7069
}
7170

7271
/**
73-
* @param Class_ $node
72+
* @param ClassMethod $node
7473
*/
7574
public function refactor(Node $node): ?Node
7675
{
76+
if ($node->isStatic()) {
77+
return null;
78+
}
79+
7780
if (! $this->testsNodeAnalyzer->isInTestClass($node)) {
7881
return null;
7982
}
8083

8184
$hasChanged = false;
8285

8386
$this->traverseNodesWithCallable($node, function (Node $node) use (&$hasChanged): int|null|MethodCall {
84-
$isInsideStaticFunctionLike = ($node instanceof ClassMethod && $node->isStatic()) || (($node instanceof Closure || $node instanceof ArrowFunction) && $node->static);
85-
if ($isInsideStaticFunctionLike) {
87+
if (($node instanceof Closure || $node instanceof ArrowFunction) && $node->static) {
8688
return NodeVisitor::DONT_TRAVERSE_CURRENT_AND_CHILDREN;
8789
}
8890

0 commit comments

Comments
 (0)