Skip to content

Commit a51400f

Browse files
sirbrilligsirbrillig
authored
Add awareness of nested named functions (#30)
* Add test for nested functions * Add additional test cases * isFunctionDefined: only search within scope * Add global function call to test * Examine only certain scopes instead * Search within the inner-most scope * Include global scope and skip some scopes * Examine all scoping blocks * Add more test cases * Remove nullable type
1 parent b2bbc43 commit a51400f

File tree

5 files changed

+146
-17
lines changed

5 files changed

+146
-17
lines changed

‎ImportDetection/SniffHelpers.php‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -219,12 +219,6 @@ public function getConstantName(File $phpcsFile, $stackPtr) {
219219
return $tokens[$nextStringPtr]['content'];
220220
}
221221

222-
public function isFunctionAMethod(File $phpcsFile, $stackPtr): bool {
223-
$tokens = $phpcsFile->getTokens();
224-
$currentToken = $tokens[$stackPtr];
225-
return ! empty($currentToken['conditions']);
226-
}
227-
228222
public function isSymbolADefinition(File $phpcsFile, Symbol $symbol): bool {
229223
// if the previous non-whitespace token is const, function, class, or trait, it is a definition
230224
// Note: this does not handle use statements, for that use isWithinImportStatement

‎ImportDetection/Sniffs/Imports/RequireImportsSniff.php‎

Lines changed: 68 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ private function isSymbolDefined(File $phpcsFile, Symbol $symbol): bool {
157157
return $this->isNamespaceImported($phpcsFile, $namespace);
158158
}
159159
// If the symbol has no namespace and is itself is imported or defined, ignore it
160-
return $this->isNamespaceImportedOrDefined($phpcsFile, $symbol->getName());
160+
return $this->isNamespaceImportedOrDefined($phpcsFile, $symbol->getName(), $symbol->getSymbolConditions());
161161
}
162162

163163
private function isNamespaceImported(File $phpcsFile, string $namespace): bool {
@@ -168,12 +168,12 @@ private function isNamespaceImported(File $phpcsFile, string $namespace): bool {
168168
);
169169
}
170170

171-
private function isNamespaceImportedOrDefined(File $phpcsFile, string $namespace): bool {
171+
private function isNamespaceImportedOrDefined(File $phpcsFile, string $namespace, array $conditions): bool {
172172
return (
173173
$this->isClassImported($phpcsFile, $namespace)
174174
|| $this->isClassDefined($phpcsFile, $namespace)
175175
|| $this->isFunctionImported($phpcsFile, $namespace)
176-
|| $this->isFunctionDefined($phpcsFile, $namespace)
176+
|| $this->isFunctionDefined($phpcsFile, $namespace, $conditions)
177177
|| $this->isConstImported($phpcsFile, $namespace)
178178
|| $this->isConstDefined($phpcsFile, $namespace)
179179
);
@@ -250,19 +250,76 @@ private function isClassDefined(File $phpcsFile, string $className): bool {
250250
return false;
251251
}
252252

253-
private function isFunctionDefined(File $phpcsFile, string $functionName): bool {
254-
$helper = new SniffHelpers();
255-
$functionPtr = $phpcsFile->findNext([T_FUNCTION], 0);
256-
while ($functionPtr) {
257-
$thisFunctionName = $phpcsFile->getDeclarationName($functionPtr);
258-
if ($functionName === $thisFunctionName && ! $helper->isFunctionAMethod($phpcsFile, $functionPtr)) {
259-
return true;
253+
private function isFunctionDefined(File $phpcsFile, string $functionName, array $conditions): bool {
254+
$tokens = $phpcsFile->getTokens();
255+
$scopesToEnter = array_filter(array_keys($conditions), function ($conditionPtr) use ($conditions) {
256+
return $conditions[$conditionPtr] === T_FUNCTION;
257+
});
258+
$this->debug("looking for definition for function {$functionName}");
259+
$this->debug("my conditions are " . json_encode($conditions));
260+
$this->debug("scopes to enter " . implode(',', $scopesToEnter));
261+
if (empty($scopesToEnter)) {
262+
return false;
263+
}
264+
// Only look at the inner-most scope and global scope
265+
$scopesToEnter = [end($scopesToEnter), 0];
266+
267+
foreach ($scopesToEnter as $scopeStart) {
268+
$functionToken = $tokens[$scopeStart];
269+
$scopeEnd = $functionToken['scope_closer'] ?? null;
270+
271+
// Within each function scope, find all the function definitions and
272+
// compare their names to the name we are looking for.
273+
$functionDefinitionsInScope = $this->findAllFunctionDefinitionsInScope($phpcsFile, $scopeStart, $scopeEnd);
274+
275+
foreach ($functionDefinitionsInScope as $thisFunctionName) {
276+
$this->debug("is this function the one we want? " . $thisFunctionName);
277+
if ($functionName === $thisFunctionName) {
278+
$this->debug("yes indeed");
279+
return true;
280+
}
260281
}
261-
$functionPtr = $phpcsFile->findNext([T_FUNCTION], $functionPtr + 1);
262282
}
263283
return false;
264284
}
265285

286+
/**
287+
* Return an array of function names defined in a scope
288+
*/
289+
private function findAllFunctionDefinitionsInScope(File $phpcsFile, int $scopeStart, int $scopeEnd = null): array {
290+
$this->debug("looking for functions defined between {$scopeStart} and {$scopeEnd}");
291+
$tokens = $phpcsFile->getTokens();
292+
$functionNames = [];
293+
294+
$tokensToInvestigate = [T_FUNCTION, T_CLASS, T_TRAIT, T_INTERFACE];
295+
296+
// Skip the function we are in, but not the global scope
297+
$functionToken = $tokens[$scopeStart];
298+
$scopeOffset = $functionToken['type'] === 'T_FUNCTION' ? 2 : 0;
299+
$functionPtr = $phpcsFile->findNext($tokensToInvestigate, $scopeStart + $scopeOffset, $scopeEnd);
300+
301+
while ($functionPtr) {
302+
$functionName = $phpcsFile->getDeclarationName($functionPtr);
303+
$functionToken = $tokens[$functionPtr];
304+
$thisFunctionScopeEnd = $functionToken['scope_closer'] ?? 0;
305+
306+
// Skip things other than IF that have their own scope
307+
if ($functionToken['type'] !== 'T_FUNCTION') {
308+
if (! $thisFunctionScopeEnd) {
309+
$this->debug("function at {$functionPtr} has no end:" . $functionName);
310+
break;
311+
}
312+
$functionPtr = $phpcsFile->findNext($tokensToInvestigate, $thisFunctionScopeEnd, $scopeEnd);
313+
continue;
314+
}
315+
316+
$this->debug("found function at {$functionPtr}:" . $functionName);
317+
$functionNames[] = $functionName;
318+
$functionPtr = $phpcsFile->findNext($tokensToInvestigate, $thisFunctionScopeEnd, $scopeEnd);
319+
}
320+
return $functionNames;
321+
}
322+
266323
private function isConstDefined(File $phpcsFile, string $functionName): bool {
267324
$helper = new SniffHelpers();
268325
$functionPtr = $phpcsFile->findNext([T_CONST], 0);

‎ImportDetection/Symbol.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,10 @@ public function getSymbolPosition(): int {
6666
return $this->tokens[0]['tokenPosition'] ?? 1;
6767
}
6868

69+
public function getSymbolConditions(): array {
70+
return $this->tokens[0]['conditions'] ?? [];
71+
}
72+
6973
public function markUsed() {
7074
$this->isUsed = true;
7175
}

‎tests/Sniffs/Imports/NestedFunctionsFixture.php‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
<?php
2+
function doThing() {
3+
4+
function nestedFuncA( $arg ) {
5+
echo $arg . ' world';
6+
}
7+
8+
nestedFuncA( 'here' );
9+
nestedFuncB( 'hello' ); // warning: undefined
10+
doAThing( 'dothingfunc' ); // warning: undefined
11+
}
12+
13+
class MyThing() {
14+
public function doAThing() {
15+
function nestedFuncB( $arg ) {
16+
echo $arg . ' world';
17+
}
18+
19+
nestedFuncB( 'hello' );
20+
nestedFuncA( 'live' ); // warning: undefined
21+
$this->nestedFuncA( 'hello' );
22+
23+
if (true) {
24+
nestedFuncB( 'boo' );
25+
}
26+
27+
if (true) {
28+
function nestedFuncC() {
29+
echo 'we are deep now';
30+
}
31+
32+
nestedFuncC();
33+
}
34+
}
35+
36+
public function doANestedThing() {
37+
function nestedFuncA() {
38+
echo 'nope';
39+
}
40+
41+
\registerThing(new class {
42+
public function subClassFunc() {
43+
function nestedFuncC( $arg ) {
44+
echo $arg . ' world';
45+
}
46+
47+
nestedFuncC( 'hello' );
48+
nestedFuncA( 'blarg' ); // warning: undefined
49+
}
50+
});
51+
52+
doANestedThing('thisfunc'); // warning: undefined
53+
}
54+
}
55+
56+
nestedFuncA( 'hi' ); // warning: undefined

‎tests/Sniffs/Imports/RequireImportsSniffTest.php‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,4 +374,22 @@ public function testRequireImportsSniffTreatsFileImportAsUsedWhenUsed() {
374374
$expectedLines = [];
375375
$this->assertEquals($expectedLines, $lines);
376376
}
377+
378+
public function testRequireImportsNoticesNestedFunctions() {
379+
$fixtureFile = __DIR__ . '/NestedFunctionsFixture.php';
380+
$sniffFile = __DIR__ . '/../../../ImportDetection/Sniffs/Imports/RequireImportsSniff.php';
381+
$helper = new SniffTestHelper();
382+
$phpcsFile = $helper->prepareLocalFileForSniffs($sniffFile, $fixtureFile);
383+
$phpcsFile->process();
384+
$lines = $helper->getWarningLineNumbersFromFile($phpcsFile);
385+
$expectedLines = [
386+
9,
387+
10,
388+
20,
389+
48,
390+
52,
391+
56,
392+
];
393+
$this->assertEquals($expectedLines, $lines);
394+
}
377395
}

0 commit comments

Comments
 (0)