Merge pull request #1712 from WordPress-Coding-Standards/feature/vali…

…dhookname-bugfix ValidHookName: bug fix / skip over array keys
WordPress · Jun 3, 2019 · 744b379 · 744b379
2 parents ea20e20 + c6cb5c8
commit 744b379
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 30 deletions.
diff --git a/WordPress/Sniffs/NamingConventions/ValidHookNameSniff.php b/WordPress/Sniffs/NamingConventions/ValidHookNameSniff.php
@@ -10,6 +10,7 @@
 namespace WordPressCS\WordPress\Sniffs\NamingConventions;
 
 use WordPressCS\WordPress\AbstractFunctionParameterSniff;
+use PHP_CodeSniffer\Util\Tokens;
 
 /**
  * Use lowercase letters in action and filter names. Separate words via underscores.
@@ -108,39 +109,60 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p
 			$content[ $i ]  = $this->tokens[ $i ]['content'];
 			$expected[ $i ] = $this->tokens[ $i ]['content'];
 
-			if ( \in_array( $this->tokens[ $i ]['code'], array( \T_CONSTANT_ENCAPSED_STRING, \T_DOUBLE_QUOTED_STRING ), true ) ) {
-				$string = $this->strip_quotes( $this->tokens[ $i ]['content'] );
-
-				/*
-				 * Here be dragons - a double quoted string can contain extrapolated variables
-				 * which don't have to comply with these rules.
-				 */
-				if ( \T_DOUBLE_QUOTED_STRING === $this->tokens[ $i ]['code'] ) {
-					$transform       = $this->transform_complex_string( $string, $regex );
-					$case_transform  = $this->transform_complex_string( $string, $regex, 'case' );
-					$punct_transform = $this->transform_complex_string( $string, $regex, 'punctuation' );
-				} else {
-					$transform       = $this->transform( $string, $regex );
-					$case_transform  = $this->transform( $string, $regex, 'case' );
-					$punct_transform = $this->transform( $string, $regex, 'punctuation' );
-				}
+			// Skip past potential variable array access: $var['Key'].
+			if ( \T_VARIABLE === $this->tokens[ $i ]['code'] ) {
+				do {
+					$open_bracket = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $i + 1 ), null, true );
+					if ( false === $open_bracket
+						|| \T_OPEN_SQUARE_BRACKET !== $this->tokens[ $open_bracket ]['code']
+						|| ! isset( $this->tokens[ $open_bracket ]['bracket_closer'] )
+					) {
+						continue 2;
+					}
 
-				if ( $string === $transform ) {
-					continue;
-				}
+					$i = $this->tokens[ $open_bracket ]['bracket_closer'];
 
-				if ( \T_DOUBLE_QUOTED_STRING === $this->tokens[ $i ]['code'] ) {
-					$expected[ $i ] = '"' . $transform . '"';
-				} else {
-					$expected[ $i ] = '\'' . $transform . '\'';
-				}
+				} while ( isset( $this->tokens[ $i ] ) && $i <= $parameters[1]['end'] );
 
-				if ( $string !== $case_transform ) {
-					$case_errors++;
-				}
-				if ( $string !== $punct_transform ) {
-					$underscores++;
-				}
+				continue;
+			}
+
+			// Skip past non-string tokens.
+			if ( isset( Tokens::$stringTokens[ $this->tokens[ $i ]['code'] ] ) === false ) {
+				continue;
+			}
+
+			$string = $this->strip_quotes( $this->tokens[ $i ]['content'] );
+
+			/*
+			 * Here be dragons - a double quoted string can contain extrapolated variables
+			 * which don't have to comply with these rules.
+			 */
+			if ( \T_DOUBLE_QUOTED_STRING === $this->tokens[ $i ]['code'] ) {
+				$transform       = $this->transform_complex_string( $string, $regex );
+				$case_transform  = $this->transform_complex_string( $string, $regex, 'case' );
+				$punct_transform = $this->transform_complex_string( $string, $regex, 'punctuation' );
+			} else {
+				$transform       = $this->transform( $string, $regex );
+				$case_transform  = $this->transform( $string, $regex, 'case' );
+				$punct_transform = $this->transform( $string, $regex, 'punctuation' );
+			}
+
+			if ( $string === $transform ) {
+				continue;
+			}
+
+			if ( \T_DOUBLE_QUOTED_STRING === $this->tokens[ $i ]['code'] ) {
+				$expected[ $i ] = '"' . $transform . '"';
+			} else {
+				$expected[ $i ] = '\'' . $transform . '\'';
+			}
+
+			if ( $string !== $case_transform ) {
+				$case_errors++;
+			}
+			if ( $string !== $punct_transform ) {
+				$underscores++;
 			}
 		}
 

diff --git a/WordPress/Tests/NamingConventions/ValidHookNameUnitTest.1.inc b/WordPress/Tests/NamingConventions/ValidHookNameUnitTest.1.inc
@@ -83,3 +83,8 @@ do_action( "admin_Head_{${$object->getName()}}_Action_{${$object->getName()}}_Ac
 // Make sure that deprecated hook names are ignored for this sniff.
 do_action_deprecated( "admin_Head_$Post admin_Head_$Post" ); // Ok.
 apply_filters_deprecated( "admin_Head_$Post->ID admin_Head_$Post->ID" ); // Ok.
+
+// Ignore array keys.
+do_action( 'prefix_block_' . $block['blockName'] ); // Ok.
+do_action( 'prefix_block_' . $block  [  'blockName'  ] . '_More_hookname' ); // Error - use lowercase (second part of the hook name).
+do_action( "prefix_block_{$block['blockName']}" ); // Ok.
diff --git a/WordPress/Tests/NamingConventions/ValidHookNameUnitTest.php b/WordPress/Tests/NamingConventions/ValidHookNameUnitTest.php
@@ -68,6 +68,7 @@ public function getErrorList( $testFile = 'ValidHookNameUnitTest.1.inc' ) {
 					79 => 1,
 					80 => 1,
 					81 => 1,
+					89 => 1,
 				);
 
 			case 'ValidHookNameUnitTest.2.inc':