Security/StaticStrreplace: extend AbstractFunctionParameterSniff

As things were, the determination of whether or not a `T_STRING` is a call to the global PHP native `str_replace()` function was severely flawed.

By switching the sniff over to be based on the WordPressCS `AbstractFunctionParameterSniff` class, this flaw is mitigated.

Includes adding a slew of additional tests, some of which (line 8 - 13) are specific to the flaw being addressed in this commit.

Additionally, the tests have been made more comprehensive and varied by:
* Testing against false positives for calls to methods or namespaced function calls (= the issue being addressed in this PR).
* Testing against false positives for attribute class using the same name as the function.
* Ensure function import `use` statements are not flagged. We're not interested in those.
* Safeguarding that function calls using PHP 5.6+ argument unpacking are not flagged.
* Safeguarding that the function is not flagged when used as a PHP 8.1+ first class callable.
* Adding more variations to the pre-existing tests:
    - Non-lowercase function call(s).
    - Fully qualified function calls.
    - Use PHP 7.3+ trailing comma's in a few function calls.
    - Use both single quoted as well as double quoted text strings.
    - Use other non-plain-text tokens in the passed parameters.
    - Multi-line function call.
    - Comments in function call.
    - `$subject` parameter passed as array.

Author: jrfnl

WordPressVIPMinimum/Sniffs/Security/StaticStrreplaceSniff.php

@@ -10,34 +10,41 @@
 namespace WordPressVIPMinimum\Sniffs\Security;
 
 use PHP_CodeSniffer\Util\Tokens;
-use WordPressVIPMinimum\Sniffs\Sniff;
+use WordPressCS\WordPress\AbstractFunctionParameterSniff;
 
 /**
  * Restricts usage of str_replace with all 3 params being static.
  */
-class StaticStrreplaceSniff extends Sniff {
+class StaticStrreplaceSniff extends AbstractFunctionParameterSniff {
 
 	/**
-	 * Returns an array of tokens this test wants to listen for.
+	 * The group name for this group of functions.
 	 *
-	 * @return array<int|string>
+	 * @var string
 	 */
-	public function register() {
-		return [ T_STRING ];
-	}
+	protected $group_name = 'str_replace';
+
+	/**
+	 * Functions this sniff is looking for.
+	 *
+	 * @var array<string, bool> Key is the function name, value irrelevant.
+	 */
+	protected $target_functions = [
+		'str_replace' => true,
+	];
 
 	/**
-	 * Process this test when one of its tokens is encountered
+	 * Process the parameters of a matched function.
 	 *
-	 * @param int $stackPtr The position of the current token in the stack passed in $tokens.
+	 * @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.
+	 * @param array  $parameters      Array with information about the parameters.
 	 *
 	 * @return void
 	 */
-	public function process_token( $stackPtr ) {
-
-		if ( $this->tokens[ $stackPtr ]['content'] !== 'str_replace' ) {
-			return;
-		}
+	public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) {
 
 		$openBracket = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true );
 

WordPressVIPMinimum/Tests/Security/StaticStrreplaceUnitTest.inc

@@ -1,13 +1,56 @@
 <?php
 
-str_replace( 'foo', 'bar', 'foobar' ); // Bad.
+/*
+ * Not the sniff target.
+ */
+use str_replace;
+
+my\ns\str_replace('foo', 'bar', 'foobar');
+$this->str_replace('foo', 'bar', 'foobar');
+$this?->str_replace('foo', 'bar', 'foobar');
+MyClass::str_replace('foo', 'bar', 'foobar');
+echo STR_REPLACE;
+namespace\str_replace('foo', 'bar', 'foobar');
+
+// Looks like a function call, but is a PHP 8.0+ class instantiation via an attribute.
+#[Str_Replace('foo', 'bar', 'foobar')]
+function foo() {}
+
+// Ignore PHP 5.6 argument unpacking.
+str_replace('foo', 'bar', ...$params);
 
-str_replace( 'foo', 'bar', $foobar ); // Ok.
+// Ignore PHP 8.1 first class callable.
+add_filter('my_filter', str_replace(...));
 
-str_replace( array( 'foo', 'bar' ), array( 'bar', 'foo' ), 'foobar' ); // Bad.
+// Incomplete function calls, should be ignored by the sniff.
+str_replace();
+str_replace('foo', 'bar');
 
-str_replace( array( 'foo', 'bar' ), array( 'bar', 'foo' ), $foobar ); // Ok.
 
-str_replace( array( 'foo', 'bar' ), array( $foo, $bar ), $foobar ); // Ok.
+/*
+ * These should all be okay.
+ */
+str_replace( $foo->getSearch(), 'bar', 'foobar' );
+\str_replace( 'foo', $bar, 'foobar' );
+str_replace( 'foo', 'bar', $foobar, );
+STR_REPLACE( $foo, $bar, 'foobar' );
+str_replace( 'foo', get_replacements(), $foobar );
 
-str_replace( array( $foo, $bar ), array( 'bar', 'foo' ), $foobar ); // Ok.
+str_replace( array( 'foo', "$bar" ), $bar, 'foobar' );
+
+str_replace( array( $foo, $bar ), array( 'bar', 'foo' ), array( 'foobar', 'foobar' ) );
+\Str_Replace( [ 'foo', 'bar' ], array( $foo, SOME_CONSTANT ), 'foobar' );
+str_replace( array( 'foo', "bar" ), [ 'bar', 'foo' ], $foobar, );
+str_replace( [ $foo, $bar->prop ], /*comment*/ [ 'bar', 'foo' ], $foobar );
+
+
+/*
+ * These should all be flagged with a warning.
+ */
+str_replace( 'foo', 'bar', 'foobar' ); // Bad.
+Str_replace(
+	'foo', // Comment.
+	'bar', /* comment */
+	'foobar'
+); // Bad.
+str_replace( array( 'foo', 'bar' ), array( 'bar', 'foo' ), array( 'foobar', 'foobar' ) ); // Bad.

WordPressVIPMinimum/Tests/Security/StaticStrreplaceUnitTest.php

@@ -23,8 +23,9 @@ class StaticStrreplaceUnitTest extends AbstractSniffUnitTest {
 	 */
 	public function getErrorList() {
 		return [
-			3 => 1,
-			7 => 1,
+			50 => 1,
+			51 => 1,
+			56 => 1,
 		];
 	}