Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix function call detection to exclude non-function tokens #279

Merged
merged 6 commits into from
Sep 8, 2022
Merged

Fix function call detection to exclude non-function tokens #279

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7ae3b6b
Add test for static var inside anon function
sirbrillig Sep 7, 2022
fe07578
Do not consider non-function names to be function names
sirbrillig Sep 7, 2022
f930814
Fix small for loop test
sirbrillig Sep 8, 2022
358637e
Add test for unused init var in for loop
sirbrillig Sep 8, 2022
52d668d
Add test for static var inside anonymous func inside closure
sirbrillig Sep 8, 2022
22fe39d
Do not consider variables in closures as part of func args
sirbrillig Sep 8, 2022
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
4 changes: 4 additions & 0 deletions Tests/VariableAnalysisSniff/ForLoopTest.php
View file
Open in desktop
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']);
}
}
15 changes: 15 additions & 0 deletions Tests/VariableAnalysisSniff/fixtures/FunctionWithClosureFixture.php
View file
Open in desktop
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;
});
}
17 changes: 16 additions & 1 deletion Tests/VariableAnalysisSniff/fixtures/FunctionWithForLoopFixture.php
View file
Open in desktop
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
View file
Open in desktop
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