Skip to content

Commit 82cb7c3

Browse files
committed
Fix anonym variables to be excluded.
1 parent 9370585 commit 82cb7c3

File tree

4 files changed

+163
-0
lines changed

4 files changed

+163
-0
lines changed

src/Annotator/Template/VariableExtractor.php

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,9 @@ protected function getExcludeReason(File $file, array $result): ?string {
146146
if ($this->isTryCatchVar($file, $result)) {
147147
return 'Try catch';
148148
}
149+
if ($this->isAnonymousFunctionParameter($file, $result)) {
150+
return 'Anonymous function parameter';
151+
}
149152

150153
if ($this->isAssignment($file, $result)) {
151154
return 'Assignment';
@@ -220,6 +223,33 @@ protected function isTryCatchVar(File $file, array $result): bool {
220223
return (bool)$file->findPrevious(T_CATCH, $startIndex - 1, $startIndex - 3);
221224
}
222225

226+
/**
227+
* @param \PHP_CodeSniffer\Files\File $file
228+
* @param array<string, mixed> $result
229+
* @return bool
230+
*/
231+
protected function isAnonymousFunctionParameter(File $file, array $result): bool {
232+
if (empty($result['context']['nested_parenthesis'])) {
233+
return false;
234+
}
235+
236+
$tokens = $file->getTokens();
237+
238+
// Check each level of nested parenthesis
239+
foreach ($result['context']['nested_parenthesis'] as $openParen => $closeParen) {
240+
// Check if this is immediately preceded by 'function' or 'fn' keyword
241+
if ($openParen > 0) {
242+
$prevToken = $file->findPrevious(T_WHITESPACE, $openParen - 1, (int)max(0, $openParen - 2), true);
243+
if ($prevToken !== false && in_array($tokens[$prevToken]['code'], [T_CLOSURE, T_FN], true)) {
244+
// This variable is inside an anonymous function's parameter list
245+
return true;
246+
}
247+
}
248+
}
249+
250+
return false;
251+
}
252+
223253
/**
224254
* @param \PHP_CodeSniffer\Files\File $file
225255
* @param array<string, mixed> $result

tests/TestCase/Annotator/Template/VariableExtractorTest.php

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,4 +208,51 @@ public function testExtractFromStrings(): void {
208208
$this->assertSame(['title', 'url'], array_keys($result));
209209
}
210210

211+
/**
212+
* Test that anonymous function parameters are excluded
213+
*
214+
* @return void
215+
*/
216+
public function testExtractExcludesAnonymousFunctionParameters() {
217+
$content = <<<'PHP'
218+
<?php
219+
$yourMoodIds = array_map(function($m) { return $m->mood_id; }, $participantMoods ?? []);
220+
$filtered = array_filter($items, function($item) {
221+
return $item->active;
222+
});
223+
usort($data, function($a, $b) {
224+
return $a->sort_order <=> $b->sort_order;
225+
});
226+
$doubled = array_map(fn($x) => $x * 2, $numbers);
227+
228+
// Test with 'use' clause
229+
$multiplier = 5;
230+
$result = array_map(function($n) use ($multiplier) {
231+
return $n * $multiplier;
232+
}, $values);
233+
PHP;
234+
235+
$file = $this->getFile('', $content);
236+
237+
$result = $this->variableExtractor->extract($file);
238+
239+
// Check that regular variables are found with correct exclusion reason
240+
$regularVars = ['yourMoodIds', 'participantMoods', 'filtered', 'items', 'data', 'doubled', 'numbers', 'multiplier', 'result', 'values'];
241+
foreach ($regularVars as $var) {
242+
$this->assertArrayHasKey($var, $result, "Variable \$$var should be found");
243+
if (in_array($var, ['yourMoodIds', 'filtered', 'doubled', 'multiplier', 'result'])) {
244+
$this->assertEquals('Assignment', $result[$var]['excludeReason'], "Variable \$$var should be excluded as assignment");
245+
} else {
246+
$this->assertNull($result[$var]['excludeReason'], "Variable \$$var should not be excluded");
247+
}
248+
}
249+
250+
// Check that anonymous function parameters are excluded
251+
$functionParams = ['m', 'item', 'a', 'b', 'x', 'n'];
252+
foreach ($functionParams as $param) {
253+
$this->assertArrayHasKey($param, $result, "Parameter \$$param should be found");
254+
$this->assertEquals('Anonymous function parameter', $result[$param]['excludeReason'], "Parameter \$$param should be excluded as anonymous function parameter");
255+
}
256+
}
257+
211258
}

tests/TestCase/Annotator/TemplateAnnotatorTest.php

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,55 @@ public function testAnnotateWithMultilineArray() {
465465
$this->assertTextContains(' -> 1 annotation added.', $output);
466466
}
467467

468+
/**
469+
* Tests that anonymous function parameters are excluded from annotations.
470+
*
471+
* @return void
472+
*/
473+
public function testAnnotateWithAnonymousFunctions() {
474+
Configure::write('IdeHelper.autoCollect', 'mixed');
475+
$annotator = $this->_getAnnotatorMock([]);
476+
477+
$expectedVariables = [
478+
'$this',
479+
'$participantMoods',
480+
'$yourMoodIds',
481+
'$items',
482+
'$filtered',
483+
'$data',
484+
'$numbers',
485+
'$doubled',
486+
'$multiplier',
487+
'$values',
488+
'$result',
489+
];
490+
491+
// Variables that should NOT get annotations (anonymous function parameters)
492+
$excludedVariables = ['$m', '$item', '$a', '$b', '$x', '$n'];
493+
// Note: $id is excluded too, but it's a foreach loop variable, not an anonymous function parameter
494+
495+
$callback = function($value) use ($expectedVariables, $excludedVariables) {
496+
// Extract just the PHPDoc block
497+
if (preg_match('/\/\*\*(.*?)\*\//s', $value, $matches)) {
498+
$docBlock = $matches[1];
499+
500+
foreach ($excludedVariables as $var) {
501+
// Check if the variable appears in an @var annotation in the doc block
502+
if (preg_match('/@var\s+[^\s]+\s+\\' . preg_quote($var, '/') . '/', $docBlock)) {
503+
$this->fail("Variable {$var} should not have an annotation (it's an anonymous function parameter)");
504+
}
505+
}
506+
}
507+
508+
return true;
509+
};
510+
511+
$annotator->expects($this->once())->method('storeFile')->with($this->anything(), $this->callback($callback));
512+
513+
$path = APP_ROOT . DS . 'templates/Foos/anonymous.php';
514+
$annotator->annotate($path);
515+
}
516+
468517
/**
469518
* @param array $params
470519
* @return \IdeHelper\Annotator\TemplateAnnotator|\PHPUnit\Framework\MockObject\MockObject
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<?php
2+
/**
3+
* @var \Cake\View\View $this
4+
* @var array<\App\Model\Entity\ParticipantMood> $participantMoods
5+
*/
6+
?>
7+
<div>
8+
<?php
9+
// This should NOT trigger an annotation for $m
10+
$yourMoodIds = array_map(function($m) { return $m->mood_id; }, $participantMoods ?? []);
11+
12+
// This should NOT trigger an annotation for $item
13+
$filtered = array_filter($items, function($item) {
14+
return $item->active;
15+
});
16+
17+
// This should NOT trigger annotations for $a and $b
18+
usort($data, function($a, $b) {
19+
return $a->sort_order <=> $b->sort_order;
20+
});
21+
22+
// Arrow function - should NOT trigger annotation for $x
23+
$doubled = array_map(fn($x) => $x * 2, $numbers);
24+
25+
// Variables with 'use' clause - $multiplier should still get annotation
26+
$multiplier = 5;
27+
$result = array_map(function($n) use ($multiplier) {
28+
return $n * $multiplier;
29+
}, $values);
30+
?>
31+
32+
<ul>
33+
<?php foreach ($yourMoodIds as $id): ?>
34+
<li><?= h($id) ?></li>
35+
<?php endforeach; ?>
36+
</ul>
37+
</div>

0 commit comments

Comments
 (0)