Make WordPress Core

Opened 7 years ago

Closed 5 years ago

#43345 closed defect (bug) (fixed)

term_exists() may return 0 which is not a text-documented return type

Reported by: dotancohen's profile dotancohen Owned by: johnbillion's profile johnbillion
Milestone: 5.4 Priority: low
Severity: normal Version:
Component: Taxonomy Keywords: needs-docs
Focuses: docs Cc:

Description

The PHPDoc for term_exists() reads as follows:

Returns null if the term does not exist. Returns the term ID if no taxonomy is specified and the term ID exists. Returns an array of the term ID and the term taxonomy ID the taxonomy is specified and the pairing exists.

Despite this, under some conditions ($term===0) the method return int 0. The attached patch rectifies this to return null, and additionally updates the @return tag datatype from mixed to string|array|null for better IDE support.

Attachments (3)

43345.diff (1.5 KB) - added by dotancohen 7 years ago.
Patch to resolve issue.
43345.1.patch (1.2 KB) - added by sathyapulse 5 years ago.
Adds documentation of the undocumented return 0.
43345.2.patch (1.2 KB) - added by sathyapulse 5 years ago.
Adds detailed documentation of the return 0.

Download all attachments as: .zip

Change History (16)

@dotancohen
7 years ago

Patch to resolve issue.

#1 @johnbillion
7 years ago

  • Keywords has-patch needs-testing added
  • Milestone changed from Awaiting Review to 5.0
  • Owner set to johnbillion
  • Status changed from new to reviewing
  • Type changed from enhancement to defect (bug)

#2 @johnbillion
6 years ago

  • Milestone changed from 5.0 to 5.1

#3 @pento
6 years ago

  • Keywords dev-feedback added
  • Milestone changed from 5.1 to 5.2

This patch needs to be reviewed for side effects of changing the return value.

#4 @johnbillion
6 years ago

  • Milestone changed from 5.2 to 5.3

#5 @johnbillion
5 years ago

I traced this condition back to [5739]. It hasn't been changed since its introduction.

#6 @johnbillion
5 years ago

  • Keywords dev-feedback removed

I've been poking around and this change doesn't seem to break anything. I suspect this can be treated as legacy code and the condition no longer gets triggered during post creation (both with the classic editor and the block editor).

Tentative +1 on the change but some more eyes would be good.

#7 @johnbillion
5 years ago

  • Version 4.9.4 deleted

#8 @johnbillion
5 years ago

Clarification: This condition was originally introduced to prevent a category with the name 0 being inserted when a post is saved with no categories selected (presumably due to the <input type="hidden" name="post_category[]" value="0"> in the Categories meta box.

#9 @jipmoors
5 years ago

Looking at this during a contributor day for the "Core Testing" tasks to be picked up.
Test instructions are very unclear, suggest to remove the "needs testing" tag from this issue.

Looks to me that having unit-tests for this change would be a much better way of ensuring functionality.

#10 @johnbillion
5 years ago

  • Keywords needs-docs added; has-patch needs-testing removed

I'm honestly not sure what to do with this change. It seems like something could unexpectedly break so I'm hesitant to change that return value. Let's just correct the docs instead.

#11 @johnbillion
5 years ago

  • Focuses docs added
  • Milestone changed from 5.3 to 5.4
  • Priority changed from normal to low

@sathyapulse
5 years ago

Adds documentation of the undocumented return 0.

@sathyapulse
5 years ago

Adds detailed documentation of the return 0.

#12 @donmhico
5 years ago

As @johnbillion mentioned, I also think that making the return 0 clear in the docs is good enough. There are quite a number of core's unit tests that asserts to 0 and there might be 3rd party plugins that are operating with the return 0.

#13 @johnbillion
5 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 47205:

Taxonomy: Clarify the docs for the return values of the term_exists() function.

Props dotancohen, sathyapulse

Fixes #43345

Note: See TracTickets for help on using tickets.