Skip to content

Commit ab9e383

Browse files
authored
Ensure that rule PreferPHPUnitSelfCall refactor call only for static methods, not any (#482)
* 📦 Move check for static above the method reflection resolving logic Avoid unnecessary resolving * 🐛 [9138] Ensure that rule `PreferPHPUnitSelfCall` refactor call only for static methods, not any rectorphp/rector#9138 * 📖 [9138] Explain fixture behavior with comment rectorphp/rector#9138 * 📦 Rename fixture to be clear about semantics * 📦 Rename class in fixture to be consistent with filename * 📦 Rename fixture file * 📦 Rename enum to ensure that there is no static methods inside, check for static call before checking with enum As suggested in review comment #482 (comment) * 📦 Rename enum to be clear about semantics and short As suggested in the review comment #482 (comment) * 📦 Revert check for static call as unneeded here As of review comment #482 (comment)
1 parent 10f60e1 commit ab9e383

File tree

5 files changed

+48
-44
lines changed

5 files changed

+48
-44
lines changed

rules-tests/CodeQuality/Rector/Class_/PreferPHPUnitSelfCallRector/Fixture/include_exceptions.php.inc

Lines changed: 0 additions & 35 deletions
This file was deleted.
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?php
2+
3+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\PreferPHPUnitSelfCallRector\Fixture;
4+
5+
use PHPUnit\Framework\TestCase;
6+
7+
final class SkipExceptions extends TestCase
8+
{
9+
public function testMe()
10+
{
11+
// this calls should be preserved with $this, because methods are not static
12+
$this->expectException(\RuntimeException::class);
13+
$this->expectExceptionMessage('foo');
14+
$this->expectExceptionCode(123);
15+
}
16+
}

rules/CodeQuality/Enum/NonAssertStaticableMethods.php renamed to rules/CodeQuality/Enum/NonAssertNonStaticMethods.php

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

55
namespace Rector\PHPUnit\CodeQuality\Enum;
66

7-
final class NonAssertStaticableMethods
7+
final class NonAssertNonStaticMethods
88
{
99
/**
1010
* @var string[]

rules/CodeQuality/NodeAnalyser/AssertMethodAnalyzer.php

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,11 @@
77
use PhpParser\Node\Expr\MethodCall;
88
use PhpParser\Node\Expr\StaticCall;
99
use PHPStan\Reflection\ClassReflection;
10+
use PHPStan\Reflection\ExtendedMethodReflection;
1011
use PHPStan\Type\ObjectType;
1112
use Rector\NodeNameResolver\NodeNameResolver;
1213
use Rector\NodeTypeResolver\NodeTypeResolver;
13-
use Rector\PHPUnit\CodeQuality\Enum\NonAssertStaticableMethods;
14+
use Rector\PHPUnit\CodeQuality\Enum\NonAssertNonStaticMethods;
1415
use Rector\PHPUnit\Enum\PHPUnitClassName;
1516
use Rector\Reflection\ReflectionResolver;
1617

@@ -36,26 +37,48 @@ public function detectTestCaseCall(MethodCall|StaticCall $call): bool
3637
$methodName = $this->nodeNameResolver->getName($call->name);
3738
if (! str_starts_with((string) $methodName, 'assert') && ! in_array(
3839
$methodName,
39-
NonAssertStaticableMethods::ALL
40+
NonAssertNonStaticMethods::ALL,
41+
true
4042
)) {
4143
return false;
4244
}
4345

44-
$classReflection = $this->reflectionResolver->resolveClassReflection($call);
45-
if (! $classReflection instanceof ClassReflection) {
46+
if ($call instanceof StaticCall && ! $this->nodeNameResolver->isNames($call->class, ['static', 'self'])) {
4647
return false;
4748
}
4849

49-
if ($call instanceof StaticCall && ! $this->nodeNameResolver->isNames($call->class, ['static', 'self'])) {
50+
$extendedMethodReflection = $this->resolveMethodReflection($call);
51+
if (! $extendedMethodReflection instanceof ExtendedMethodReflection) {
5052
return false;
5153
}
5254

53-
$extendedMethodReflection = $classReflection->getNativeMethod($methodName);
54-
5555
// only handle methods in TestCase or Assert class classes
5656
$declaringClassName = $extendedMethodReflection->getDeclaringClass()
5757
->getName();
5858

5959
return in_array($declaringClassName, [PHPUnitClassName::TEST_CASE, PHPUnitClassName::ASSERT]);
6060
}
61+
62+
public function detectTestCaseCallForStatic(MethodCall $methodCall): bool
63+
{
64+
if (! $this->detectTestCaseCall($methodCall)) {
65+
return false;
66+
}
67+
68+
$extendedMethodReflection = $this->resolveMethodReflection($methodCall);
69+
70+
return $extendedMethodReflection instanceof ExtendedMethodReflection && $extendedMethodReflection->isStatic();
71+
}
72+
73+
private function resolveMethodReflection(MethodCall|StaticCall $call): ?ExtendedMethodReflection
74+
{
75+
$methodName = $this->nodeNameResolver->getName($call->name);
76+
77+
$classReflection = $this->reflectionResolver->resolveClassReflection($call);
78+
if (! $classReflection instanceof ClassReflection) {
79+
return null;
80+
}
81+
82+
return $classReflection->getNativeMethod($methodName);
83+
}
6184
}

rules/CodeQuality/Rector/Class_/PreferPHPUnitSelfCallRector.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public function refactor(Node $node): ?Node
8383
return null;
8484
}
8585

86-
if (! $this->assertMethodAnalyzer->detectTestCaseCall($node)) {
86+
if (! $this->assertMethodAnalyzer->detectTestCaseCallForStatic($node)) {
8787
return null;
8888
}
8989

0 commit comments

Comments
 (0)