diff --git a/Yoast/Docs/Yoast/AlternativeFunctionsStandard.xml b/Yoast/Docs/Yoast/JsonEncodeAlternativeStandard.xml similarity index 81% rename from Yoast/Docs/Yoast/AlternativeFunctionsStandard.xml rename to Yoast/Docs/Yoast/JsonEncodeAlternativeStandard.xml index 4487f80b..4e44e2cf 100644 --- a/Yoast/Docs/Yoast/AlternativeFunctionsStandard.xml +++ b/Yoast/Docs/Yoast/JsonEncodeAlternativeStandard.xml @@ -1,11 +1,11 @@ diff --git a/Yoast/Sniffs/Yoast/AlternativeFunctionsSniff.php b/Yoast/Sniffs/Yoast/AlternativeFunctionsSniff.php deleted file mode 100644 index d43dcdce..00000000 --- a/Yoast/Sniffs/Yoast/AlternativeFunctionsSniff.php +++ /dev/null @@ -1,92 +0,0 @@ - [ - 'type' => 'error', - 'message' => 'Detected a call to %s(). Use %s() instead.', - 'functions' => [ - 'json_encode', - 'wp_json_encode', - ], - 'replacement' => 'WPSEO_Utils::format_json_encode', - ], - ]; - } - - /** - * Process a matched token. - * - * @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. - * - * @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 = [ - $matched_content, - $replacement, - ]; - - /* - * Deal with specific situations. - */ - 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'; - } - - break; - } - - if ( $fixable === false ) { - MessageHelper::addMessage( $this->phpcsFile, $message, $stackPtr, $is_error, $error_code, $data ); - return; - } - - $fix = MessageHelper::addFixableMessage( $this->phpcsFile, $message, $stackPtr, $is_error, $error_code, $data ); - if ( $fix === true ) { - $namespaced = Namespaces::determineNamespace( $this->phpcsFile, $stackPtr ); - - if ( empty( $namespaced ) || empty( $replacement ) ) { - $this->phpcsFile->fixer->replaceToken( $stackPtr, $replacement ); - } - else { - $this->phpcsFile->fixer->replaceToken( $stackPtr, '\\' . $replacement ); - } - } - } -} diff --git a/Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php b/Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php new file mode 100644 index 00000000..05aa48fe --- /dev/null +++ b/Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php @@ -0,0 +1,179 @@ + 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> + */ + public function getGroups() { + return [ + 'json_encode' => [ + 'functions' => [ + 'json_encode', + 'wp_json_encode', + ], + ], + ]; + } + + /** + * Process a matched token. + * + * @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. + * + * @return void + */ + public function process_matched_token( $stackPtr, $group_name, $matched_content ) { + $error = 'Detected a call to %s(). Use %s() instead.'; + $error_code = 'Found'; + $data = [ + $matched_content, + self::REPLACEMENT, + ]; + + $params = PassedParameters::getParameters( $this->phpcsFile, $stackPtr ); + + /* + * 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). + */ + if ( empty( $params ) ) { + /* + * Make sure this is not a PHP 8.1+ first class callable. If it is, throw the error, but don't autofix. + */ + $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 ) { + $error_code .= 'InFirstClassCallable'; + $this->phpcsFile->addError( $error, $stackPtr, $error_code, $data ); + return; + } + + $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 ) { + /* + * Okay, so this is a function call with only the first/$value parameter passed. + * This can be safely replaced. + */ + $fix = $this->phpcsFile->addFixableError( $error, $stackPtr, $error_code, $data ); + if ( $fix === true ) { + $this->fix_it( $stackPtr, $value_param ); + } + + return; + } + } + + /* + * In all other cases, we cannot auto-fix, only flag. + */ + $error_code .= 'WithAdditionalParams'; + + $this->phpcsFile->addError( $error, $stackPtr, $error_code, $data ); + } + + /** + * 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(); + + // 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, '' ); + } + + // 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/AlternativeFunctionsUnitTest.inc b/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc deleted file mode 100644 index d4790b4a..00000000 --- a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc +++ /dev/null @@ -1,22 +0,0 @@ -wp_json_encode( $thing ); // OK. -$json = MyNamespace\json_encode( $thing ); // OK. -$json = namespace\wp_json_encode( $thing ); // OK. - -// The sniff should trigger on these. -$json = json_encode( $thing ); // Error. -$json = wp_json_encode( $thing ); // Error. -return function_call(nested_call(json_encode( $thing ))); // Error. - -$json = json_encode ($value, $options); // Error, non-fixable. -$json = wp_json_encode( $data, $options, $depth ); // Error, non-fixable. - -namespace Yoast\CMS\Plugin\Dir; // Non-relevant parse error. - -$json = json_encode( $thing ); // Error. -$json = wp_json_encode( $thing ); // Error. diff --git a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc.fixed b/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc.fixed deleted file mode 100644 index 6a16d0f5..00000000 --- a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc.fixed +++ /dev/null @@ -1,22 +0,0 @@ -wp_json_encode( $thing ); // OK. -$json = MyNamespace\json_encode( $thing ); // OK. -$json = namespace\wp_json_encode( $thing ); // OK. - -// The sniff should trigger on these. -$json = WPSEO_Utils::format_json_encode( $thing ); // Error. -$json = WPSEO_Utils::format_json_encode( $thing ); // Error. -return function_call(nested_call(WPSEO_Utils::format_json_encode( $thing ))); // Error. - -$json = json_encode ($value, $options); // Error, non-fixable. -$json = wp_json_encode( $data, $options, $depth ); // Error, non-fixable. - -namespace Yoast\CMS\Plugin\Dir; // Non-relevant parse error. - -$json = \WPSEO_Utils::format_json_encode( $thing ); // Error. -$json = \WPSEO_Utils::format_json_encode( $thing ); // Error. diff --git a/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc b/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc new file mode 100644 index 00000000..4b6a254e --- /dev/null +++ b/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc @@ -0,0 +1,52 @@ +wp_json_encode( $thing ); // OK. +$json = MyNamespace\json_encode( $thing ); // OK. +$json = namespace\wp_json_encode( $thing ); // OK. +$json = $obj?->wp_json_encode( $thing ); // OK. + +// The sniff should trigger on these. +$json = \json_encode( $thing, ); // Error. +$json = wp_json_encode( $thing ); // Error. +return function_call(nested_call(json_encode( $thing ))); // Error. + +$json = json_encode ($value, $options,); // Error, non-fixable. +$json = wp_json_encode( $data, $options, $depth ); // Error, non-fixable. + +namespace Yoast\CMS\Plugin\Dir; // Non-relevant parse error. + +$json = json_encode( $thing ); // Error. +$json = wp_json_encode( $thing ); // Error. + +// Safeguard that the leading \ does not get doubled. +$json = \json_encode( $thing ); // Error. +$json = \wp_json_encode( $thing ); // Error. + +// Safeguard that parameter unpacking gets recognized and makes the error non-fixable. +$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. + +// Safeguard handling of the functions when used as PHP 8.1+ first class callables. +call_user_func(json_encode(...), $something); // Error, non-fixable. +\call_user_func(\wp_json_encode(...), $something); // 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 new file mode 100644 index 00000000..ac0724ba --- /dev/null +++ b/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc.fixed @@ -0,0 +1,52 @@ +wp_json_encode( $thing ); // OK. +$json = MyNamespace\json_encode( $thing ); // OK. +$json = namespace\wp_json_encode( $thing ); // OK. +$json = $obj?->wp_json_encode( $thing ); // OK. + +// The sniff should trigger on these. +$json = WPSEO_Utils::format_json_encode( $thing, ); // Error. +$json = WPSEO_Utils::format_json_encode( $thing ); // Error. +return function_call(nested_call(WPSEO_Utils::format_json_encode( $thing ))); // Error. + +$json = json_encode ($value, $options,); // Error, non-fixable. +$json = wp_json_encode( $data, $options, $depth ); // Error, non-fixable. + +namespace Yoast\CMS\Plugin\Dir; // Non-relevant parse error. + +$json = \WPSEO_Utils::format_json_encode( $thing ); // Error. +$json = \WPSEO_Utils::format_json_encode( $thing ); // Error. + +// Safeguard that the leading \ does not get doubled. +$json = \WPSEO_Utils::format_json_encode( $thing ); // Error. +$json = \WPSEO_Utils::format_json_encode( $thing ); // Error. + +// Safeguard that parameter unpacking gets recognized and makes the error non-fixable. +$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. + +// Safeguard handling of the functions when used as PHP 8.1+ first class callables. +call_user_func(json_encode(...), $something); // Error, non-fixable. +\call_user_func(\wp_json_encode(...), $something); // Error, non-fixable. + +// Live coding/parse error test. +// This must be the last test in the file. +$json = \WPSEO_Utils::format_json_encode( diff --git a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.php b/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.php similarity index 59% rename from Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.php rename to Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.php index 432e9de3..e42a8735 100644 --- a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.php +++ b/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.php @@ -5,13 +5,13 @@ use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest; /** - * Unit test class for the AlternativeFunctions sniff. + * Unit test class for the JsonEncodeAlternative sniff. * * @since 1.3.0 * - * @covers YoastCS\Yoast\Sniffs\Yoast\AlternativeFunctionsSniff + * @covers YoastCS\Yoast\Sniffs\Yoast\JsonEncodeAlternativeSniff */ -final class AlternativeFunctionsUnitTest extends AbstractSniffUnitTest { +final class JsonEncodeAlternativeUnitTest extends AbstractSniffUnitTest { /** * Returns the lines where errors should occur. @@ -20,13 +20,29 @@ final class AlternativeFunctionsUnitTest extends AbstractSniffUnitTest { */ public function getErrorList(): array { return [ - 12 => 1, 13 => 1, 14 => 1, - 16 => 1, + 15 => 1, 17 => 1, - 21 => 1, + 18 => 1, 22 => 1, + 23 => 1, + 26 => 1, + 27 => 1, + 30 => 1, + 31 => 1, + 34 => 1, + 35 => 1, + 36 => 1, + 37 => 1, + 38 => 1, + 40 => 1, + 41 => 1, + 43 => 1, + 44 => 1, + 47 => 1, + 48 => 1, + 52 => 1, ]; }