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

NamedArgumentForDataProviderRector changes data providers that it doesn't need to #346

Closed
gnutix opened this issue Jun 29, 2024 · 3 comments · Fixed by #348
Closed

NamedArgumentForDataProviderRector changes data providers that it doesn't need to #346

gnutix opened this issue Jun 29, 2024 · 3 comments · Fixed by #348

Comments

@gnutix
Copy link

gnutix commented Jun 29, 2024

Hey ya,

Let me know if I'm wrong, but I think this kind of change is unnecessary (I tried reverting the change and PHPUnit does not report any deprecation) :

image

While this one is probably necessary, as there cannot be un-named arguments after named arguments (yet PHPUnit does not report the deprecation either... probably a miss on their side) :

image

It doesn't really hurt though, so I get it if you don't want to fix it. But it might generate quite of lot of useless diff for some projects, which could be avoided.

gnutix

@TomasVotruba
Copy link
Member

Hey, I agree. There should not be changed.
Could you send PR with fix and test?

@gnutix
Copy link
Author

gnutix commented Jun 29, 2024

Sorry, too much on my plate these days.

@TomasVotruba
Copy link
Member

No worries. The rule works as intended, it's just an optional way to use a feature: sebastianbergmann/phpunit#5225

I'll remove the rule from main set, so it's not enforcing an optional feature.

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