WordPress.org

Make WordPress Core

Opened 10 months ago

Last modified 6 months ago

#45643 reviewing defect (bug)

PHPDoc errors in functions.php

Reported by: subrataemfluence Owned by: SergeyBiryukov
Milestone: Future Release Priority: normal
Severity: normal Version: 5.0.1
Component: General Keywords: has-patch needs-refresh
Focuses: docs Cc:
PR Number:

Description

File: wp-includes/functions.php

Issues:

PHPDoc: non-existing argument (line nos. 775, 776, 777)
PHPDoc: Unhandled Exception (line no. 3082)
PHPDoc: Argument type mismatch (line no. 3542)

Attachments (1)

45643.patch (1.7 KB) - added by subrataemfluence 10 months ago.
Proposed patch

Download all attachments as: .zip

Change History (9)

@subrataemfluence
10 months ago

Proposed patch

#1 @subrataemfluence
10 months ago

Also there are some areas where unused parameters are reported.
Is this a good idea to get rid of those?

Example: $title parameter is not used anywhere in the function

function _xmlrpc_wp_die_handler( $message, $title = '', $args = array() ) { ... }

#2 follow-up: @SergeyBiryukov
10 months ago

  • Focuses docs added; coding-standards removed
  • Milestone changed from Awaiting Review to 5.1
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

Thanks for the patch!

add_query_arg() arguments are correct, it uses func_get_args() to get them. The rest looks good.

In _xmlrpc_wp_die_handler(), even though the $title argument is not used, we cannot remove it. The function signature needs to remain consistent with _default_wp_die_handler() and _ajax_wp_die_handler().

#3 in reply to: ↑ 2 ; follow-up: @subrataemfluence
10 months ago

Replying to SergeyBiryukov:

add_query_arg() arguments are correct, it uses func_get_args() to get them

In this case, I think it might be more logical to mention how it actually gets the arguments. To me it looks a bit misleading to have them here without mentioning the actual source. May be not super important, but might be given a thought! :)

In _xmlrpc_wp_die_handler(), even though the $title argument is not used, we cannot remove it. The function signature needs to remain consistent with _default_wp_die_handler() and _ajax_wp_die_handler().

Understood!

Last edited 10 months ago by subrataemfluence (previous) (diff)

#4 @pento
9 months ago

  • Keywords needs-refresh added
  • Milestone changed from 5.1 to 5.2

#5 @johnbillion
7 months ago

Partially related: #37402

#6 in reply to: ↑ 3 @johnbillion
7 months ago

Replying to subrataemfluence:

In this case, I think it might be more logical to mention how it actually gets the arguments. To me it looks a bit misleading to have them here without mentioning the actual source. May be not super important, but might be given a thought! :)

The function's usage is documented in the docblock summary, but granted it's not clear how its parameters are populated just by looking at the docs. Suggestions for improvements to the summary or its parameter docs welcome.

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


6 months ago

#8 @audrasjb
6 months ago

  • Milestone changed from 5.2 to Future Release

As per today's bug scrub, we are going to move this one to Future Release since beta 3 and RC are approaching.

Note: See TracTickets for help on using tickets.