From 74ba5b62e381a81cdc6d83f8ec7af45ab3b1af03 Mon Sep 17 00:00:00 2001
From: Gary Jones <gary.jones@automattic.com>
Date: Sun, 22 Mar 2026 13:51:30 +0000
Subject: [PATCH] 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
---
 .../Sniffs/Hooks/AlwaysReturnInFilterSniff.php   | 16 ++++++++++------
 .../Tests/Hooks/AlwaysReturnInFilterUnitTest.inc |  9 +++++++++
 .../Tests/Hooks/AlwaysReturnInFilterUnitTest.php |  1 +
 3 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/WordPressVIPMinimum/Sniffs/Hooks/AlwaysReturnInFilterSniff.php b/WordPressVIPMinimum/Sniffs/Hooks/AlwaysReturnInFilterSniff.php
index 2cf1060c..4d45c9d3 100644
--- a/WordPressVIPMinimum/Sniffs/Hooks/AlwaysReturnInFilterSniff.php
+++ b/WordPressVIPMinimum/Sniffs/Hooks/AlwaysReturnInFilterSniff.php
@@ -12,6 +12,7 @@
 use PHP_CodeSniffer\Util\Tokens;
 use PHPCSUtils\Utils\Arrays;
 use PHPCSUtils\Utils\FunctionDeclarations;
+use PHPCSUtils\Utils\TextStrings;
 use WordPressVIPMinimum\Sniffs\Sniff;
 
 /**
@@ -80,6 +81,9 @@ public function process_token( $stackPtr ) {
 
 		if ( $this->tokens[ $callbackPtr ]['code'] === T_CLOSURE ) {
 			$this->processFunctionBody( $callbackPtr );
+		} elseif ( $this->tokens[ $callbackPtr ]['code'] === T_FN ) {
+			// Arrow functions always return a value implicitly. No check needed.
+			return;
 		} elseif ( $this->tokens[ $callbackPtr ]['code'] === T_ARRAY
 			|| $this->tokens[ $callbackPtr ]['code'] === T_OPEN_SHORT_ARRAY
 		) {
@@ -133,7 +137,7 @@ private function processArray( $stackPtr ) {
 	 */
 	private function processString( $stackPtr, $start = 0, $end = null ) {
 
-		$callbackFunctionName = substr( $this->tokens[ $stackPtr ]['content'], 1, -1 );
+		$callbackFunctionName = TextStrings::stripQuotes( $this->tokens[ $stackPtr ]['content'] );
 
 		$callbackFunctionPtr = $this->phpcsFile->findNext(
 			T_STRING,
@@ -165,10 +169,10 @@ private function processFunction( $stackPtr, $start = 0, $end = null ) {
 		$functionName = $this->tokens[ $stackPtr ]['content'];
 
 		$offset = $start;
-		while ( $this->phpcsFile->findNext( [ T_FUNCTION ], $offset, $end ) !== false ) {
-			$functionStackPtr = $this->phpcsFile->findNext( [ T_FUNCTION ], $offset, $end );
-			$functionNamePtr  = $this->phpcsFile->findNext( Tokens::$emptyTokens, $functionStackPtr + 1, null, true, null, true );
-			if ( $this->tokens[ $functionNamePtr ]['code'] === T_STRING && $this->tokens[ $functionNamePtr ]['content'] === $functionName ) {
+		// phpcs:ignore Generic.CodeAnalysis.AssignmentInCondition.FoundInWhileCondition -- Valid usage.
+		while ( ( $functionStackPtr = $this->phpcsFile->findNext( T_FUNCTION, $offset, $end ) ) !== false ) {
+			$declaredName = FunctionDeclarations::getName( $this->phpcsFile, $functionStackPtr );
+			if ( $declaredName === $functionName ) {
 				$this->processFunctionBody( $functionStackPtr );
 				return;
 			}
@@ -294,7 +298,7 @@ private function isInsideIfConditonal( $stackPtr ) {
 	private function isReturningVoid( $stackPtr ) {
 
 		$nextToReturnTokenPtr = $this->phpcsFile->findNext(
-			[ Tokens::$emptyTokens ],
+			Tokens::$emptyTokens,
 			$stackPtr + 1,
 			null,
 			true
diff --git a/WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.inc b/WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.inc
index 2748e074..0e9cc609 100644
--- a/WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.inc
+++ b/WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.inc
@@ -188,6 +188,15 @@ class tokenizer_bug_test {
 	public function tokenizer_bug( $param ): namespace\Foo {} // Error.
 }
 
+// Arrow function callbacks always return implicitly. Ok.
+add_filter( 'good_arrow_function', fn( $x ) => $x . ' suffix' );
+
+add_filter( 'good_arrow_function_null', fn( $x ) => null ); // Ok, still returns a value (null).
+
+add_filter( 'bad_void_return_with_space', function() { // Error.
+	return ;
+} );
+
 // Intentional parse error. This has to be the last test in the file!
 class parse_error_test {
 	public function __construct() {
diff --git a/WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.php b/WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.php
index fe022949..0c913191 100644
--- a/WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.php
+++ b/WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.php
@@ -31,6 +31,7 @@ public function getErrorList() {
 			129 => 1,
 			163 => 1,
 			188 => 1,
+			196 => 1,
 		];
 	}