Merge pull request #881 from Automattic/GaryJones/fix-always-return

Author: GaryJones

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

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() {

WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.php

@@ -31,6 +31,7 @@ public function getErrorList() {
 			129 => 1,
 			163 => 1,
 			188 => 1,
+			196 => 1,
 		];
 	}