Skip to content

Commit aaf9022

Browse files
authored
Fix function call detection to exclude non-function tokens (#279)
* 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
1 parent 9f70390 commit aaf9022

File tree

4 files changed

+56
-2
lines changed

4 files changed

+56
-2
lines changed

Tests/VariableAnalysisSniff/ForLoopTest.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ public function testFunctionWithForLoopWarningsReportsCorrectLines()
2323
118,
2424
123,
2525
129,
26+
143,
27+
145,
2628
];
2729
$this->assertSame($expectedWarnings, $lines);
2830
}
@@ -43,5 +45,7 @@ public function testFunctionWithForLoopWarningsHasCorrectSniffCodes()
4345
$this->assertSame(self::UNUSED_ERROR_CODE, $warnings[118][5][0]['source']);
4446
$this->assertSame(self::UNDEFINED_ERROR_CODE, $warnings[123][5][0]['source']);
4547
$this->assertSame(self::UNDEFINED_ERROR_CODE, $warnings[129][14][0]['source']);
48+
$this->assertSame(self::UNUSED_ERROR_CODE, $warnings[143][5][0]['source']);
49+
$this->assertSame(self::UNUSED_ERROR_CODE, $warnings[145][3][0]['source']);
4650
}
4751
}

Tests/VariableAnalysisSniff/fixtures/FunctionWithClosureFixture.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,3 +101,18 @@ function function_with_static_closure() {
101101
echo $inner_param;
102102
}, $params);
103103
}
104+
105+
function function_with_static_variable_inside_anonymous_function() {
106+
$anon = (function() {
107+
static $test;
108+
echo $test;
109+
});
110+
$anon();
111+
}
112+
113+
function function_with_static_variable_inside_anonymous_function_inside_arguments() {
114+
add_action('test', function () {
115+
static $providerId;
116+
echo $providerId;
117+
});
118+
}

Tests/VariableAnalysisSniff/fixtures/FunctionWithForLoopFixture.php

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,8 +138,23 @@ function veryBoringForLoop() {
138138
}
139139
}
140140

141+
function reallySmallForLoopWithUnusedInitVar() {
142+
for ($i = 1,
143+
$j = 0; // Unused variable $j
144+
$i <= 10;
145+
$j += $i, // Unused variable $j
146+
print $i,
147+
$i++);
148+
}
149+
141150
function reallySmallForLoop() {
142-
for ($i = 1, $j = 0; $i <= 10; $j += $i, print $i, $i++);
151+
for ($i = 1,
152+
$j = 0;
153+
$i <= 10;
154+
$j += $i,
155+
print $i +
156+
$j,
157+
$i++);
143158
}
144159

145160
function colonSyntaxForLoop() {

VariableAnalysis/Lib/Helpers.php

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1092,12 +1092,32 @@ public static function getFunctionIndexForFunctionCallArgument(File $phpcsFile,
10921092
if (! is_int($functionPtr) || ! isset($tokens[$functionPtr]['code'])) {
10931093
return null;
10941094
}
1095-
if ($tokens[$functionPtr]['content'] === 'function' || ($tokens[$functionPtr]['content'] === 'fn' && self::isArrowFunction($phpcsFile, $functionPtr))) {
1095+
if (
1096+
$tokens[$functionPtr]['content'] === 'function'
1097+
|| ($tokens[$functionPtr]['content'] === 'fn' && self::isArrowFunction($phpcsFile, $functionPtr))
1098+
) {
1099+
// If there is a function/fn keyword before the beginning of the parens,
1100+
// this is a function definition and not a function call.
10961101
return null;
10971102
}
10981103
if (! empty($tokens[$functionPtr]['scope_opener'])) {
1104+
// If the alleged function name has a scope, this is not a function call.
1105+
return null;
1106+
}
1107+
1108+
$functionNameType = $tokens[$functionPtr]['code'];
1109+
if (! in_array($functionNameType, Tokens::$functionNameTokens, true)) {
1110+
// If the alleged function name is not a variable or a string, this is
1111+
// not a function call.
10991112
return null;
11001113
}
1114+
1115+
if ($tokens[$functionPtr]['level'] !== $tokens[$stackPtr]['level']) {
1116+
// If the variable is inside a different scope than the function name,
1117+
// the function call doesn't apply to the variable.
1118+
return null;
1119+
}
1120+
11011121
return $functionPtr;
11021122
}
11031123

0 commit comments

Comments
 (0)