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

End restricting usage of term_exists() #720

Closed
Chouby opened this issue Aug 23, 2022 · 3 comments · Fixed by #812
Closed

End restricting usage of term_exists() #720

Chouby opened this issue Aug 23, 2022 · 3 comments · Fixed by #812

Comments

@Chouby
Copy link

Chouby commented Aug 23, 2022

What problem would the enhancement address for VIP?

Since WP 6.0.0, term_exists() uses get_terms() internally`. See https://core.trac.wordpress.org/ticket/36949. This means that reporting this function as not being cached is not true anymore.

Describe the solution you'd like

WordPressVIPMinimum coding standards could stop reporting terms_exists() as a restricted function.

@GaryJones
Copy link
Contributor

GaryJones commented Aug 23, 2022

Hi @Chouby - good catch!

Looking at the code here, VIPCS does mark the term_exists() function usage as an PHPCS Error.

We still have some VIP customers using WP < 6.0, so we wouldn't be able to remove the code entirely just yet, but we can certainly update the messaging to be more accurate, and/or use the MinimumWPVersionTrait from WPCS to adjust the violation accordingly.

Rough idea:

Adding the use trait statement to the class, and then adding to here, something like:

$this->get_wp_version_from_cli( $this->phpcsFile );

if ( version_compare( $this->minimum_supported_version, '6.0', '<' ) ) {
    unset( $groups['term_exists'] );
}

With the idea that if the minimum supported version (currently 5.1 by default) is below 6.0, then we still report about term_exists() usage.

@GaryJones GaryJones added this to the 3.x milestone Feb 16, 2024
GaryJones added a commit that referenced this issue Feb 16, 2024
Since WP 6.0.0, term_exists() uses get_terms() internally`. See core.trac.wordpress.org/ticket/36949. This means that reporting this function as not being cached is not true anymore.

Fixes #720.
rebeccahum pushed a commit that referenced this issue May 2, 2024
Since WP 6.0.0, term_exists() uses get_terms() internally`. See core.trac.wordpress.org/ticket/36949. This means that reporting this function as not being cached is not true anymore.

Fixes #720.
@benlk
Copy link

benlk commented May 22, 2024

It's good to see that this has been fixed in the upstream VIP PHPCS rules. How long until @wpcomvip-vipgoci-bot starts using the updated standards? It flagged a bunch of term_exists calls in a PR just today.

@GaryJones
Copy link
Contributor

https://github.com/Automattic/vip-go-ci/releases/tag/1.3.12 includes an update to use VIPCS 3.0.1, which is the version which incorporates #812.

This was made available on customer repositories yesterday.

If you're finding it's still getting flagged, please open a support ticket with VIP, so we can look at your specific code and the feedback on your private repo.

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.

3 participants