From 3a372229d943a9489ec9aa3da15b431b21d6a7c2 Mon Sep 17 00:00:00 2001 From: o0h Date: Fri, 5 Apr 2024 03:01:01 +0900 Subject: [PATCH 1/3] Update AssertSameWithCountRule for assertSameSize Expanded the AssertSameWithCountRule in PHPUnit rules to handle additional scenarios. Now the rule checks if both sides of the assertSame are counts, and suggests assertSameSize instead when necessary. The modification enhances the rule's applicability and accuracy. --- rules.neon | 7 +- src/Rules/PHPUnit/AssertSameWithCountRule.php | 104 ++++++++++++++---- .../PHPUnit/AssertSameWithCountRuleTest.php | 22 +++- .../Rules/PHPUnit/data/assert-same-count.php | 31 ++++++ 4 files changed, 139 insertions(+), 25 deletions(-) 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 { From 11d7b3591d304a71aa39d7b9c1dfc95273e94030 Mon Sep 17 00:00:00 2001 From: o0h Date: Sat, 6 Apr 2024 04:08:41 +0900 Subject: [PATCH 2/3] Update README --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 205cbe4..2039163 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 [bleedingEdge](/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? From 3f833d26ad60690355a209cb05a409aa85b0c0e9 Mon Sep 17 00:00:00 2001 From: o0h Date: Sat, 6 Apr 2024 17:55:00 +0900 Subject: [PATCH 3/3] Update link and fix typo in README.md The link in the README file's "bleeding edge" reference has been updated to accurately direct users to the relevant blog post. Also corrected the typo by capitalizing the initial letter of the sentence. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 2039163..094c539 100644 --- a/README.md +++ b/README.md @@ -22,7 +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 [bleedingEdge](/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. + * 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?