Skip to content

Commit b73700a

Browse files
staabmstaabm
authored
Make AssertSameWithCountRule auto-fixable (#259)
* Make AssertSameWithCountRule auto-fixable * skip named args * skip inner named args * refactor * skip named args for auto-fixing for now * Update AssertSameWithCountRule.php * simplify * Update AssertSameWithCountRule.php * we don't expect any fixes for named arguments * Updated AssertSameWithCountRule to unwrap only the actual-value argument. * skip unpack * reuse helper * Update AssertSameWithCountRule.php
1 parent ac3237e commit b73700a

6 files changed

Lines changed: 248 additions & 0 deletions

File tree

‎src/Rules/PHPUnit/AssertRuleHelper.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ public static function isMethodOrStaticCallOnAssert(Node $node, Scope $scope): b
5050
public static function hasNamedOrUnpackedArguments(CallLike $call): bool
5151
{
5252
foreach ($call->getArgs() as $arg) {
53+
// PHPUnit does not support named arguments for most of its APIs, e.g. assert*.
5354
if ($arg->name !== null || $arg->unpack) {
5455
return true;
5556
}

‎src/Rules/PHPUnit/AssertSameWithCountRule.php‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use Countable;
66
use PhpParser\Node;
77
use PhpParser\Node\Expr\CallLike;
8+
use PhpParser\NodeAbstract;
89
use PHPStan\Analyser\Scope;
910
use PHPStan\Rules\Rule;
1011
use PHPStan\Rules\RuleErrorBuilder;
@@ -50,6 +51,21 @@ public function processNode(Node $node, Scope $scope): array
5051
return [
5152
RuleErrorBuilder::message('You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).')
5253
->identifier('phpunit.assertCount')
54+
->fixNode($node, static function (CallLike $node) use ($scope) {
55+
if (AssertRuleHelper::hasNamedOrUnpackedArguments($node)) {
56+
return $node;
57+
}
58+
59+
$newArgs = self::rewriteArgs($node->args, $scope);
60+
if ($newArgs === null) {
61+
return $node;
62+
}
63+
64+
$node->name = new Node\Identifier('assertCount');
65+
$node->args = $newArgs;
66+
67+
return $node;
68+
})
5369
->build(),
5470
];
5571
}
@@ -58,6 +74,21 @@ public function processNode(Node $node, Scope $scope): array
5874
return [
5975
RuleErrorBuilder::message('You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, $variable->count()).')
6076
->identifier('phpunit.assertCount')
77+
->fixNode($node, static function (CallLike $node) use ($scope) {
78+
if (AssertRuleHelper::hasNamedOrUnpackedArguments($node)) {
79+
return $node;
80+
}
81+
82+
$newArgs = self::rewriteArgs($node->args, $scope);
83+
if ($newArgs === null) {
84+
return $node;
85+
}
86+
87+
$node->name = new Node\Identifier('assertCount');
88+
$node->args = $newArgs;
89+
90+
return $node;
91+
})
6192
->build(),
6293
];
6394
}
@@ -109,4 +140,48 @@ private static function isNormalCount(Node\Expr\FuncCall $countFuncCall, Type $c
109140
return $isNormalCount;
110141
}
111142

143+
/**
144+
* @template T of NodeAbstract
145+
* @param array<T> $args
146+
* @return list<T|Node\Arg>|null
147+
*/
148+
private static function rewriteArgs(array $args, Scope $scope): ?array
149+
{
150+
$newArgs = [];
151+
foreach ($args as $i => $arg) {
152+
if (!$arg instanceof Node\Arg) {
153+
$newArgs[] = $arg;
154+
continue;
155+
}
156+
157+
if ($i !== 1 || !$arg->value instanceof CallLike) {
158+
$newArgs[] = $arg;
159+
continue;
160+
}
161+
162+
$callLike = $arg->value;
163+
164+
// for now skip more complex cases
165+
if (AssertRuleHelper::hasNamedOrUnpackedArguments($callLike)) {
166+
return null;
167+
}
168+
169+
if (self::isCountFunctionCall($callLike, $scope)) {
170+
if (count($callLike->getArgs()) !== 1) {
171+
return null;
172+
}
173+
174+
$newArgs[] = new Node\Arg($callLike->getArgs()[0]->value);
175+
continue;
176+
} elseif (self::isCountableMethodCall($callLike, $scope)) {
177+
$newArgs[] = new Node\Arg($callLike->var);
178+
continue;
179+
}
180+
181+
return null;
182+
}
183+
184+
return $newArgs;
185+
}
186+
112187
}

‎tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use PHPStan\Rules\Rule;
66
use PHPStan\Testing\RuleTestCase;
7+
use const PHP_VERSION_ID;
78

89
/**
910
* @extends RuleTestCase<AssertSameWithCountRule>
@@ -42,6 +43,39 @@ public function testRule(): void
4243
]);
4344
}
4445

46+
public function testFix(): void
47+
{
48+
$this->fix(__DIR__ . '/data/assert-same-count-fixable.php', __DIR__ . '/data/assert-same-count-fixable.php.fixed');
49+
// we don't expect any fixes for named arguments
50+
$this->fix(__DIR__ . '/data/assert-same-count-named-arguments.php', __DIR__ . '/data/assert-same-count-named-arguments.php');
51+
}
52+
53+
public function testNamedArguments(): void
54+
{
55+
if (PHP_VERSION_ID < 80000) {
56+
self::markTestSkipped('Named arguments require PHP 8.0.');
57+
}
58+
59+
$this->analyse([__DIR__ . '/data/assert-same-count-named-arguments.php'], [
60+
[
61+
'You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).',
62+
12,
63+
],
64+
[
65+
'You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).',
66+
13,
67+
],
68+
[
69+
'You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).',
70+
15,
71+
],
72+
[
73+
'You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).',
74+
17,
75+
],
76+
]);
77+
}
78+
4579
/**
4680
* @return string[]
4781
*/

‎tests/Rules/PHPUnit/data/assert-same-count-fixable.php‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace ExampleTestCaseFix;
4+
5+
use const COUNT_RECURSIVE;
6+
7+
class AssertSameWithCountTestCase extends \PHPUnit\Framework\TestCase
8+
{
9+
10+
public function testAssertSameWithCount()
11+
{
12+
$this->assertSame(5, count([1, 2, 3]));
13+
}
14+
15+
public function testAssertSameWithCountCallsInOtherArguments($expected, $actual, \Countable $countable)
16+
{
17+
$this->assertSame(count($expected), count($actual));
18+
$this->assertSame(count($expected), count($actual), getMessage());
19+
$this->assertSame(count($expected), $countable->count(), getMessage());
20+
}
21+
22+
public function testAssertSameWithCountUnpackedArguments(array $args)
23+
{
24+
$this->assertSame(5, count(...$args));
25+
}
26+
27+
public function testAssertSameWithCountRecursive($x)
28+
{
29+
$this->assertSame(5, count([1, 2, 3, $x], COUNT_RECURSIVE));
30+
}
31+
32+
public function testAssertSameWithCountMethodForCountableVariableIsNotOK()
33+
{
34+
$bar = new \ExampleTestCaseFix\Bar ();
35+
36+
$this->assertSame(5, $bar->count());
37+
}
38+
39+
public function testAssertSameWithCountMethodForCountablePropertyFetchIsNotOK()
40+
{
41+
$foo = new \stdClass();
42+
$foo->bar = new Bar ();
43+
44+
$this->assertSame(5, $foo->bar->count());
45+
}
46+
47+
}
48+
49+
class Bar implements \Countable {
50+
public function count(): int
51+
{
52+
return 1;
53+
}
54+
}

‎tests/Rules/PHPUnit/data/assert-same-count-fixable.php.fixed‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace ExampleTestCaseFix;
4+
5+
use const COUNT_RECURSIVE;
6+
7+
class AssertSameWithCountTestCase extends \PHPUnit\Framework\TestCase
8+
{
9+
10+
public function testAssertSameWithCount()
11+
{
12+
$this->assertCount(5, [1, 2, 3]);
13+
}
14+
15+
public function testAssertSameWithCountCallsInOtherArguments($expected, $actual, \Countable $countable)
16+
{
17+
$this->assertCount(count($expected), $actual);
18+
$this->assertCount(count($expected), $actual, getMessage());
19+
$this->assertCount(count($expected), $countable, getMessage());
20+
}
21+
22+
public function testAssertSameWithCountUnpackedArguments(array $args)
23+
{
24+
$this->assertSame(5, count(...$args));
25+
}
26+
27+
public function testAssertSameWithCountRecursive($x)
28+
{
29+
$this->assertSame(5, count([1, 2, 3, $x], COUNT_RECURSIVE));
30+
}
31+
32+
public function testAssertSameWithCountMethodForCountableVariableIsNotOK()
33+
{
34+
$bar = new \ExampleTestCaseFix\Bar ();
35+
36+
$this->assertCount(5, $bar);
37+
}
38+
39+
public function testAssertSameWithCountMethodForCountablePropertyFetchIsNotOK()
40+
{
41+
$foo = new \stdClass();
42+
$foo->bar = new Bar ();
43+
44+
$this->assertCount(5, $foo->bar);
45+
}
46+
47+
}
48+
49+
class Bar implements \Countable {
50+
public function count(): int
51+
{
52+
return 1;
53+
}
54+
}

‎tests/Rules/PHPUnit/data/assert-same-count-named-arguments.php‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php // lint >= 8.0
2+
3+
namespace ExampleTestCaseFixNamedArguments;
4+
5+
use function count;
6+
7+
class AssertSameWithCountTestCase extends \PHPUnit\Framework\TestCase
8+
{
9+
10+
public function skipNamedArguments(Bar $bar): void
11+
{
12+
$this->assertSame(expected: 5, actual: count([1, 2, 3]), message: 'message');
13+
$this->assertSame(message: 'message', actual: count(value: [1, 2, 3]), expected: 5);
14+
self::assertSame(actual: $bar->count(), expected: 5);
15+
$this->assertSame(5, actual: count([1, 2, 3]), message: 'message');
16+
17+
$this->assertSame(5, count(value: [1, 2, 3]), 'message');
18+
}
19+
20+
}
21+
22+
class Bar implements \Countable
23+
{
24+
25+
public function count(): int
26+
{
27+
return 1;
28+
}
29+
30+
}

0 commit comments

Comments
 (0)