Skip to content

Commit b59ffbd

Browse files
committed
fix: @return void on abstract functions weren't being removed.
When an abstract function has a `@return void`, the return doesn't get removed but it should. To fix we need to explicitly check for abstract functions in the FunctionCommentSniff class. Remove it's return void tag if it has one, and ensure that it is skipped when processing the missing return tag violation. Added: - `abstractFunctionHasVoidReturnTag` method to check if an abstract function has a `@return void` tag in it's docblock. - `isAbstractFunctionWithoutExplicitReturn` method to check if this is an abstract function without an explicit return type. - Checks to the `process` and `hasVoidReturn` methods to determine whether to run the abstract function methods. - New test abstract function in both the `TestFile`s.
1 parent 371d67b commit b59ffbd

File tree

3 files changed

+138
-3
lines changed

3 files changed

+138
-3
lines changed

test_utils/TestFile.php

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
* @method mixed get($name) Description
3939
*/
4040

41-
class TestClass
41+
abstract class TestClass
4242
{
4343
/************************
4444
* CLASS PROPERTY TESTS *
@@ -383,6 +383,16 @@ public function testImplicitVoidFunctionWithDocblockReturn($message)
383383
echo $message;
384384
}
385385

386+
/**
387+
* Abstract function that returns void with a `@return void` tag (should be flagged).
388+
*
389+
* The following should be fixed:
390+
* - The `@return` tag should be removed.
391+
*
392+
* @return void
393+
*/
394+
abstract public function testAbstractVoidFunction();
395+
386396
/*****************************************
387397
* VOID MAGIC METHODS WITH @RETURN TESTS *
388398
*****************************************/

test_utils/TestFile_WithErrors_DoNotFix.php

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
* @method mixed get($name) Description
3939
*/
4040

41-
class TestClass
41+
abstract class TestClass
4242
{
4343
/************************
4444
* CLASS PROPERTY TESTS *
@@ -383,6 +383,16 @@ public function testImplicitVoidFunctionWithDocblockReturn($message)
383383
echo $message;
384384
}
385385

386+
/**
387+
* Abstract function that returns void with a `@return void` tag (should be flagged).
388+
*
389+
* The following should be fixed:
390+
* - The `@return` tag should be removed.
391+
*
392+
* @return void
393+
*/
394+
abstract public function testAbstractVoidFunction();
395+
386396
/*****************************************
387397
* VOID MAGIC METHODS WITH @RETURN TESTS *
388398
*****************************************/

yCodeTech/Sniffs/Commenting/FunctionCommentSniff.php

Lines changed: 116 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,12 @@ public function process(File $phpcsFile, $stackPtr)
9797
// If so, then it should have @return tag with type iterable.
9898
$isGeneratorFunction = $this->isGeneratorFunction($phpcsFile, $stackPtr);
9999

100+
// Check if this is an abstract function without explicit return type
101+
$isAbstractWithoutExplicitReturn = $this->isAbstractFunctionWithoutExplicitReturn($phpcsFile, $stackPtr);
102+
100103
// If function doesn't return void, it must have @return tag
101-
if ((!$hasVoidReturn || $isGeneratorFunction) && !$hasReturnTag) {
104+
// Skip this check for abstract functions without explicit return types
105+
if ((!$hasVoidReturn || $isGeneratorFunction) && !$hasReturnTag && !$isAbstractWithoutExplicitReturn) {
102106
$error = 'Missing @return tag in function comment';
103107
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'MissingReturn');
104108
if ($fix === true) {
@@ -165,6 +169,16 @@ private function hasVoidReturn(File $phpcsFile, $stackPtr)
165169
}
166170
}
167171

172+
// For abstract functions, check if they have @return void in their docblock
173+
// If so, treat them as void functions for the purpose of flagging unnecessary @return tags
174+
$scopeOpener = $tokens[$stackPtr]['scope_opener'] ?? null;
175+
$scopeCloser = $tokens[$stackPtr]['scope_closer'] ?? null;
176+
177+
if ($scopeOpener === null || $scopeCloser === null) {
178+
// This is an abstract function or interface method
179+
return $this->abstractFunctionHasVoidReturnTag($phpcsFile, $stackPtr);
180+
}
181+
168182
// Check if function implicitly returns void by analysing the function body
169183
return $this->hasImplicitVoidReturn($phpcsFile, $stackPtr);
170184
}
@@ -280,6 +294,107 @@ private function shouldProcessMagicMethod($functionName)
280294
return in_array($functionName, $voidMagicMethods, true);
281295
}
282296

297+
/**
298+
* Check if an abstract function has a @return void tag in its docblock.
299+
*
300+
* For abstract functions, if they have @return void in their docblock,
301+
* we should treat them as void functions for flagging purposes.
302+
*
303+
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
304+
* @param int $stackPtr The position of the function token.
305+
*
306+
* @return bool
307+
*/
308+
private function abstractFunctionHasVoidReturnTag(File $phpcsFile, $stackPtr)
309+
{
310+
$tokens = $phpcsFile->getTokens();
311+
312+
// Find the docblock for this function (reuse existing logic from process method)
313+
$commentEnd = $phpcsFile->findPrevious(T_DOC_COMMENT_CLOSE_TAG, ($stackPtr - 1));
314+
if ($commentEnd === false) {
315+
return false;
316+
}
317+
318+
// Check if this docblock actually belongs to this function
319+
$tokenAfterComment = $phpcsFile->findNext(
320+
[T_WHITESPACE, T_COMMENT, T_PUBLIC, T_PRIVATE, T_PROTECTED, T_STATIC, T_FINAL, T_ABSTRACT],
321+
($commentEnd + 1),
322+
$stackPtr,
323+
true
324+
);
325+
326+
if ($tokenAfterComment !== false && $tokenAfterComment !== $stackPtr) {
327+
return false;
328+
}
329+
330+
$commentStart = $tokens[$commentEnd]['comment_opener'];
331+
332+
// Look for @return void tag in the docblock
333+
for ($i = $commentStart; $i <= $commentEnd; $i++) {
334+
if ($tokens[$i]['code'] === T_DOC_COMMENT_TAG && $tokens[$i]['content'] === '@return') {
335+
// Check if the next token after @return is 'void'
336+
$nextToken = $phpcsFile->findNext([T_DOC_COMMENT_WHITESPACE], $i + 1, $commentEnd, true);
337+
if ($nextToken !== false && $tokens[$nextToken]['code'] === T_DOC_COMMENT_STRING) {
338+
$returnContent = trim($tokens[$nextToken]['content']);
339+
if (strpos($returnContent, 'void') === 0) {
340+
return true;
341+
}
342+
}
343+
}
344+
}
345+
346+
return false;
347+
}
348+
349+
/**
350+
* Check if this is an abstract function without explicit return type.
351+
*
352+
* Abstract functions without explicit return types should not be required
353+
* to have @return tags.
354+
*
355+
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
356+
* @param int $stackPtr The position of the function token.
357+
*
358+
* @return bool
359+
*/
360+
private function isAbstractFunctionWithoutExplicitReturn(File $phpcsFile, $stackPtr)
361+
{
362+
$tokens = $phpcsFile->getTokens();
363+
364+
// Check if this is an abstract function
365+
$scopeOpener = $tokens[$stackPtr]['scope_opener'] ?? null;
366+
$scopeCloser = $tokens[$stackPtr]['scope_closer'] ?? null;
367+
368+
if ($scopeOpener !== null || $scopeCloser !== null) {
369+
// This is not an abstract function (has a body)
370+
return false;
371+
}
372+
373+
// Check if it has an explicit return type
374+
$openParen = $phpcsFile->findNext(T_OPEN_PARENTHESIS, $stackPtr);
375+
if ($openParen === false) {
376+
return false;
377+
}
378+
379+
$closeParen = $tokens[$openParen]['parenthesis_closer'] ?? null;
380+
if ($closeParen === null) {
381+
return false;
382+
}
383+
384+
// Look for a colon (indicating return type) after the closing parenthesis
385+
$colonPtr = $phpcsFile->findNext(T_COLON, $closeParen + 1);
386+
387+
// If no colon found, or colon is after a semicolon, then no explicit return type
388+
$semicolonPtr = $phpcsFile->findNext(T_SEMICOLON, $closeParen + 1);
389+
390+
if ($colonPtr === false || ($semicolonPtr !== false && $colonPtr > $semicolonPtr)) {
391+
// No explicit return type
392+
return true;
393+
}
394+
395+
return false;
396+
}
397+
283398
/**
284399
* Check if function is a generator function (contains yield statements).
285400
*

0 commit comments

Comments
 (0)