search

rectorphp / rector-phpunit

Rector upgrade rules for PHPUnit
http://getrector.com
MIT License
61 stars 46 forks source link

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

Closed gnutix closed 1 week ago

gnutix commented 1 week ago

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 commented 1 week ago

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

gnutix commented 1 week ago

Sorry, too much on my plate these days.

TomasVotruba commented 1 week ago

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

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