Skip to content

Commit

Permalink
AbstractArrayAssignmentRestrictions: bug fix - improve value capturing
Browse files Browse the repository at this point in the history
This commit improves the way the `AbstractArrayAssignmentRestrictionsSniff` captures the value of an array key assignment before it gets passed on to the callback for further examination.

As things were previously, the sniff would go wrong in two cases:
1. Without a trailing comma after an array item, the long/short array close bracket would be included in the value, while it shouldn't be.
    This was reported in issue 2211.
2. The sniff would not always capture a complete value when the value contained a function call, sub-array, closure etc; basically anything which can contain a comma or semi-colon, but where that comma/semi-colon is in an "inner" construct and not the "outer" construct which is being captured.
    This was reported in issue 2027

The fix included in this commit fixes the first issue completely and a partial fix for the second issue.

Includes tests via the `WordPress.WP.PostsPerPage` sniff.

Fixes 2211
  • Loading branch information
jrfnl committed Jun 25, 2023
1 parent 4723506 commit fc01694
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 4 deletions.
16 changes: 12 additions & 4 deletions WordPress/AbstractArrayAssignmentRestrictionsSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,18 @@ public function process_token( $stackPtr ) {
if ( isset( Tokens::$stringTokens[ $this->tokens[ $keyIdx ]['code'] ] )
&& ! is_numeric( $this->tokens[ $keyIdx ]['content'] )
) {
$key = TextStrings::stripQuotes( $this->tokens[ $keyIdx ]['content'] );
$valStart = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $operator + 1 ), null, true );
$valEnd = $this->phpcsFile->findNext( array( \T_COMMA, \T_SEMICOLON ), ( $valStart + 1 ), null, false, null, true );
$val = trim( GetTokensAsString::compact( $this->phpcsFile, $valStart, ( $valEnd - 1 ), true ) );
$key = TextStrings::stripQuotes( $this->tokens[ $keyIdx ]['content'] );
$valStart = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $operator + 1 ), null, true );
$valEnd = $this->phpcsFile->findEndOfStatement( $valStart, \T_COLON );
if ( \T_COMMA === $this->tokens[ $valEnd ]['code']
|| \T_SEMICOLON === $this->tokens[ $valEnd ]['code']
) {
// FindEndOfStatement includes the comma/semi-colon if that's the end of the statement.
// That's not what we want (and inconsistent), so remove it.
--$valEnd;
}

$val = trim( GetTokensAsString::compact( $this->phpcsFile, $valStart, $valEnd, true ) );
$val = TextStrings::stripQuotes( $val );
$inst[ $key ] = array(
'value' => $val,
Expand Down
6 changes: 6 additions & 0 deletions WordPress/Tests/WP/PostsPerPageUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,9 @@ $query_args['posts_per_page'] ??= 200; // Bad.
$val = match($val) {
'posts_per_page' => 999, // OK, not an array assignment.
};

// Verify handling of arrays without trailing comma after the last array item.
$args = array( 'posts_per_page' => 999 ); // Bad.
$args = [
'posts_per_page' => 999
]; // Bad.
2 changes: 2 additions & 0 deletions WordPress/Tests/WP/PostsPerPageUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ public function getWarningList() {
59 => 1,
67 => 1,
72 => 1,
80 => 1,
82 => 1,
);
}
}

0 comments on commit fc01694

Please sign in to comment.