Skip to content

Commit 74ba5b6

Browse files
committed
fix: resolve bugs and adopt PHPCSUtils in AlwaysReturnInFilterSniff
The `isReturningVoid()` method wrapped `Tokens::$emptyTokens` in an extra array, preventing whitespace tokens from being skipped. This meant `return ;` (with a space) was not detected as a void return. Additional improvements: - Eliminate duplicate `findNext()` call in `processFunction()` - Use `FunctionDeclarations::getName()` for reliable name matching - Use `TextStrings::stripQuotes()` instead of `substr()` for stripping quotes from callback function names - Handle arrow function (`T_FN`) callbacks, which always return a value implicitly Ref #520
1 parent b19c94b commit 74ba5b6

File tree

3 files changed

+20
-6
lines changed

3 files changed

+20
-6
lines changed

WordPressVIPMinimum/Sniffs/Hooks/AlwaysReturnInFilterSniff.php

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use PHP_CodeSniffer\Util\Tokens;
1313
use PHPCSUtils\Utils\Arrays;
1414
use PHPCSUtils\Utils\FunctionDeclarations;
15+
use PHPCSUtils\Utils\TextStrings;
1516
use WordPressVIPMinimum\Sniffs\Sniff;
1617

1718
/**
@@ -80,6 +81,9 @@ public function process_token( $stackPtr ) {
8081

8182
if ( $this->tokens[ $callbackPtr ]['code'] === T_CLOSURE ) {
8283
$this->processFunctionBody( $callbackPtr );
84+
} elseif ( $this->tokens[ $callbackPtr ]['code'] === T_FN ) {
85+
// Arrow functions always return a value implicitly. No check needed.
86+
return;
8387
} elseif ( $this->tokens[ $callbackPtr ]['code'] === T_ARRAY
8488
|| $this->tokens[ $callbackPtr ]['code'] === T_OPEN_SHORT_ARRAY
8589
) {
@@ -133,7 +137,7 @@ private function processArray( $stackPtr ) {
133137
*/
134138
private function processString( $stackPtr, $start = 0, $end = null ) {
135139

136-
$callbackFunctionName = substr( $this->tokens[ $stackPtr ]['content'], 1, -1 );
140+
$callbackFunctionName = TextStrings::stripQuotes( $this->tokens[ $stackPtr ]['content'] );
137141

138142
$callbackFunctionPtr = $this->phpcsFile->findNext(
139143
T_STRING,
@@ -165,10 +169,10 @@ private function processFunction( $stackPtr, $start = 0, $end = null ) {
165169
$functionName = $this->tokens[ $stackPtr ]['content'];
166170

167171
$offset = $start;
168-
while ( $this->phpcsFile->findNext( [ T_FUNCTION ], $offset, $end ) !== false ) {
169-
$functionStackPtr = $this->phpcsFile->findNext( [ T_FUNCTION ], $offset, $end );
170-
$functionNamePtr = $this->phpcsFile->findNext( Tokens::$emptyTokens, $functionStackPtr + 1, null, true, null, true );
171-
if ( $this->tokens[ $functionNamePtr ]['code'] === T_STRING && $this->tokens[ $functionNamePtr ]['content'] === $functionName ) {
172+
// phpcs:ignore Generic.CodeAnalysis.AssignmentInCondition.FoundInWhileCondition -- Valid usage.
173+
while ( ( $functionStackPtr = $this->phpcsFile->findNext( T_FUNCTION, $offset, $end ) ) !== false ) {
174+
$declaredName = FunctionDeclarations::getName( $this->phpcsFile, $functionStackPtr );
175+
if ( $declaredName === $functionName ) {
172176
$this->processFunctionBody( $functionStackPtr );
173177
return;
174178
}
@@ -294,7 +298,7 @@ private function isInsideIfConditonal( $stackPtr ) {
294298
private function isReturningVoid( $stackPtr ) {
295299

296300
$nextToReturnTokenPtr = $this->phpcsFile->findNext(
297-
[ Tokens::$emptyTokens ],
301+
Tokens::$emptyTokens,
298302
$stackPtr + 1,
299303
null,
300304
true

WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.inc

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,15 @@ class tokenizer_bug_test {
188188
public function tokenizer_bug( $param ): namespace\Foo {} // Error.
189189
}
190190

191+
// Arrow function callbacks always return implicitly. Ok.
192+
add_filter( 'good_arrow_function', fn( $x ) => $x . ' suffix' );
193+
194+
add_filter( 'good_arrow_function_null', fn( $x ) => null ); // Ok, still returns a value (null).
195+
196+
add_filter( 'bad_void_return_with_space', function() { // Error.
197+
return ;
198+
} );
199+
191200
// Intentional parse error. This has to be the last test in the file!
192201
class parse_error_test {
193202
public function __construct() {

WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ public function getErrorList() {
3131
129 => 1,
3232
163 => 1,
3333
188 => 1,
34+
196 => 1,
3435
];
3536
}
3637

0 commit comments

Comments
 (0)