Allow `get_terms()` to return all terms, make `$taxonomy` parameter optional

[flixos90]

Now that term IDs are unique, we can continue work on making term functions more flexible. One thing I would consider useful would be to make the $taxonomy parameter of get_terms() optional, furthermore allowing to not specify any taxonomy, resulting in all terms of any taxonomy being returned.

I will upload an initial patch which provides this functionality. There might be areas in the function that require additional focus to apply this change, but in general it works with this approach.

[flixos90]

Initial patch allowing an empty $taxonomies parameter

[swissspidy]

I guess we can't just use function get_terms( $args = array() ) {…} and func_get_args() to maintain backwards compatibility? Would look more clean than get_terms( null, $args ).

[flixos90]

Replying to swissspidy:

I guess we can't just use function get_terms( $args = array() ) {…} and func_get_args() to maintain backwards compatibility? Would look more clean than get_terms( null, $args ).

+1 for this idea, it's definitely cleaner. So I assume that would also mean we would need to add a taxonomies key in the $args array. If a user uses both parameters, but still adds taxonomies in the $args array (makes no sense to me, but it would be possible), what takes precedence there? Or do you mean that the syntax with 1 parameters is only if you don't need a specific taxonomy while the syntax with 2 parameters should be used otherwise?

[boonebgorges]

I like @swissspidy's approach more. Could we see a patch for this? And unit tests that demonstrate the conflict between 'taxonomies' parameters? (For the record, I am not concerned about this; if anyone is passing an array of args to the first param, then they clearly intend for the second param to be ignored.)

[boonebgorges]

35495.2.diff​ is a first pass at get_terms( $args ). When a non-empty second param is passed to the function, or when an array is passed to the first param that doesn't look like an $args array, the function falls back on the old get_terms( $taxonomies, $args ) format.

I don't see the possibility for conflicts between legacy and new arg formats. If the second param is present, the first will be parsed as a taxonomy array. $legacy_args['taxonomy'] is effectively ignored. This should result in total backward compatibility for anyone who happens to be passing $args['taxonomy'] in an existing plugin.

What do people think? Especially regarding the documentation. (ping @DrewAPicture)

[swissspidy]

Thanks for the patch! I was already mentally preparing myself to dive into this, glad I don't have to now :-)

The logic looks good, though $args = array(), $legacy_args = '' sounds a bit confusing. Can't we just name this parameter $deprecated, so that it's clear you should really only use get_terms( $args )?

return new WP_Error( 'invalid_taxonomy', __( 'Invalid taxonomy' ) );

Is it possible to pass $taxonomy here, to make it obvious which taxonomy is invalid?

Also, could we perhaps use array_filter() for these taxonomy_exists() and is_taxonomy_hierarchical() checks? It should be just as fast and a bit cleaner to read.

---

Note: We may still change the milestone to 4.5 if we get a commit-ready patch

[flixos90]

@boonebgorges:
The patch looks good to me as well, 35495.3.diff​ fixes two issues I found when querying terms without specifying any taxonomy (a PHP notice thrown and an invalid SQL query).

@swissspidy:
Sounds like a good idea to me, this will provide more clarity, especially passing which taxonomy is invalid. Should I write another patch with your proposal included? I'm on it right now anyway :)

[swissspidy]

By the way, what about making the same changes to get_term()?

[boonebgorges]

Stripping the leading AND off the clause drives me nuts. Let's assemble the WHERE clause in get_terms() properly so that we don't have to do this. (We've already made the corresponding change in other query classes.)

[boonebgorges]

In 36598:

In get_terms(), assemble WHERE conditions in an array instead of concatenating.

This method is more reliable when adding new WHERE conditions.

See #35495.

[boonebgorges]

Is it possible to pass $taxonomy here, to make it obvious which taxonomy is invalid?

Yes, let's add this, in a separate changeset.

Also, could we perhaps use array_filter() for these taxonomy_exists() and is_taxonomy_hierarchical() checks? It should be just as fast and a bit cleaner to read.

OK, but I would want to verify that there's full test coverage before making changes like this. May be worth a separate ticket.

By the way, what about making the same changes to get_term()?

get_term() hasn't required the $taxonomy param since [34997].

Can't we just name this parameter $deprecated, so that it's clear you should really only use get_terms( $args )?

I didn't do this originally because I didn't want to throw a _deprecated_argument() notice. This would be extremely annoying for developers, especially this late in the cycle. (Maybe after 4.5 comes out.) But yes, I'm OK with the variable $deprecated.

35495.4.diff​ is an updated patch.

[TimothyBlynJacobs]

I don't think this should throw a deprecated argument for a few releases. For anyone who has to support less than 4.5 it will liter the codebase with WP version checks. Additionally, the prior syntax is more clean when doing a simple query for only one taxonomy.

[DrewAPicture]

Talked with @boonebgorges a bit about approach and I think this is realistically the best we're going to get in terms of documentation. It's definitely parameter gymnastics.

Initially I was thinking adding the $deprecated parameter might be overkill but I think we need to do it in case we ever decide to add a third parameter (yay future-proofing). In fact, I would even suggest adding an @internal comment explaining the presence of the second "dead" parameter.

[boonebgorges]

In 36614:

Allow get_terms() to fetch terms regardless of taxonomy.

get_terms() has historically required that a taxonomy be specified when
querying terms. This requirement is related to the fact that terms could
formerly be shared between taxonomies, making $taxonomies critical for
disambiguation. Since terms can no longer be shared as of 4.4, it'
s desirable to be able to query for terms regardless of what taxonomy they're in.

Because it's now optional to pass taxonomies, it's no longer necessary to have
$taxonomies as the first (and required) parameter for get_terms(). The new
function signature is get_terms( $args ), where 'taxonomy' can (optionally) be
passed as part of the $args array. This syntax is more consistent with
functions like get_users() and get_posts().

We've maintained backward compatibility by always giving precedence to the old
argument format. If a second parameter is detected, or if it's detected that
the first parameter is a list of taxonomy names rather than an $args array,
get_terms() will parse the function arguments in the legacy fashion.

Props flixos90, swissspidy, DrewAPicture, boonebgorges.
Fixes #35495.

[boonebgorges]

In 45888:

Taxonomy: Ensure consistency of hide_empty in term queries when taxonomy is excluded.

When querying for terms in hierarchical categories using hide_empty=true,
results have historically included parent terms which are themselves
unattached to any objects (are "empty") but which have non-empty descendent
terms. Because this process involves walking the descendant tree, we avoid it
when we detect that the queried taxonomies are not hierarchical. (This
behavior was introduced in [5525].)

When the taxonomy parameter of get_terms() was made optional - see #35495,
[36614] - it affected the mechanism for avoiding unneccessary tree walks,
since there may not be any explicitly declared taxonomies to run through
is_taxonomy_hierarchical(). As a result, term queries excluding taxonomy
did not check descendants, and empty parents with non-empty children were not
included in hide_empty results.

We correct the behavior by crawling term descendants when the taxonomy
argument is absent, which means that we're querying for terms in all taxonomies.

Props smerriman.
Fixes #37728.