Skip to content

Commit 4f7b5a4

Browse files
yCodeTechyCodeTech
committed
fix: void functions being wrongly flagged for missing @return tags
If a void function has a nested anonymous function or closure which returns a value, the `FunctionComment` sniff was wrongly flagging the void function has missing a `@return` tag. Fixed by adding logic to check if the `return` statement is within a nested function/closure, and skipping it if it does. Added: - Added `isReturnInNestedFunction` method to `FunctionCommentSniff` class to check if `return` is in a nested function/closure. - Added new test functions in both `TestFile`s. Changed: - Various `TestFile`s comments. - Readme. Removed: - Auto formatting removals, superfluous whitespace and `@return void`.
1 parent fd72054 commit 4f7b5a4

File tree

4 files changed

+158
-25
lines changed

4 files changed

+158
-25
lines changed

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ Enforces proper spacing and formatting in docblocks.
185185

186186
### yCodeTech.Commenting.FunctionComment
187187

188-
Functions that return a value must have a `@return` docblock tag.
188+
Functions that return a value must have a `@return` docblock tag (nested anonymous function and closure returns will be ignored).
189189

190190
<table>
191191
<tr>

‎test_utils/TestFile.php‎

Lines changed: 51 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -247,8 +247,13 @@ public function testMissingGeneratorReturn()
247247
yield 1;
248248
}
249249

250+
/***********************
251+
* VOID FUNCTION TESTS *
252+
***********************/
253+
250254
/**
251-
* Function that returns void with an explicit `void` typing (should NOT be flagged).
255+
* Function that returns void with an explicit `void` typing
256+
* (should NOT be flagged for missing `@return` tag).
252257
*
253258
* The following should NOT be fixed:
254259
* - A `@return` tag should not be added for an explicit `void` return.
@@ -262,7 +267,7 @@ public function testExplicitVoidFunction($message): void
262267

263268
/**
264269
* Function that returns void implicitly, ie. no return statement in the body
265-
* (should NOT be flagged).
270+
* (should NOT be flagged for missing `@return` tag).
266271
*
267272
* The following should NOT be fixed:
268273
* - A `@return` tag should not be added for an implicit `void` return.
@@ -275,10 +280,11 @@ public function testImplicitVoidFunction($message)
275280
}
276281

277282
/**
278-
* Function with empty return statement (should NOT be flagged).
283+
* Function with empty return statement
284+
* (should NOT be flagged for missing `@return` tag).
279285
*
280286
* The following should NOT be fixed:
281-
* - A `@return` tag should not be added for an implicit `void` return.
287+
* - A `@return` tag should not be added for an empty `return`.
282288
*
283289
* @param string $condition Some condition
284290
*/
@@ -290,6 +296,47 @@ public function testEmptyReturnFunction($condition)
290296
echo "Continuing...";
291297
}
292298

299+
/**
300+
* Function that returns void implicitly and has a nested closure that returns a value
301+
* (should NOT be flagged for missing `@return` tag).
302+
*
303+
* The following should NOT be fixed:
304+
* - A `@return` tag should not be added for a `void` function that has a
305+
* nested returnable closure.
306+
*/
307+
function testVoidFunctionWithNestedClosure()
308+
{
309+
$array = [1, 2, 3];
310+
311+
// This closure returns a value, but it's not in the immediate function scope
312+
$result = array_map(function($item) {
313+
return $item * 2; // This return should be ignored
314+
}, $array);
315+
316+
echo "Result: " . implode(', ', $result);
317+
}
318+
319+
/**
320+
* Function that returns void implicitly and has a nested anonymous function that returns a
321+
* value (should NOT be flagged for missing `@return` tag).
322+
*
323+
* The following should NOT be fixed:
324+
* - A `@return` tag should not be added for a `void` function that has a
325+
* nested returnable anonymous function.
326+
*/
327+
function testVoidFunctionWithAnonymousFunction()
328+
{
329+
$callback = function($value) {
330+
return strtoupper($value); // This return should be ignored
331+
};
332+
333+
$callback('test');
334+
}
335+
336+
/*************************************
337+
* VOID FUNCTIONS WITH @RETURN TESTS *
338+
*************************************/
339+
293340
/**
294341
* Function that returns void that has a `@return` tag (should be flagged).
295342
*

‎test_utils/TestFile_WithErrors_DoNotFix.php‎

Lines changed: 51 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -247,8 +247,13 @@ public function testMissingGeneratorReturn()
247247
yield 1;
248248
}
249249

250+
/***********************
251+
* VOID FUNCTION TESTS *
252+
***********************/
253+
250254
/**
251-
* Function that returns void with an explicit `void` typing (should NOT be flagged).
255+
* Function that returns void with an explicit `void` typing
256+
* (should NOT be flagged for missing `@return` tag).
252257
*
253258
* The following should NOT be fixed:
254259
* - A `@return` tag should not be added for an explicit `void` return.
@@ -262,7 +267,7 @@ public function testExplicitVoidFunction($message): void
262267

263268
/**
264269
* Function that returns void implicitly, ie. no return statement in the body
265-
* (should NOT be flagged).
270+
* (should NOT be flagged for missing `@return` tag).
266271
*
267272
* The following should NOT be fixed:
268273
* - A `@return` tag should not be added for an implicit `void` return.
@@ -275,10 +280,11 @@ public function testImplicitVoidFunction($message)
275280
}
276281

277282
/**
278-
* Function with empty return statement (should NOT be flagged).
283+
* Function with empty return statement
284+
* (should NOT be flagged for missing `@return` tag).
279285
*
280286
* The following should NOT be fixed:
281-
* - A `@return` tag should not be added for an implicit `void` return.
287+
* - A `@return` tag should not be added for an empty `return`.
282288
*
283289
* @param string $condition Some condition
284290
*/
@@ -290,6 +296,47 @@ public function testEmptyReturnFunction($condition)
290296
echo "Continuing...";
291297
}
292298

299+
/**
300+
* Function that returns void implicitly and has a nested closure that returns a value
301+
* (should NOT be flagged for missing `@return` tag).
302+
*
303+
* The following should NOT be fixed:
304+
* - A `@return` tag should not be added for a `void` function that has a
305+
* nested returnable closure.
306+
*/
307+
function testVoidFunctionWithNestedClosure()
308+
{
309+
$array = [1, 2, 3];
310+
311+
// This closure returns a value, but it's not in the immediate function scope
312+
$result = array_map(function($item) {
313+
return $item * 2; // This return should be ignored
314+
}, $array);
315+
316+
echo "Result: " . implode(', ', $result);
317+
}
318+
319+
/**
320+
* Function that returns void implicitly and has a nested anonymous function that returns a
321+
* value (should NOT be flagged for missing `@return` tag).
322+
*
323+
* The following should NOT be fixed:
324+
* - A `@return` tag should not be added for a `void` function that has a
325+
* nested returnable anonymous function.
326+
*/
327+
function testVoidFunctionWithAnonymousFunction()
328+
{
329+
$callback = function($value) {
330+
return strtoupper($value); // This return should be ignored
331+
};
332+
333+
$callback('test');
334+
}
335+
336+
/*************************************
337+
* VOID FUNCTIONS WITH @RETURN TESTS *
338+
*************************************/
339+
293340
/**
294341
* Function that returns void that has a `@return` tag (should be flagged).
295342
*

‎yCodeTech/Sniffs/Commenting/FunctionCommentSniff.php‎

Lines changed: 55 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,6 @@ public function register()
3939
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
4040
* @param int $stackPtr The position of the current token in the
4141
* stack passed in $tokens.
42-
*
43-
* @return void
4442
*/
4543
public function process(File $phpcsFile, $stackPtr)
4644
{
@@ -73,7 +71,7 @@ public function process(File $phpcsFile, $stackPtr)
7371
$stackPtr,
7472
true
7573
);
76-
74+
7775
if ($tokenAfterComment !== false && $tokenAfterComment !== $stackPtr) {
7876
// There are other tokens between the docblock and the function,
7977
// so this docblock doesn't belong to this function. So the function doesn't have a docblock, and found a previous function's docblock instead. Skipping.
@@ -116,7 +114,7 @@ public function process(File $phpcsFile, $stackPtr)
116114
break;
117115
}
118116
}
119-
117+
120118
if ($returnTagPtr !== false) {
121119
$error = 'Void function should not have @return tag';
122120
$fix = $phpcsFile->addFixableError($error, $returnTagPtr, 'VoidReturnTagFound');
@@ -191,6 +189,12 @@ private function hasImplicitVoidReturn(File $phpcsFile, $stackPtr)
191189
// Look for return statements within the function scope
192190
$returnPtr = $scopeOpener;
193191
while (($returnPtr = $phpcsFile->findNext(T_RETURN, $returnPtr + 1, $scopeCloser)) !== false) {
192+
// Check if this return statement is inside a nested function/closure
193+
if ($this->isReturnInNestedFunction($phpcsFile, $returnPtr, $scopeOpener, $scopeCloser)) {
194+
// Skip this return statement as it belongs to a nested function
195+
continue;
196+
}
197+
194198
// Check what follows the return statement (skip whitespace)
195199
$nextToken = $phpcsFile->findNext(T_WHITESPACE, $returnPtr + 1, null, true);
196200

@@ -208,6 +212,45 @@ private function hasImplicitVoidReturn(File $phpcsFile, $stackPtr)
208212
return true;
209213
}
210214

215+
/**
216+
* Check if a return statement is inside a nested function/closure.
217+
*
218+
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
219+
* @param int $returnPtr The position of the return statement.
220+
* @param int $functionScopeOpener The scope opener of the parent function.
221+
* @param int $functionScopeCloser The scope closer of the parent function.
222+
*
223+
* @return bool
224+
*/
225+
private function isReturnInNestedFunction(File $phpcsFile, $returnPtr, $functionScopeOpener, $functionScopeCloser)
226+
{
227+
$tokens = $phpcsFile->getTokens();
228+
229+
// Look backwards from the return statement to find any nested function/closure
230+
$searchPtr = $returnPtr;
231+
while ($searchPtr > $functionScopeOpener) {
232+
$searchPtr = $phpcsFile->findPrevious([T_FUNCTION, T_CLOSURE], $searchPtr - 1, $functionScopeOpener);
233+
234+
if ($searchPtr === false) {
235+
// No nested function found between return and parent function start
236+
return false;
237+
}
238+
239+
// Check if this nested function's scope contains our return statement
240+
$nestedScopeOpener = $tokens[$searchPtr]['scope_opener'] ?? null;
241+
$nestedScopeCloser = $tokens[$searchPtr]['scope_closer'] ?? null;
242+
243+
if ($nestedScopeOpener !== null && $nestedScopeCloser !== null) {
244+
if ($returnPtr > $nestedScopeOpener && $returnPtr < $nestedScopeCloser) {
245+
// The return statement is inside this nested function's scope
246+
return true;
247+
}
248+
}
249+
}
250+
251+
return false;
252+
}
253+
211254
/**
212255
* Check if function is a generator function (contains yield statements).
213256
*
@@ -245,8 +288,6 @@ private function isGeneratorFunction(File $phpcsFile, $stackPtr)
245288
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
246289
* @param int $commentEnd The position of the docblock closing tag.
247290
* @param string $returnType The return type to use.
248-
*
249-
* @return void
250291
*/
251292
private function addReturnTag(File $phpcsFile, $commentEnd, $returnType)
252293
{
@@ -315,29 +356,27 @@ private function addReturnTag(File $phpcsFile, $commentEnd, $returnType)
315356
*
316357
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
317358
* @param int $returnTagPtr The position of the @return tag.
318-
*
319-
* @return void
320359
*/
321360
private function removeReturnTag(File $phpcsFile, $returnTagPtr)
322361
{
323362
$tokens = $phpcsFile->getTokens();
324-
363+
325364
$phpcsFile->fixer->beginChangeset();
326-
365+
327366
// Find all tokens on the @return line
328367
$returnLine = $tokens[$returnTagPtr]['line'];
329368
$tokensToRemove = [];
330-
369+
331370
// Collect all tokens on the @return line
332371
for ($i = 0; $i < count($tokens); $i++) {
333372
if ($tokens[$i]['line'] === $returnLine) {
334373
$tokensToRemove[] = $i;
335374
}
336375
}
337-
376+
338377
// Check if there's an empty line before @return that should also be removed
339378
$prevLine = $returnLine - 1;
340-
379+
341380
// Look for tokens on the previous line to see if it's an empty docblock line
342381
for ($i = 0; $i < count($tokens); $i++) {
343382
if ($tokens[$i]['line'] === $prevLine) {
@@ -348,12 +387,12 @@ private function removeReturnTag(File $phpcsFile, $returnTagPtr)
348387
}
349388
}
350389
}
351-
390+
352391
// Remove all collected tokens
353392
foreach ($tokensToRemove as $tokenIndex) {
354393
$phpcsFile->fixer->replaceToken($tokenIndex, '');
355394
}
356-
395+
357396
$phpcsFile->fixer->endChangeset();
358397
}
359-
}
398+
}

0 commit comments

Comments
 (0)