Skip to content

Commit b356ae5

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

File tree

5 files changed

+130
-104
lines changed

5 files changed

+130
-104
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/skip_inside_static_closure.php.inc‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,23 @@ final class AssertBoolInTest extends TestCase
1313
};
1414
}
1515
}
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 AssertBoolInTest 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_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/FuncCall/AssertFuncCallToPHPUnitAssertRector.php‎

Lines changed: 94 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,13 @@
1212
use PhpParser\Node\Expr\BinaryOp\NotIdentical;
1313
use PhpParser\Node\Expr\Cast\Bool_;
1414
use PhpParser\Node\Expr\ClassConstFetch;
15-
use PhpParser\Node\Expr\Closure;
1615
use PhpParser\Node\Expr\FuncCall;
1716
use PhpParser\Node\Expr\Instanceof_;
1817
use PhpParser\Node\Expr\MethodCall;
1918
use PhpParser\Node\Expr\StaticCall;
2019
use PhpParser\Node\Expr\Variable;
2120
use PhpParser\Node\Name\FullyQualified;
22-
use PhpParser\Node\Stmt\ClassMethod;
23-
use PhpParser\NodeVisitor;
21+
use PhpParser\Node\Stmt\Class_;
2422
use PHPStan\Reflection\ClassReflection;
2523
use Rector\PhpParser\Node\Value\ValueResolver;
2624
use Rector\PHPStan\ScopeFetcher;
@@ -81,105 +79,115 @@ public function test()
8179
*/
8280
public function getNodeTypes(): array
8381
{
84-
return [ClassMethod::class, Closure::class, FuncCall::class];
82+
return [Class_::class];
8583
}
8684

8785
/**
88-
* @param ClassMethod|Closure|FuncCall $node
89-
* @return StaticCall|MethodCall|null|NodeVisitor::DONT_TRAVERSE_CHILDREN
86+
* @param Class_ $node
9087
*/
91-
public function refactor(Node $node): StaticCall|MethodCall|null|int
88+
public function refactor(Node $node): ?Class_
9289
{
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()) {
90+
if (! $this->testsNodeAnalyzer->isInTestClass($node) && ! $this->isBehatContext($node)) {
11591
return null;
11692
}
11793

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;
94+
$hasChanged = false;
12895

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;
96+
foreach ($node->getMethods() as $classMethod) {
97+
if ($classMethod->stmts === null) {
98+
continue;
16599
}
166100

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

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

177-
return $this->createCall($node, $methodName, $exprs);
185+
return $node;
178186
}
179187

180-
private function isBehatContext(FuncCall $funcCall): bool
188+
private function isBehatContext(Class_ $class): bool
181189
{
182-
$scope = ScopeFetcher::fetch($funcCall);
190+
$scope = ScopeFetcher::fetch($class);
183191
if (! $scope->getClassReflection() instanceof ClassReflection) {
184192
return false;
185193
}
@@ -191,33 +199,17 @@ private function isBehatContext(FuncCall $funcCall): bool
191199
return str_ends_with($className, 'Context');
192200
}
193201

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-
210202
/**
211203
* @param Expr[] $exprs
212204
*/
213-
private function createCall(FuncCall $funcCall, string $methodName, array $exprs): MethodCall|StaticCall
205+
private function createAssertCall(string $methodName, array $exprs, bool $useStaticAssert): MethodCall|StaticCall
214206
{
215207
$args = [];
216208
foreach ($exprs as $expr) {
217209
$args[] = new Arg($expr);
218210
}
219211

220-
if ($this->isBehatContext($funcCall)) {
212+
if ($useStaticAssert) {
221213
$assertFullyQualified = new FullyQualified(PHPUnitClassName::ASSERT);
222214
return new StaticCall($assertFullyQualified, $methodName, $args);
223215
}

0 commit comments

Comments
 (0)