From ae9c55cfa73a39a9576c39d7b6f9fc00df4a2986 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 17 Oct 2023 06:24:25 +0200 Subject: [PATCH] Yoast/JsonEncodeAlternative: refactor the sniff to support PHP 8.0+ function calls with named parameters PHP 8.0 added support for parameter labels in function calls. This has two significant consequences for this sniff: 1. We can no longer rely on the parameter _count_ to determine whether the function call can be auto-fixed. Instead we need to specifically check for the `$value` parameter having been passed and no other parameters being passed. 2. For the auto-fixer, we can no longer just swop out the function call for its replacement. We also need to check if the passed parameter was a named parameter and if so, we may need to fix the parameter name as well. This commit refactors the sniff to take both the above into account. The refactor also removes some flexibility from the sniff, as, now the sniff has been renamed and will only check for the `[wp_]json_encode()` function calls, we no longer need to take the potential of additional function groups being checked via this sniff into account. Includes tests. --- .../Yoast/JsonEncodeAlternativeSniff.php | 163 ++++++++++++------ .../Yoast/JsonEncodeAlternativeUnitTest.inc | 13 ++ .../JsonEncodeAlternativeUnitTest.inc.fixed | 15 +- .../Yoast/JsonEncodeAlternativeUnitTest.php | 9 + 4 files changed, 142 insertions(+), 58 deletions(-) diff --git a/Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php b/Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php index da63fadb..52227499 100644 --- a/Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php +++ b/Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php @@ -3,34 +3,59 @@ namespace YoastCS\Yoast\Sniffs\Yoast; use PHP_CodeSniffer\Util\Tokens; -use PHPCSUtils\Utils\MessageHelper; use PHPCSUtils\Utils\Namespaces; use PHPCSUtils\Utils\PassedParameters; use WordPressCS\WordPress\AbstractFunctionRestrictionsSniff; /** - * Discourages the use of various functions and suggests (Yoast) alternatives. + * Discourages the use of the PHP and WP native [wp_]json_encode() functions in favour of a Yoast native alternative. * * @since 1.3.0 * @since 3.0.0 Renamed from `AlternativeFunctionsSniff` to `JsonEncodeAlternativeSniff`. */ final class JsonEncodeAlternativeSniff extends AbstractFunctionRestrictionsSniff { + /** + * Name of the replacement function (method). + * + * @since 3.0.0 + * + * @var string + */ + private const REPLACEMENT = 'WPSEO_Utils::format_json_encode'; + + /** + * Function call parameter details. + * + * @since 3.0.0 + * + * @var array Function names as the keys and the name of the first declared parameter + * as the value. + * There can be multiple parameter names if the parameter + * was renamed over time. + */ + private const PARAM_INFO = [ + 'json_encode' => 'value', + + /* + * The current parameter name is `$data`, but this is expected to be changed to `$value` in WP 6.5. + * See: https://core.trac.wordpress.org/ticket/59630 + */ + 'wp_json_encode' => [ 'data', 'value' ], + ]; + /** * Groups of functions to restrict. * - * @return array> + * @return array> */ public function getGroups() { return [ 'json_encode' => [ - 'type' => 'error', - 'message' => 'Detected a call to %s(). Use %s() instead.', - 'functions' => [ + 'functions' => [ 'json_encode', 'wp_json_encode', ], - 'replacement' => 'WPSEO_Utils::format_json_encode', ], ]; } @@ -46,72 +71,96 @@ public function getGroups() { * @return void */ public function process_matched_token( $stackPtr, $group_name, $matched_content ) { - - $replacement = ( $this->groups[ $group_name ]['replacement'] ?? '' ); - $fixable = true; - $message = $this->groups[ $group_name ]['message']; - $is_error = ( $this->groups[ $group_name ]['type'] === 'error' ); - $error_code = MessageHelper::stringToErrorcode( $group_name . '_' . $matched_content ); - $data = [ + $error = 'Detected a call to %s(). Use %s() instead.'; + $error_code = 'Found'; + $data = [ $matched_content, - $replacement, + self::REPLACEMENT, ]; + $params = PassedParameters::getParameters( $this->phpcsFile, $stackPtr ); + /* - * Deal with specific situations. + * If no parameters were passed, we can safely replace the function call, even though + * the function call itself, as-is, is not correct/working (but that's not the concern of + * this sniff). */ - switch ( $matched_content ) { - case 'json_encode': - case 'wp_json_encode': - /* - * The function `WPSEO_Utils:format_json_encode()` is only a valid alternative - * when only the first parameter is passed. - */ - if ( PassedParameters::getParameterCount( $this->phpcsFile, $stackPtr ) !== 1 ) { - $fixable = false; - $error_code .= 'WithAdditionalParams'; - } + if ( empty( $params ) ) { + $fix = $this->phpcsFile->addFixableError( $error, $stackPtr, $error_code, $data ); + if ( $fix === true ) { + $this->fix_it( $stackPtr ); + } + return; + } + + /* + * If there are function parameters, we need to verify that only the first ($value) parameter + * was passed, taking PHP 8.0+ function calls with named parameters into account. + * + * We also need to check for parameter unpacking being used as in that case, the + * parameter count will be unreliable. + */ + $value_param = PassedParameters::getParameterFromStack( $params, 1, self::PARAM_INFO[ $matched_content ] ); + if ( \is_array( $value_param ) && \count( $params ) === 1 ) { + $first_token = $this->phpcsFile->findNext( Tokens::$emptyTokens, $value_param['start'], ( $value_param['end'] + 1 ), true ); + if ( $first_token === false || $this->tokens[ $first_token ]['code'] !== \T_ELLIPSIS ) { /* - * When the function call uses parameter unpacking, the parameter count is unreliable. + * Okay, so this is a function call with only the first/$value parameter passed. + * This can be safely replaced. */ - $ignore = Tokens::$emptyTokens; - $ignore[ \T_OPEN_PARENTHESIS ] = \T_OPEN_PARENTHESIS; - - $first_in_call = $this->phpcsFile->findNext( $ignore, ( $stackPtr + 1 ), null, true ); - if ( $first_in_call !== false && $this->tokens[ $first_in_call ]['code'] === \T_ELLIPSIS ) { - $fixable = false; - $error_code .= 'WithAdditionalParams'; + $fix = $this->phpcsFile->addFixableError( $error, $stackPtr, $error_code, $data ); + if ( $fix === true ) { + $this->fix_it( $stackPtr, $value_param ); } - break; + return; + } } - if ( $fixable === false ) { - MessageHelper::addMessage( $this->phpcsFile, $message, $stackPtr, $is_error, $error_code, $data ); - return; - } + /* + * In all other cases, we cannot auto-fix, only flag. + */ + $error_code .= 'WithAdditionalParams'; - $fix = MessageHelper::addFixableMessage( $this->phpcsFile, $message, $stackPtr, $is_error, $error_code, $data ); - if ( $fix === true ) { - $this->phpcsFile->fixer->beginChangeset(); + $this->phpcsFile->addError( $error, $stackPtr, $error_code, $data ); + } - // Remove potential leading namespace separator for fully qualified function call. - $prev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true ); - if ( $this->tokens[ $prev ]['code'] === \T_NS_SEPARATOR ) { - $this->phpcsFile->fixer->replaceToken( $prev, '' ); - } + /** + * Auto-fix the function call to use the replacement function. + * + * @since 3.0.0 + * + * @param int $stackPtr The position of the current token in the stack. + * @param array|false $value_param Optional. Parameter information for the first/$value + * parameter if available, or false if not. + * + * @return void + */ + private function fix_it( $stackPtr, $value_param = false ) { + $this->phpcsFile->fixer->beginChangeset(); - // Replace the function call with a, potentially fully qualified, call to the replacement. - $namespaced = Namespaces::determineNamespace( $this->phpcsFile, $stackPtr ); - if ( empty( $namespaced ) || empty( $replacement ) ) { - $this->phpcsFile->fixer->replaceToken( $stackPtr, $replacement ); - } - else { - $this->phpcsFile->fixer->replaceToken( $stackPtr, '\\' . $replacement ); - } + // Remove potential leading namespace separator for fully qualified function call. + $prev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true ); + if ( $prev !== false && $this->tokens[ $prev ]['code'] === \T_NS_SEPARATOR ) { + $this->phpcsFile->fixer->replaceToken( $prev, '' ); + } - $this->phpcsFile->fixer->endChangeset(); + // Replace the function call with a, potentially fully qualified, call to the replacement. + $namespaced = Namespaces::determineNamespace( $this->phpcsFile, $stackPtr ); + if ( empty( $namespaced ) ) { + $this->phpcsFile->fixer->replaceToken( $stackPtr, self::REPLACEMENT ); + } + else { + $this->phpcsFile->fixer->replaceToken( $stackPtr, '\\' . self::REPLACEMENT ); } + + if ( \is_array( $value_param ) && isset( $value_param['name_token'] ) ) { + // Update the parameter name when the function call uses named parameters. + // `$data` is the parameter name used in the WPSEO_Utils::format_json_encode() function. + $this->phpcsFile->fixer->replaceToken( $value_param['name_token'], 'data' ); + } + + $this->phpcsFile->fixer->endChangeset(); } } diff --git a/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc b/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc index 27962e53..18da3a67 100644 --- a/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc +++ b/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc @@ -30,6 +30,19 @@ $json = \wp_json_encode( $thing ); // Error. $json = json_encode (...$params); // Error, non-fixable. $json = wp_json_encode( ...$params ); // Error, non-fixable. +// Safeguard support for PHP 8.0+ function calls using named parameters. +$json = wp_json_encode(); // Error - not useful as required param is missing, but that's not the concern of this sniff. +$json = json_encode( something: $thing ); // Error, non-fixable - not useful as required param is missing, but that's not the concern of this sniff. +$json = \wp_json_encode( depth: 256, options: 0 ); // Error, non-fixable - not useful as required param is missing, but that's not the concern of this sniff. +$json = json_encode( depths: 256, value: $thing ); // Error, non-fixable - typo in optional param, but that's not the concern of this sniff. +$json = json_encode( value: ); // Error, - missing the actual parameter value, but that's not the concern of this sniff. + +$json = \json_encode( value: $thing ); // Error. +$json = wp_json_encode( data: $thing ); // Error. + +$json = json_encode( flags: 0, depth: 256, value: $thing ); // Error, non-fixable. +$json = wp_json_encode( depth: 256, options: 0, data: $thing ); // Error, non-fixable. + // Live coding/parse error test. // This must be the last test in the file. $json = wp_json_encode( diff --git a/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc.fixed b/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc.fixed index 11c068f6..50cfa6f2 100644 --- a/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc.fixed +++ b/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc.fixed @@ -30,6 +30,19 @@ $json = \WPSEO_Utils::format_json_encode( $thing ); // Error. $json = json_encode (...$params); // Error, non-fixable. $json = wp_json_encode( ...$params ); // Error, non-fixable. +// Safeguard support for PHP 8.0+ function calls using named parameters. +$json = \WPSEO_Utils::format_json_encode(); // Error - not useful as required param is missing, but that's not the concern of this sniff. +$json = json_encode( something: $thing ); // Error, non-fixable - not useful as required param is missing, but that's not the concern of this sniff. +$json = \wp_json_encode( depth: 256, options: 0 ); // Error, non-fixable - not useful as required param is missing, but that's not the concern of this sniff. +$json = json_encode( depths: 256, value: $thing ); // Error, non-fixable - typo in optional param, but that's not the concern of this sniff. +$json = \WPSEO_Utils::format_json_encode( data: ); // Error, - missing the actual parameter value, but that's not the concern of this sniff. + +$json = \WPSEO_Utils::format_json_encode( data: $thing ); // Error. +$json = \WPSEO_Utils::format_json_encode( data: $thing ); // Error. + +$json = json_encode( flags: 0, depth: 256, value: $thing ); // Error, non-fixable. +$json = wp_json_encode( depth: 256, options: 0, data: $thing ); // Error, non-fixable. + // Live coding/parse error test. // This must be the last test in the file. -$json = wp_json_encode( +$json = \WPSEO_Utils::format_json_encode( diff --git a/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.php b/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.php index 348bb561..af18e3de 100644 --- a/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.php +++ b/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.php @@ -31,7 +31,16 @@ public function getErrorList(): array { 27 => 1, 30 => 1, 31 => 1, + 34 => 1, 35 => 1, + 36 => 1, + 37 => 1, + 38 => 1, + 40 => 1, + 41 => 1, + 43 => 1, + 44 => 1, + 48 => 1, ]; }