diff --git a/config/phpunit-rules.neon b/config/phpunit-rules.neon index c600dbf7..c0c0ee35 100644 --- a/config/phpunit-rules.neon +++ b/config/phpunit-rules.neon @@ -8,3 +8,7 @@ rules: - Symplify\PHPStanRules\Rules\Doctrine\NoEntityMockingRule - Symplify\PHPStanRules\Rules\PHPUnit\NoMockObjectAndRealObjectPropertyRule - Symplify\PHPStanRules\Rules\PHPUnit\NoDoubleConsecutiveTestMockRule + + # @todo test first + # ever method() must have expects() call to define how many times it is expected + # - Symplify\PHPStanRules\Rules\PHPUnit\ExplicitExpectsMockMethodRule diff --git a/phpunit.xml b/phpunit.xml index 60461d5b..2dddc2d1 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -13,5 +13,7 @@ tests/Rules/PHPUnit/PublicStaticDataProviderRule tests/Rules/PHPUnit/NoAssertFuncCallInTestsRule/Fixture tests/Rules/PHPUnit/NoMockOnlyTestRule/Fixture/SkipConstraintValidatorTest.php + tests/Rules/PHPUnit/ExplicitExpectsMockMethodRule/Fixture/SkipMockWithExpectsTest.php + tests/Rules/PHPUnit/ExplicitExpectsMockMethodRule/Fixture/MockWithoutExpectsTest.php diff --git a/src/PHPUnit/TestClassDetector.php b/src/PHPUnit/TestClassDetector.php new file mode 100644 index 00000000..cfe4315c --- /dev/null +++ b/src/PHPUnit/TestClassDetector.php @@ -0,0 +1,30 @@ +getFile(), $testFileSuffix)) { + return true; + } + } + + return false; + } +} diff --git a/src/Rules/PHPUnit/ExplicitExpectsMockMethodRule.php b/src/Rules/PHPUnit/ExplicitExpectsMockMethodRule.php new file mode 100644 index 00000000..1c18d278 --- /dev/null +++ b/src/Rules/PHPUnit/ExplicitExpectsMockMethodRule.php @@ -0,0 +1,62 @@ + + * + * @see \Symplify\PHPStanRules\Tests\Rules\PHPUnit\ExplicitExpectsMockMethodRule\ExplicitExpectsMockMethodRuleTest + */ +final class ExplicitExpectsMockMethodRule implements Rule +{ + public const string ERROR_MESSAGE = 'PHPUnit mock method is missing explicit expects(), e.g. $this->mock->expects($this->once())->...'; + + public function getNodeType(): string + { + return MethodCall::class; + } + + /** + * @param MethodCall $node + * @return IdentifierRuleError[] + */ + public function processNode(Node $node, Scope $scope): array + { + if (! NamingHelper::isName($node->name, 'method')) { + return []; + } + + if (! TestClassDetector::isTestClass($scope)) { + return []; + } + + if (! $node->var instanceof Variable && ! $node->var instanceof PropertyFetch) { + return []; + } + + $callerType = $scope->getType($node->var); + if (! $callerType->hasMethod('expects')->yes()) { + return []; + } + + $identifierRuleError = RuleErrorBuilder::message(self::ERROR_MESSAGE) + ->identifier(PHPUnitRuleIdentifier::NO_ASSERT_FUNC_CALL_IN_TESTS) + ->build(); + + return [$identifierRuleError]; + } +} diff --git a/src/Rules/PHPUnit/NoAssertFuncCallInTestsRule.php b/src/Rules/PHPUnit/NoAssertFuncCallInTestsRule.php index 7b6cb4f8..27a0ada7 100644 --- a/src/Rules/PHPUnit/NoAssertFuncCallInTestsRule.php +++ b/src/Rules/PHPUnit/NoAssertFuncCallInTestsRule.php @@ -10,6 +10,7 @@ use PHPStan\Rules\RuleErrorBuilder; use Symplify\PHPStanRules\Enum\RuleIdentifier\PHPUnitRuleIdentifier; use Symplify\PHPStanRules\Helper\NamingHelper; +use Symplify\PHPStanRules\PHPUnit\TestClassDetector; /** * @implements Rule @@ -18,12 +19,6 @@ final class NoAssertFuncCallInTestsRule implements Rule { public const string ERROR_MESSAGE = 'Instead of assert() that can miss important checks, use native PHPUnit assert call'; - private const array TEST_FILE_SUFFIXES = [ - 'Test.php', - 'TestCase.php', - 'Context.php', - ]; - public function getNodeType(): string { return FuncCall::class; @@ -39,7 +34,7 @@ public function processNode(Node $node, Scope $scope): array return []; } - if (! $this->isTestFile($scope)) { + if (! TestClassDetector::isTestClass($scope)) { return []; } @@ -49,15 +44,4 @@ public function processNode(Node $node, Scope $scope): array return [$identifierRuleError]; } - - private function isTestFile(Scope $scope): bool - { - foreach (self::TEST_FILE_SUFFIXES as $testFileSuffix) { - if (str_ends_with($scope->getFile(), $testFileSuffix)) { - return true; - } - } - - return false; - } } diff --git a/tests/Rules/PHPUnit/ExplicitExpectsMockMethodRule/ExplicitExpectsMockMethodRuleTest.php b/tests/Rules/PHPUnit/ExplicitExpectsMockMethodRule/ExplicitExpectsMockMethodRuleTest.php new file mode 100644 index 00000000..35f59ee1 --- /dev/null +++ b/tests/Rules/PHPUnit/ExplicitExpectsMockMethodRule/ExplicitExpectsMockMethodRuleTest.php @@ -0,0 +1,38 @@ +> $expectedErrorsWithLines + */ + #[DataProvider('provideData')] + public function testRule(string $filePath, array $expectedErrorsWithLines): void + { + $this->analyse([$filePath], $expectedErrorsWithLines); + } + + /** + * @return Iterator, mixed>> + */ + public static function provideData(): Iterator + { + yield [__DIR__ . '/Fixture/MockWithoutExpectsTest.php', [[ExplicitExpectsMockMethodRule::ERROR_MESSAGE, 12]]]; + + yield [__DIR__ . '/Fixture/SkipMockWithExpectsTest.php', []]; + } + + protected function getRule(): Rule + { + return new ExplicitExpectsMockMethodRule(); + } +} diff --git a/tests/Rules/PHPUnit/ExplicitExpectsMockMethodRule/Fixture/MockWithoutExpectsTest.php b/tests/Rules/PHPUnit/ExplicitExpectsMockMethodRule/Fixture/MockWithoutExpectsTest.php new file mode 100644 index 00000000..9cdbc7db --- /dev/null +++ b/tests/Rules/PHPUnit/ExplicitExpectsMockMethodRule/Fixture/MockWithoutExpectsTest.php @@ -0,0 +1,14 @@ +createMock(\stdClass::class); + $mock->method('someMethod')->willReturn('value'); + } +} diff --git a/tests/Rules/PHPUnit/ExplicitExpectsMockMethodRule/Fixture/SkipMockWithExpectsTest.php b/tests/Rules/PHPUnit/ExplicitExpectsMockMethodRule/Fixture/SkipMockWithExpectsTest.php new file mode 100644 index 00000000..3cf634f2 --- /dev/null +++ b/tests/Rules/PHPUnit/ExplicitExpectsMockMethodRule/Fixture/SkipMockWithExpectsTest.php @@ -0,0 +1,17 @@ +createMock(\stdClass::class); + + $mock->expects($this->atLeastOnce()) + ->method('someMethod') + ->willReturn('value'); + } +}