diff --git a/Yoast/Sniffs/NamingConventions/ValidHookNameSniff.php b/Yoast/Sniffs/NamingConventions/ValidHookNameSniff.php index 94e9e4a6..ea0274b6 100644 --- a/Yoast/Sniffs/NamingConventions/ValidHookNameSniff.php +++ b/Yoast/Sniffs/NamingConventions/ValidHookNameSniff.php @@ -4,6 +4,7 @@ use PHP_CodeSniffer\Util\Tokens; use PHPCSUtils\Utils\TextStrings; +use WordPressCS\WordPress\Helpers\WPHookHelper; use WordPressCS\WordPress\Sniffs\NamingConventions\ValidHookNameSniff as WPCS_ValidHookNameSniff; use YoastCS\Yoast\Utils\CustomPrefixesTrait; @@ -22,7 +23,7 @@ * Check the number of words in hook names and the use of the correct prefix-type, * but only when plugin specific prefixes have been passed. * - * {@internal For now allows for an array of both old-style as well as new-style + * {@internal For now, allows for an array of both old-style as well as new-style * prefixes during the transition period. * Once all plugins have been transitioned over to use the new-style * namespace-like prefix for hooks, the `WrongPrefix` warning should be @@ -71,7 +72,7 @@ final class ValidHookNameSniff extends WPCS_ValidHookNameSniff { * Keep track of the content of first text string which was passed to the `transform()` * method as it may be repeatedly called for the same token. * - * @var bool + * @var string */ private $first_string = ''; @@ -89,14 +90,21 @@ final class ValidHookNameSniff extends WPCS_ValidHookNameSniff { /** * Process the parameters of a matched function. * - * @param int $stackPtr The position of the current token in the stack. - * @param string $group_name The name of the group which was matched. - * @param string $matched_content The token content (function name) which was matched. - * @param array $parameters Array with information about the parameters. + * @param int $stackPtr The position of the current token in the stack. + * @param string $group_name The name of the group which was matched. + * @param string $matched_content The token content (function name) which was + * matched in lowercase. + * @param array> $parameters Array with information about the parameters. * * @return void */ public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) { + $hook_name_param = WPHookHelper::get_hook_name_param( $matched_content, $parameters ); + if ( $hook_name_param === false ) { + // If we can't find the hook name parameter, there's nothing to do, so bow out. + return; + } + /* * The custom prefix should be in the first text passed to `transform()` for each * matched function call. @@ -113,8 +121,7 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p * If any prefixes were passed, check if this is a hook belonging to the plugin being checked. */ if ( empty( $this->validated_prefixes ) === false ) { - $param = $parameters[1]; - $first_non_empty = $this->phpcsFile->findNext( Tokens::$emptyTokens, $param['start'], ( $param['end'] + 1 ), true ); + $first_non_empty = $this->phpcsFile->findNext( Tokens::$emptyTokens, $hook_name_param['start'], ( $hook_name_param['end'] + 1 ), true ); $found_prefix = ''; if ( isset( Tokens::$stringTokens[ $this->tokens[ $first_non_empty ]['code'] ] ) ) { @@ -157,7 +164,7 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p } // Do the YoastCS specific hook name length and prefix check. - $this->verify_yoast_hook_name( $stackPtr, $parameters ); + $this->verify_yoast_hook_name( $stackPtr, $hook_name_param ); } /** @@ -199,15 +206,18 @@ protected function transform( $text_string, $regex, $transform_type = 'full' ) { /** * Additional YoastCS specific hook name checks. * - * @param int $stackPtr The position of the current token in the stack. - * @param array $parameters Array with information about the parameters. + * @param int $stackPtr The position of the current token in the stack. + * @param array $hook_name_param Array with information about the hook name parameter. * * @return void */ - public function verify_yoast_hook_name( $stackPtr, $parameters ) { + private function verify_yoast_hook_name( $stackPtr, $hook_name_param ) { - $param = $parameters[1]; - $first_non_empty = $this->phpcsFile->findNext( Tokens::$emptyTokens, $param['start'], ( $param['end'] + 1 ), true ); + $first_non_empty = $this->phpcsFile->findNext( Tokens::$emptyTokens, $hook_name_param['start'], ( $hook_name_param['end'] + 1 ), true ); + if ( $first_non_empty === false ) { + // Shouldn't be possible as we've checked this before. + return; // @codeCoverageIgnore + } /* * Check that the namespace-like prefix is used for hooks. @@ -270,7 +280,7 @@ public function verify_yoast_hook_name( $stackPtr, $parameters ) { $allow = [ \T_CONSTANT_ENCAPSED_STRING ]; $allow += Tokens::$emptyTokens; - $has_non_string = $this->phpcsFile->findNext( $allow, $param['start'], ( $param['end'] + 1 ), true ); + $has_non_string = $this->phpcsFile->findNext( $allow, $hook_name_param['start'], ( $hook_name_param['end'] + 1 ), true ); if ( $has_non_string !== false ) { /* * Double quoted string or a hook name concatenated together, checking the word count for the @@ -281,10 +291,13 @@ public function verify_yoast_hook_name( $stackPtr, $parameters ) { * be thrown when PHPCS is explicitly requested to check with a lower severity. */ $this->phpcsFile->addWarning( - 'Hook name could not reliably be examined for maximum word count. Please verify this hook name manually. Found: %s', + 'Hook name could not reliably be examined for maximum word count (max is %d words). Please verify this hook name manually. Found: %s', $first_non_empty, 'NonString', - [ $param['raw'] ], + [ + $this->max_words, + $hook_name_param['raw'], + ], 3 ); @@ -305,10 +318,6 @@ public function verify_yoast_hook_name( $stackPtr, $parameters ) { $this->phpcsFile->recordMetric( $stackPtr, 'Nr of words in hook name', $part_count ); - if ( $part_count <= $this->recommended_max_words && $part_count <= $this->max_words ) { - return; - } - if ( $part_count > $this->max_words ) { $error = 'A hook name is not allowed to consist of more than %d words after the plugin prefix. Words found: %d in %s'; $data = [ diff --git a/Yoast/Tests/NamingConventions/ValidHookNameUnitTest.inc b/Yoast/Tests/NamingConventions/ValidHookNameUnitTest.inc index 80882350..6704b7dc 100644 --- a/Yoast/Tests/NamingConventions/ValidHookNameUnitTest.inc +++ b/Yoast/Tests/NamingConventions/ValidHookNameUnitTest.inc @@ -140,5 +140,13 @@ apply_filters( "Yoast\\WP\\Plugin\\hookname", $var); // OK. apply_filters( "Yoast\WP\Plugin\hookname", $var); // Warning, missing escaping x 3. apply_filters( "Yoast\WP\\Plugin\hook{$name}", $var); // Warning, missing escaping x 2 + warning at severity 3 for word count. + +// Safeguard support for PHP 8.0+ named parameters. +do_action_ref_array( hook: 'My-Hook', args: $args ); // OK. Well, not really, but using the wrong parameter name, so not our concern. +do_action_ref_array( args: $args, hook_name: 'Yoast\WP\Plugin\Test\some_hook_name', ); // OK. +do_action_ref_array( args: $args, hook_name: "yoast_plugin_some_hook_name", ); // Warning - wrong prefix. +do_action_ref_array( args: $args, hook_name: 'Yoast\WP\Plugin\some_hook', ); // OK. +do_action_ref_array( args: $args, hook_name: 'Yoast\WP\Plugin\some_hook_name_which_is_too_long', ); // Error - too long. + // Reset to default settings. // phpcs:set Yoast.NamingConventions.ValidHookName prefixes[] diff --git a/Yoast/Tests/NamingConventions/ValidHookNameUnitTest.php b/Yoast/Tests/NamingConventions/ValidHookNameUnitTest.php index 8aa47639..762b6e46 100644 --- a/Yoast/Tests/NamingConventions/ValidHookNameUnitTest.php +++ b/Yoast/Tests/NamingConventions/ValidHookNameUnitTest.php @@ -50,6 +50,7 @@ public function getErrorList(): array { 108 => 1, 111 => 1, 119 => 1, + 149 => 1, ]; } @@ -79,6 +80,7 @@ public function getWarningList(): array { 136 => 1, // Severity: 3. 140 => 1, 141 => 2, // Severity: 3 + 5. + 147 => 1, ]; } }