Closed felixarntz closed 1 month ago
Thanks @felixarntz the AC's look good to me.
I think we might want to consider how additional attributes are added for these checks. I've noted in #203 that the PHPCodeSniffer_Sniff_Check
constructor would need to accept a category parameter, but what about any other attributes such as stability flags (#92)?
Although this approach will reduce repeated code it does bring some restrictions. However, we could take this approach for now and adjust as we need.
I left a reply to your other comment, for the class here we could use traits probably for both the category and stability.
For stability, we could even consider to always make these PHPCS checks stable for now, since they would always use logic that is already established (e.g. through WPCS) anyway.
Every check now has a dedicated description & documentation URL. Not sure this refactoring is worth it anymore, closing as wontfix/maybelater
Is your enhancement related to a problem? Please describe.
The following checks currently rely on an extremely similar class implementation:
Enqueued_Scripts_In_Footer_Check
I18n_Usage_Check
Late_Escaping_Check
Performant_WP_Query_Params_Check
Implementing a new class for every single sniff check is unnecessarily tedious, and it will just get worse in the future as we add more PHPCS sniff-based checks.
Therefore, we should introduce a single class for that purpose, which can simply be passed the sniff to use in a constructor parameter.
Designs
PHPCodeSniffer_Sniff_Check
class that extendsAbstract_PHP_CodeSniffer_Check
.array $sniffs
.get_args()
method as follows:extensions
as "php".sniffs
as a comma-separated list of$sniffs
.standard
as a comma-separated list of the unique first segments of each element in$sniffs
(e.g. for a single sniff "WordPress.DB.SlowDBQuery", it should use "WordPress" as the standard to use).Abstract_Check_Runner::register_checks()
to use instances of the new class, passing the relevant sniffs. At the time of writing, this applies to the following checks:enqueued_scripts_in_footer
i18n_usage
late_escaping
performant_wp_query_params
PHPCodeSniffer_Sniff_Check
. It can use any sniff, e.g. one of the ones we already had tests for (likeWordPress.WP.I18n
), so that we can basically just copy over the tests rather than writing new ones from scratch.Describe alternatives you've considered
No response
Code of Conduct