Skip to content

Commit a915378

Browse files
committed
fix: void magic methods should be flagged if they have @return tag.
If void magic methods have a `@return` tag, they weren't being flagged because all magic methods were being skipped. Fixed by introducing a list of void magic methods which allows them to be processed, while all other magic methods will be skipped. Added: - Added `shouldProcessMagicMethod` method to define the array of void magic methods, and determine if the current function is within the array. - Added new test functions in both `TestFile`s. Changed: - Changed the `VoidReturnTagFound` violation error message to include the function name too. - Changed readme to document the void magic methods.
1 parent a7bf639 commit a915378

File tree

4 files changed

+124
-12
lines changed

4 files changed

+124
-12
lines changed

README.md

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -210,14 +210,25 @@ Functions that return a value must have a `@return` docblock tag (nested anonymo
210210
<td>Functions with <code>void</code> return types must NOT have <code>@return</code> tags, except generator functions.
211211
</td>
212212
<td>✔️</td>
213-
<td></td>
214-
</tr>
215-
<tr>
216-
<td>Generator functions must have a <code>@return</code> tag.
217-
</td>
218-
<td>✔️</td>
219213
<td>
220214

215+
- Most magic methods are exempt, except for those that return `void`:
216+
<code>\_\_construct</code>,
217+
<code>\_\_destruct</code>,
218+
<code>\_\_clone</code>,
219+
<code>\_\_set</code>,
220+
<code>\_\_unset</code>,
221+
<code>\_\_wakeup</code>, and
222+
<code>\_\_unserialize</code>.
223+
224+
</td>
225+
</tr>
226+
<tr>
227+
<td>Generator functions must have a <code>@return</code> tag.
228+
</td>
229+
<td>✔️</td>
230+
<td>
231+
221232
- Fixes with an <code>iterable</code> return type.
222233

223234
</td>

test_utils/TestFile.php

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,42 @@ public function testImplicitVoidFunctionWithDocblockReturn($message)
365365
echo $message;
366366
}
367367

368+
/*****************************************
369+
* VOID MAGIC METHODS WITH @RETURN TESTS *
370+
*****************************************/
371+
372+
/**
373+
* Construct magic method with unnecessary `@return void` tag (should be flagged).
374+
*
375+
* The following should be fixed:
376+
* - The `@return` tag should be removed.
377+
*
378+
* @return void
379+
*/
380+
public function __construct() {
381+
}
382+
383+
/**
384+
* Destruct magic method with unnecessary `@return void` tag (should be flagged).
385+
*
386+
* The following should be fixed:
387+
* - The `@return` tag should be removed.
388+
*
389+
* @return void
390+
*/
391+
public function __destruct() {
392+
}
393+
394+
/**
395+
* Set magic method with unnecessary `@return void` tag (should be flagged).
396+
*
397+
* The following should be fixed:
398+
* - The `@return` tag should be removed.
399+
*
400+
* @return void
401+
*/
402+
public function __set($name, $value) {
403+
}
368404

369405
/************************************
370406
* ALL OTHER @ TAGS FORMATTING TEST *

test_utils/TestFile_WithErrors_DoNotFix.php

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,42 @@ public function testImplicitVoidFunctionWithDocblockReturn($message)
365365
echo $message;
366366
}
367367

368+
/*****************************************
369+
* VOID MAGIC METHODS WITH @RETURN TESTS *
370+
*****************************************/
371+
372+
/**
373+
* Construct magic method with unnecessary `@return void` tag (should be flagged).
374+
*
375+
* The following should be fixed:
376+
* - The `@return` tag should be removed.
377+
*
378+
* @return void
379+
*/
380+
public function __construct() {
381+
}
382+
383+
/**
384+
* Destruct magic method with unnecessary `@return void` tag (should be flagged).
385+
*
386+
* The following should be fixed:
387+
* - The `@return` tag should be removed.
388+
*
389+
* @return void
390+
*/
391+
public function __destruct() {
392+
}
393+
394+
/**
395+
* Set magic method with unnecessary `@return void` tag (should be flagged).
396+
*
397+
* The following should be fixed:
398+
* - The `@return` tag should be removed.
399+
*
400+
* @return void
401+
*/
402+
public function __set($name, $value) {
403+
}
368404

369405
/************************************
370406
* ALL OTHER @ TAGS FORMATTING TEST *

yCodeTech/Sniffs/Commenting/FunctionCommentSniff.php

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,9 @@ public function process(File $phpcsFile, $stackPtr)
5050
return;
5151
}
5252

53-
// Skip magic methods and constructors/destructors
53+
// Get the function name for later processing
5454
$functionName = $tokens[$namePtr]['content'];
55-
if (substr($functionName, 0, 2) === '__') {
56-
return;
57-
}
55+
$isMagicMethod = substr($functionName, 0, 2) === '__';
5856

5957
// Find the docblock for this function
6058
$commentEnd = $phpcsFile->findPrevious(T_DOC_COMMENT_CLOSE_TAG, ($stackPtr - 1));
@@ -90,6 +88,11 @@ public function process(File $phpcsFile, $stackPtr)
9088
}
9189
}
9290

91+
// Skip magic methods unless they are void magic methods that should be processed
92+
if ($isMagicMethod && !$this->shouldProcessMagicMethod($functionName)) {
93+
return;
94+
}
95+
9396
// Check if the function is a generator.
9497
// If so, then it should have @return tag with type iterable.
9598
$isGeneratorFunction = $this->isGeneratorFunction($phpcsFile, $stackPtr);
@@ -116,8 +119,8 @@ public function process(File $phpcsFile, $stackPtr)
116119
}
117120

118121
if ($returnTagPtr !== false) {
119-
$error = 'Void function should not have @return tag';
120-
$fix = $phpcsFile->addFixableError($error, $returnTagPtr, 'VoidReturnTagFound');
122+
$error = 'Void function (%s) should not have @return tag';
123+
$fix = $phpcsFile->addFixableError($error, $returnTagPtr, 'VoidReturnTagFound', [$functionName]);
121124
if ($fix === true) {
122125
$this->removeReturnTag($phpcsFile, $returnTagPtr);
123126
}
@@ -251,6 +254,32 @@ private function isReturnInNestedFunction(File $phpcsFile, $returnPtr, $function
251254
return false;
252255
}
253256

257+
/**
258+
* Check if a magic method should be processed.
259+
*
260+
* Only void magic methods should be processed to flag unnecessary @return void tags.
261+
* Magic methods that return values should be skipped entirely.
262+
*
263+
* @param string $functionName The name of the function.
264+
*
265+
* @return bool
266+
*/
267+
private function shouldProcessMagicMethod($functionName)
268+
{
269+
// Magic methods that are typically void and should be processed for @return void violations
270+
$voidMagicMethods = [
271+
'__construct',
272+
'__destruct',
273+
'__clone',
274+
'__set',
275+
'__unset',
276+
'__wakeup',
277+
'__unserialize',
278+
];
279+
280+
return in_array($functionName, $voidMagicMethods, true);
281+
}
282+
254283
/**
255284
* Check if function is a generator function (contains yield statements).
256285
*

0 commit comments

Comments
 (0)