Skip to content

Accept namespaces in sitePassByRefFunctions #351

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions Tests/VariableAnalysisSniff/VariableAnalysisTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ public function testFunctionWithReferenceWarnings()
77,
81,
98,
106,
];
$this->assertSame($expectedWarnings, $lines);
}
Expand Down Expand Up @@ -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');
Expand All @@ -357,6 +388,7 @@ public function testFunctionWithReferenceWarningsAllowsWordPressFunctionsIfSet()
76,
77,
98,
106,
];
$this->assertSame($expectedWarnings, $lines);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
35 changes: 35 additions & 0 deletions VariableAnalysis/Lib/Helpers.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down
32 changes: 30 additions & 2 deletions VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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));
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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);
Expand Down