Skip to content

Commit 0f4de98

Browse files
committed
feat: warn on exit/die/throw in filter callbacks
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.
1 parent af54d9b commit 0f4de98

File tree

3 files changed

+78
-3
lines changed

3 files changed

+78
-3
lines changed

WordPressVIPMinimum/Sniffs/Hooks/AlwaysReturnInFilterSniff.php

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -246,9 +246,15 @@ private function processFunctionBody( $stackPtr ) {
246246
}
247247

248248
if ( $outsideConditionalReturn === 0 ) {
249-
$message = 'Please, make sure that a callback to `%s` filter is always returning some value.';
250-
$data = [ $filterName ];
251-
$this->phpcsFile->addError( $message, $functionBodyScopeStart, 'MissingReturnStatement', $data );
249+
if ( $this->hasTerminatingStatement( $functionBodyScopeStart, $functionBodyScopeEnd ) ) {
250+
$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.';
251+
$data = [ $filterName ];
252+
$this->phpcsFile->addWarning( $message, $functionBodyScopeStart, 'TerminatingInsteadOfReturn', $data );
253+
} else {
254+
$message = 'Please, make sure that a callback to `%s` filter is always returning some value.';
255+
$data = [ $filterName ];
256+
$this->phpcsFile->addError( $message, $functionBodyScopeStart, 'MissingReturnStatement', $data );
257+
}
252258
}
253259
}
254260

@@ -288,6 +294,25 @@ private function isInsideIfConditonal( $stackPtr ) {
288294
return false;
289295
}
290296

297+
/**
298+
* Check whether the function body contains an exit, die, or throw statement.
299+
*
300+
* @param int $scopeStart The scope opener of the function body.
301+
* @param int $scopeEnd The scope closer of the function body.
302+
*
303+
* @return bool
304+
*/
305+
private function hasTerminatingStatement( $scopeStart, $scopeEnd ) {
306+
307+
$terminatingPtr = $this->phpcsFile->findNext(
308+
[ T_EXIT, T_THROW ],
309+
$scopeStart + 1,
310+
$scopeEnd
311+
);
312+
313+
return $terminatingPtr !== false;
314+
}
315+
291316
/**
292317
* Is the token returning void
293318
*

WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.inc

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,51 @@ add_filter( 'bad_void_return_with_space', function() { // Error.
197197
return ;
198198
} );
199199

200+
// === exit/die/throw in filter callbacks (issue #719) ===
201+
202+
// Exit used as unconditional termination path — warning, not error.
203+
function exit_in_filter( $redirect_url ) { // Warning.
204+
if ( some_condition() ) {
205+
return $redirect_url;
206+
}
207+
wp_redirect( $redirect_url );
208+
exit;
209+
}
210+
add_filter( 'redirect_canonical', 'exit_in_filter' );
211+
212+
// Die used as unconditional termination path — warning, not error.
213+
add_filter( 'die_in_filter', function( $url ) { // Warning.
214+
if ( some_condition() ) {
215+
return $url;
216+
}
217+
die( 'Terminated' );
218+
} );
219+
220+
// Throw used as unconditional termination path — warning, not error.
221+
class throw_in_filter_class {
222+
public function __construct() {
223+
add_filter( 'throw_in_filter', [ $this, 'validate' ] );
224+
}
225+
226+
public function validate( $value ) { // Warning.
227+
if ( is_valid( $value ) ) {
228+
return $value;
229+
}
230+
throw new \RuntimeException( 'Invalid value' );
231+
}
232+
}
233+
234+
// Exit with no return at all — warning, not error.
235+
add_filter( 'exit_no_return', function( $input ) { // Warning.
236+
do_something( $input );
237+
exit;
238+
} );
239+
240+
// No return and no exit/die/throw — remains an error.
241+
add_filter( 'still_missing_return', function( $input ) { // Error.
242+
do_something( $input );
243+
} );
244+
200245
// Intentional parse error. This has to be the last test in the file!
201246
class parse_error_test {
202247
public function __construct() {

WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ public function getErrorList() {
3232
163 => 1,
3333
188 => 1,
3434
196 => 1,
35+
241 => 1,
3536
];
3637
}
3738

@@ -43,6 +44,10 @@ public function getErrorList() {
4344
public function getWarningList() {
4445
return [
4546
180 => 1,
47+
203 => 1,
48+
213 => 1,
49+
226 => 1,
50+
235 => 1,
4651
];
4752
}
4853
}

0 commit comments

Comments
 (0)