Create a better solution for severity levels

[barrykooij]

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]

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).

[swissspidy]

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:

So you can have warnings with severity 10 and errors with severity 0? 🤔

[joemcgill]

So you can have warnings with severity 10 and errors with severity 0?

Correct. This is similar to the way PHPCS handles severity levels as I understand it. I assume it's so severity level (0–10) and classification (Warning or Error) can be determined separately, rather than mapping severity levels to classification (e.g., anything over a severity level of 7 is an Error).

The benefit of this type of system is so you can filter out issues that are not that important in your reports (low severity issues), but still see both Errors and Warnings.

[ernilambar]

Ref: #441 (comment)

But our main objective is to help developers to develop better and more secure. We actually give two kind of results and pass. Errors and Warnings. I think that we should show warnings, that the developer could work on that, and obviously it won't count as a pass fail.

To achieve this I think we need to introduce two separate severity level like what PHPCS does: --error-severity, and --warning-severity.

[davidperezgar]

Ok