From 0f4de98334f47ec8a19a86292902ff848d65f0b8 Mon Sep 17 00:00:00 2001
From: Gary Jones <gary.jones@automattic.com>
Date: Sun, 22 Mar 2026 16:58:53 +0000
Subject: [PATCH] feat: warn on exit/die/throw in filter callbacks
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When a filter callback uses exit, die, or throw instead of returning a
value, the sniff previously flagged it as MissingReturnStatement. This
is technically correct but misleading — the developer has intentionally
chosen to terminate execution rather than accidentally forgetting a
return.

The sniff now distinguishes between these cases: callbacks containing
terminating statements receive a TerminatingInsteadOfReturn warning
with a more specific message, while genuinely missing returns remain
errors. This gives users a distinct error code to suppress when the
pattern is intentional, without silently ignoring problematic code.

Refs #719.
---
 .../Hooks/AlwaysReturnInFilterSniff.php       | 31 +++++++++++--
 .../Hooks/AlwaysReturnInFilterUnitTest.inc    | 45 +++++++++++++++++++
 .../Hooks/AlwaysReturnInFilterUnitTest.php    |  5 +++
 3 files changed, 78 insertions(+), 3 deletions(-)

diff --git a/WordPressVIPMinimum/Sniffs/Hooks/AlwaysReturnInFilterSniff.php b/WordPressVIPMinimum/Sniffs/Hooks/AlwaysReturnInFilterSniff.php
index 4d45c9d3..dea2730b 100644
--- a/WordPressVIPMinimum/Sniffs/Hooks/AlwaysReturnInFilterSniff.php
+++ b/WordPressVIPMinimum/Sniffs/Hooks/AlwaysReturnInFilterSniff.php
@@ -246,9 +246,15 @@ private function processFunctionBody( $stackPtr ) {
 		}
 
 		if ( $outsideConditionalReturn === 0 ) {
-			$message = 'Please, make sure that a callback to `%s` filter is always returning some value.';
-			$data    = [ $filterName ];
-			$this->phpcsFile->addError( $message, $functionBodyScopeStart, 'MissingReturnStatement', $data );
+			if ( $this->hasTerminatingStatement( $functionBodyScopeStart, $functionBodyScopeEnd ) ) {
+				$message = 'The callback for the `%s` filter uses a terminating statement (`exit`, `die`, or `throw`) instead of returning a value. Filter callbacks should always return a value.';
+				$data    = [ $filterName ];
+				$this->phpcsFile->addWarning( $message, $functionBodyScopeStart, 'TerminatingInsteadOfReturn', $data );
+			} else {
+				$message = 'Please, make sure that a callback to `%s` filter is always returning some value.';
+				$data    = [ $filterName ];
+				$this->phpcsFile->addError( $message, $functionBodyScopeStart, 'MissingReturnStatement', $data );
+			}
 		}
 	}
 
@@ -288,6 +294,25 @@ private function isInsideIfConditonal( $stackPtr ) {
 		return false;
 	}
 
+	/**
+	 * Check whether the function body contains an exit, die, or throw statement.
+	 *
+	 * @param int $scopeStart The scope opener of the function body.
+	 * @param int $scopeEnd   The scope closer of the function body.
+	 *
+	 * @return bool
+	 */
+	private function hasTerminatingStatement( $scopeStart, $scopeEnd ) {
+
+		$terminatingPtr = $this->phpcsFile->findNext(
+			[ T_EXIT, T_THROW ],
+			$scopeStart + 1,
+			$scopeEnd
+		);
+
+		return $terminatingPtr !== false;
+	}
+
 	/**
 	 * Is the token returning void
 	 *
diff --git a/WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.inc b/WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.inc
index 0e9cc609..086b0eab 100644
--- a/WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.inc
+++ b/WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.inc
@@ -197,6 +197,51 @@ add_filter( 'bad_void_return_with_space', function() { // Error.
 	return ;
 } );
 
+// === exit/die/throw in filter callbacks (issue #719) ===
+
+// Exit used as unconditional termination path — warning, not error.
+function exit_in_filter( $redirect_url ) { // Warning.
+	if ( some_condition() ) {
+		return $redirect_url;
+	}
+	wp_redirect( $redirect_url );
+	exit;
+}
+add_filter( 'redirect_canonical', 'exit_in_filter' );
+
+// Die used as unconditional termination path — warning, not error.
+add_filter( 'die_in_filter', function( $url ) { // Warning.
+	if ( some_condition() ) {
+		return $url;
+	}
+	die( 'Terminated' );
+} );
+
+// Throw used as unconditional termination path — warning, not error.
+class throw_in_filter_class {
+	public function __construct() {
+		add_filter( 'throw_in_filter', [ $this, 'validate' ] );
+	}
+
+	public function validate( $value ) { // Warning.
+		if ( is_valid( $value ) ) {
+			return $value;
+		}
+		throw new \RuntimeException( 'Invalid value' );
+	}
+}
+
+// Exit with no return at all — warning, not error.
+add_filter( 'exit_no_return', function( $input ) { // Warning.
+	do_something( $input );
+	exit;
+} );
+
+// No return and no exit/die/throw — remains an error.
+add_filter( 'still_missing_return', function( $input ) { // Error.
+	do_something( $input );
+} );
+
 // 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 0c913191..9201985f 100644
--- a/WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.php
+++ b/WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.php
@@ -32,6 +32,7 @@ public function getErrorList() {
 			163 => 1,
 			188 => 1,
 			196 => 1,
+			241 => 1,
 		];
 	}
 
@@ -43,6 +44,10 @@ public function getErrorList() {
 	public function getWarningList() {
 		return [
 			180 => 1,
+			203 => 1,
+			213 => 1,
+			226 => 1,
+			235 => 1,
 		];
 	}
 }