Skip to content

Commit 1d2f1b6

Browse files
TomasVotrubaTomasVotruba
committed
tidy up AssertFuncCallToPHPUnitAssertRector
1 parent d22b617 commit 1d2f1b6

File tree

7 files changed

+149
-119
lines changed

7 files changed

+149
-119
lines changed

‎phpstan.neon‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,3 +51,7 @@ parameters:
5151
message: '#Property PhpParser\\Node\\Identifier\:\:\$name \(non\-empty\-string\) does not accept string#'
5252
path: rules/CodeQuality/Rector/ClassMethod/ReplaceTestFunctionPrefixWithAttributeRector.php
5353

54+
# handle next
55+
-
56+
identifier: symplify.forbiddenNode
57+
message: '#switch#'

‎rules-tests/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector/Fixture/assert_inside_static_closure.php.inc‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<?php
2+
3+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\FuncCall\AssertFuncCallToPHPUnitAssertRector\Fixture;
4+
5+
use PHPUnit\Framework\TestCase;
6+
7+
final class AssertInsideStaticClosure extends TestCase
8+
{
9+
public function some($response)
10+
{
11+
static function () use ($response) {
12+
assert((bool) $response);
13+
};
14+
}
15+
}
16+
17+
?>
18+
-----
19+
<?php
20+
21+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\FuncCall\AssertFuncCallToPHPUnitAssertRector\Fixture;
22+
23+
use PHPUnit\Framework\TestCase;
24+
25+
final class AssertInsideStaticClosure extends TestCase
26+
{
27+
public function some($response)
28+
{
29+
static function () use ($response) {
30+
\PHPUnit\Framework\Assert::assertTrue((bool) $response);
31+
};
32+
}
33+
}
34+
35+
?>

‎rules-tests/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector/Fixture/skip_inside_static_closure.php.inc‎

Lines changed: 0 additions & 15 deletions
This file was deleted.

‎rules-tests/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector/Fixture/skip_outside_test_case.php.inc‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?php
2+
3+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\FuncCall\AssertFuncCallToPHPUnitAssertRector\Fixture;
4+
5+
final class SkipOutsideTestCase
6+
{
7+
public function some($response)
8+
{
9+
assert((bool) $response);
10+
}
11+
}

‎rules-tests/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector/Fixture/static_assert_in_static_method.php.inc‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,14 @@ final class StaticAssertInStaticMethodTest extends TestCase
2121

2222
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\FuncCall\AssertFuncCallToPHPUnitAssertRector\Fixture;
2323

24-
use PHPUnit\Framework\Assert;
2524
use PHPUnit\Framework\TestCase;
2625

2726
final class StaticAssertInStaticMethodTest extends TestCase
2827
{
2928
private static function getExceptionMessage(string $fqcn): string
3029
{
3130
$exception = new $fqcn();
32-
Assert::assertInstanceOf($exception, \Exception::class);
31+
\PHPUnit\Framework\Assert::assertInstanceOf(\Exception::class, $exception);
3332

3433
return $exception->getMessage();
3534
}

‎rules/CodeQuality/Rector/Class_/AddParamTypeFromDependsRector.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66

77
use PhpParser\Node;
88
use PhpParser\Node\Attribute;
9+
use PhpParser\Node\ComplexType;
10+
use PhpParser\Node\Identifier;
11+
use PhpParser\Node\Name;
912
use PhpParser\Node\Scalar\String_;
1013
use PhpParser\Node\Stmt\Class_;
1114
use PhpParser\Node\Stmt\ClassMethod;
@@ -134,7 +137,7 @@ public function refactor(Node $node): ?Node
134137
private function resolveReturnTypeOfDependsMethod(
135138
ClassMethod $classMethod,
136139
Class_ $class
137-
): Node\ComplexType|Node\Identifier|Node\Name|null {
140+
): ComplexType|Identifier|Name|null {
138141
$dependsMethodName = $this->resolveDependsAnnotationOrAttributeMethod($classMethod);
139142

140143
if ($dependsMethodName === null || $dependsMethodName === '') {

‎rules/CodeQuality/Rector/FuncCall/AssertFuncCallToPHPUnitAssertRector.php‎

Lines changed: 94 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,7 @@
1919
use PhpParser\Node\Expr\StaticCall;
2020
use PhpParser\Node\Expr\Variable;
2121
use PhpParser\Node\Name\FullyQualified;
22-
use PhpParser\Node\Stmt\ClassMethod;
23-
use PhpParser\NodeVisitor;
22+
use PhpParser\Node\Stmt\Class_;
2423
use PHPStan\Reflection\ClassReflection;
2524
use Rector\PhpParser\Node\Value\ValueResolver;
2625
use Rector\PHPStan\ScopeFetcher;
@@ -81,105 +80,115 @@ public function test()
8180
*/
8281
public function getNodeTypes(): array
8382
{
84-
return [ClassMethod::class, Closure::class, FuncCall::class];
83+
return [Class_::class];
8584
}
8685

8786
/**
88-
* @param ClassMethod|Closure|FuncCall $node
89-
* @return StaticCall|MethodCall|null|NodeVisitor::DONT_TRAVERSE_CHILDREN
87+
* @param Class_ $node
9088
*/
91-
public function refactor(Node $node): StaticCall|MethodCall|null|int
89+
public function refactor(Node $node): ?Class_
9290
{
93-
// @todo handle only in test classes!
94-
95-
// most liekly hook to ClassMethod :) + traverse then
96-
$this->testsNodeAnalyzer->isInTestClass($node);
97-
98-
// if ($node instanceof ClassMethod) {
99-
// if ($node->isStatic()) {
100-
// return NodeVisitor::DONT_TRAVERSE_CHILDREN;
101-
// }
102-
//
103-
// return null;
104-
// }
105-
106-
// if ($node instanceof Closure) {
107-
// if ($node->static) {
108-
// return NodeVisitor::DONT_TRAVERSE_CHILDREN;
109-
// }
110-
//
111-
// return null;
112-
// }
113-
114-
if ($node->isFirstClassCallable()) {
91+
if (! $this->testsNodeAnalyzer->isInTestClass($node) && ! $this->isBehatContext($node)) {
11592
return null;
11693
}
11794

118-
if (! $this->isName($node, 'assert')) {
119-
return null;
120-
}
121-
122-
if (! $this->isTestFilePath($node) && ! $this->isBehatContext($node)) {
123-
return null;
124-
}
125-
126-
$comparedExpr = $node->getArgs()[0]
127-
->value;
95+
$hasChanged = false;
12896

129-
if ($comparedExpr instanceof Equal) {
130-
$methodName = AssertMethod::ASSERT_EQUALS;
131-
$exprs = [$comparedExpr->right, $comparedExpr->left];
132-
133-
} elseif ($comparedExpr instanceof Identical) {
134-
$methodName = AssertMethod::ASSERT_SAME;
135-
$exprs = [$comparedExpr->right, $comparedExpr->left];
136-
137-
} elseif ($comparedExpr instanceof NotIdentical) {
138-
if ($this->valueResolver->isNull($comparedExpr->right)) {
139-
$methodName = 'assertNotNull';
140-
$exprs = [$comparedExpr->left];
141-
} else {
142-
return null;
143-
}
144-
145-
} elseif ($comparedExpr instanceof Bool_) {
146-
$methodName = 'assertTrue';
147-
$exprs = [$comparedExpr];
148-
} elseif ($comparedExpr instanceof FuncCall) {
149-
if ($this->isName($comparedExpr, 'method_exists')) {
150-
$methodName = 'assertTrue';
151-
$exprs = [$comparedExpr];
152-
} else {
153-
return null;
154-
}
155-
} elseif ($comparedExpr instanceof Instanceof_) {
156-
// outside TestCase
157-
$methodName = 'assertInstanceOf';
158-
$exprs = [];
159-
160-
if ($comparedExpr->class instanceof FullyQualified) {
161-
$classConstFetch = new ClassConstFetch($comparedExpr->class, 'class');
162-
$exprs[] = $classConstFetch;
163-
} else {
164-
return null;
97+
foreach ($node->getMethods() as $classMethod) {
98+
if ($classMethod->stmts === null) {
99+
continue;
165100
}
166101

167-
$exprs[] = $comparedExpr->expr;
168-
} else {
169-
return null;
102+
$useStaticAssert = $classMethod->isStatic() ?: $this->isBehatContext($node);
103+
104+
$this->traverseNodesWithCallable($classMethod->stmts, function (Node $node) use (
105+
&$useStaticAssert,
106+
&$hasChanged
107+
): null|MethodCall|StaticCall {
108+
if ($node instanceof Closure && $node->static) {
109+
$useStaticAssert = true;
110+
return null;
111+
}
112+
113+
if (! $node instanceof FuncCall) {
114+
return null;
115+
}
116+
117+
if ($node->isFirstClassCallable()) {
118+
return null;
119+
}
120+
121+
if (! $this->isName($node, 'assert')) {
122+
return null;
123+
}
124+
125+
$comparedExpr = $node->getArgs()[0]
126+
->value;
127+
128+
if ($comparedExpr instanceof Equal) {
129+
$methodName = AssertMethod::ASSERT_EQUALS;
130+
$exprs = [$comparedExpr->right, $comparedExpr->left];
131+
132+
} elseif ($comparedExpr instanceof Identical) {
133+
$methodName = AssertMethod::ASSERT_SAME;
134+
$exprs = [$comparedExpr->right, $comparedExpr->left];
135+
136+
} elseif ($comparedExpr instanceof NotIdentical) {
137+
if ($this->valueResolver->isNull($comparedExpr->right)) {
138+
$methodName = 'assertNotNull';
139+
$exprs = [$comparedExpr->left];
140+
} else {
141+
return null;
142+
}
143+
144+
} elseif ($comparedExpr instanceof Bool_) {
145+
$methodName = 'assertTrue';
146+
$exprs = [$comparedExpr];
147+
} elseif ($comparedExpr instanceof FuncCall) {
148+
if ($this->isName($comparedExpr, 'method_exists')) {
149+
$methodName = 'assertTrue';
150+
$exprs = [$comparedExpr];
151+
} else {
152+
return null;
153+
}
154+
} elseif ($comparedExpr instanceof Instanceof_) {
155+
// outside TestCase
156+
$methodName = 'assertInstanceOf';
157+
$exprs = [];
158+
159+
if ($comparedExpr->class instanceof FullyQualified) {
160+
$classConstFetch = new ClassConstFetch($comparedExpr->class, 'class');
161+
$exprs[] = $classConstFetch;
162+
} else {
163+
return null;
164+
}
165+
166+
$exprs[] = $comparedExpr->expr;
167+
} else {
168+
return null;
169+
}
170+
171+
// is there a comment message
172+
if (isset($node->getArgs()[1])) {
173+
$exprs[] = $node->getArgs()[1]->value;
174+
}
175+
176+
$hasChanged = true;
177+
178+
return $this->createAssertCall($methodName, $exprs, $useStaticAssert);
179+
});
170180
}
171181

172-
// is there a comment message
173-
if (isset($node->getArgs()[1])) {
174-
$exprs[] = $node->getArgs()[1]->value;
182+
if (! $hasChanged) {
183+
return null;
175184
}
176185

177-
return $this->createCall($node, $methodName, $exprs);
186+
return $node;
178187
}
179188

180-
private function isBehatContext(FuncCall $funcCall): bool
189+
private function isBehatContext(Class_ $class): bool
181190
{
182-
$scope = ScopeFetcher::fetch($funcCall);
191+
$scope = ScopeFetcher::fetch($class);
183192
if (! $scope->getClassReflection() instanceof ClassReflection) {
184193
return false;
185194
}
@@ -191,33 +200,17 @@ private function isBehatContext(FuncCall $funcCall): bool
191200
return str_ends_with($className, 'Context');
192201
}
193202

194-
private function isTestFilePath(FuncCall $funcCall): bool
195-
{
196-
$scope = ScopeFetcher::fetch($funcCall);
197-
if (! $scope->getClassReflection() instanceof ClassReflection) {
198-
return false;
199-
}
200-
201-
$className = $scope->getClassReflection()
202-
->getName();
203-
if (str_ends_with($className, 'Test')) {
204-
return true;
205-
}
206-
207-
return str_ends_with($className, 'TestCase');
208-
}
209-
210203
/**
211204
* @param Expr[] $exprs
212205
*/
213-
private function createCall(FuncCall $funcCall, string $methodName, array $exprs): MethodCall|StaticCall
206+
private function createAssertCall(string $methodName, array $exprs, bool $useStaticAssert): MethodCall|StaticCall
214207
{
215208
$args = [];
216209
foreach ($exprs as $expr) {
217210
$args[] = new Arg($expr);
218211
}
219212

220-
if ($this->isBehatContext($funcCall)) {
213+
if ($useStaticAssert) {
221214
$assertFullyQualified = new FullyQualified(PHPUnitClassName::ASSERT);
222215
return new StaticCall($assertFullyQualified, $methodName, $args);
223216
}

0 commit comments

Comments
 (0)