Automattic / VIP-Coding-Standards

PHP_CodeSniffer ruleset to enforce WordPress VIP coding standards.
https://wpvip.com/documentation/how-to-install-php-code-sniffer-for-wordpress-com-vip/
Other
236 stars 40 forks source link

Remove Code Sniff for Super Global Usage #427

Closed mdbitz closed 5 years ago

mdbitz commented 5 years ago

What problem would the enhancement address for VIP?

Reduce the amount of noise and/or code comments clients add to reduce noise :)

Describe the solution you'd like

Lets remove the code sniff that warns that a Super Global was used.

WordPress.VIP.SuperGlobalInputUsage.AccessDetected

Additional context

Clients are I would say spamming their code with comments to ignore phpcs warning rules

// phpcs:ignore WordPress.VIP.SuperGlobalInputUsage.AccessDetected

If clients are not finding this info helpful then we should remove it.

GaryJones commented 5 years ago

VIPCS 1.0.0 doesn't use WordPress.VIP - it would only be used where a client has been using an old version of VIPCS.

WordPress.VIP was removed in WPCS 2.0.

WPCS 2.0.0 and 2.1.0 made improvements regarding access of superglobals, and VIPCS 2.0 . can take advantage of these improvements.

Here are some examples that would previously have been caught / need ignoring:

if ( ! isset( $_POST[ self::NONCE_KEY ] ) ) {
    return;
}
if ( ! wp_verify_nonce( sanitize_text_field( wp_unslash( $_POST[ self::NONCE_KEY ] ) ), 'save-ads' ) ) {
    return;
}
if ( isset( $_POST[ self::DISABLED_META_KEY ] ) ) {
    $ads_disabled = intval( sanitize_text_field( wp_unslash( $_POST[ self::DISABLED_META_KEY ] ) ) );
}

I tested these against VIPCS 1.0.0, and they don't cause a violation for simply accessing the superglobal. As such, I'm going to mark this one as invalid. Please re-open if I've missed anything.