Skip to content

Commit f10c972

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 ac9c5fc commit f10c972

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
@@ -10,6 +10,7 @@
1010
use PHP_CodeSniffer\Util\Tokens;
1111
use PHPCSUtils\Utils\Arrays;
1212
use PHPCSUtils\Utils\FunctionDeclarations;
13+
use PHPCSUtils\Utils\TextStrings;
1314
use WordPressVIPMinimum\Sniffs\Sniff;
1415

1516
/**
@@ -78,6 +79,9 @@ public function process_token( $stackPtr ) {
7879

7980
if ( $this->tokens[ $callbackPtr ]['code'] === 'PHPCS_T_CLOSURE' ) {
8081
$this->processFunctionBody( $callbackPtr );
82+
} elseif ( $this->tokens[ $callbackPtr ]['code'] === T_FN ) {
83+
// Arrow functions always return a value implicitly. No check needed.
84+
return;
8185
} elseif ( $this->tokens[ $callbackPtr ]['code'] === T_ARRAY
8286
|| $this->tokens[ $callbackPtr ]['code'] === T_OPEN_SHORT_ARRAY
8387
) {
@@ -131,7 +135,7 @@ private function processArray( $stackPtr ) {
131135
*/
132136
private function processString( $stackPtr, $start = 0, $end = null ) {
133137

134-
$callbackFunctionName = substr( $this->tokens[ $stackPtr ]['content'], 1, -1 );
138+
$callbackFunctionName = TextStrings::stripQuotes( $this->tokens[ $stackPtr ]['content'] );
135139

136140
$callbackFunctionPtr = $this->phpcsFile->findNext(
137141
T_STRING,
@@ -163,10 +167,10 @@ private function processFunction( $stackPtr, $start = 0, $end = null ) {
163167
$functionName = $this->tokens[ $stackPtr ]['content'];
164168

165169
$offset = $start;
166-
while ( $this->phpcsFile->findNext( [ T_FUNCTION ], $offset, $end ) !== false ) {
167-
$functionStackPtr = $this->phpcsFile->findNext( [ T_FUNCTION ], $offset, $end );
168-
$functionNamePtr = $this->phpcsFile->findNext( Tokens::$emptyTokens, $functionStackPtr + 1, null, true, null, true );
169-
if ( $this->tokens[ $functionNamePtr ]['code'] === T_STRING && $this->tokens[ $functionNamePtr ]['content'] === $functionName ) {
170+
// phpcs:ignore Generic.CodeAnalysis.AssignmentInCondition.FoundInWhileCondition -- Valid usage.
171+
while ( ( $functionStackPtr = $this->phpcsFile->findNext( T_FUNCTION, $offset, $end ) ) !== false ) {
172+
$declaredName = FunctionDeclarations::getName( $this->phpcsFile, $functionStackPtr );
173+
if ( $declaredName === $functionName ) {
170174
$this->processFunctionBody( $functionStackPtr );
171175
return;
172176
}
@@ -292,7 +296,7 @@ private function isInsideIfConditonal( $stackPtr ) {
292296
private function isReturningVoid( $stackPtr ) {
293297

294298
$nextToReturnTokenPtr = $this->phpcsFile->findNext(
295-
[ Tokens::$emptyTokens ],
299+
Tokens::$emptyTokens,
296300
$stackPtr + 1,
297301
null,
298302
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 {} // Ok (tokenizer bug PHPCS < 3.5.7) / Error in PHPCS >= 3.5.7.
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
@@ -32,6 +32,7 @@ public function getErrorList() {
3232
129 => 1,
3333
163 => 1,
3434
188 => version_compare( Config::VERSION, '3.5.7', '>=' ) ? 1 : 0,
35+
196 => 1,
3536
];
3637
}
3738

0 commit comments

Comments
 (0)