diff --git a/README.md b/README.md index 205cbe4..094c539 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,7 @@ It also contains this strict framework-specific rules (can be enabled separately * Check that you are not using `assertSame()` with `false` as expected value. `assertFalse()` should be used instead. * Check that you are not using `assertSame()` with `null` as expected value. `assertNull()` should be used instead. * Check that you are not using `assertSame()` with `count($variable)` as second parameter. `assertCount($variable)` should be used instead. + * If you enable [bleeding edge](https://phpstan.org/blog/what-is-bleeding-edge), check that you are not using `assertSame()` with `count($variable)` as first and second parameter. `assertSameSize()`should be used instead. ## How to document mock objects in phpDocs? diff --git a/rules.neon b/rules.neon index 8dc7056..a9e2505 100644 --- a/rules.neon +++ b/rules.neon @@ -1,11 +1,16 @@ rules: - PHPStan\Rules\PHPUnit\AssertSameBooleanExpectedRule - PHPStan\Rules\PHPUnit\AssertSameNullExpectedRule - - PHPStan\Rules\PHPUnit\AssertSameWithCountRule - PHPStan\Rules\PHPUnit\MockMethodCallRule - PHPStan\Rules\PHPUnit\ShouldCallParentMethodsRule services: + - + class: PHPStan\Rules\PHPUnit\AssertSameWithCountRule + arguments: + bleedingEdge: %featureToggles.bleedingEdge% + tags: + - phpstan.rules.rule - class: PHPStan\Rules\PHPUnit\ClassCoversExistsRule - class: PHPStan\Rules\PHPUnit\ClassMethodCoversExistsRule - diff --git a/src/Rules/PHPUnit/AssertSameWithCountRule.php b/src/Rules/PHPUnit/AssertSameWithCountRule.php index 38fdf9a..716d12a 100644 --- a/src/Rules/PHPUnit/AssertSameWithCountRule.php +++ b/src/Rules/PHPUnit/AssertSameWithCountRule.php @@ -4,6 +4,7 @@ use Countable; use PhpParser\Node; +use PhpParser\Node\Expr\MethodCall; use PhpParser\NodeAbstract; use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; @@ -17,6 +18,14 @@ class AssertSameWithCountRule implements Rule { + /** @var bool */ + private $bleedingEdge; + + public function __construct(bool $bleedingEdge) + { + $this->bleedingEdge = $bleedingEdge; + } + public function getNodeType(): string { return NodeAbstract::class; @@ -37,36 +46,89 @@ 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' - ) { - return [ - RuleErrorBuilder::message('You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).') - ->identifier('phpunit.assertCount') - ->build(), - ]; + $rightIsCountFuncCall = $this->isCountFuncCall($right); + $rightIsCountMethodCall = $this->isCountMethodCall($right) && $this->argIsCountable($right, $scope); + if (!($rightIsCountFuncCall || $rightIsCountMethodCall)) { + return []; } - if ( - $right instanceof Node\Expr\MethodCall - && $right->name instanceof Node\Identifier - && $right->name->toLowerString() === 'count' - && count($right->getArgs()) === 0 - ) { - $type = $scope->getType($right->var); + $leftIsCountFuncCall = $leftIsCountMethodCall = false; + if ($this->bleedingEdge) { + $left = $node->getArgs()[0]->value; + $leftIsCountFuncCall = $this->isCountFuncCall($left); + $leftIsCountMethodCall = $this->isCountMethodCall($left) && $this->argIsCountable($left, $scope); + } - if ((new ObjectType(Countable::class))->isSuperTypeOf($type)->yes()) { + if ($rightIsCountFuncCall) { + if ($leftIsCountFuncCall) { return [ - RuleErrorBuilder::message('You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, $variable->count()).') - ->identifier('phpunit.assertCount') + RuleErrorBuilder::message('You should use assertSameSize($expected, $variable) instead of assertSame(count($expected), count($variable)).') + ->identifier('phpunit.assertSameSize') + ->build(), + ]; + } elseif ($leftIsCountMethodCall) { + return [ + RuleErrorBuilder::message('You should use assertSameSize($expected, $variable) instead of assertSame($expected->count(), count($variable)).') + ->identifier('phpunit.assertSameSize') ->build(), ]; } + + return [ + RuleErrorBuilder::message('You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).') + ->identifier('phpunit.assertCount') + ->build(), + ]; + } + + if ($leftIsCountFuncCall) { + return [ + RuleErrorBuilder::message('You should use assertSameSize($expected, $variable) instead of assertSame(count($expected), $variable->count()).') + ->identifier('phpunit.assertSameSize') + ->build(), + ]; + } elseif ($leftIsCountMethodCall) { + return [ + RuleErrorBuilder::message('You should use assertSameSize($expected, $variable) instead of assertSame($expected->count(), $variable->count()).') + ->identifier('phpunit.assertSameSize') + ->build(), + ]; } - return []; + return [ + RuleErrorBuilder::message('You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, $variable->count()).') + ->identifier('phpunit.assertCount') + ->build(), + ]; + } + + /** + * @phpstan-assert-if-true Node\Expr\FuncCall $expr + */ + private function isCountFuncCall(Node\Expr $expr): bool + { + return $expr instanceof Node\Expr\FuncCall + && $expr->name instanceof Node\Name + && $expr->name->toLowerString() === 'count'; + } + + /** + * @phpstan-assert-if-true Node\Expr\MethodCall $expr + */ + private function isCountMethodCall(Node\Expr $expr): bool + { + return $expr instanceof Node\Expr\MethodCall + && $expr->name instanceof Node\Identifier + && $expr->name->toLowerString() === 'count' + && count($expr->getArgs()) === 0; + } + + private function argIsCountable(MethodCall $methodCall, Scope $scope): bool + { + $type = $scope->getType($methodCall->var); + $countableType = new ObjectType(Countable::class); + + return $countableType->isSuperTypeOf($type)->yes(); } } diff --git a/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php b/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php index 32f564d..ff9af79 100644 --- a/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php +++ b/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php @@ -13,7 +13,7 @@ class AssertSameWithCountRuleTest extends RuleTestCase protected function getRule(): Rule { - return new AssertSameWithCountRule(); + return new AssertSameWithCountRule(true); } public function testRule(): void @@ -23,13 +23,29 @@ public function testRule(): void 'You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).', 10, ], + [ + 'You should use assertSameSize($expected, $variable) instead of assertSame(count($expected), count($variable)).', + 15, + ], + [ + 'You should use assertSameSize($expected, $variable) instead of assertSame($expected->count(), count($variable)).', + 23, + ], [ 'You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).', - 22, + 35, ], [ 'You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, $variable->count()).', - 30, + 43, + ], + [ + 'You should use assertSameSize($expected, $variable) instead of assertSame(count($expected), $variable->count()).', + 51, + ], + [ + 'You should use assertSameSize($expected, $variable) instead of assertSame($expected->count(), $variable->count()).', + 61, ], ]); } diff --git a/tests/Rules/PHPUnit/data/assert-same-count.php b/tests/Rules/PHPUnit/data/assert-same-count.php index 73df333..a149fcb 100644 --- a/tests/Rules/PHPUnit/data/assert-same-count.php +++ b/tests/Rules/PHPUnit/data/assert-same-count.php @@ -10,6 +10,19 @@ public function testAssertSameWithCount() $this->assertSame(5, count([1, 2, 3])); } + public function testAssertSameWithCountExpectedWithCount() + { + $this->assertSame(count([10, 20]), count([1, 2, 3])); + } + + public function testAssertSameWithCountExpectedMethodWithCountMethod() + { + $foo = new \stdClass(); + $foo->bar = new Bar (); + + $this->assertSame($foo->bar->count(), count([1, 2, 3])); + } + public function testAssertSameWithCountMethodIsOK() { $foo = new \stdClass(); @@ -30,6 +43,24 @@ public function testAssertSameWithCountMethodForCountableVariableIsNotOK() $this->assertSame(5, $foo->bar->count()); } + public function testAssertSameWithCountExpectedWithCountMethodForCountableVariableIsNot() + { + $foo = new \stdClass(); + $foo->bar = new Bar (); + + $this->assertSame(count([10, 20]), $foo->bar->count()); + } + + public function testAssertSameWithCountExpectedMethodWithCountMethodForCountableVariableIsNot() + { + $foo = new \stdClass(); + $foo->bar = new Bar (); + $foo2 = new \stdClass(); + $foo2->bar = new Bar (); + + $this->assertSame($foo2->bar->count(), $foo->bar->count()); + } + } class Bar implements \Countable {