[phpunit] Add ExplicitExpectsMockMethodRule (#260)

Author: TomasVotruba

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

phpunit.xml

@@ -13,5 +13,7 @@
         <exclude>tests/Rules/PHPUnit/PublicStaticDataProviderRule</exclude>
         <exclude>tests/Rules/PHPUnit/NoAssertFuncCallInTestsRule/Fixture</exclude>
         <exclude>tests/Rules/PHPUnit/NoMockOnlyTestRule/Fixture/SkipConstraintValidatorTest.php</exclude>
+        <exclude>tests/Rules/PHPUnit/ExplicitExpectsMockMethodRule/Fixture/SkipMockWithExpectsTest.php</exclude>
+        <exclude>tests/Rules/PHPUnit/ExplicitExpectsMockMethodRule/Fixture/MockWithoutExpectsTest.php</exclude>
     </testsuite>
 </phpunit>

src/PHPUnit/TestClassDetector.php

@@ -0,0 +1,30 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\PHPUnit;
+
+use PHPStan\Analyser\Scope;
+
+final class TestClassDetector
+{
+    /**
+     * @var string[]
+     */
+    private const array TEST_FILE_SUFFIXES = [
+        'Test.php',
+        'TestCase.php',
+        'Context.php',
+    ];
+
+    public static function isTestClass(Scope $scope): bool
+    {
+        foreach (self::TEST_FILE_SUFFIXES as $testFileSuffix) {
+            if (str_ends_with($scope->getFile(), $testFileSuffix)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+}

src/Rules/PHPUnit/ExplicitExpectsMockMethodRule.php

@@ -0,0 +1,62 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Rules\PHPUnit;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Expr\PropertyFetch;
+use PhpParser\Node\Expr\Variable;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\IdentifierRuleError;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+use Symplify\PHPStanRules\Enum\RuleIdentifier\PHPUnitRuleIdentifier;
+use Symplify\PHPStanRules\Helper\NamingHelper;
+use Symplify\PHPStanRules\PHPUnit\TestClassDetector;
+
+/**
+ * @implements Rule<MethodCall>
+ *
+ * @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];
+    }
+}

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<FuncCall>
@@ -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;
-    }
 }

tests/Rules/PHPUnit/ExplicitExpectsMockMethodRule/ExplicitExpectsMockMethodRuleTest.php

@@ -0,0 +1,38 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\PHPUnit\ExplicitExpectsMockMethodRule;
+
+use Iterator;
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+use PHPUnit\Framework\Attributes\DataProvider;
+use Symplify\PHPStanRules\Rules\PHPUnit\ExplicitExpectsMockMethodRule;
+
+final class ExplicitExpectsMockMethodRuleTest extends RuleTestCase
+{
+    /**
+     * @param array<int, array<string|int>> $expectedErrorsWithLines
+     */
+    #[DataProvider('provideData')]
+    public function testRule(string $filePath, array $expectedErrorsWithLines): void
+    {
+        $this->analyse([$filePath], $expectedErrorsWithLines);
+    }
+
+    /**
+     * @return Iterator<array<array<int, mixed>, 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();
+    }
+}

tests/Rules/PHPUnit/ExplicitExpectsMockMethodRule/Fixture/MockWithoutExpectsTest.php

@@ -0,0 +1,14 @@
+<?php
+
+namespace Symplify\PHPStanRules\Tests\Rules\PHPUnit\ExplicitExpectsMockMethodRule\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+final class MockWithoutExpectsTest extends TestCase
+{
+    public function test(): void
+    {
+        $mock = $this->createMock(\stdClass::class);
+        $mock->method('someMethod')->willReturn('value');
+    }
+}

tests/Rules/PHPUnit/ExplicitExpectsMockMethodRule/Fixture/SkipMockWithExpectsTest.php

@@ -0,0 +1,17 @@
+<?php
+
+namespace Symplify\PHPStanRules\Tests\Rules\PHPUnit\ExplicitExpectsMockMethodRule\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+final class SkipMockWithExpectsTest extends TestCase
+{
+    public function test(): void
+    {
+        $mock = $this->createMock(\stdClass::class);
+
+        $mock->expects($this->atLeastOnce())
+            ->method('someMethod')
+            ->willReturn('value');
+    }
+}