Add support for severity levels for warnings and errors

[ernilambar]

Fixes #479

• Default severity level for check is 5
• --severity argument is added for wp plugin check

Example:

# Show errors and warnings with severity greater or equal to 7
wp plugin check foo-sample --severity=7   

[ernilambar]

Since we don't have different severity level in our checks currently I have not added any tests. After this PR is merged #518 (some strange issue has appeared), we can add tests with multiple severity levels.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ernilambar <[email protected]>
Co-authored-by: davidperezgar <[email protected]>
Co-authored-by: swissspidy <[email protected]>
Co-authored-by: barrykooij <[email protected]>
Co-authored-by: joemcgill <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[ernilambar]

PR has been updated with feature tests.

[davidperezgar]

For me it seems correct. Where do we put the severity in check? I'd update some documentation in the plugin about severity levels.

[ernilambar]

For sniffer rules, we can assign severity like this in phpcs-rulesets/plugin-review.xml:

<rule ref="WordPress.WP.PostsPerPage">
  <severity>9</severity>
</rule>

For other type of check like add_result_error_for_file() or add_result_warning_for_file(), there is addition of new parameter for severity

$this->add_result_error_for_file(
  $result,
  __( 'Prohibited text found.', 'pcp-addon' ),
  'prohibited_text_detected',
  $file,
  0,
  0,
  8
);

In this example, 8 is set as severity for the prohibited_text_detected check.

For checks which are extended from Abstract_PHP_CodeSniffer_Check, like following, I could not find the way to change the severity level:

protected function get_args() {
	return array(
		'extensions' => 'php',
		'standard'   => 'WordPress',
		'sniffs'     => 'WordPress.WP.I18n',
	);
}

[ernilambar]

Currently several readme warnings are kept under same error code readme_parser_warnings. If we need to have separate severity level for separate error code then we need to keep unique error code for each warnings.
Ref: https://github.com/WordPress/plugin-check/blob/trunk/includes/Checker/Checks/Plugin_Repo/Plugin_Readme_Check.php#L538

[davidperezgar]

I think they are all Warnings.

[davidperezgar]

Thanks Nilambar, you have solved the conflicts and we have now new argument for severity.

[swissspidy]

What about the suggestion at #479 (comment) to have separate severity levels for warnings and errors

[ernilambar]

PR updated with feature tests and also with separate severity arguments.

[davidperezgar]

Why did you separate the severity?

I undestood that 0-5 is warning, and 6-10 is error. After that, we have different severity levels. I think is less complicated.

[swissspidy]

This was suggested at #479 (comment)

let‘s discuss it there

seperate levels brings more flexibility

[ernilambar]

Why did you separate the severity?

I undestood that 0-5 is warning, and 6-10 is error. After that, we have different severity levels. I think is less complicated.

This is not the approach this PR has implemented. Since our significant checks used PHPCS, it'd better we follow the similar approach it follows.

[davidperezgar]

Ok

[swissspidy]

Looks good to me but would appreciate more reviews.

[davidperezgar]

Ok, I ping to others.

[barrykooij]

Look great. I agree keeping separate levels for warnings and errors for that extra bit of flexibility. For plugin repo specific checks, we'll have an extra look afterward if all levels are the way we like, but that's non-blocking for this PR.

[davidperezgar]

Perfect!