diff --git a/Tests/VariableAnalysisSniff/VariableAnalysisTest.php b/Tests/VariableAnalysisSniff/VariableAnalysisTest.php index ed11f48..a8c5c51 100644 --- a/Tests/VariableAnalysisSniff/VariableAnalysisTest.php +++ b/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); } diff --git a/Tests/VariableAnalysisSniff/fixtures/FunctionWithReferenceFixture.php b/Tests/VariableAnalysisSniff/fixtures/FunctionWithReferenceFixture.php index 44c0f10..baf88e7 100644 --- a/Tests/VariableAnalysisSniff/fixtures/FunctionWithReferenceFixture.php +++ b/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 +} diff --git a/VariableAnalysis/Lib/Helpers.php b/VariableAnalysis/Lib/Helpers.php index ae06129..4a9c162 100644 --- a/VariableAnalysis/Lib/Helpers.php +++ b/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 * diff --git a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php index 5eaea94..b1d7e46 100644 --- a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php +++ b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php @@ -153,6 +153,13 @@ class VariableAnalysisSniff implements Sniff */ public $allowUnusedVariablesBeforeRequire = false; + /** + * A cache for getPassByReferenceFunctions + * + * @var array>|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> + */ + 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);