Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create a rector rule that will rename the array keys from the PHPUnit Dataprovider #330

Merged

Conversation

marcelthole
Copy link
Contributor

@marcelthole marcelthole commented May 17, 2024

Create a rector rule that will rename the array keys from the PHPUnit Dataprovider to the argument name of the called test method to be prepared for the usage of named arguments in PHPUnit 11. This was already deprecated in PHPUnit 10.5

I think this is a risky rule because you could use the same dataProvider for multiple tests and the name of the arguments could be differ between the methods. (That's the point why i didn't added it to a new PHPUnit110 list)

Maybe it make sense to add an configurable option to remove the array key names completly to not use the named argument feature at all?

… DataProvider to the argument name of the called test method to be prepared for the usage of named arguments in PHPUnit 11. This was already deprecated in PHPUnit 10.5
@samsonasik
Copy link
Member

Please run:

vendor/bin/rector && composer fix-cs

To fix CI

Co-authored-by: Abdul Malik Ikhsan <[email protected]>
@marcelthole marcelthole force-pushed the change-dataprovider-namedarguments branch from 1aed14a to 6a5acb0 Compare May 22, 2024 08:45
@marcelthole
Copy link
Contributor Author

Hey,
is there something i could do to get this PR merged? or is something missing here?

Copy link
Member

@samsonasik samsonasik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as it is not included in config/ sets, I am fine with it so you can include yourself as your needs ;)

use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

/**
* @see \Rector\PHPUnit\Tests\PHPUnit100\Rector\Class_\StaticDataProviderClassMethodRector\StaticDataProviderClassMethodRectorTest
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you add @see https://github.com/sebastianbergmann/phpunit ... , which exact part to exact changelog/documentation for this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, added the changelog and the PR

Comment on lines 129 to 132
$wasChanged = $this->refactorArrayKey(
$dataProviderArrayItem,
$namedArgumentsFromTestClass
) || $wasChanged;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it can be more readable with:

if ($this->refactorArrayKey(
                    $dataProviderArrayItem,
                    $namedArgumentsFromTestClass
                )) {
    $wasChanged = true;
}

@samsonasik
Copy link
Member

LGTM, @TomasVotruba let's merge it ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants