Merge pull request #338 from Yoast/JRF/yoastcs-alternativefunctions-v…

…arious-improvements

Yoast/AlternativeFunctions: rename sniff, bug fix for fixer, support modern PHP and other improvements

Yoast/Docs/Yoast/AlternativeFunctionsStandard.xml → Yoast/Docs/Yoast/JsonEncodeAlternativeStandard.xml

@@ -1,11 +1,11 @@
 <?xml version="1.0"?>
 <documentation xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     xsi:noNamespaceSchemaLocation="https://phpcsstandards.github.io/PHPCSDevTools/phpcsdocs.xsd"
-    title="Alternative Functions"
+    title="Json Encode Alternative"
     >
     <standard>
     <![CDATA[
-    Discourages the use of certain PHP or WP native functions in favour of Yoast native alternatives.
+    Discourages the use of the PHP and WP native [wp_]json_encode() functions in favour of a Yoast native alternative.
     ]]>
     </standard>
     <code_comparison>

Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php

@@ -0,0 +1,179 @@
+<?php
+
+namespace YoastCS\Yoast\Sniffs\Yoast;
+
+use PHP_CodeSniffer\Util\Tokens;
+use PHPCSUtils\Utils\Namespaces;
+use PHPCSUtils\Utils\PassedParameters;
+use WordPressCS\WordPress\AbstractFunctionRestrictionsSniff;
+
+/**
+ * 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[]>>
+	 */
+	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<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();
+
+		// 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();
+	}
+}

Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc

@@ -0,0 +1,52 @@
+<?php
+
+$json = WPSEO_Utils::format_json_encode( $thing ); // OK.
+
+// Don't trigger on calls to functions which are not the PHP or WP native functions.
+$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.
+$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(