Skip to content

Commit

Permalink
Yoast/JsonEncodeAlternative: refactor the sniff to support PHP 8.0+ f…
Browse files Browse the repository at this point in the history 
…unction 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.
  • Loading branch information
@jrfnl
jrfnl committed Nov 4, 2023
1 parent 87acf1f commit ae9c55c
Show file tree
Hide file tree
Showing 4 changed files with 142 additions and 58 deletions.
163 changes: 106 additions & 57 deletions Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string|string[]> 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<string, array<string, string|string[]>>
* @return array<string, array<string, string[]>>
*/
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',
],
];
}
Expand All @@ -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<string, int|string>|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();
}
}
13 changes: 13 additions & 0 deletions Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
15 changes: 14 additions & 1 deletion Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc.fixed
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
9 changes: 9 additions & 0 deletions Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
];
}

Expand Down

0 comments on commit ae9c55c

Please sign in to comment.