From 0deed7b4cf1961785171c304f31b2e52cec73f84 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Sun, 16 Mar 2025 18:17:57 -0400 Subject: [PATCH 1/5] Add test for using a namespace in sitePassByRefFunctions --- .../VariableAnalysisTest.php | 32 +++++++++++++++++++ .../fixtures/FunctionWithReferenceFixture.php | 5 +++ 2 files changed, 37 insertions(+) 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 +} From 2e0ecf239eb90f1fec7781e3198881fbf66f3ff9 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Sun, 16 Mar 2025 18:51:17 -0400 Subject: [PATCH 2/5] Add namespace to function names when looking for pass-by-reference --- VariableAnalysis/Lib/Helpers.php | 35 +++++++++++++++++++ .../CodeAnalysis/VariableAnalysisSniff.php | 29 +++++++++++++-- 2 files changed, 62 insertions(+), 2 deletions(-) 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..7e7bb24 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> + */ + private $passByRefFunctionsCache = []; + 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 ($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,12 @@ 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); + $refArgs = $this->getPassByReferenceFunction($functionName); + if (! $refArgs) { + return false; + } } $argPtrs = Helpers::findFunctionCallArguments($phpcsFile, $stackPtr); From 10e2dc005f78124aafd86b5226ad6388242cbfb0 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Sun, 16 Mar 2025 18:59:26 -0400 Subject: [PATCH 3/5] Make sure cache is not used before it is ready --- .../Sniffs/CodeAnalysis/VariableAnalysisSniff.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php index 7e7bb24..c5948eb 100644 --- a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php +++ b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php @@ -158,7 +158,7 @@ class VariableAnalysisSniff implements Sniff * * @var array> */ - private $passByRefFunctionsCache = []; + private $passByRefFunctionsCache; public function __construct() { @@ -211,7 +211,7 @@ private function getPassByReferenceFunction($functionName) */ private function getPassByReferenceFunctions() { - if ($this->passByRefFunctionsCache) { + if (isset($this->passByRefFunctionsCache)) { return $this->passByRefFunctionsCache; } $passByRefFunctions = Constants::getPassByReferenceFunctions(); From 1fd2685a8a30bb4aac711654904fe71beba85ba4 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Sun, 16 Mar 2025 19:00:10 -0400 Subject: [PATCH 4/5] Make sure we don't try to use null in getPassByReferenceFunction --- VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php index c5948eb..780fed2 100644 --- a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php +++ b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php @@ -1507,6 +1507,9 @@ protected function processVariableAsPassByReferenceFunctionCall(File $phpcsFile, if (! $refArgs) { // 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; From fb9f3b61bfb59f4987819c826569858cb6a4d81e Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Sun, 16 Mar 2025 19:02:10 -0400 Subject: [PATCH 5/5] Init passByRefFunctionsCache to null explicitly --- .../Sniffs/CodeAnalysis/VariableAnalysisSniff.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php index 780fed2..b1d7e46 100644 --- a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php +++ b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php @@ -156,9 +156,9 @@ class VariableAnalysisSniff implements Sniff /** * A cache for getPassByReferenceFunctions * - * @var array> + * @var array>|null */ - private $passByRefFunctionsCache; + private $passByRefFunctionsCache = null; public function __construct() { @@ -211,7 +211,7 @@ private function getPassByReferenceFunction($functionName) */ private function getPassByReferenceFunctions() { - if (isset($this->passByRefFunctionsCache)) { + if (! is_null($this->passByRefFunctionsCache)) { return $this->passByRefFunctionsCache; } $passByRefFunctions = Constants::getPassByReferenceFunctions();