Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 2 years ago

#38462 closed task (blessed) (fixed)

Fix identical hooks with differing parameters

Reported by: johnbillion's profile johnbillion Owned by:
Milestone: 4.9 Priority: normal
Severity: normal Version:
Component: General Keywords:
Focuses: docs Cc:

Description

Two recent instances where a hook that is triggered in multiple places in core has ended up having differing parameters:

It would be ideal to have unit test coverage which scans core's PHP files for all instances of do_action and apply_filters, filters hooks that occur multiple times, and then verifies that every instance of the hook accepts the same number of parameters.

Change History (16)

#1 @jdgrimes
7 years ago

The docs parser could probably do this easily, since it already scans for all of the hooks anyway. Maybe there is a way to integrate it into the tests process, but just to scan for the hooks?

#3 @johnbillion
7 years ago

Thanks for that @keesiemeijer! Some of the inconsistencies there are a complete mess.

#4 @keesiemeijer
7 years ago

Yeah, maybe this should be changed into a task instead of a feature request. Should I create a separate ticket for that?

Meanwhile I've found some more oddities that should be cleaned up.

This one (in a deprecated function) is missing a parameter in the database because it's not documented.
https://developer.wordpress.org/reference/hooks/the_content_rss/

These have differing parameter names in the DocBlock

hook function
the_commentsWP_Comment_Query::get_comments
the_sitesWP_Site_Query::get_sites
posts_joinWP_Query::get_posts
the_networksWP_Network_Query::get_networks

These have missing parameters in the DocBlock

hook function
get_avatarget_avatar
show_post_locked_dialog_admin_notice_post_locked
fallback_intermediate_image_sizeswp_generate_attachment_metadata

These have Concatenated dynamic hooks

hook function
the_author_{$field}the_author_meta
get_the_author_{$field}get_the_author_meta
customize_save_{$this->id_data[‘base’}]WP_Customize_Setting::save
{$field}sanitize_bookmark_field
{$field}sanitize_user_field
{$field}sanitize_post_field
default_option_{$option}update_option
default_option_{$option}add_option
uninstall_{$file}uninstall_plugin
{$type}_send_to_editor_urlwp_ajax_send_link_to_editor
{$option}WP_Screen::render_per_page_options
{$option}WP_List_Table::get_items_per_page
handle_bulk_actions-{current_screen_id}*
handle_network_bulk_actions-{current_screen_id}*
Last edited 7 years ago by keesiemeijer (previous) (diff)

#5 @keesiemeijer
7 years ago

I've created a ticket for the concatenated dynamic hooks here: #39148

#6 @johnbillion
7 years ago

#40636 was marked as a duplicate.

#7 @johnbillion
7 years ago

  • Component changed from Plugins to General
  • Focuses docs added
  • Keywords needs-patch added; needs-unit-tests removed
  • Milestone changed from Awaiting Review to Future Release
  • Summary changed from Unit test to detect identical hooks with differing parameters to Fix identical hooks with differing parameters
  • Type changed from feature request to task (blessed)

I think we can drop the idea of unit testing this for now, and concentrate on fixing the differing parameters.

#8 @johnbillion
7 years ago

In 41215:

Docs: Fix various filter documentation.

See #38462, #41017

#9 @johnbillion
7 years ago

In 41216:

Docs: Add a missing docblock for the the_content_rss filter.

See #38462, #41017

#10 @johnbillion
7 years ago

In 41217:

General: Add missing parameters to instances of the the_permalink filter.

This ensures every instance of this filter receives the same parameters.

Props keesiemeijer for identifying the issue

See #38462

#11 @johnbillion
7 years ago

In 41219:

General: Fix various instances of incorrect filter docs and incorrect filter and action parameters.

Props keesiemeijer for identifying the issues

See #38462

#12 @johnbillion
7 years ago

In 41220:

General: Fix a typo introduced in [41219].

See #38462

#13 @johnbillion
7 years ago

  • Milestone changed from Future Release to 4.9

#14 @johnbillion
7 years ago

In 41221:

General: Fix more instances of inconsistent parameters passed to various filters, plus fix some filter docs.

See #38462, #41017

#15 @johnbillion
7 years ago

  • Keywords needs-patch removed
  • Resolution set to fixed
  • Status changed from new to closed

Thanks again for those scans, @keesiemeijer . I've fixed all of the issues. The remaining issue is in #38057.

#16 @SergeyBiryukov
2 years ago

#40757 was marked as a duplicate.

Note: See TracTickets for help on using tickets.