Skip to content

Commit

Permalink
Fix function call detection to exclude non-function tokens (#279)
Browse files Browse the repository at this point in the history
* Add test for static var inside anon function

* Do not consider non-function names to be function names

* Fix small for loop test

* Add test for unused init var in for loop

* Add test for static var inside anonymous func inside closure

* Do not consider variables in closures as part of func args
  • Loading branch information
sirbrillig authored Sep 8, 2022
1 parent 9f70390 commit aaf9022
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 2 deletions.
4 changes: 4 additions & 0 deletions Tests/VariableAnalysisSniff/ForLoopTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ public function testFunctionWithForLoopWarningsReportsCorrectLines()
118,
123,
129,
143,
145,
];
$this->assertSame($expectedWarnings, $lines);
}
Expand All @@ -43,5 +45,7 @@ public function testFunctionWithForLoopWarningsHasCorrectSniffCodes()
$this->assertSame(self::UNUSED_ERROR_CODE, $warnings[118][5][0]['source']);
$this->assertSame(self::UNDEFINED_ERROR_CODE, $warnings[123][5][0]['source']);
$this->assertSame(self::UNDEFINED_ERROR_CODE, $warnings[129][14][0]['source']);
$this->assertSame(self::UNUSED_ERROR_CODE, $warnings[143][5][0]['source']);
$this->assertSame(self::UNUSED_ERROR_CODE, $warnings[145][3][0]['source']);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -101,3 +101,18 @@ function function_with_static_closure() {
echo $inner_param;
}, $params);
}

function function_with_static_variable_inside_anonymous_function() {
$anon = (function() {
static $test;
echo $test;
});
$anon();
}

function function_with_static_variable_inside_anonymous_function_inside_arguments() {
add_action('test', function () {
static $providerId;
echo $providerId;
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,23 @@ function veryBoringForLoop() {
}
}

function reallySmallForLoopWithUnusedInitVar() {
for ($i = 1,
$j = 0; // Unused variable $j
$i <= 10;
$j += $i, // Unused variable $j
print $i,
$i++);
}

function reallySmallForLoop() {
for ($i = 1, $j = 0; $i <= 10; $j += $i, print $i, $i++);
for ($i = 1,
$j = 0;
$i <= 10;
$j += $i,
print $i +
$j,
$i++);
}

function colonSyntaxForLoop() {
Expand Down
22 changes: 21 additions & 1 deletion VariableAnalysis/Lib/Helpers.php
Original file line number Diff line number Diff line change
Expand Up @@ -1092,12 +1092,32 @@ public static function getFunctionIndexForFunctionCallArgument(File $phpcsFile,
if (! is_int($functionPtr) || ! isset($tokens[$functionPtr]['code'])) {
return null;
}
if ($tokens[$functionPtr]['content'] === 'function' || ($tokens[$functionPtr]['content'] === 'fn' && self::isArrowFunction($phpcsFile, $functionPtr))) {
if (
$tokens[$functionPtr]['content'] === 'function'
|| ($tokens[$functionPtr]['content'] === 'fn' && self::isArrowFunction($phpcsFile, $functionPtr))
) {
// If there is a function/fn keyword before the beginning of the parens,
// this is a function definition and not a function call.
return null;
}
if (! empty($tokens[$functionPtr]['scope_opener'])) {
// If the alleged function name has a scope, this is not a function call.
return null;
}

$functionNameType = $tokens[$functionPtr]['code'];
if (! in_array($functionNameType, Tokens::$functionNameTokens, true)) {
// If the alleged function name is not a variable or a string, this is
// not a function call.
return null;
}

if ($tokens[$functionPtr]['level'] !== $tokens[$stackPtr]['level']) {
// If the variable is inside a different scope than the function name,
// the function call doesn't apply to the variable.
return null;
}

return $functionPtr;
}

Expand Down

0 comments on commit aaf9022

Please sign in to comment.