diff --git a/src/Rules/PHPUnit/AssertRuleHelper.php b/src/Rules/PHPUnit/AssertRuleHelper.php index 5971fc0..5fb958d 100644 --- a/src/Rules/PHPUnit/AssertRuleHelper.php +++ b/src/Rules/PHPUnit/AssertRuleHelper.php @@ -50,6 +50,7 @@ public static function isMethodOrStaticCallOnAssert(Node $node, Scope $scope): b public static function hasNamedOrUnpackedArguments(CallLike $call): bool { foreach ($call->getArgs() as $arg) { + // PHPUnit does not support named arguments for most of its APIs, e.g. assert*. if ($arg->name !== null || $arg->unpack) { return true; } diff --git a/src/Rules/PHPUnit/AssertSameWithCountRule.php b/src/Rules/PHPUnit/AssertSameWithCountRule.php index 2a5a765..196037c 100644 --- a/src/Rules/PHPUnit/AssertSameWithCountRule.php +++ b/src/Rules/PHPUnit/AssertSameWithCountRule.php @@ -5,6 +5,7 @@ use Countable; use PhpParser\Node; use PhpParser\Node\Expr\CallLike; +use PhpParser\NodeAbstract; use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; @@ -50,6 +51,21 @@ public function processNode(Node $node, Scope $scope): array return [ RuleErrorBuilder::message('You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).') ->identifier('phpunit.assertCount') + ->fixNode($node, static function (CallLike $node) use ($scope) { + if (AssertRuleHelper::hasNamedOrUnpackedArguments($node)) { + return $node; + } + + $newArgs = self::rewriteArgs($node->args, $scope); + if ($newArgs === null) { + return $node; + } + + $node->name = new Node\Identifier('assertCount'); + $node->args = $newArgs; + + return $node; + }) ->build(), ]; } @@ -58,6 +74,21 @@ public function processNode(Node $node, Scope $scope): array return [ RuleErrorBuilder::message('You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, $variable->count()).') ->identifier('phpunit.assertCount') + ->fixNode($node, static function (CallLike $node) use ($scope) { + if (AssertRuleHelper::hasNamedOrUnpackedArguments($node)) { + return $node; + } + + $newArgs = self::rewriteArgs($node->args, $scope); + if ($newArgs === null) { + return $node; + } + + $node->name = new Node\Identifier('assertCount'); + $node->args = $newArgs; + + return $node; + }) ->build(), ]; } @@ -109,4 +140,48 @@ private static function isNormalCount(Node\Expr\FuncCall $countFuncCall, Type $c return $isNormalCount; } + /** + * @template T of NodeAbstract + * @param array $args + * @return list|null + */ + private static function rewriteArgs(array $args, Scope $scope): ?array + { + $newArgs = []; + foreach ($args as $i => $arg) { + if (!$arg instanceof Node\Arg) { + $newArgs[] = $arg; + continue; + } + + if ($i !== 1 || !$arg->value instanceof CallLike) { + $newArgs[] = $arg; + continue; + } + + $callLike = $arg->value; + + // for now skip more complex cases + if (AssertRuleHelper::hasNamedOrUnpackedArguments($callLike)) { + return null; + } + + if (self::isCountFunctionCall($callLike, $scope)) { + if (count($callLike->getArgs()) !== 1) { + return null; + } + + $newArgs[] = new Node\Arg($callLike->getArgs()[0]->value); + continue; + } elseif (self::isCountableMethodCall($callLike, $scope)) { + $newArgs[] = new Node\Arg($callLike->var); + continue; + } + + return null; + } + + return $newArgs; + } + } diff --git a/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php b/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php index dfb940c..188a9cd 100644 --- a/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php +++ b/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php @@ -4,6 +4,7 @@ use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; +use const PHP_VERSION_ID; /** * @extends RuleTestCase @@ -42,6 +43,39 @@ public function testRule(): void ]); } + public function testFix(): void + { + $this->fix(__DIR__ . '/data/assert-same-count-fixable.php', __DIR__ . '/data/assert-same-count-fixable.php.fixed'); + // we don't expect any fixes for named arguments + $this->fix(__DIR__ . '/data/assert-same-count-named-arguments.php', __DIR__ . '/data/assert-same-count-named-arguments.php'); + } + + public function testNamedArguments(): void + { + if (PHP_VERSION_ID < 80000) { + self::markTestSkipped('Named arguments require PHP 8.0.'); + } + + $this->analyse([__DIR__ . '/data/assert-same-count-named-arguments.php'], [ + [ + 'You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).', + 12, + ], + [ + 'You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).', + 13, + ], + [ + 'You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).', + 15, + ], + [ + 'You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).', + 17, + ], + ]); + } + /** * @return string[] */ diff --git a/tests/Rules/PHPUnit/data/assert-same-count-fixable.php b/tests/Rules/PHPUnit/data/assert-same-count-fixable.php new file mode 100644 index 0000000..85e4163 --- /dev/null +++ b/tests/Rules/PHPUnit/data/assert-same-count-fixable.php @@ -0,0 +1,54 @@ +assertSame(5, count([1, 2, 3])); + } + + public function testAssertSameWithCountCallsInOtherArguments($expected, $actual, \Countable $countable) + { + $this->assertSame(count($expected), count($actual)); + $this->assertSame(count($expected), count($actual), getMessage()); + $this->assertSame(count($expected), $countable->count(), getMessage()); + } + + public function testAssertSameWithCountUnpackedArguments(array $args) + { + $this->assertSame(5, count(...$args)); + } + + public function testAssertSameWithCountRecursive($x) + { + $this->assertSame(5, count([1, 2, 3, $x], COUNT_RECURSIVE)); + } + + public function testAssertSameWithCountMethodForCountableVariableIsNotOK() + { + $bar = new \ExampleTestCaseFix\Bar (); + + $this->assertSame(5, $bar->count()); + } + + public function testAssertSameWithCountMethodForCountablePropertyFetchIsNotOK() + { + $foo = new \stdClass(); + $foo->bar = new Bar (); + + $this->assertSame(5, $foo->bar->count()); + } + +} + +class Bar implements \Countable { + public function count(): int + { + return 1; + } +} diff --git a/tests/Rules/PHPUnit/data/assert-same-count-fixable.php.fixed b/tests/Rules/PHPUnit/data/assert-same-count-fixable.php.fixed new file mode 100644 index 0000000..8a2dc42 --- /dev/null +++ b/tests/Rules/PHPUnit/data/assert-same-count-fixable.php.fixed @@ -0,0 +1,54 @@ +assertCount(5, [1, 2, 3]); + } + + public function testAssertSameWithCountCallsInOtherArguments($expected, $actual, \Countable $countable) + { + $this->assertCount(count($expected), $actual); + $this->assertCount(count($expected), $actual, getMessage()); + $this->assertCount(count($expected), $countable, getMessage()); + } + + public function testAssertSameWithCountUnpackedArguments(array $args) + { + $this->assertSame(5, count(...$args)); + } + + public function testAssertSameWithCountRecursive($x) + { + $this->assertSame(5, count([1, 2, 3, $x], COUNT_RECURSIVE)); + } + + public function testAssertSameWithCountMethodForCountableVariableIsNotOK() + { + $bar = new \ExampleTestCaseFix\Bar (); + + $this->assertCount(5, $bar); + } + + public function testAssertSameWithCountMethodForCountablePropertyFetchIsNotOK() + { + $foo = new \stdClass(); + $foo->bar = new Bar (); + + $this->assertCount(5, $foo->bar); + } + +} + +class Bar implements \Countable { + public function count(): int + { + return 1; + } +} diff --git a/tests/Rules/PHPUnit/data/assert-same-count-named-arguments.php b/tests/Rules/PHPUnit/data/assert-same-count-named-arguments.php new file mode 100644 index 0000000..7f0402e --- /dev/null +++ b/tests/Rules/PHPUnit/data/assert-same-count-named-arguments.php @@ -0,0 +1,30 @@ += 8.0 + +namespace ExampleTestCaseFixNamedArguments; + +use function count; + +class AssertSameWithCountTestCase extends \PHPUnit\Framework\TestCase +{ + + public function skipNamedArguments(Bar $bar): void + { + $this->assertSame(expected: 5, actual: count([1, 2, 3]), message: 'message'); + $this->assertSame(message: 'message', actual: count(value: [1, 2, 3]), expected: 5); + self::assertSame(actual: $bar->count(), expected: 5); + $this->assertSame(5, actual: count([1, 2, 3]), message: 'message'); + + $this->assertSame(5, count(value: [1, 2, 3]), 'message'); + } + +} + +class Bar implements \Countable +{ + + public function count(): int + { + return 1; + } + +}