Skip to content

Commit 2fe9fbe

Browse files
staabmondrejmirtes
authored andcommitted
Fix AssertSameWithCountRule for recursive count()
1 parent d935297 commit 2fe9fbe

File tree

3 files changed

+84
-17
lines changed

3 files changed

+84
-17
lines changed

src/Rules/PHPUnit/AssertSameWithCountRule.php

Lines changed: 51 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,12 @@
88
use PHPStan\Analyser\Scope;
99
use PHPStan\Rules\Rule;
1010
use PHPStan\Rules\RuleErrorBuilder;
11+
use PHPStan\TrinaryLogic;
12+
use PHPStan\Type\Constant\ConstantIntegerType;
1113
use PHPStan\Type\ObjectType;
14+
use PHPStan\Type\Type;
1215
use function count;
16+
use const COUNT_NORMAL;
1317

1418
/**
1519
* @implements Rule<CallLike>
@@ -42,36 +46,67 @@ public function processNode(Node $node, Scope $scope): array
4246
}
4347

4448
$right = $node->getArgs()[1]->value;
45-
if (
46-
$right instanceof Node\Expr\FuncCall
47-
&& $right->name instanceof Node\Name
48-
&& $right->name->toLowerString() === 'count'
49-
) {
49+
if (self::isCountFunctionCall($right, $scope)) {
5050
return [
5151
RuleErrorBuilder::message('You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).')
5252
->identifier('phpunit.assertCount')
5353
->build(),
5454
];
5555
}
5656

57+
if (self::isCountableMethodCall($right, $scope)) {
58+
return [
59+
RuleErrorBuilder::message('You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, $variable->count()).')
60+
->identifier('phpunit.assertCount')
61+
->build(),
62+
];
63+
}
64+
65+
return [];
66+
}
67+
68+
/**
69+
* @phpstan-assert-if-true Node\Expr\FuncCall $expr
70+
*/
71+
private static function isCountFunctionCall(Node\Expr $expr, Scope $scope): bool
72+
{
73+
return $expr instanceof Node\Expr\FuncCall
74+
&& $expr->name instanceof Node\Name
75+
&& $expr->name->toLowerString() === 'count'
76+
&& count($expr->getArgs()) >= 1
77+
&& self::isNormalCount($expr, $scope->getType($expr->getArgs()[0]->value), $scope)->yes();
78+
}
79+
80+
/**
81+
* @phpstan-assert-if-true Node\Expr\MethodCall $expr
82+
*/
83+
private static function isCountableMethodCall(Node\Expr $expr, Scope $scope): bool
84+
{
5785
if (
58-
$right instanceof Node\Expr\MethodCall
59-
&& $right->name instanceof Node\Identifier
60-
&& $right->name->toLowerString() === 'count'
61-
&& count($right->getArgs()) === 0
86+
$expr instanceof Node\Expr\MethodCall
87+
&& $expr->name instanceof Node\Identifier
88+
&& $expr->name->toLowerString() === 'count'
89+
&& count($expr->getArgs()) === 0
6290
) {
63-
$type = $scope->getType($right->var);
91+
$type = $scope->getType($expr->var);
6492

6593
if ((new ObjectType(Countable::class))->isSuperTypeOf($type)->yes()) {
66-
return [
67-
RuleErrorBuilder::message('You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, $variable->count()).')
68-
->identifier('phpunit.assertCount')
69-
->build(),
70-
];
94+
return true;
7195
}
7296
}
7397

74-
return [];
98+
return false;
99+
}
100+
101+
private static function isNormalCount(Node\Expr\FuncCall $countFuncCall, Type $countedType, Scope $scope): TrinaryLogic
102+
{
103+
if (count($countFuncCall->getArgs()) === 1) {
104+
$isNormalCount = TrinaryLogic::createYes();
105+
} else {
106+
$mode = $scope->getType($countFuncCall->getArgs()[1]->value);
107+
$isNormalCount = (new ConstantIntegerType(COUNT_NORMAL))->isSuperTypeOf($mode)->result->or($countedType->getIterableValueType()->isArray()->negate());
108+
}
109+
return $isNormalCount;
75110
}
76111

77112
}

tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,14 @@ public function testRule(): void
3131
'You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, $variable->count()).',
3232
30,
3333
],
34+
[
35+
'You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).',
36+
40,
37+
],
38+
[
39+
'You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).',
40+
45,
41+
],
3442
]);
3543
}
3644

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

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,35 @@ public function testAssertSameWithCountMethodForCountableVariableIsNotOK()
3030
$this->assertSame(5, $foo->bar->count());
3131
}
3232

33+
public function testRecursiveCount($x)
34+
{
35+
$this->assertSame(5, count([1, 2, 3, $x], COUNT_RECURSIVE)); // OK
36+
}
37+
38+
public function testNormalCount($x)
39+
{
40+
$this->assertSame(5, count([1, 2, 3, $x], COUNT_NORMAL));
41+
}
42+
43+
public function testImplicitNormalCount($mode)
44+
{
45+
$this->assertSame(5, count([1, 2, 3], $mode));
46+
}
47+
48+
public function testUnknownCountable($x, $mode)
49+
{
50+
$this->assertSame(5, count($x, $mode)); // OK
51+
}
52+
53+
public function testUnknownCountMode($x, $mode)
54+
{
55+
$this->assertSame(5, count([1, 2, 3, $x], $mode)); // OK
56+
}
3357
}
3458

3559
class Bar implements \Countable {
3660
public function count(): int
3761
{
3862
return 1;
3963
}
40-
};
64+
}

0 commit comments

Comments
 (0)