From 141c46d820a9666fac911c354d2b1cb8008a263c Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Wed, 12 Aug 2026 16:27:33 +0200 Subject: [PATCH] Deprecate AllowMockObjectsWithoutExpectationsAttributeRector and remove it from set The attribute only silences the "mock created without expectations" notice. The correct fix is to add the missing expects() to the test methods that use the mock, which depends on the test intent and cannot be automated. --- config/sets/composer-based.php | 2 - ...WithoutExpectationsAttributeRectorTest.php | 28 -- .../add_to_setup_when_no_expects.php.inc | 54 ---- ..._to_setup_when_no_expects_variable.php.inc | 50 ---- .../Fixture/final_property_as_well.php.inc | 56 ---- ..._tests_methods_define_expectations.php.inc | 25 -- ...if_mock_not_used_in_2_test_methods.php.inc | 23 -- .../Fixture/skip_not_mock_object.php.inc | 26 -- .../Fixture/skip_only_single_test.php.inc | 19 -- .../Fixture/some_class.php.inc | 54 ---- .../Source/ClassToStub.php | 11 - .../Source/SomeFinalClass.php | 8 - .../config/configured_rule.php | 9 - ...ectsWithoutExpectationsAttributeRector.php | 255 +----------------- 14 files changed, 14 insertions(+), 606 deletions(-) delete mode 100644 rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/AllowMockObjectsWithoutExpectationsAttributeRectorTest.php delete mode 100644 rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/add_to_setup_when_no_expects.php.inc delete mode 100644 rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/add_to_setup_when_no_expects_variable.php.inc delete mode 100644 rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/final_property_as_well.php.inc delete mode 100644 rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/skip_if_all_tests_methods_define_expectations.php.inc delete mode 100644 rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/skip_if_mock_not_used_in_2_test_methods.php.inc delete mode 100644 rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/skip_not_mock_object.php.inc delete mode 100644 rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/skip_only_single_test.php.inc delete mode 100644 rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Fixture/some_class.php.inc delete mode 100644 rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Source/ClassToStub.php delete mode 100644 rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/Source/SomeFinalClass.php delete mode 100644 rules-tests/PHPUnit120/Rector/Class_/AllowMockObjectsWithoutExpectationsAttributeRector/config/configured_rule.php 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, + )); } }