From 3704682747f340eb579fd266052e8c1542f40fb7 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 18 Sep 2023 05:56:53 +0200 Subject: [PATCH 1/8] Yoast/AlternativeFunctions: minor documentation improvements --- Yoast/Sniffs/Yoast/AlternativeFunctionsSniff.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Yoast/Sniffs/Yoast/AlternativeFunctionsSniff.php b/Yoast/Sniffs/Yoast/AlternativeFunctionsSniff.php index d43dcdce..b4a606b0 100644 --- a/Yoast/Sniffs/Yoast/AlternativeFunctionsSniff.php +++ b/Yoast/Sniffs/Yoast/AlternativeFunctionsSniff.php @@ -17,7 +17,7 @@ final class AlternativeFunctionsSniff extends AbstractFunctionRestrictionsSniff /** * Groups of functions to restrict. * - * @return array + * @return array> */ public function getGroups() { return [ @@ -38,7 +38,8 @@ public function getGroups() { * * @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 string $matched_content The token content (function name) which was matched + * in lowercase. * * @return void */ From cda0ba4c235e07fa9e11754021d75becb1761864 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 17 Oct 2023 06:33:30 +0200 Subject: [PATCH 2/8] Yoast/AlternativeFunctions: safeguard handling of PHP 7.3+ function calls with trailing commas As of PHP 7.3, the parameter list for a function call can have a trailing comma. This is already handled correctly by the sniff. This commit just adjusts some pre-existing tests to safeguard this for the future. --- Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc | 4 ++-- Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc.fixed | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc b/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc index d4790b4a..51727e1f 100644 --- a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc +++ b/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc @@ -9,11 +9,11 @@ $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 = 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 = 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. diff --git a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc.fixed b/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc.fixed index 6a16d0f5..ff25755d 100644 --- a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc.fixed +++ b/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc.fixed @@ -9,11 +9,11 @@ $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. $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 = 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. From 10a24ac30063e06fdf93a98c65ec516f7c7e84bb Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 16 Oct 2023 06:07:48 +0200 Subject: [PATCH 3/8] Yoast/AlternativeFunctions: add test with PHP 8.0+ nullsafe object operator ... to safeguard that the sniff does not recognize method calls prefixed with the nullsafe object operator as calls to the global functions. Note: this is already handled by the WPCS parent sniff, this test just safeguards it. --- Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc | 1 + Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc.fixed | 1 + Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.php | 6 +++--- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc b/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc index 51727e1f..a1686cc7 100644 --- a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc +++ b/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc @@ -7,6 +7,7 @@ $json = Class_Other_Plugin::json_encode( $thing ); // OK. $json = $obj->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. diff --git a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc.fixed b/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc.fixed index ff25755d..2a70b420 100644 --- a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc.fixed +++ b/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc.fixed @@ -7,6 +7,7 @@ $json = Class_Other_Plugin::json_encode( $thing ); // OK. $json = $obj->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. diff --git a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.php b/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.php index 432e9de3..172be717 100644 --- a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.php +++ b/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.php @@ -20,13 +20,13 @@ 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, ]; } From 93888749a40caccb502e843cfc35991eb04232e6 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 16 Oct 2023 06:25:09 +0200 Subject: [PATCH 4/8] Yoast/AlternativeFunctions: bug fix - fixer could create parse error If the original function call was already fully qualified in a namespaced file, the fixer would add a superfluous extra namespace separator - `\\WPSEO_Utils::format_json_encode()`, effectively creating a parse error in the resulting file. As fully qualified calls to global functions in namespaced files are now the norm in the Yoast repos, this is a clear bug which should be prevented. Fixed now by removing a potential leading namespace separator before doing the replacement. Includes additional tests. --- Yoast/Sniffs/Yoast/AlternativeFunctionsSniff.php | 13 ++++++++++++- Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc | 6 +++++- .../Yoast/AlternativeFunctionsUnitTest.inc.fixed | 4 ++++ Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.php | 2 ++ 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/Yoast/Sniffs/Yoast/AlternativeFunctionsSniff.php b/Yoast/Sniffs/Yoast/AlternativeFunctionsSniff.php index b4a606b0..33d3d158 100644 --- a/Yoast/Sniffs/Yoast/AlternativeFunctionsSniff.php +++ b/Yoast/Sniffs/Yoast/AlternativeFunctionsSniff.php @@ -2,6 +2,7 @@ namespace YoastCS\Yoast\Sniffs\Yoast; +use PHP_CodeSniffer\Util\Tokens; use PHPCSUtils\Utils\MessageHelper; use PHPCSUtils\Utils\Namespaces; use PHPCSUtils\Utils\PassedParameters; @@ -80,14 +81,24 @@ public function process_matched_token( $stackPtr, $group_name, $matched_content $fix = MessageHelper::addFixableMessage( $this->phpcsFile, $message, $stackPtr, $is_error, $error_code, $data ); if ( $fix === true ) { - $namespaced = Namespaces::determineNamespace( $this->phpcsFile, $stackPtr ); + $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 ( $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 ) || empty( $replacement ) ) { $this->phpcsFile->fixer->replaceToken( $stackPtr, $replacement ); } else { $this->phpcsFile->fixer->replaceToken( $stackPtr, '\\' . $replacement ); } + + $this->phpcsFile->fixer->endChangeset(); } } } diff --git a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc b/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc index a1686cc7..ced1d223 100644 --- a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc +++ b/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc @@ -10,7 +10,7 @@ $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 = \json_encode( $thing, ); // Error. $json = wp_json_encode( $thing ); // Error. return function_call(nested_call(json_encode( $thing ))); // Error. @@ -21,3 +21,7 @@ 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. diff --git a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc.fixed b/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc.fixed index 2a70b420..750a2039 100644 --- a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc.fixed +++ b/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc.fixed @@ -21,3 +21,7 @@ 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. diff --git a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.php b/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.php index 172be717..26c437a2 100644 --- a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.php +++ b/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.php @@ -27,6 +27,8 @@ public function getErrorList(): array { 18 => 1, 22 => 1, 23 => 1, + 26 => 1, + 27 => 1, ]; } From 2709e2647d26e05b459d399d70f03862d95a8092 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 17 Oct 2023 06:50:08 +0200 Subject: [PATCH 5/8] Yoast/AlternativeFunctions: bug fix - allow for PHP 5.6+ parameter unpacking When a function call uses parameter unpacking, the exact parameter count is undetermined, so the auto-fixer should not kick in. The sniff did not take this into account. Fixed now. Includes tests. --- Yoast/Sniffs/Yoast/AlternativeFunctionsSniff.php | 12 ++++++++++++ Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc | 8 ++++++++ .../Yoast/AlternativeFunctionsUnitTest.inc.fixed | 8 ++++++++ Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.php | 3 +++ 4 files changed, 31 insertions(+) diff --git a/Yoast/Sniffs/Yoast/AlternativeFunctionsSniff.php b/Yoast/Sniffs/Yoast/AlternativeFunctionsSniff.php index 33d3d158..52771728 100644 --- a/Yoast/Sniffs/Yoast/AlternativeFunctionsSniff.php +++ b/Yoast/Sniffs/Yoast/AlternativeFunctionsSniff.php @@ -71,6 +71,18 @@ public function process_matched_token( $stackPtr, $group_name, $matched_content $error_code .= 'WithAdditionalParams'; } + /* + * When the function call uses parameter unpacking, the parameter count is unreliable. + */ + $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'; + } + break; } diff --git a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc b/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc index ced1d223..27962e53 100644 --- a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc +++ b/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc @@ -25,3 +25,11 @@ $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. + +// Live coding/parse error test. +// This must be the last test in the file. +$json = wp_json_encode( diff --git a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc.fixed b/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc.fixed index 750a2039..11c068f6 100644 --- a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc.fixed +++ b/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc.fixed @@ -25,3 +25,11 @@ $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. + +// Live coding/parse error test. +// This must be the last test in the file. +$json = wp_json_encode( diff --git a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.php b/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.php index 26c437a2..1f77a374 100644 --- a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.php +++ b/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.php @@ -29,6 +29,9 @@ public function getErrorList(): array { 23 => 1, 26 => 1, 27 => 1, + 30 => 1, + 31 => 1, + 35 => 1, ]; } From 87acf1f162f7974abf589817fa73bf41ffc0ce91 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 17 Oct 2023 00:05:05 +0200 Subject: [PATCH 6/8] Yoast/AlternativeFunctions: rename the sniff to `JsonEncodeAlternative` ... in anticipation of adding support for PHP 8.0+ function calls using named parameters. Supporting this would too complex if there is an open ended possibility of detection of additional function groups being added to this sniff. The renamed sniff will only handle the `[wp_]json_encode()` function call detection and replacement. If in the future additional functions would have Yoast specific alternative and need sniffs, this should be handled by a separate sniff/separate sniffs. --- ...ctionsStandard.xml => JsonEncodeAlternativeStandard.xml} | 4 ++-- ...iveFunctionsSniff.php => JsonEncodeAlternativeSniff.php} | 3 ++- ...ctionsUnitTest.inc => JsonEncodeAlternativeUnitTest.inc} | 0 ...st.inc.fixed => JsonEncodeAlternativeUnitTest.inc.fixed} | 0 ...ctionsUnitTest.php => JsonEncodeAlternativeUnitTest.php} | 6 +++--- 5 files changed, 7 insertions(+), 6 deletions(-) rename Yoast/Docs/Yoast/{AlternativeFunctionsStandard.xml => JsonEncodeAlternativeStandard.xml} (81%) rename Yoast/Sniffs/Yoast/{AlternativeFunctionsSniff.php => JsonEncodeAlternativeSniff.php} (95%) rename Yoast/Tests/Yoast/{AlternativeFunctionsUnitTest.inc => JsonEncodeAlternativeUnitTest.inc} (100%) rename Yoast/Tests/Yoast/{AlternativeFunctionsUnitTest.inc.fixed => JsonEncodeAlternativeUnitTest.inc.fixed} (100%) rename Yoast/Tests/Yoast/{AlternativeFunctionsUnitTest.php => JsonEncodeAlternativeUnitTest.php} (78%) 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/JsonEncodeAlternativeSniff.php similarity index 95% rename from Yoast/Sniffs/Yoast/AlternativeFunctionsSniff.php rename to Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php index 52771728..da63fadb 100644 --- a/Yoast/Sniffs/Yoast/AlternativeFunctionsSniff.php +++ b/Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php @@ -12,8 +12,9 @@ * Discourages the use of various functions and suggests (Yoast) alternatives. * * @since 1.3.0 + * @since 3.0.0 Renamed from `AlternativeFunctionsSniff` to `JsonEncodeAlternativeSniff`. */ -final class AlternativeFunctionsSniff extends AbstractFunctionRestrictionsSniff { +final class JsonEncodeAlternativeSniff extends AbstractFunctionRestrictionsSniff { /** * Groups of functions to restrict. diff --git a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc b/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc similarity index 100% rename from Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc rename to Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc diff --git a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc.fixed b/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc.fixed similarity index 100% rename from Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc.fixed rename to Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc.fixed diff --git a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.php b/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.php similarity index 78% rename from Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.php rename to Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.php index 1f77a374..348bb561 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. From ae9c55cfa73a39a9576c39d7b6f9fc00df4a2986 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 17 Oct 2023 06:24:25 +0200 Subject: [PATCH 7/8] 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, ]; } From 0501528c32ce4b6220f50be8943e2dcc3efd0eb4 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 17 Oct 2023 07:21:32 +0200 Subject: [PATCH 8/8] Yoast/JsonEncodeAlternative: handle PHP 8.1+ first class callables PHP 8.1 added support for "first class callables". As the number of parameters which will be passed to the callable is unknown, the sniff should flag these functions when used as first class callable, but should not auto-fix. Includes tests. --- Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php | 13 +++++++++++++ Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc | 4 ++++ .../Yoast/JsonEncodeAlternativeUnitTest.inc.fixed | 4 ++++ Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.php | 2 ++ 4 files changed, 23 insertions(+) diff --git a/Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php b/Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php index 52227499..05aa48fe 100644 --- a/Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php +++ b/Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php @@ -86,6 +86,19 @@ public function process_matched_token( $stackPtr, $group_name, $matched_content * 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 ); diff --git a/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc b/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc index 18da3a67..4b6a254e 100644 --- a/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc +++ b/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc @@ -43,6 +43,10 @@ $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 index 50cfa6f2..ac0724ba 100644 --- a/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc.fixed +++ b/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc.fixed @@ -43,6 +43,10 @@ $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/JsonEncodeAlternativeUnitTest.php b/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.php index af18e3de..e42a8735 100644 --- a/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.php +++ b/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.php @@ -40,7 +40,9 @@ public function getErrorList(): array { 41 => 1, 43 => 1, 44 => 1, + 47 => 1, 48 => 1, + 52 => 1, ]; }