Make WordPress Core

Opened 21 months ago

Closed 6 weeks ago

Last modified 6 weeks ago

#45486 closed defect (bug) (fixed)

Docs: Correct the inline documentation for rest_sanitize_value_from_schema()

Reported by: birgire Owned by: SergeyBiryukov
Milestone: 5.5 Priority: normal
Severity: normal Version: 4.7
Component: REST API Keywords: good-first-bug has-patch
Focuses: docs Cc:


According to the inline documentation for rest_sanitize_value_from_schema() it can return true or WP_Error:

* @return true|WP_Error

See here:


But it looks like it can return an array, boolean, string, int and float.

See e.g.


It would also be informative to check if WP_Error can really be returned. It's not obvious from the function's code, but could be implicitly returned from the other function used there. But I haven't checked them all, so I'm not convinced yet :-) Looking at the unit tests in WP_Test_REST_Schema_Sanitization there are no tests for WP_Error.

So it seems that the inline documentation for @return is accidentally the same for rest_sanitize_value_from_schema() and rest_validate_value_from_schema():


So the inline documentation needs to be corrected for rest_sanitize_value_from_schema().

Attachments (4)

patch-45486.diff (509 bytes) - added by Jean-David 21 months ago.
Fix inline documentation
45486.patch (539 bytes) - added by mukesh27 21 months ago.
Updated patch.
patch-45486.2.diff (1.1 KB) - added by Jean-David 20 months ago.
Not sure of the returns formatting.
454836.3.diff (946 bytes) - added by priyankkpatel 15 months ago.
Patch for wp-includes/rest-api.php

Download all attachments as: .zip

Change History (18)

21 months ago

Fix inline documentation

#1 @Jean-David
21 months ago

First timer here, I hope I did everything right.

Actually, none of the code in the function returns a WP_Error; the rest_sanitize_boolean and sanitize_text_field don't.

21 months ago

Updated patch.

#2 @birgire
21 months ago

Thanks for the patches @Jean-David and @mukesh27

I've also seen @return mixed ... being used in core.

According to


the primitive types in PHPDoc are: string, int, float, bool, array, resource, null, callable.

And regarding the mixed keyword it says:

The PHPDoc Standard also describes several keywords that are not native to PHP but are found to be often used or are representations of situations that are convenient to describe.

A value with this type can be literally anything; the author of the documentation is unable to predict which type it will be.

I'm not sure how strict the definition of mixed is in core, if it means that it must be "literally anything", i.e. all the primitive types and keywords, and that the author must be unable to predict the type?

We have the following on @return in the Core Handbook:


@return: Should contain all possible return types, and a description for each. Use a period at the end. 
Note: @return void should not be used outside of the default bundled themes.

So according to this it seems we are on the right path regarding the patches, but it seems we need to update it with a description for each type.

@mukesh27, @Jean-David What do you think?

#3 @SergeyBiryukov
21 months ago

  • Component changed from General to REST API
  • Milestone changed from Awaiting Review to 5.1
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

20 months ago

Not sure of the returns formatting.

#4 @Jean-David
20 months ago

Actually, the last line of this function returns the original $value passed in first parameters.
@birgire In this case, is it considered mixed type as we don't know the value passed in parameters ?

This ticket was mentioned in Slack in #core-restapi by desrosj. View the logs.

19 months ago

#6 @pento
19 months ago

  • Type changed from defect (bug) to task (blessed)

Converting to task, as it's only a docs change.

#7 @pento
18 months ago

  • Milestone changed from 5.1 to 5.2

There's still some discussion on the right way to do this. Moving to 5.2.

This ticket was mentioned in Slack in #core-restapi by kadamwhite. View the logs.

17 months ago

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.

16 months ago

#10 @JeffPaul
16 months ago

  • Milestone changed from 5.2 to Future Release

Per today's bugscrub, we're punting this as not enough time remains to get this into 5.2. Given the minimal movement during the 5.2 release cycle, I'm punting this to Future Release so that we consciously milestone this for a numbered release when we're ready to take it on.

15 months ago

Patch for wp-includes/rest-api.php

#11 @splitti
14 months ago

  • Keywords has-patch added; needs-patch removed

#12 @TimothyBlynJacobs
3 months ago

I think this makes more sense as mixed. As @Jean-David points out, the input is returned as a fall through, so mixed is the most accurate.

Additionally, the docblock is meant to provide value, and I don't think documenting every possible return type separately makes the documentation anymore valuable to the reader.

#13 @TimothyBlynJacobs
6 weeks ago

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

In 48307:

REST API: Correct the return type of rest_sanitize_value_from_schema.

Fixes #45486.
Props birgire, Jean-David, mukesh27, priyankkpatel.

#14 @TimothyBlynJacobs
6 weeks ago

  • Milestone changed from Future Release to 5.5
  • Type changed from task (blessed) to defect (bug)
Note: See TracTickets for help on using tickets.