Conversation
The sniff warned on every call to get_posts(), wp_get_recent_posts() and get_children() without reading the arguments, so code that already followed the advice in the message still got the warning. Calls where the args are a variable or the value is not a literal false still warn, as the sniff cannot know what they hold. Fixes Automattic#899.
GaryJones
requested changes
Oct 1, 2026
GaryJones
left a comment
GaryJones
left a comment
Contributor
There was a problem hiding this comment.
Thanks for picking this up, Tom, and for following #899 all the way through to a fix. It's good to finally have the sniff read the arguments rather than nagging on every call, and the tests for the awkward cases (duplicate keys, unpacking, nested keys) are a nice touch.
I've requested changes for one thing really: the exemption currently covers get_children() too, which quietly removes the no-LIMIT error that the VIPMinimum ruleset hangs off that error code (details inline). The warning message could do with a small tweak now as well.
Everything else is optional, so feel free to pick and choose or leave them for a follow-up. Thanks again for contributing!
Both rulesets reuse the get_posts_get_children code for the no-LIMIT query that get_children() runs by default. Exempting it removed that error under WordPressVIPMinimum, and the warning under WordPress-VIP-Go. The ruleset tests now cover the call, which shifts the expected line numbers after it.
The sniff now skips calls it can see set suppress_filters to false, so the warning only reaches calls where it could not tell. "This can be safely ignored" no longer fits those without saying when.
Three changes to how the sniff reads the args array: - The array has to be the whole argument, so `[ 'suppress_filters' => false ] ?: $args` warns again. - A lone `0` counts as well as `false`. WP_Query only checks that the value is falsy. - Unpacking is found by its token, without building a string from the whole item.
Contributor
Author
|
@GaryJones all addressed and ready for another review! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
get_posts(),wp_get_recent_posts()andget_children()got the "uncached unless suppress_filters is false" warning on every call, including calls that already pass'suppress_filters' => false. The sniff never read the arguments. It now skips the warning forget_posts()andwp_get_recent_posts()when the args array sets the key tofalseor0.get_children()still warns, because both rulesets use its error code for the no-LIMIT query it runs by default.It still warns when it cannot tell what the value is: args passed as a variable, a non-literal value, a query string, an array unpacked after the key, or an array that is only part of the argument.
In #899 I guessed the
phpcs:ignorecomment was breaking the array parsing. It was not. The same call warns without the comment.Fixes #899