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

NoPagingSniff: False negative when there is no comma after the parameter #713

Closed rebeccahum closed 1 year ago

rebeccahum commented 2 years ago

Bug Description

There's a false negative when there's no comma after a single item array. For example, this will flag as expected:

$args = array(
    'nopaging'       => true, // Bad.
);

However, this won't:

$args = array(
    'nopaging'       => true // Bad.
);

Minimal Code Snippet

This should flag as an error:

$args = array( 'nopaging' => true );

Error Code

WordPressVIPMinimum.Performance.NoPaging.nopaging_nopaging

Environment

Use php -v and composer show to get versions.

Question Answer
PHP version 7.4.27
PHP_CodeSniffer version 3.6.2
VIPCS version 2.3.3

Tested Against master branch?

rebeccahum commented 2 years ago

@jrfnl Is this expected of the AbstractArrayAssignmentRestrictionsSniff?

GaryJones commented 1 year ago

This seems like an upstream issue. Doesn't seem to be fixed with WPCS develop (pre-3.0) branch.

The abstract class is tested with https://github.com/WordPress/WordPress-Coding-Standards/blob/develop/WordPress/Tests/DB/SlowDBQueryUnitTest.inc and https://github.com/WordPress/WordPress-Coding-Standards/blob/develop/WordPress/Tests/WP/PostsPerPageUnitTest.inc and neither of those have an array key and value without a trailing comma. cc @jrfnl

jrfnl commented 1 year ago

I agree, this should be reported upstream.

Also note that the AbstractArrayAssignmentRestrictionsSniff class is one I've barely ever touched (original was created before my time) as the premise of that sniff is iffy to say the least, so it's an abstract which is likely to have quite a few more flaws.

GaryJones commented 1 year ago

Closing in favour of https://github.com/WordPress/WordPress-Coding-Standards/issues/2211.