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
feat: @PhpCsFixer
ruleset - enable php_unit_data_provider_static
#7685
feat: @PhpCsFixer
ruleset - enable php_unit_data_provider_static
#7685
Conversation
php_unit_data_provider_static
to PhpCsFixer
ruleset@PhpCsFixer
ruleset - enable php_unit_data_provider_static
80f482e
to
1b9fbb4
Compare
doc/ruleSets/PhpCsFixerRisky.rst
Outdated
@@ -31,6 +31,7 @@ Rules | |||
- `no_unset_on_property <./../rules/language_construct/no_unset_on_property.rst>`_ | |||
- `php_unit_data_provider_name <./../rules/php_unit/php_unit_data_provider_name.rst>`_ | |||
- `php_unit_data_provider_return_type <./../rules/php_unit/php_unit_data_provider_return_type.rst>`_ | |||
- `php_unit_data_provider_static <./../rules/php_unit/php_unit_data_provider_static.rst>`_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👎
This is not about style, this is about migration - in Fixer we applied this only because of migration purposes.
All migration rules shall be part of PHPUnit migration ruleset.
I would rather recommend projects to keep config like following:
[
'@PHP{xy}Migration' => true,
'@PHPUnit{xy}Migration' => true,
'@PhpCsFixer' => true, // or @Symfony, @PSR, ...
]
if there is a non-migration rule, it could be part of vendor style (eg @Fixer...
, @Symfony
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even if static providers make sense in all PHPUnits?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Were static data providers always available? If yes, then I would consider it as style rule rather than migration rule. But if they were introduced in some specific version, then it surely is migration rule 😉. Personally: I don't know 😅.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are supported since "always": https://github.com/sebastianbergmann/phpunit/blob/4.8.36/src/Util/Test.php#L440
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls update https://github.com/PHP-CS-Fixer/PHP-CS-Fixer/pull/7685/files#r1459845883 ,
ready to merge afterwards
5fc0d83
to
f8555c1
Compare
php_unit_data_provider_static
is good even for PHPUnit < 10, only static methods were not required there