Skip to content

Warn instead of error for exit/die/throw in filter callbacks#883

Merged
GaryJones merged 1 commit intodevelopfrom
GaryJones/exit-die-throw-in-filter
Mar 22, 2026
Merged

Warn instead of error for exit/die/throw in filter callbacks#883
GaryJones merged 1 commit intodevelopfrom
GaryJones/exit-die-throw-in-filter

Conversation

@GaryJones
Copy link
Copy Markdown
Contributor

Summary

When a filter callback uses exit, die, or throw instead of returning a value, the AlwaysReturnInFilter sniff previously flagged it with the MissingReturnStatement error code. While technically correct — the function doesn't return a value to the filter chain — this is misleading in practice. Developers who use exit or throw in a filter callback have made a deliberate choice to terminate execution; they haven't accidentally forgotten a return.

The canonical example is a redirect filter that either returns the URL or calls wp_redirect() followed by exit. The current sniff treats this identically to a callback that simply forgot to return, which makes the error message unhelpful and forces developers to suppress a generic code that might mask genuine issues elsewhere.

This PR introduces a hasTerminatingStatement() check that scans the function body for T_EXIT (covering both exit and die) and T_THROW tokens. When the sniff finds no unconditional return but does find a terminating statement, it now emits a TerminatingInsteadOfReturn warning with a specific message, rather than the generic MissingReturnStatement error. Callbacks with genuinely missing returns (no terminating statements either) continue to receive the original error.

This gives users a distinct, suppressible code (WordPressVIPMinimum.Hooks.AlwaysReturnInFilter.TerminatingInsteadOfReturn) for cases where the pattern is intentional, without silently ignoring potentially problematic code.

Refs #719.

Test plan

  • composer test passes
  • Filter callbacks with exit/die/throw receive the TerminatingInsteadOfReturn warning (not error)
  • Filter callbacks with no return and no terminating statement still receive the MissingReturnStatement error
  • Existing test cases remain unaffected

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.
@GaryJones GaryJones requested a review from a team as a code owner March 22, 2026 17:04
@GaryJones GaryJones merged commit 3513402 into develop Mar 22, 2026
26 checks passed
@GaryJones GaryJones deleted the GaryJones/exit-die-throw-in-filter branch March 22, 2026 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant