diff --git a/config/sets/phpunit120.php b/config/sets/phpunit120.php index 611a42b8..5990bd7b 100644 --- a/config/sets/phpunit120.php +++ b/config/sets/phpunit120.php @@ -14,5 +14,9 @@ // stubs over mocks CreateStubOverCreateMockArgRector::class, + + // experimental, from PHPUnit 12.5.2 + // @see https://github.com/sebastianbergmann/phpunit/commit/24c208d6a340c3071f28a9b5cce02b9377adfd43 + // AllowMockObjectsWithoutExpectationsAttributeRector::class, ]); }; diff --git a/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/AllowMockObjectsWithoutExpectationsAttributeRectorTest.php b/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/AllowMockObjectsWithoutExpectationsAttributeRectorTest.php new file mode 100644 index 00000000..1d6c75c6 --- /dev/null +++ b/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/AllowMockObjectsWithoutExpectationsAttributeRectorTest.php @@ -0,0 +1,28 @@ +doTestFile($filePath); + } + + public static function provideData(): Iterator + { + return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule.php'; + } +} diff --git a/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/skip_if_all_tests_methods_define_expectations.php.inc b/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/skip_if_all_tests_methods_define_expectations.php.inc new file mode 100644 index 00000000..da0e0722 --- /dev/null +++ b/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/skip_if_all_tests_methods_define_expectations.php.inc @@ -0,0 +1,25 @@ +someMock = $this->createMock(\stdClass::class); + } + + public function testOne() + { + $this->someMock->method('doSomething')->willReturn('value'); + } + + public function testTwo() + { + $this->someMock->method('doSomethingElse')->willReturn('another value'); + } +} diff --git a/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/skip_if_mock_not_used_in_2_test_methods.php.inc b/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/skip_if_mock_not_used_in_2_test_methods.php.inc new file mode 100644 index 00000000..2d61aa44 --- /dev/null +++ b/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/skip_if_mock_not_used_in_2_test_methods.php.inc @@ -0,0 +1,24 @@ +someMock = $this->createMock(\stdClass::class); + } + + public function testOne() + { + } + + public function testTwo() + { + + } +} diff --git a/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/skip_only_single_test.php.inc b/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/skip_only_single_test.php.inc new file mode 100644 index 00000000..106dfbbe --- /dev/null +++ b/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/skip_only_single_test.php.inc @@ -0,0 +1,19 @@ +someMock = $this->createMock(\stdClass::class); + } + + public function testOne() + { + } +} diff --git a/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/some_class.php.inc b/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/some_class.php.inc new file mode 100644 index 00000000..18f0548d --- /dev/null +++ b/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/some_class.php.inc @@ -0,0 +1,54 @@ +someMock = $this->createMock(\stdClass::class); + } + + public function testOne() + { + $this->someMock->method('doSomething')->willReturn('value'); + } + + public function testTwo() + { + } +} + +?> +----- +someMock = $this->createMock(\stdClass::class); + } + + public function testOne() + { + $this->someMock->method('doSomething')->willReturn('value'); + } + + public function testTwo() + { + } +} + +?> diff --git a/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/config/configured_rule.php b/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/config/configured_rule.php new file mode 100644 index 00000000..baae2bb5 --- /dev/null +++ b/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/config/configured_rule.php @@ -0,0 +1,9 @@ +withRules([AllowMockObjectsWithoutExpectationsAttributeRector::class]); diff --git a/rules/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector.php b/rules/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector.php new file mode 100644 index 00000000..91ba39f9 --- /dev/null +++ b/rules/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector.php @@ -0,0 +1,239 @@ +shouldSkipClass($node)) { + return null; + } + + $mockObjectPropertyNames = $this->matchMockObjectPropertyNames($node); + + // there are no mock object properties + if ($mockObjectPropertyNames === []) { + return null; + } + + // @todo add the attribute if has more than 1 public test* method + $missedTestMethodsByMockPropertyName = []; + $testMethodCount = 0; + + foreach ($mockObjectPropertyNames as $mockObjectPropertyName) { + $missedTestMethodsByMockPropertyName[$mockObjectPropertyName] = []; + + foreach ($node->getMethods() as $classMethod) { + if (! $this->testsNodeAnalyzer->isTestClassMethod($classMethod)) { + continue; + } + + // is a mock property used in the class method, as part of some method call? guessing mock expectation is set + // skip if so + if ($this->isClassMethodUsingMethodCallOnPropertyNamed($classMethod, $mockObjectPropertyName)) { + continue; + } + + $missedTestMethodsByMockPropertyName[][] = $this->getName($classMethod); + ++$testMethodCount; + } + } + + if (! $this->shouldAddAttribute($missedTestMethodsByMockPropertyName)) { + return null; + } + + // add attribute + $node->attrGroups[] = new AttributeGroup([ + new Attribute(new FullyQualified(PHPUnitAttribute::ALLOW_MOCK_OBJECTS_WITHOUT_EXPECTATIONS)), + ]); + + return $node; + } + + public function getRuleDefinition(): RuleDefinition + { + return new RuleDefinition( + 'Add #[AllowMockObjectsWithoutExpectations] attribute to PHPUnit test classes with mock properties used in multiple methods', + [ + new CodeSample( + <<<'CODE_SAMPLE' +use PHPUnit\Framework\TestCase; +final class SomeTest extends TestCase +{ + private \PHPUnit\Framework\MockObject\MockObject $someServiceMock; + + protected function setUp(): void + { + $this->someServiceMock = $this->createMock(SomeService::class); + } + + public function testOne(): void + { + // use $this->someServiceMock + } + + public function testTwo(): void + { + // use $this->someServiceMock + } +} +CODE_SAMPLE + , + <<<'CODE_SAMPLE' +use PHPUnit\Framework\TestCase; +use PHPUnit\Framework\Attributes\AllowMockObjectsWithoutExpectations; + +#[AllowMockObjectsWithoutExpectations] +final class SomeTest extends TestCase +{ + private \PHPUnit\Framework\MockObject\MockObject $someServiceMock; + + protected function setUp(): void + { + $this->someServiceMock = $this->createMock(SomeService::class); + } + + public function testOne(): void + { + // use $this->someServiceMock + } + + public function testTwo(): void + { + // use $this->someServiceMock + } +} +CODE_SAMPLE + ), + + ] + ); + + } + + /** + * @return string[] + */ + private function matchMockObjectPropertyNames(Class_ $class): array + { + $propertyNames = []; + + foreach ($class->getProperties() as $property) { + if (! $property->type instanceof Name) { + continue; + } + + if (! $this->isName($property->type, PHPUnitClassName::MOCK_OBJECT)) { + continue; + } + + $propertyNames[] = $this->getName($property->props[0]); + } + + return $propertyNames; + } + + private function shouldSkipClass(Class_ $class): bool + { + if (! $this->testsNodeAnalyzer->isInTestClass($class)) { + return true; + } + + // attribute must exist for the rule to work + if (! $this->reflectionProvider->hasClass(PHPUnitAttribute::ALLOW_MOCK_OBJECTS_WITHOUT_EXPECTATIONS)) { + return true; + } + + // already filled + if ($this->attributeFinder->hasAttributeByClasses( + $class, + [PHPUnitAttribute::ALLOW_MOCK_OBJECTS_WITHOUT_EXPECTATIONS] + )) { + return true; + } + + // has mock objects properties and setUp() method? + + $setupClassMethod = $class->getMethod(MethodName::SET_UP); + return ! $setupClassMethod instanceof ClassMethod; + } + + private function isClassMethodUsingMethodCallOnPropertyNamed(ClassMethod $classMethod, string $mockObjectPropertyName): bool + { + /** @var MethodCall[] $methodCalls */ + $methodCalls = $this->betterNodeFinder->findInstancesOfScoped([$classMethod], [MethodCall::class]); + foreach ($methodCalls as $methodCall) { + if (!$methodCall->var instanceof PropertyFetch) { + continue; + } + + $propertyFetch = $methodCall->var; + + // we found a method call on a property fetch named + if ($this->isName($propertyFetch, $mockObjectPropertyName)) { + return true; + } + } + + return false; + } + + private function shouldAddAttribute(array $missedTestMethodsByMockPropertyName): bool + { + foreach ($missedTestMethodsByMockPropertyName as $missedTestMethods) { + // all test methods are using method calls on the mock property, so skip + if (count($missedTestMethods) === 0) { + continue; + } + + return true; + } + + return false; + } +} diff --git a/src/Enum/PHPUnitAttribute.php b/src/Enum/PHPUnitAttribute.php index bf09c0fa..a540f7f6 100644 --- a/src/Enum/PHPUnitAttribute.php +++ b/src/Enum/PHPUnitAttribute.php @@ -23,4 +23,9 @@ final class PHPUnitAttribute public const string REQUIRES_SETTING = 'PHPUnit\Framework\Attributes\RequiresSetting'; public const string TEST = 'PHPUnit\Framework\Attributes\Test'; + + /** + * @see https://github.com/sebastianbergmann/phpunit/commit/24c208d6a340c3071f28a9b5cce02b9377adfd43 + */ + public const string ALLOW_MOCK_OBJECTS_WITHOUT_EXPECTATIONS = 'PHPUnit\Framework\Attributes\AllowMockObjectsWithoutExpectations'; } diff --git a/src/Enum/PHPUnitClassName.php b/src/Enum/PHPUnitClassName.php index 51a4ee21..5171edfa 100644 --- a/src/Enum/PHPUnitClassName.php +++ b/src/Enum/PHPUnitClassName.php @@ -23,6 +23,8 @@ final class PHPUnitClassName public const string TEST_LISTENER = 'PHPUnit\Framework\TestListener'; + public const string MOCK_OBJECT = 'PHPUnit\Framework\MockObject\MockObject'; + /** * @var string[] */