Security/StaticStrreplace: use pre-parsed function call argument information / support PHP 8.0+ named arguments

As things were, the sniff walked the tokens in the function call itself, but what with the switch to extending the `AbstractFunctionParameterSniff` class, we already have the start and end pointers of each passed parameter available, so let's start using that information instead of doing the token walking within the sniff.

By using the pre-parsed information, the sniff automatically will start supporting the use of PHP 8.0+ named arguments in function calls.

Includes tests.

Author: jrfnl

WordPressVIPMinimum/Sniffs/Security/StaticStrreplaceSniff.php

@@ -10,8 +10,8 @@
 namespace WordPressVIPMinimum\Sniffs\Security;
 
 use PHP_CodeSniffer\Util\Tokens;
+use PHPCSUtils\Exceptions\UnexpectedTokenType;
 use PHPCSUtils\Tokens\Collections;
-use PHPCSUtils\Utils\Arrays;
 use PHPCSUtils\Utils\PassedParameters;
 use WordPressCS\WordPress\AbstractFunctionParameterSniff;
 
@@ -48,49 +48,50 @@ class StaticStrreplaceSniff extends AbstractFunctionParameterSniff {
 	 * @return void
 	 */
 	public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) {
-
-		$openBracket = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true );
-
-		if ( $this->tokens[ $openBracket ]['code'] !== T_OPEN_PARENTHESIS ) {
+		$search_param  = PassedParameters::getParameterFromStack( $parameters, 1, 'search' );
+		$replace_param = PassedParameters::getParameterFromStack( $parameters, 2, 'replace' );
+		$subject_param = PassedParameters::getParameterFromStack( $parameters, 3, 'subject' );
+
+		if ( $search_param === false || $replace_param === false || $subject_param === false ) {
+			/*
+			 * Either an invalid function call (missing PHP required parameter); or function call
+			 * with argument unpacking; or live coding.
+			 * In all these cases, this is not the code pattern this sniff is looking for, so bow out.
+			 */
 			return;
 		}
 
 		$static_text_tokens                               = Tokens::$emptyTokens;
 		$static_text_tokens[ T_CONSTANT_ENCAPSED_STRING ] = T_CONSTANT_ENCAPSED_STRING;
 
-		$next_start_ptr = $openBracket + 1;
-		for ( $i = 0; $i < 3; $i++ ) {
-			$param_ptr = $this->phpcsFile->findNext( array_merge( Tokens::$emptyTokens, [ T_COMMA ] ), $next_start_ptr, null, true );
-			if ( $param_ptr === false ) {
-				// Live coding or parse error. Ignore.
-				return;
+		foreach ( [ $search_param, $replace_param, $subject_param ] as $param ) {
+			$has_non_static_text = $this->phpcsFile->findNext( $static_text_tokens, $param['start'], ( $param['end'] + 1 ), true );
+			if ( $has_non_static_text === false ) {
+				// The parameter contained only tokens which could be considered static text.
+				continue;
 			}
 
-			if ( isset( Collections::arrayOpenTokensBC()[ $this->tokens[ $param_ptr ]['code'] ] ) ) {
-				$arrayOpenClose = Arrays::getOpenClose( $this->phpcsFile, $param_ptr );
-				if ( $arrayOpenClose === false ) {
+			if ( isset( Collections::arrayOpenTokensBC()[ $this->tokens[ $has_non_static_text ]['code'] ] ) ) {
+				try {
+					$array_items = PassedParameters::getParameters( $this->phpcsFile, $has_non_static_text );
+				} catch ( UnexpectedTokenType $e ) {
 					// Short list, parse error or live coding, bow out.
 					return;
 				}
 
-				$array_items = PassedParameters::getParameters( $this->phpcsFile, $param_ptr );
 				foreach ( $array_items as $array_item ) {
 					$has_non_static_text = $this->phpcsFile->findNext( $static_text_tokens, $array_item['start'], $array_item['end'], true );
 					if ( $has_non_static_text !== false ) {
 						return;
 					}
 				}
 
-				$next_start_ptr = $arrayOpenClose['closer'] + 1;
+				// The array only contained items with tokens which could be considered static text.
 				continue;
 			}
 
-			if ( $this->tokens[ $param_ptr ]['code'] !== T_CONSTANT_ENCAPSED_STRING ) {
-				return;
-			}
-
-			$next_start_ptr = $param_ptr + 1;
-
+			// Non-static text token found. Not what we're looking for.
+			return;
 		}
 
 		$message = 'This code pattern is often used to run a very dangerous shell programs on your server. The code in these files needs to be reviewed, and possibly cleaned.';

WordPressVIPMinimum/Tests/Security/StaticStrreplaceUnitTest.inc

@@ -61,3 +61,11 @@ str_replace( 'foo', 'bar', 'foobar', $count ); // Bad.
 
 // Don't confuse PHP 7.1+ short list with short array.
 str_replace( [ $a, $b ] = $search, [ 'bar', 'foo' ], 'foobar', ); // OK.
+
+// Safeguard support for PHP 8.0+ named parameters.
+str_replace( search: [ 'foo', 'bar' ], replace: 'baz', count: $count ); // OK, well not really, missing required $subject param, but not our concern.
+str_replace( search: 'foo', replace: 'baz', subjects: [ 'foobar', 'foobar' ] ); // OK, well not really, typo in $subject param name, not our concern.
+str_replace( subject: $subject, count: $count, replace: 'baz', search: 'foo', ); // OK, different parameter order.
+
+str_replace( count: $count, subject: [ 'foobar', 'foobar' ], search: 'foo', replace: 'baz', ); // Bad with different parameter order.
+str_replace( [ 'foo', 'bar' ], 'baz', count: $count, subject: 'foobar' ); // Bad, with mixed named and unnamed params.

WordPressVIPMinimum/Tests/Security/StaticStrreplaceUnitTest.php

@@ -28,6 +28,8 @@ public function getErrorList() {
 			56 => 1,
 			57 => 1,
 			60 => 1,
+			70 => 1,
+			71 => 1,
 		];
 	}