Accept namespaces in sitePassByRefFunctions (#351)

* Add test for using a namespace in sitePassByRefFunctions

* Add namespace to function names when looking for pass-by-reference

* Make sure cache is not used before it is ready

* Make sure we don't try to use null in getPassByReferenceFunction

* Init passByRefFunctionsCache to null explicitly

Author: sirbrillig

Tests/VariableAnalysisSniff/VariableAnalysisTest.php

@@ -298,6 +298,7 @@ public function testFunctionWithReferenceWarnings()
 			77,
 			81,
 			98,
+			106,
 		];
 		$this->assertSame($expectedWarnings, $lines);
 	}
@@ -331,6 +332,36 @@ public function testFunctionWithReferenceWarningsAllowsCustomFunctions()
 		$this->assertSame($expectedWarnings, $lines);
 	}
 
+	public function testFunctionWithReferenceWarningsAllowsCustomFunctionsNamespaced()
+	{
+		$fixtureFile = $this->getFixture('FunctionWithReferenceFixture.php');
+		$phpcsFile = $this->prepareLocalFileForSniffs($fixtureFile);
+		$this->setSniffProperty($phpcsFile, 'sitePassByRefFunctions', '\My\Functions\my_reference_function:2,3 another_reference_function:2,...');
+		$phpcsFile->process();
+		$lines = $this->getWarningLineNumbersFromFile($phpcsFile);
+		$expectedWarnings = [
+			10,
+			11,
+			12,
+			13,
+			14,
+			16,
+			29,
+			41,
+			42,
+			43,
+			46,
+			52,
+			56,
+			57,
+			63,
+			76,
+			81,
+			98,
+		];
+		$this->assertSame($expectedWarnings, $lines);
+	}
+
 	public function testFunctionWithReferenceWarningsAllowsWordPressFunctionsIfSet()
 	{
 		$fixtureFile = $this->getFixture('FunctionWithReferenceFixture.php');
@@ -357,6 +388,7 @@ public function testFunctionWithReferenceWarningsAllowsWordPressFunctionsIfSet()
 			76,
 			77,
 			98,
+			106,
 		];
 		$this->assertSame($expectedWarnings, $lines);
 	}

Tests/VariableAnalysisSniff/fixtures/FunctionWithReferenceFixture.php

@@ -100,3 +100,8 @@ function function_with_foreach_with_reference($derivatives, $base_plugin_definit
   }
   return $derivatives;
 }
+
+function function_with_ignored_reference_call_with_namespace() {
+    $foo = 'bar';
+    \My\Functions\my_reference_function($foo, $baz, $bip); // Undefined variable $bar, Undefined variable $bip
+}

VariableAnalysis/Lib/Helpers.php

@@ -1658,6 +1658,41 @@ public static function isConstructorPromotion(File $phpcsFile, $stackPtr)
 		return false;
 	}
 
+	/**
+	 * If looking at a function call token, return a string for the full function
+	 * name including any inline namespace.
+	 *
+	 * So for example, if the call looks like `\My\Namespace\doSomething($bar)`
+	 * and `$stackPtr` refers to `doSomething`, this will return
+	 * `\My\Namespace\doSomething`.
+	 *
+	 * @param File $phpcsFile
+	 * @param int  $stackPtr
+	 *
+	 * @return string|null
+	 */
+	public static function getFunctionNameWithNamespace(File $phpcsFile, $stackPtr)
+	{
+		$tokens = $phpcsFile->getTokens();
+
+		if (! isset($tokens[$stackPtr])) {
+			return null;
+		}
+		$startOfScope = self::findVariableScope($phpcsFile, $stackPtr);
+		$functionName = $tokens[$stackPtr]['content'];
+
+		// Move backwards from the token, collecting namespace separators and
+		// strings, until we encounter whitespace or something else.
+		$partOfNamespace = [T_NS_SEPARATOR, T_STRING];
+		for ($i = $stackPtr - 1; $i > $startOfScope; $i--) {
+			if (! in_array($tokens[$i]['code'], $partOfNamespace, true)) {
+				break;
+			}
+			$functionName = "{$tokens[$i]['content']}{$functionName}";
+		}
+		return $functionName;
+	}
+
 	/**
 	 * Return false if the token is definitely not part of a typehint
 	 *

VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php

@@ -153,6 +153,13 @@ class VariableAnalysisSniff implements Sniff
 	 */
 	public $allowUnusedVariablesBeforeRequire = false;
 
+	/**
+	 * A cache for getPassByReferenceFunctions
+	 *
+	 * @var array<array<int|string>>|null
+	 */
+	private $passByRefFunctionsCache = null;
+
 	public function __construct()
 	{
 		$this->scopeManager = new ScopeManager();
@@ -195,6 +202,18 @@ public function register()
 	 */
 	private function getPassByReferenceFunction($functionName)
 	{
+		$passByRefFunctions = $this->getPassByReferenceFunctions();
+		return isset($passByRefFunctions[$functionName]) ? $passByRefFunctions[$functionName] : [];
+	}
+
+	/**
+	 * @return array<array<int|string>>
+	 */
+	private function getPassByReferenceFunctions()
+	{
+		if (! is_null($this->passByRefFunctionsCache)) {
+			return $this->passByRefFunctionsCache;
+		}
 		$passByRefFunctions = Constants::getPassByReferenceFunctions();
 		if (!empty($this->sitePassByRefFunctions)) {
 			$lines = Helpers::splitStringToArray('/\s+/', trim($this->sitePassByRefFunctions));
@@ -206,7 +225,8 @@ private function getPassByReferenceFunction($functionName)
 		if ($this->allowWordPressPassByRefFunctions) {
 			$passByRefFunctions = array_merge($passByRefFunctions, Constants::getWordPressPassByReferenceFunctions());
 		}
-		return isset($passByRefFunctions[$functionName]) ? $passByRefFunctions[$functionName] : [];
+		$this->passByRefFunctionsCache = $passByRefFunctions;
+		return $passByRefFunctions;
 	}
 
 	/**
@@ -1485,7 +1505,15 @@ protected function processVariableAsPassByReferenceFunctionCall(File $phpcsFile,
 		$functionName = $tokens[$functionPtr]['content'];
 		$refArgs = $this->getPassByReferenceFunction($functionName);
 		if (! $refArgs) {
-			return false;
+			// Check again with the fully namespaced function name.
+			$functionName = Helpers::getFunctionNameWithNamespace($phpcsFile, $functionPtr);
+			if (! $functionName) {
+				return false;
+			}
+			$refArgs = $this->getPassByReferenceFunction($functionName);
+			if (! $refArgs) {
+				return false;
+			}
 		}
 
 		$argPtrs = Helpers::findFunctionCallArguments($phpcsFile, $stackPtr);