From dfd6b09d120c54b8d7bd36e8950116cfdf560ba7 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 11 Nov 2025 07:26:10 +0100 Subject: [PATCH 01/13] Make AssertSameWithCountRule auto-fixable --- src/Rules/PHPUnit/AssertSameWithCountRule.php | 54 +++++++++++++++++++ .../PHPUnit/AssertSameWithCountRuleTest.php | 5 ++ .../data/assert-same-count-fixable.php | 42 +++++++++++++++ .../data/assert-same-count-fixable.php.fixed | 42 +++++++++++++++ 4 files changed, 143 insertions(+) create mode 100644 tests/Rules/PHPUnit/data/assert-same-count-fixable.php create mode 100644 tests/Rules/PHPUnit/data/assert-same-count-fixable.php.fixed diff --git a/src/Rules/PHPUnit/AssertSameWithCountRule.php b/src/Rules/PHPUnit/AssertSameWithCountRule.php index 2a5a7651..744641a4 100644 --- a/src/Rules/PHPUnit/AssertSameWithCountRule.php +++ b/src/Rules/PHPUnit/AssertSameWithCountRule.php @@ -50,6 +50,17 @@ 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) { + $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 +69,17 @@ 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) { + $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 +131,36 @@ private static function isNormalCount(Node\Expr\FuncCall $countFuncCall, Type $c return $isNormalCount; } + /** + * @param array $args + * @return list + */ + private static function rewriteArgs(array $args, Scope $scope): ?array + { + $newArgs = []; + for ($i = 0; $i < count($args); $i++) { + + if ( + $args[$i] instanceof Node\Arg + && $args[$i]->value instanceof CallLike + ) { + $value = $args[$i]->value; + if (self::isCountFunctionCall($value, $scope)) { + if (count($value->getArgs()) !== 1) { + return null; + } + + $newArgs[] = new Node\Arg($value->getArgs()[0]->value); + continue; + } elseif (self::isCountableMethodCall($value, $scope)) { + $newArgs[] = new Node\Arg($value->var); + continue; + } + } + + $newArgs[] = $args[$i]; + } + return $newArgs; + } + } diff --git a/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php b/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php index dfb940cf..c90b2d69 100644 --- a/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php +++ b/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php @@ -42,6 +42,11 @@ 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'); + } + /** * @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 00000000..a15c16d2 --- /dev/null +++ b/tests/Rules/PHPUnit/data/assert-same-count-fixable.php @@ -0,0 +1,42 @@ +assertSame(5, count([1, 2, 3])); + } + + 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 00000000..c2e9f96b --- /dev/null +++ b/tests/Rules/PHPUnit/data/assert-same-count-fixable.php.fixed @@ -0,0 +1,42 @@ +assertCount(5, [1, 2, 3]); + } + + 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; + } +} From 7110421f1df661f715bd3cbf3daee3d51d6957ba Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 5 Oct 2026 17:18:25 +0200 Subject: [PATCH 02/13] skip named args --- src/Rules/PHPUnit/AssertSameWithCountRule.php | 11 ++++++-- .../PHPUnit/AssertSameWithCountRuleTest.php | 10 +++++++ .../assert-same-count-named-arguments.php | 26 +++++++++++++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 tests/Rules/PHPUnit/data/assert-same-count-named-arguments.php diff --git a/src/Rules/PHPUnit/AssertSameWithCountRule.php b/src/Rules/PHPUnit/AssertSameWithCountRule.php index 744641a4..3f22febb 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; @@ -40,6 +41,11 @@ public function processNode(Node $node, Scope $scope): array if (!$node->name instanceof Node\Identifier || $node->name->toLowerString() !== 'assertsame') { return []; } + foreach ($node->getArgs() as $arg) { + if ($arg->name !== null) { + return []; + } + } if (!AssertRuleHelper::isMethodOrStaticCallOnAssert($node, $scope)) { return []; @@ -132,8 +138,9 @@ private static function isNormalCount(Node\Expr\FuncCall $countFuncCall, Type $c } /** - * @param array $args - * @return list + * @template T of NodeAbstract + * @param array $args + * @return list */ private static function rewriteArgs(array $args, Scope $scope): ?array { diff --git a/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php b/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php index c90b2d69..907ecbd3 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 @@ -47,6 +48,15 @@ public function testFix(): void $this->fix(__DIR__ . '/data/assert-same-count-fixable.php', __DIR__ . '/data/assert-same-count-fixable.php.fixed'); } + 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'], []); + } + /** * @return string[] */ 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 00000000..d0b4b8f8 --- /dev/null +++ b/tests/Rules/PHPUnit/data/assert-same-count-named-arguments.php @@ -0,0 +1,26 @@ += 8.0 + +namespace ExampleTestCaseFixNamedArguments; + +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'); + } + +} + +class Bar implements \Countable +{ + + public function count(): int + { + return 1; + } + +} From 7a0377ac67455420f69f6800f37eb0a708d2a2a3 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 5 Oct 2026 17:31:18 +0200 Subject: [PATCH 03/13] skip inner named args --- src/Rules/PHPUnit/AssertSameWithCountRule.php | 21 ++++++++++++++----- .../assert-same-count-named-arguments.php | 4 ++++ 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/Rules/PHPUnit/AssertSameWithCountRule.php b/src/Rules/PHPUnit/AssertSameWithCountRule.php index 3f22febb..0decd001 100644 --- a/src/Rules/PHPUnit/AssertSameWithCountRule.php +++ b/src/Rules/PHPUnit/AssertSameWithCountRule.php @@ -98,11 +98,22 @@ public function processNode(Node $node, Scope $scope): array */ private static function isCountFunctionCall(Node\Expr $expr, Scope $scope): bool { - return $expr instanceof Node\Expr\FuncCall - && $expr->name instanceof Node\Name - && $expr->name->toLowerString() === 'count' - && count($expr->getArgs()) >= 1 - && self::isNormalCount($expr, $scope->getType($expr->getArgs()[0]->value), $scope)->yes(); + if (!$expr instanceof Node\Expr\FuncCall + || !$expr->name instanceof Node\Name + || $expr->name->toLowerString() !== 'count' + ) { + return false; + } + + $args = $expr->getArgs(); + foreach ($args as $arg) { + if ($arg->name !== null) { + return false; + } + } + + return count($args) >= 1 + && self::isNormalCount($expr, $scope->getType($args[0]->value), $scope)->yes(); } /** diff --git a/tests/Rules/PHPUnit/data/assert-same-count-named-arguments.php b/tests/Rules/PHPUnit/data/assert-same-count-named-arguments.php index d0b4b8f8..7f0402ee 100644 --- a/tests/Rules/PHPUnit/data/assert-same-count-named-arguments.php +++ b/tests/Rules/PHPUnit/data/assert-same-count-named-arguments.php @@ -2,6 +2,8 @@ namespace ExampleTestCaseFixNamedArguments; +use function count; + class AssertSameWithCountTestCase extends \PHPUnit\Framework\TestCase { @@ -11,6 +13,8 @@ public function skipNamedArguments(Bar $bar): void $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'); } } From 4eb576f3cac78f5f737b738df5311515b397a8b9 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 5 Oct 2026 17:45:19 +0200 Subject: [PATCH 04/13] refactor --- src/Rules/PHPUnit/AssertSameWithCountRule.php | 40 ++++++++++++------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/src/Rules/PHPUnit/AssertSameWithCountRule.php b/src/Rules/PHPUnit/AssertSameWithCountRule.php index 0decd001..8e2e8cdb 100644 --- a/src/Rules/PHPUnit/AssertSameWithCountRule.php +++ b/src/Rules/PHPUnit/AssertSameWithCountRule.php @@ -41,10 +41,8 @@ public function processNode(Node $node, Scope $scope): array if (!$node->name instanceof Node\Identifier || $node->name->toLowerString() !== 'assertsame') { return []; } - foreach ($node->getArgs() as $arg) { - if ($arg->name !== null) { - return []; - } + if (self::hasNamedArgs($node->getArgs())) { + return []; } if (!AssertRuleHelper::isMethodOrStaticCallOnAssert($node, $scope)) { @@ -52,7 +50,7 @@ public function processNode(Node $node, Scope $scope): array } $right = $node->getArgs()[1]->value; - if (self::isCountFunctionCall($right, $scope)) { + if (self::isFixableCountFunctionCall($right, $scope)) { return [ RuleErrorBuilder::message('You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).') ->identifier('phpunit.assertCount') @@ -71,7 +69,7 @@ public function processNode(Node $node, Scope $scope): array ]; } - if (self::isCountableMethodCall($right, $scope)) { + if (self::isFixableCountableMethodCall($right, $scope)) { return [ RuleErrorBuilder::message('You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, $variable->count()).') ->identifier('phpunit.assertCount') @@ -96,7 +94,7 @@ public function processNode(Node $node, Scope $scope): array /** * @phpstan-assert-if-true Node\Expr\FuncCall $expr */ - private static function isCountFunctionCall(Node\Expr $expr, Scope $scope): bool + private static function isFixableCountFunctionCall(Node\Expr $expr, Scope $scope): bool { if (!$expr instanceof Node\Expr\FuncCall || !$expr->name instanceof Node\Name @@ -106,10 +104,8 @@ private static function isCountFunctionCall(Node\Expr $expr, Scope $scope): bool } $args = $expr->getArgs(); - foreach ($args as $arg) { - if ($arg->name !== null) { - return false; - } + if (self::hasNamedArgs($args)) { + return false; } return count($args) >= 1 @@ -119,7 +115,7 @@ private static function isCountFunctionCall(Node\Expr $expr, Scope $scope): bool /** * @phpstan-assert-if-true Node\Expr\MethodCall $expr */ - private static function isCountableMethodCall(Node\Expr $expr, Scope $scope): bool + private static function isFixableCountableMethodCall(Node\Expr $expr, Scope $scope): bool { if ( $expr instanceof Node\Expr\MethodCall @@ -148,6 +144,20 @@ private static function isNormalCount(Node\Expr\FuncCall $countFuncCall, Type $c return $isNormalCount; } + /** + * @param array $args + */ + private static function hasNamedArgs(array $args): bool + { + foreach ($args as $arg) { + if ($arg->name !== null) { + return true; + } + } + + return false; + } + /** * @template T of NodeAbstract * @param array $args @@ -163,17 +173,19 @@ private static function rewriteArgs(array $args, Scope $scope): ?array && $args[$i]->value instanceof CallLike ) { $value = $args[$i]->value; - if (self::isCountFunctionCall($value, $scope)) { + if (self::isFixableCountFunctionCall($value, $scope)) { if (count($value->getArgs()) !== 1) { return null; } $newArgs[] = new Node\Arg($value->getArgs()[0]->value); continue; - } elseif (self::isCountableMethodCall($value, $scope)) { + } elseif (self::isFixableCountableMethodCall($value, $scope)) { $newArgs[] = new Node\Arg($value->var); continue; } + + return null; } $newArgs[] = $args[$i]; From ff49176919b0ef10e98e994362bdd7c4ffe0675f Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 6 Oct 2026 07:32:26 +0200 Subject: [PATCH 05/13] skip named args for auto-fixing for now --- src/Rules/PHPUnit/AssertSameWithCountRule.php | 46 +++++++++++-------- .../PHPUnit/AssertSameWithCountRuleTest.php | 11 ++++- 2 files changed, 37 insertions(+), 20 deletions(-) diff --git a/src/Rules/PHPUnit/AssertSameWithCountRule.php b/src/Rules/PHPUnit/AssertSameWithCountRule.php index 8e2e8cdb..03704192 100644 --- a/src/Rules/PHPUnit/AssertSameWithCountRule.php +++ b/src/Rules/PHPUnit/AssertSameWithCountRule.php @@ -41,16 +41,13 @@ public function processNode(Node $node, Scope $scope): array if (!$node->name instanceof Node\Identifier || $node->name->toLowerString() !== 'assertsame') { return []; } - if (self::hasNamedArgs($node->getArgs())) { - return []; - } if (!AssertRuleHelper::isMethodOrStaticCallOnAssert($node, $scope)) { return []; } $right = $node->getArgs()[1]->value; - if (self::isFixableCountFunctionCall($right, $scope)) { + if (self::isCountFunctionCall($right, $scope)) { return [ RuleErrorBuilder::message('You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).') ->identifier('phpunit.assertCount') @@ -69,7 +66,7 @@ public function processNode(Node $node, Scope $scope): array ]; } - if (self::isFixableCountableMethodCall($right, $scope)) { + if (self::isCountableMethodCall($right, $scope)) { return [ RuleErrorBuilder::message('You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, $variable->count()).') ->identifier('phpunit.assertCount') @@ -94,7 +91,7 @@ public function processNode(Node $node, Scope $scope): array /** * @phpstan-assert-if-true Node\Expr\FuncCall $expr */ - private static function isFixableCountFunctionCall(Node\Expr $expr, Scope $scope): bool + private static function isCountFunctionCall(Node\Expr $expr, Scope $scope): bool { if (!$expr instanceof Node\Expr\FuncCall || !$expr->name instanceof Node\Name @@ -115,7 +112,7 @@ private static function isFixableCountFunctionCall(Node\Expr $expr, Scope $scope /** * @phpstan-assert-if-true Node\Expr\MethodCall $expr */ - private static function isFixableCountableMethodCall(Node\Expr $expr, Scope $scope): bool + private static function isCountableMethodCall(Node\Expr $expr, Scope $scope): bool { if ( $expr instanceof Node\Expr\MethodCall @@ -167,25 +164,36 @@ private static function rewriteArgs(array $args, Scope $scope): ?array { $newArgs = []; for ($i = 0; $i < count($args); $i++) { - if ( $args[$i] instanceof Node\Arg - && $args[$i]->value instanceof CallLike ) { - $value = $args[$i]->value; - if (self::isFixableCountFunctionCall($value, $scope)) { - if (count($value->getArgs()) !== 1) { + // skip named args for auto-fixing for now + if ($args[$i]->name !== null) { + return null; + } + + if ($args[$i]->value instanceof CallLike) { + $callLike = $args[$i]->value; + + // skip named args for auto-fixing for now + if (self::hasNamedArgs($callLike->getArgs())) { return null; } - $newArgs[] = new Node\Arg($value->getArgs()[0]->value); - continue; - } elseif (self::isFixableCountableMethodCall($value, $scope)) { - $newArgs[] = new Node\Arg($value->var); - continue; - } + if (self::isCountFunctionCall($callLike, $scope)) { + if (count($callLike->getArgs()) !== 1) { + return null; + } - 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; + } } $newArgs[] = $args[$i]; diff --git a/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php b/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php index 907ecbd3..b6754f39 100644 --- a/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php +++ b/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php @@ -54,7 +54,16 @@ public function testNamedArguments(): void self::markTestSkipped('Named arguments require PHP 8.0.'); } - $this->analyse([__DIR__ . '/data/assert-same-count-named-arguments.php'], []); + $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)).', + 15, + ], + ]); } /** From d8c602d2ee0becf1b33fe712f37034de3d2e2eb8 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 6 Oct 2026 07:39:05 +0200 Subject: [PATCH 06/13] Update AssertSameWithCountRule.php --- src/Rules/PHPUnit/AssertSameWithCountRule.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Rules/PHPUnit/AssertSameWithCountRule.php b/src/Rules/PHPUnit/AssertSameWithCountRule.php index 03704192..f50aae35 100644 --- a/src/Rules/PHPUnit/AssertSameWithCountRule.php +++ b/src/Rules/PHPUnit/AssertSameWithCountRule.php @@ -167,7 +167,7 @@ private static function rewriteArgs(array $args, Scope $scope): ?array if ( $args[$i] instanceof Node\Arg ) { - // skip named args for auto-fixing for now + // skip named args for auto-fixing. PHPUnit does not support named arguments for assert*. if ($args[$i]->name !== null) { return null; } @@ -175,7 +175,7 @@ private static function rewriteArgs(array $args, Scope $scope): ?array if ($args[$i]->value instanceof CallLike) { $callLike = $args[$i]->value; - // skip named args for auto-fixing for now + // skip named args for auto-fixing. PHPUnit does not support named arguments for assert*. if (self::hasNamedArgs($callLike->getArgs())) { return null; } From 61b3061c7f09e0091b8f356914927acceef85d6b Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 6 Oct 2026 07:41:23 +0200 Subject: [PATCH 07/13] simplify --- src/Rules/PHPUnit/AssertSameWithCountRule.php | 19 +++++-------------- .../PHPUnit/AssertSameWithCountRuleTest.php | 8 ++++++++ 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/Rules/PHPUnit/AssertSameWithCountRule.php b/src/Rules/PHPUnit/AssertSameWithCountRule.php index f50aae35..a5c19e99 100644 --- a/src/Rules/PHPUnit/AssertSameWithCountRule.php +++ b/src/Rules/PHPUnit/AssertSameWithCountRule.php @@ -93,20 +93,11 @@ public function processNode(Node $node, Scope $scope): array */ private static function isCountFunctionCall(Node\Expr $expr, Scope $scope): bool { - if (!$expr instanceof Node\Expr\FuncCall - || !$expr->name instanceof Node\Name - || $expr->name->toLowerString() !== 'count' - ) { - return false; - } - - $args = $expr->getArgs(); - if (self::hasNamedArgs($args)) { - return false; - } - - return count($args) >= 1 - && self::isNormalCount($expr, $scope->getType($args[0]->value), $scope)->yes(); + return $expr instanceof Node\Expr\FuncCall + && $expr->name instanceof Node\Name + && $expr->name->toLowerString() === 'count' + && count($expr->getArgs()) >= 1 + && self::isNormalCount($expr, $scope->getType($expr->getArgs()[0]->value), $scope)->yes(); } /** diff --git a/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php b/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php index b6754f39..6d2ff429 100644 --- a/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php +++ b/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php @@ -59,10 +59,18 @@ public function testNamedArguments(): void '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, + ], ]); } From 13a5a597baa12bd0b6c99e6c66f97b50bc872a8e Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 6 Oct 2026 07:48:33 +0200 Subject: [PATCH 08/13] Update AssertSameWithCountRule.php --- src/Rules/PHPUnit/AssertSameWithCountRule.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Rules/PHPUnit/AssertSameWithCountRule.php b/src/Rules/PHPUnit/AssertSameWithCountRule.php index a5c19e99..479b9ea9 100644 --- a/src/Rules/PHPUnit/AssertSameWithCountRule.php +++ b/src/Rules/PHPUnit/AssertSameWithCountRule.php @@ -135,7 +135,7 @@ private static function isNormalCount(Node\Expr\FuncCall $countFuncCall, Type $c /** * @param array $args */ - private static function hasNamedArgs(array $args): bool + private static function hasNamedArg(array $args): bool { foreach ($args as $arg) { if ($arg->name !== null) { @@ -167,7 +167,7 @@ private static function rewriteArgs(array $args, Scope $scope): ?array $callLike = $args[$i]->value; // skip named args for auto-fixing. PHPUnit does not support named arguments for assert*. - if (self::hasNamedArgs($callLike->getArgs())) { + if (self::hasNamedArg($callLike->getArgs())) { return null; } From e560171a1e0d174c04428c0e5addbdeb76f722f3 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 6 Oct 2026 07:53:47 +0200 Subject: [PATCH 09/13] we don't expect any fixes for named arguments --- tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php b/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php index 6d2ff429..188a9cd4 100644 --- a/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php +++ b/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php @@ -46,6 +46,8 @@ 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 From c5fbf61172ab19a484ac95779706af3c7e8da948 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 6 Oct 2026 08:03:22 +0200 Subject: [PATCH 10/13] Updated AssertSameWithCountRule to unwrap only the actual-value argument. --- src/Rules/PHPUnit/AssertSameWithCountRule.php | 71 ++++++++----------- .../data/assert-same-count-fixable.php | 7 ++ .../data/assert-same-count-fixable.php.fixed | 7 ++ 3 files changed, 45 insertions(+), 40 deletions(-) diff --git a/src/Rules/PHPUnit/AssertSameWithCountRule.php b/src/Rules/PHPUnit/AssertSameWithCountRule.php index 479b9ea9..8f32ce79 100644 --- a/src/Rules/PHPUnit/AssertSameWithCountRule.php +++ b/src/Rules/PHPUnit/AssertSameWithCountRule.php @@ -132,63 +132,54 @@ private static function isNormalCount(Node\Expr\FuncCall $countFuncCall, Type $c return $isNormalCount; } - /** - * @param array $args - */ - private static function hasNamedArg(array $args): bool - { - foreach ($args as $arg) { - if ($arg->name !== null) { - return true; - } - } - - return false; - } - /** * @template T of NodeAbstract * @param array $args - * @return list + * @return list|null */ private static function rewriteArgs(array $args, Scope $scope): ?array { $newArgs = []; - for ($i = 0; $i < count($args); $i++) { - if ( - $args[$i] instanceof Node\Arg - ) { - // skip named args for auto-fixing. PHPUnit does not support named arguments for assert*. - if ($args[$i]->name !== null) { - return null; - } + foreach ($args as $i => $arg) { + if (!$arg instanceof Node\Arg) { + $newArgs[] = $arg; + continue; + } - if ($args[$i]->value instanceof CallLike) { - $callLike = $args[$i]->value; + // PHPUnit does not support named arguments for assert*. + if ($arg->name !== null) { + return null; + } - // skip named args for auto-fixing. PHPUnit does not support named arguments for assert*. - if (self::hasNamedArg($callLike->getArgs())) { - return null; - } + if ($i !== 1 || !$arg->value instanceof CallLike) { + $newArgs[] = $arg; + continue; + } - if (self::isCountFunctionCall($callLike, $scope)) { - if (count($callLike->getArgs()) !== 1) { - return null; - } + $callLike = $arg->value; - $newArgs[] = new Node\Arg($callLike->getArgs()[0]->value); - continue; - } elseif (self::isCountableMethodCall($callLike, $scope)) { - $newArgs[] = new Node\Arg($callLike->var); - continue; - } + // The count call itself must not use named arguments. + foreach ($callLike->getArgs() as $callArg) { + if ($callArg->name !== null) { + 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; } - $newArgs[] = $args[$i]; + return null; } + return $newArgs; } diff --git a/tests/Rules/PHPUnit/data/assert-same-count-fixable.php b/tests/Rules/PHPUnit/data/assert-same-count-fixable.php index a15c16d2..5db85e58 100644 --- a/tests/Rules/PHPUnit/data/assert-same-count-fixable.php +++ b/tests/Rules/PHPUnit/data/assert-same-count-fixable.php @@ -12,6 +12,13 @@ public function testAssertSameWithCount() $this->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 testAssertSameWithCountRecursive($x) { $this->assertSame(5, count([1, 2, 3, $x], COUNT_RECURSIVE)); diff --git a/tests/Rules/PHPUnit/data/assert-same-count-fixable.php.fixed b/tests/Rules/PHPUnit/data/assert-same-count-fixable.php.fixed index c2e9f96b..56ee41ae 100644 --- a/tests/Rules/PHPUnit/data/assert-same-count-fixable.php.fixed +++ b/tests/Rules/PHPUnit/data/assert-same-count-fixable.php.fixed @@ -12,6 +12,13 @@ class AssertSameWithCountTestCase extends \PHPUnit\Framework\TestCase $this->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 testAssertSameWithCountRecursive($x) { $this->assertSame(5, count([1, 2, 3, $x], COUNT_RECURSIVE)); From 5e889eb5177df4a55ec3589898dcb131200357b4 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 6 Oct 2026 08:08:28 +0200 Subject: [PATCH 11/13] skip unpack --- src/Rules/PHPUnit/AssertSameWithCountRule.php | 6 +++--- tests/Rules/PHPUnit/data/assert-same-count-fixable.php | 5 +++++ .../Rules/PHPUnit/data/assert-same-count-fixable.php.fixed | 5 +++++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/Rules/PHPUnit/AssertSameWithCountRule.php b/src/Rules/PHPUnit/AssertSameWithCountRule.php index 8f32ce79..482a89e9 100644 --- a/src/Rules/PHPUnit/AssertSameWithCountRule.php +++ b/src/Rules/PHPUnit/AssertSameWithCountRule.php @@ -147,7 +147,7 @@ private static function rewriteArgs(array $args, Scope $scope): ?array } // PHPUnit does not support named arguments for assert*. - if ($arg->name !== null) { + if ($arg->name !== null || $arg->unpack) { return null; } @@ -158,9 +158,9 @@ private static function rewriteArgs(array $args, Scope $scope): ?array $callLike = $arg->value; - // The count call itself must not use named arguments. + // The count call itself must not use named arguments or unpacking. foreach ($callLike->getArgs() as $callArg) { - if ($callArg->name !== null) { + if ($callArg->name !== null || $callArg->unpack) { return null; } } diff --git a/tests/Rules/PHPUnit/data/assert-same-count-fixable.php b/tests/Rules/PHPUnit/data/assert-same-count-fixable.php index 5db85e58..85e41632 100644 --- a/tests/Rules/PHPUnit/data/assert-same-count-fixable.php +++ b/tests/Rules/PHPUnit/data/assert-same-count-fixable.php @@ -19,6 +19,11 @@ public function testAssertSameWithCountCallsInOtherArguments($expected, $actual, $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)); diff --git a/tests/Rules/PHPUnit/data/assert-same-count-fixable.php.fixed b/tests/Rules/PHPUnit/data/assert-same-count-fixable.php.fixed index 56ee41ae..8a2dc426 100644 --- a/tests/Rules/PHPUnit/data/assert-same-count-fixable.php.fixed +++ b/tests/Rules/PHPUnit/data/assert-same-count-fixable.php.fixed @@ -19,6 +19,11 @@ class AssertSameWithCountTestCase extends \PHPUnit\Framework\TestCase $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)); From bda4b02227877b6de92dcaf881d6d4643de8d3f2 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 6 Oct 2026 10:36:02 +0200 Subject: [PATCH 12/13] reuse helper --- src/Rules/PHPUnit/AssertRuleHelper.php | 1 + src/Rules/PHPUnit/AssertSameWithCountRule.php | 13 ++++++++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Rules/PHPUnit/AssertRuleHelper.php b/src/Rules/PHPUnit/AssertRuleHelper.php index 5971fc0b..5fb958d9 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 482a89e9..0b5f5fdf 100644 --- a/src/Rules/PHPUnit/AssertSameWithCountRule.php +++ b/src/Rules/PHPUnit/AssertSameWithCountRule.php @@ -52,6 +52,10 @@ public function processNode(Node $node, Scope $scope): array 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; @@ -71,6 +75,10 @@ public function processNode(Node $node, Scope $scope): array 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; @@ -146,11 +154,6 @@ private static function rewriteArgs(array $args, Scope $scope): ?array continue; } - // PHPUnit does not support named arguments for assert*. - if ($arg->name !== null || $arg->unpack) { - return null; - } - if ($i !== 1 || !$arg->value instanceof CallLike) { $newArgs[] = $arg; continue; From caf2e2aaeed9f032acef58e9df7066ca81d2454c Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 6 Oct 2026 10:38:50 +0200 Subject: [PATCH 13/13] Update AssertSameWithCountRule.php --- src/Rules/PHPUnit/AssertSameWithCountRule.php | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/Rules/PHPUnit/AssertSameWithCountRule.php b/src/Rules/PHPUnit/AssertSameWithCountRule.php index 0b5f5fdf..196037c1 100644 --- a/src/Rules/PHPUnit/AssertSameWithCountRule.php +++ b/src/Rules/PHPUnit/AssertSameWithCountRule.php @@ -161,11 +161,9 @@ private static function rewriteArgs(array $args, Scope $scope): ?array $callLike = $arg->value; - // The count call itself must not use named arguments or unpacking. - foreach ($callLike->getArgs() as $callArg) { - if ($callArg->name !== null || $callArg->unpack) { - return null; - } + // for now skip more complex cases + if (AssertRuleHelper::hasNamedOrUnpackedArguments($callLike)) { + return null; } if (self::isCountFunctionCall($callLike, $scope)) {