From bac8aa4b6f20c3eea47613e07ae23e299f0d106e Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 11 Nov 2025 07:26:10 +0100 Subject: [PATCH] Fix AssertSameWithCountRule for recursive count() --- src/Rules/PHPUnit/AssertSameWithCountRule.php | 67 ++++++++++++++----- .../PHPUnit/AssertSameWithCountRuleTest.php | 8 +++ .../Rules/PHPUnit/data/assert-same-count.php | 26 ++++++- 3 files changed, 84 insertions(+), 17 deletions(-) diff --git a/src/Rules/PHPUnit/AssertSameWithCountRule.php b/src/Rules/PHPUnit/AssertSameWithCountRule.php index 0e6d83cf..2a5a7651 100644 --- a/src/Rules/PHPUnit/AssertSameWithCountRule.php +++ b/src/Rules/PHPUnit/AssertSameWithCountRule.php @@ -8,8 +8,12 @@ use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; +use PHPStan\TrinaryLogic; +use PHPStan\Type\Constant\ConstantIntegerType; use PHPStan\Type\ObjectType; +use PHPStan\Type\Type; use function count; +use const COUNT_NORMAL; /** * @implements Rule @@ -42,11 +46,7 @@ public function processNode(Node $node, Scope $scope): array } $right = $node->getArgs()[1]->value; - if ( - $right instanceof Node\Expr\FuncCall - && $right->name instanceof Node\Name - && $right->name->toLowerString() === 'count' - ) { + if (self::isCountFunctionCall($right, $scope)) { return [ RuleErrorBuilder::message('You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).') ->identifier('phpunit.assertCount') @@ -54,24 +54,59 @@ public function processNode(Node $node, Scope $scope): array ]; } + if (self::isCountableMethodCall($right, $scope)) { + return [ + RuleErrorBuilder::message('You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, $variable->count()).') + ->identifier('phpunit.assertCount') + ->build(), + ]; + } + + return []; + } + + /** + * @phpstan-assert-if-true Node\Expr\FuncCall $expr + */ + 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(); + } + + /** + * @phpstan-assert-if-true Node\Expr\MethodCall $expr + */ + private static function isCountableMethodCall(Node\Expr $expr, Scope $scope): bool + { if ( - $right instanceof Node\Expr\MethodCall - && $right->name instanceof Node\Identifier - && $right->name->toLowerString() === 'count' - && count($right->getArgs()) === 0 + $expr instanceof Node\Expr\MethodCall + && $expr->name instanceof Node\Identifier + && $expr->name->toLowerString() === 'count' + && count($expr->getArgs()) === 0 ) { - $type = $scope->getType($right->var); + $type = $scope->getType($expr->var); if ((new ObjectType(Countable::class))->isSuperTypeOf($type)->yes()) { - return [ - RuleErrorBuilder::message('You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, $variable->count()).') - ->identifier('phpunit.assertCount') - ->build(), - ]; + return true; } } - return []; + return false; + } + + private static function isNormalCount(Node\Expr\FuncCall $countFuncCall, Type $countedType, Scope $scope): TrinaryLogic + { + if (count($countFuncCall->getArgs()) === 1) { + $isNormalCount = TrinaryLogic::createYes(); + } else { + $mode = $scope->getType($countFuncCall->getArgs()[1]->value); + $isNormalCount = (new ConstantIntegerType(COUNT_NORMAL))->isSuperTypeOf($mode)->result->or($countedType->getIterableValueType()->isArray()->negate()); + } + return $isNormalCount; } } diff --git a/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php b/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php index 32f564d6..dfb940cf 100644 --- a/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php +++ b/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php @@ -31,6 +31,14 @@ public function testRule(): void 'You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, $variable->count()).', 30, ], + [ + 'You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).', + 40, + ], + [ + 'You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).', + 45, + ], ]); } diff --git a/tests/Rules/PHPUnit/data/assert-same-count.php b/tests/Rules/PHPUnit/data/assert-same-count.php index 73df333d..bc40eed7 100644 --- a/tests/Rules/PHPUnit/data/assert-same-count.php +++ b/tests/Rules/PHPUnit/data/assert-same-count.php @@ -30,6 +30,30 @@ public function testAssertSameWithCountMethodForCountableVariableIsNotOK() $this->assertSame(5, $foo->bar->count()); } + public function testRecursiveCount($x) + { + $this->assertSame(5, count([1, 2, 3, $x], COUNT_RECURSIVE)); // OK + } + + public function testNormalCount($x) + { + $this->assertSame(5, count([1, 2, 3, $x], COUNT_NORMAL)); + } + + public function testImplicitNormalCount($mode) + { + $this->assertSame(5, count([1, 2, 3], $mode)); + } + + public function testUnknownCountable($x, $mode) + { + $this->assertSame(5, count($x, $mode)); // OK + } + + public function testUnknownCountMode($x, $mode) + { + $this->assertSame(5, count([1, 2, 3, $x], $mode)); // OK + } } class Bar implements \Countable { @@ -37,4 +61,4 @@ public function count(): int { return 1; } -}; +}