WordPress / plugin-check

A repository for the new Plugin Check plugin from the WordPress Performance and Plugins Team.
https://wordpress.org/plugins/plugin-check/
GNU General Public License v2.0
195 stars 39 forks source link

Create a better solution for severity levels #479

Open barrykooij opened 3 days ago

barrykooij commented 3 days ago

Another thing we'd have to change before we can integrate on w.org is a more flexible "severity levels" solutions. Currently, we have 2 array variables $errors and $warnings but we need to be able to extend this.

For the w.org integration specifically we need a higher level than error, called (something along the lines of) "blocking". If this is > 0, the submission will not be allowed to .org. This is mainly because we can't automatically block every error, because of false positives. And reducing errors to warning because of this reason feels wrong.

Also, in the future, I'd love to see something like a "notice" added with suggestions / best practices. I suggest we move away from having a variable per "level" and move into a more extendable way of setting this up via a "severity level collection" like solution. This way other people can even add (or remove) levels via a filter for example.

Relevant file: https://github.com/WordPress/plugin-check/blob/trunk/includes/Checker/Check_Result.php#L31

I'll also add this in the doc / plan. Love to hear your thoughts on this.


Talked this over with @joemcgill in Slack, we discussed the possibility for having different ruleset xml files and/or using PHPCS severity levels. This solves the PHPCS side of things, but PCP has more (and will have a lot more in the future) checks not related to PHPCS. We need to support multiple severity levels for this as well.

joemcgill commented 3 days ago

Our conversation can be found here.

To summarize, I'd suggest trying to adapt the existing severity levels strategy used by PHPCS.

We would still sort failed checks into two groups, Errors and Warning, but each would include a severity level ranging from 0–10, with 5 being the default level that is shown in reports, unless otherwise specified. This would allow the output of checks to be limited to only show errors or warnings that meet a specific severity threshold:

ex: wp plugin check ${slug} --ignore-warnings --severity=8

For any checks that are extending the Abstract_PHP_CodeSniffer_Check class, we could pass the severity levels as args to the PHPCS Runner, and/or add a new param to the Amend_Check_Result::add_result_message_for_file() so that the severity is collected along with other info, like file, line, column, etc.

We would need to adapt non PHPCS checks to make use of this severity system so that any messages not meeting the specified severity level would either be filtered out upon output or not saved to the arrays in the first place (probably the former).