[cleanup] remove per-version PHPUnit set configs and constants - #760
Merged
Conversation
Every rule and configuration these sets carried is already registered in the composer-based set, guarded by a "phpunit/phpunit" version constraint. Keeping the per-version sets around only duplicated that content.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #758 / #759.
After every per-version rule was moved into the composer-based set, the
phpunit40.php…phpunit130.phpconfigs held nothing of their own — each rule and rename they registered is already inconfig/sets/composer-based.php, guarded by aphpunit/phpunitversion constraint. So they are removed, along with theirPHPUnitSetListconstants.Verified coverage
Each removed set was checked rule by rule against
composer-based.php:phpunit40staticExpects()rename,>=4.0phpunit50GetMockRectorphpunit60DelegateExceptionArgumentsRector,ExceptionAnnotationRector,AddDoesNotPerformAssertionToNonAssertingTestRector,GetMockBuilderGetMockToCreateMockRector+ the>=6.0rename/class mapsphpunit70RemoveDataProviderTestPrefixRector,scenario→test,>=7.0phpunit80SpecificAssert*rules + param/return type declarations,>=8.0(itsPHPUnit_Framework_MockObject_MockObjectrename is part of the>=6.0class map)phpunit90TestListenerToHooksRector,ExplicitPhpErrorApiRector,SpecificAssertContainsWithoutIdentityRector,AssertRegExpRector+ the>=9.1assert renamesphpunit100withConsecutiverules +>=10.0renames and annotation-to-attribute config.PropertyExistsWithoutAssertRectorwas the one rule not yet there, so it is added in this PR (see below)phpunit110NamedArgumentForDataProviderRectorphpunit120RemoveOverrideFinalConstructTestCaseRectorand the mock-to-stub rulesphpunit125AllowMockObjects*rulesphpunit130expectExceptionMessageIsOrContains()rename,>=13.2For users
return RectorConfig::configure() ->withSets([ - PHPUnitSetList::PHPUNIT_90, + PHPUnitSetList::COMPOSER_BASED, ]);COMPOSER_BASEDpicks the applicable rules from the installedphpunit/phpunitversion, so there is no longer a version constant to pick by hand.Test fixture change
tests/Issues/PHPUnit10DataProvidermoved fromPHPUNIT_100toCOMPOSER_BASED. That set is broader, soAddDoesNotPerformAssertionToNonAssertingTestRectornow also applies to the fixture and the expected output gains one attribute:#[\PHPUnit\Framework\Attributes\DataProvider('fooProvider')] #[\PHPUnit\Framework\Attributes\Test] +#[\PHPUnit\Framework\Attributes\DoesNotPerformAssertions] public function it_should_do_stuff(string $foo): voidPropertyExistsWithoutAssertRector
This rule lived only in
phpunit100.php, so removing that set would have dropped it from every set. It already declares a>=10.0constraint, so it is now registered incomposer-based.php.Registering it surfaced a pre-existing overlap: the rule and the set's
>=10.1RenameMethodRectorboth claimedassertObjectHasAttribute(). Both were registered together in the oldphpunit100.phptoo, so the conflict is not new — it was simply never exercised by a fixture.Resolved in favour of the rename, since these two methods have a direct replacement:
assertObjectHasAttributeandassertObjectNotHasAttributeare therefore removed from the rule's map, and its fixture becomes a skip case. The rule keeps the 7 methods that have no direct replacement: