diff --git a/config/sets/composer-based.php b/config/sets/composer-based.php index 422c719e..0443b60c 100644 --- a/config/sets/composer-based.php +++ b/config/sets/composer-based.php @@ -35,7 +35,6 @@ use Rector\PHPUnit\PHPUnit120\Rector\CallLike\CreateStubOverCreateMockArgRector; use Rector\PHPUnit\PHPUnit120\Rector\Class_\AllowMockObjectsForDataProviderRector; use Rector\PHPUnit\PHPUnit120\Rector\Class_\AllowMockObjectsWhereParentClassRector; -use Rector\PHPUnit\PHPUnit120\Rector\Class_\AllowMockObjectsWithoutExpectationsAttributeRector; use Rector\PHPUnit\PHPUnit120\Rector\Class_\AssertIsTypeMethodCallRector; use Rector\PHPUnit\PHPUnit120\Rector\Class_\PropertyCreateMockToCreateStubRector; use Rector\PHPUnit\PHPUnit120\Rector\Class_\RemoveOverrideFinalConstructTestCaseRector; @@ -113,7 +112,6 @@ // the AllowMockObjectsWithoutExpectations attribute exists since PHPUnit 12.5.2 AllowMockObjectsWhereParentClassRector::class, AllowMockObjectsForDataProviderRector::class, - AllowMockObjectsWithoutExpectationsAttributeRector::class, // the upgrade rules of the per-version sets, each bound to the PHPUnit version // its target API is available from diff --git a/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/AllowMockObjectsWithoutExpectationsAttributeRectorTest.php b/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/AllowMockObjectsWithoutExpectationsAttributeRectorTest.php deleted file mode 100644 index 1d6c75c6..00000000 --- a/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/AllowMockObjectsWithoutExpectationsAttributeRectorTest.php +++ /dev/null @@ -1,28 +0,0 @@ -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/add_to_setup_when_no_expects.php.inc b/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/add_to_setup_when_no_expects.php.inc deleted file mode 100644 index a4dddd3b..00000000 --- a/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/add_to_setup_when_no_expects.php.inc +++ /dev/null @@ -1,54 +0,0 @@ -someMock = $this->createMock(\stdClass::class); - $this->someMock->method('some')->willReturn(true); - } - - public function testOne() - { - } - - public function testTwo() - { - } -} - -?> ------ -someMock = $this->createMock(\stdClass::class); - $this->someMock->method('some')->willReturn(true); - } - - public function testOne() - { - } - - public function testTwo() - { - } -} - -?> diff --git a/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/add_to_setup_when_no_expects_variable.php.inc b/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/add_to_setup_when_no_expects_variable.php.inc deleted file mode 100644 index 0ce59a7c..00000000 --- a/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/add_to_setup_when_no_expects_variable.php.inc +++ /dev/null @@ -1,50 +0,0 @@ -createMock(\stdClass::class); - $someMock->method('some')->willReturn(true); - } - - public function testOne() - { - } - - public function testTwo() - { - } -} - -?> ------ -createMock(\stdClass::class); - $someMock->method('some')->willReturn(true); - } - - public function testOne() - { - } - - public function testTwo() - { - } -} - -?> diff --git a/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/final_property_as_well.php.inc b/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/final_property_as_well.php.inc deleted file mode 100644 index 82c573a3..00000000 --- a/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/final_property_as_well.php.inc +++ /dev/null @@ -1,56 +0,0 @@ -someMock = $this->createMock(SomeFinalClass::class); - $this->someMock->method('some')->willReturn(true); - } - - public function testOne() - { - } - - public function testTwo() - { - } -} - -?> ------ -someMock = $this->createMock(SomeFinalClass::class); - $this->someMock->method('some')->willReturn(true); - } - - public function testOne() - { - } - - public function testTwo() - { - } -} - -?> 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 deleted file mode 100644 index da0e0722..00000000 --- a/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/skip_if_all_tests_methods_define_expectations.php.inc +++ /dev/null @@ -1,25 +0,0 @@ -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 deleted file mode 100644 index a0e82d7a..00000000 --- a/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/skip_if_mock_not_used_in_2_test_methods.php.inc +++ /dev/null @@ -1,23 +0,0 @@ -someMock = $this->createMock(\stdClass::class); - } - - public function testOne() - { - } - - public function testTwo() - { - } -} diff --git a/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/skip_not_mock_object.php.inc b/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/skip_not_mock_object.php.inc deleted file mode 100644 index 2562b4e9..00000000 --- a/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/skip_not_mock_object.php.inc +++ /dev/null @@ -1,26 +0,0 @@ -method('foo')->willReturn('error'); - - $this->foo = $foo; - } - - public function test() - { - $this->foo->foo(); - } -} \ No newline at end of file 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 deleted file mode 100644 index 106dfbbe..00000000 --- a/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/skip_only_single_test.php.inc +++ /dev/null @@ -1,19 +0,0 @@ -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 deleted file mode 100644 index 18f0548d..00000000 --- a/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/some_class.php.inc +++ /dev/null @@ -1,54 +0,0 @@ -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/Source/ClassToStub.php b/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Source/ClassToStub.php deleted file mode 100644 index 58360a94..00000000 --- a/rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Source/ClassToStub.php +++ /dev/null @@ -1,11 +0,0 @@ -withRules([AllowMockObjectsWithoutExpectationsAttributeRector::class]); diff --git a/rules/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector.php b/rules/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector.php index 78e6636e..7a84d3e1 100644 --- a/rules/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector.php +++ b/rules/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector.php @@ -5,133 +5,19 @@ namespace Rector\PHPUnit\PHPUnit120\Rector\Class_; use PhpParser\Node; -use PhpParser\Node\Attribute; -use PhpParser\Node\AttributeGroup; -use PhpParser\Node\Expr\MethodCall; -use PhpParser\Node\Expr\PropertyFetch; -use PhpParser\Node\Expr\Variable; -use PhpParser\Node\Name; -use PhpParser\Node\Name\FullyQualified; use PhpParser\Node\Stmt\Class_; -use PhpParser\Node\Stmt\ClassMethod; -use PHPStan\Reflection\ReflectionProvider; -use PHPStan\Type\NeverType; -use PHPStan\Type\ObjectType; -use Rector\Doctrine\NodeAnalyzer\AttributeFinder; -use Rector\PhpParser\Node\BetterNodeFinder; -use Rector\PHPUnit\Enum\PHPUnitAttribute; -use Rector\PHPUnit\Enum\PHPUnitClassName; -use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer; +use Rector\Configuration\Deprecation\Contract\DeprecatedInterface; +use Rector\Exception\ShouldNotHappenException; use Rector\Rector\AbstractRector; -use Rector\ValueObject\MethodName; -use Rector\ValueObject\PhpVersionFeature; -use Rector\VersionBonding\Contract\ComposerPackageConstraintInterface; -use Rector\VersionBonding\Contract\MinPhpVersionInterface; -use Rector\VersionBonding\ValueObject\ComposerPackageConstraint; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; /** - * The AllowMockObjectsWithoutExpectations attribute was added in PHPUnit 12.5.2 - * - * @see \Rector\PHPUnit\Tests\PHPUnit120\Rector\Class_\AllowMockObjectsWithoutExpectationsAttributeRector\AllowMockObjectsWithoutExpectationsAttributeRectorTest - * - * @see https://github.com/sebastianbergmann/phpunit/commit/24c208d6a340c3071f28a9b5cce02b9377adfd43 + * @deprecated This rule is deprecated, as the attribute only silences the notice. The correct fix is to add the missing + * expects() to the test methods that use the mock, which requires knowing the test intent and cannot be automated. */ -final class AllowMockObjectsWithoutExpectationsAttributeRector extends AbstractRector implements MinPhpVersionInterface, ComposerPackageConstraintInterface +final class AllowMockObjectsWithoutExpectationsAttributeRector extends AbstractRector implements DeprecatedInterface { - public function __construct( - private readonly TestsNodeAnalyzer $testsNodeAnalyzer, - private readonly AttributeFinder $attributeFinder, - private readonly ReflectionProvider $reflectionProvider, - private readonly BetterNodeFinder $betterNodeFinder, - ) { - } - - public function provideComposerPackageConstraint(): ComposerPackageConstraint - { - return new ComposerPackageConstraint('phpunit/phpunit', '>=12.5.2'); - } - - public function getNodeTypes(): array - { - return [Class_::class]; - } - - public function provideMinPhpVersion(): int - { - return PhpVersionFeature::ATTRIBUTES; - } - - /** - * @param Class_ $node - */ - public function refactor(Node $node): ?Class_ - { - if ($this->shouldSkipClass($node)) { - return null; - } - - // even for 0 mocked properties, the variable in setUp() can be mocked - $mockObjectPropertyNames = $this->matchMockObjectPropertyNames($node); - - $missedTestMethodsByMockPropertyName = []; - $usingTestMethodsByMockPropertyName = []; - $testMethodCount = 0; - - foreach ($mockObjectPropertyNames as $mockObjectPropertyName) { - $missedTestMethodsByMockPropertyName[$mockObjectPropertyName] = []; - $usingTestMethodsByMockPropertyName[$mockObjectPropertyName] = []; - - foreach ($node->getMethods() as $classMethod) { - if (! $this->testsNodeAnalyzer->isTestClassMethod($classMethod)) { - continue; - } - - ++$testMethodCount; - - // 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)) { - $usingTestMethodsByMockPropertyName[$mockObjectPropertyName][] = $this->getName($classMethod); - continue; - } - - $missedTestMethodsByMockPropertyName[$mockObjectPropertyName][] = $this->getName($classMethod); - } - } - - // or find a ->method() calls on a setUp() mocked property - $hasAnyMethodInSetup = $this->isMissingExpectsOnMockObjectMethodCallInSetUp($node); - if ($hasAnyMethodInSetup) { - $node->attrGroups[] = new AttributeGroup([ - new Attribute(new FullyQualified(PHPUnitAttribute::ALLOW_MOCK_OBJECTS_WITHOUT_EXPECTATIONS)), - ]); - - return $node; - } - - if (! $this->shouldAddAttribute($missedTestMethodsByMockPropertyName)) { - return null; - } - - // skip sole test method, as those are expected to use all mocks - if ($testMethodCount < 2) { - return null; - } - - if (! $this->isAtLeastOneMockPropertyMockedOnce($usingTestMethodsByMockPropertyName)) { - 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( @@ -196,134 +82,21 @@ public function testTwo(): void } /** - * @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; - } - - /** - * @param array $missedTestMethodsByMockPropertyName + * @return array> */ - private function shouldAddAttribute(array $missedTestMethodsByMockPropertyName): bool + public function getNodeTypes(): array { - 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; + return [Class_::class]; } /** - * @param array $usingTestMethodsByMockPropertyName + * @param Class_ $node */ - private function isAtLeastOneMockPropertyMockedOnce(array $usingTestMethodsByMockPropertyName): bool + public function refactor(Node $node): ?Node { - return array_any( - $usingTestMethodsByMockPropertyName, - fn (array $usingTestMethods): bool => $usingTestMethods !== [] - ); - } - - private function isMissingExpectsOnMockObjectMethodCallInSetUp(Class_ $class): bool - { - $setupClassMethod = $class->getMethod(MethodName::SET_UP); - - if (! $setupClassMethod instanceof ClassMethod) { - return false; - } - - /** @var MethodCall[] $methodCalls */ - $methodCalls = $this->betterNodeFinder->findInstancesOfScoped( - (array) $setupClassMethod->stmts, - MethodCall::class - ); - - foreach ($methodCalls as $methodCall) { - if (! $this->isName($methodCall->name, 'method')) { - continue; - } - - $type = $this->getType($methodCall->var); - if ( - // never type check is needed for fixture with mock final class, example use is possibly by dg/bypass-finals - ! $type instanceof NeverType && - ! $this->isObjectType($methodCall->var, new ObjectType(PHPUnitClassName::MOCK_OBJECT))) { - continue; - } - - if ($methodCall->var instanceof Variable || $methodCall->var instanceof PropertyFetch) { - return true; - } - } - - return false; + throw new ShouldNotHappenException(sprintf( + '"%s" is deprecated, as the attribute only silences the notice. Add the missing expects() to the test methods that use the mock instead.', + self::class, + )); } }