Plugin Review Team: Ensure top level of "Error" includes no false positives.

[bordoni]

As we talked last week @barrykooij will own this task to making sure that we have either:

• A new level of error that includes no false positives.
• Modify the current top level items to not have any false positives.

That is important to enables us to have it be part of the submission on WordPress.org, which should enable us to reduce the Queue and maintain it at a good level.

Currently there is no way to appeal specific items that might be a false positive so this is a must.

[felixarntz]

@bordoni @barrykooij Curious to learn more about this, can you provide a bit more context on what this is about? How could we possibly ensure there are no false positives? Maybe I'm misunderstanding the issue description.

[barrykooij]

@felixarntz This issue is about identifying what phpcs rules are 100% blocking for new submission. These should not have any false positives.

An example of a 100% blocking check is DisallowShortOpenTag. We never accept this, no exceptions and no false positives. This issue is to identify all rules that are like this so we can use these rules as a first step in the plugin upload process.

[felixarntz]

@barrykooij Thanks, makes sense!

Is the idea to put those rules into a new PHPCS config once they're identified? Or a new check group? How would those rules technically be annotated / grouped so that only those would run in the new submission process?

[barrykooij]

@felixarntz I'm not sure yet, whatever works best. I'm currently leaning towards creating a set (check group?) of "blocking" PHPCS rules and if any of these fails, a submission is blocked. Another thing we need to in mind is that we're going to expand the checks to other form of checks than PHPCS, like a readme parser.

I think it would be good to also show this in the "regular" PCP wp-admin result, so a developer can see that the current version of the plugin contains blocking issues.

I'm currently creating a meta trac ticket to discuss how to run only these checks in a new plugin submission. Related to this is also #441

[felixarntz]

@barrykooij

I'm currently leaning towards creating a set (check group?) of "blocking" PHPCS rules and if any of these fails, a submission is blocked. Another thing we need to in mind is that we're going to expand the checks to other form of checks than PHPCS, like a readme parser.

In that case a "check category" probably makes most sense. For instance, we could add a "Plugin Submission" or "Plugin Repo (blocking)" category to https://github.com/WordPress/plugin-check/blob/trunk/includes/Checker/Check_Categories.php#L48 and assign the relevant checks to that category.

[barrykooij]

Alright. I've created a list of checks and PHPCS rules that I think should be included in the first version.

Checks

Code_Obfuscation_Check
File_Type_Check
Localhost_Check
No_Unfiltered_Uploads_Check
Plugin_Header_Text_Domain_Check

PHPCS RULES
based on phpcs-rulesets/plugin-review.xml

Generic.PHP.BacktickOperator.Found
Generic.PHP.DiscourageGoto.Found
Generic.PHP.DisallowShortOpenTag
Generic.PHP.DisallowAlternativePHPTags
WordPress.Security.PluginMenuSlug
WordPress.DB.RestrictedClasses
WordPress.DB.RestrictedFunctions
Generic.PHP.ForbiddenFunctions
WordPress.WP.DeprecatedClasses
WordPress.WP.DeprecatedFunctions
WordPress.WP.DeprecatedParameters
WordPress.DateTime.RestrictedFunctions
WordPress.WP.DiscouragedConstants
WordPress.WP.DeprecatedParameterValues

I'm still in doubt about WordPress.WP.AlternativeFunctions.

In that case a "check category" probably makes most sense. For instance, we could add a "Plugin Submission" or "Plugin Repo (blocking)" category to https://github.com/WordPress/plugin-check/blob/trunk/includes/Checker/Check_Categories.php#L48 and assign the relevant checks to that category.
I've taken a look and I agree this would be a clean way to add this. I can create a new ruleset xml file and create a new check that uses this new ruleset.

The only thing I'm not happy about in this solution is that we don't want to have this be a publicly new check category. Ideally, I see this included in the _ Plugin Repo_ category, perhaps as a new Type of check result. Now Error is the highest find type, maybe we can introduce a level above this like Blocking. This way we can make it clear to people using the plugin that the submission will be blocked in the current state.

@felixarntz

[joemcgill]

One way we could address this problem is by implementing a "severity" system, as discussed in #479.

With that in place, we could set some guidelines for what would cause a check to meet different severity levels and make sure all blocking checks are configured with a severity level above a certain threshold (e.g. 8). WP VIP has a page dedicated to interpreting severity levels for their WordPress-VIP-Go sniffs that could be referenced for inspiration.

[davidperezgar]

So, in somewhere we should cover all severity levels for all checks. I'd suggest to make it in the documentation area.

[davidperezgar]

Closed as it was merged severity PR