Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filters should always return the first argument #229

Closed
jrfnl opened this issue Oct 7, 2020 · 1 comment · Fixed by #365
Closed

Filters should always return the first argument #229

jrfnl opened this issue Oct 7, 2020 · 1 comment · Fixed by #365
Milestone

Comments

@jrfnl
Copy link
Collaborator

jrfnl commented Oct 7, 2020

From https://yoast.slack.com/archives/C5SUKMF2T/p1595412782373600

Today is the second day in a row I've had to fix a bug in production due to incorrect usage of filters. When writing or reviewing code that involves actions or filters please always be sure about the following:

  1. Make sure the type of hook matches, never use add_action for filters or add_filter for actions. It may seem like it works on a simple test but it will lead to issues.
  2. A filter should always return the first argument if nothing needs to be done. Don't return the value originally passed into the filter as other plugins may have changed it. Always return the first argument.

All WordPress core actions and filters are documented and can easily be found on https://developer.wordpress.org/reference/hooks/. It's your responsibility to search for them when you write code that adds a new filter or action and it's your responsibility to do the same when code reviewing new additions.

Related:

Notes:

There is a sniff in the Automattic VIPCS which can check that functions which are hooked into filters always return something as long as the "hook-in" and the function being hooked are in the same file. The sniff is pretty buggy at this time, but I'm working on fixing that in the foreseeable future.

@jrfnl
Copy link
Collaborator Author

jrfnl commented Dec 14, 2023

Side-note: PHPStan is pretty darn good at detecting these type of issues and possibly the better tool for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant