Performance/FetchingRemoteData: bug fix - false negative for compound parameters

As things were, if the parameter did contain a text string token and that text string token did *not* contain the text "://", the sniff would stay silent, even when the parameter contained additional non-text string tokens, which means we cannot reliably determine whether it is a local file or not.

This commit changes the order of the checks to _first_ check if any of the text string tokens in the parameter contain the "://" text. If so, it will throw the (more informative) `FileGetContentsRemoteFile` warning.

If not, it will check if the parameter contained anything but text string related or empty tokens. If so, it will throw the (more generic) `FileGetContentsUnknown` warning.

Includes test.

Author: jrfnl

WordPressVIPMinimum/Sniffs/Performance/FetchingRemoteDataSniff.php

@@ -62,24 +62,35 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p
 			return;
 		}
 
-		$has_text_string = $this->phpcsFile->findNext( Tokens::$stringTokens, $param_start, $search_end );
-		if ( $has_text_string === false ) {
-			$this->add_contents_unknown_warning( $stackPtr, $data );
-		}
-
 		$isRemoteFile = false;
-		while ( $has_text_string !== false ) {
+		$search_start = $param_start;
+		// phpcs:ignore Generic.CodeAnalysis.AssignmentInCondition.FoundInWhileCondition -- Valid usage.
+		while ( ( $has_text_string = $this->phpcsFile->findNext( Tokens::$stringTokens, $search_start, $search_end ) ) !== false ) {
 			if ( strpos( $this->tokens[ $has_text_string ]['content'], '://' ) !== false ) {
 				$isRemoteFile = true;
 				break;
 			}
 
-			$has_text_string = $this->phpcsFile->findNext( Tokens::$stringTokens, ( $has_text_string + 1 ), $search_end );
+			$search_start = ( $has_text_string + 1 );
 		}
 
 		if ( $isRemoteFile === true ) {
 			$message = '`%s()` is highly discouraged for remote requests, please use `wpcom_vip_file_get_contents()` or `vip_safe_wp_remote_get()` instead.';
 			$this->phpcsFile->addWarning( $message, $stackPtr, 'FileGetContentsRemoteFile', $data );
+			return;
+		}
+
+		/*
+		 * Okay, so we haven't been able to determine for certain this is a remote file.
+		 * Check for tokens which would make the parameter contents dynamic.
+		 */
+		$ignore  = Tokens::$emptyTokens;
+		$ignore += Tokens::$stringTokens;
+		$ignore += [ T_STRING_CONCAT => T_STRING_CONCAT ];
+
+		$has_non_text_string = $this->phpcsFile->findNext( $ignore, $param_start, $search_end, true );
+		if ( $has_non_text_string !== false ) {
+			$this->add_contents_unknown_warning( $stackPtr, $data );
 		}
 	}
 

WordPressVIPMinimum/Tests/Performance/FetchingRemoteDataUnitTest.inc

@@ -83,3 +83,6 @@ array_walk($files, file_get_contents(...)); // PHP 8.1 first class callable. War
 
 // Bug: check all text string tokens in the parameter.
 $result = file_get_contents(( is_ssl() ? 'http' : 'https' ) . '://example.com'); // Warning FileGetContentsRemoteFile.
+
+// Bug: don't presume if the parameter contains a text string token, that will be the only token.
+\file_get_contents( $url . '/filename.css' ); // Warning FileGetContentsUnknown.

WordPressVIPMinimum/Tests/Performance/FetchingRemoteDataUnitTest.php

@@ -44,6 +44,7 @@ public function getWarningList() {
 			70 => 1,
 			79 => 1,
 			85 => 1,
+			88 => 1,
 		];
 	}
 }