Consistency of the _deprecated_function() calls and related documentation.

[jrf]

While working on ​updating the `WP.DeprecatedFunctions` sniff which is part of the ​WordPress Coding Standards project, we've come across a number of inconsistencies in the application of the function and/or the related documentation.

@jdgrimes previously already reported this concerning three functions in #41094 (has its own patch in that ticket), however since then a lot more inconsistencies have been identified.

In a series of patches which I will add to this ticket I will address these:

Patch 1: Fix deprecation info in various docblocks

• Fix incorrect deprecation comment naming the deprecated function as its own alternative
• Fix typo in docblock @deprecated tag which meant that deprecation is not indicated on the dev ref website
• Add missing @deprecated tags for two functions
• Add missing alternative in @deprecated comment for wp_get_sites()

Patch 2: Add missing replacement functions to various _deprecated_function() calls

• Add missing parentheses to a few deprecated function replacement function strings to be consistent with how this is done everywhere else.
• Add missing alternative for the WP_Customize Widget deprecated methods.
• Add missing alternative for three deprecated ms functions
• Add missing alternative for deprecated wp_cache_reset() function

Patch 3: Add missing _deprecated_function() function calls to various functions

• Add missing _deprecated_function() call for funky_javascript_callback()
• Add missing _deprecated_function() call for _save_post_hook().
• Add missing _deprecated_function() call for default_topic_count_text().
• Add missing _deprecated_function() call for screen_meta().
• Add missing _deprecated_function() call for two screen icon related functions
• Add missing _deprecated_function() call for nine wp_dashboard related functions
• Add missing _deprecated_function() call for wp_ajax_wp_fullscreen_save_post().
• Add missing _deprecated_function() call for ms_deprecated_blogs_file()

Patch 4: Add missing _deprecated_function() calls to various deprecated class methods

• Add _deprecated_function() calls to all methods within the deprecated WP_User_Search class.
• Add missing _deprecated_function() call for four methods in WP_Customize_Image_Control

Patch 5: Add missing _deprecated_function() calls for four deprecated pluggable functions.

Not all functions complied with what the docblock at the top of the files states:

Deprecated warnings are also thrown if one of these functions is being defined by a plugin.

[jrf]

Patch 1

[jrf]

Patch 2

[jrf]

Patch 3

[jrf]

Patch 4

[jrf]

Patch 5

[netweb]

Related: #41057 Update PHP codebase per WordPress PHP Coding Standards

Moving to 4.9 and 'switching to blessing task, this ticket is required as part of #41057 and is requires committing before #41057 can be committed.

[jdgrimes]

Note that some of these functions may intentionally not contain the _deprecated_function() call, for various reasons. I think I remember when screen_icon() was deprecated, in particular, that the decision was made not to add the notice. Might be worth investigating some of these to see if there was a reason that the _deprecated_function() call was omitted, and if it still holds.

[jrf]

this ticket is required as part of #41057 and is requires committing before #41057 can be committed.

Sorry, I need to correct you there @netweb.

This ticket is unrelated to ticket #41057. The sniff involved does not contain any auto-fixers, so will not impact the efforts for #41057.

Apart from that, we can update the sniff independently of whether or not this has been fixed in core yet, so these two tickets should really be seen independently of each other.

I have opened a related issue in WPCS to start detecting these type of inconsistencies automatically:
​https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/993
This is currently not (yet) done. Whether the new sniff which would be written for this, would contain any auto-fixers remains to be seen.

Independently of all that, the WP.Deprecated... sniffs are currently not part of the WPCS WordPress-Core ruleset. They are only included in the WordPress-Extra ruleset.

[jrf]

Related: #41125

[DrewAPicture]

In 40922:

Docs: Standardize and add missing deprecation notations in DocBlocks for the following functions:

• post_form_autocomplete_off()
• _rotate_image_resource()
• _flip_image_resource()
• wp_get_sites()
• deactivate_sitewide_plugin()

Props jrf.
See #41121.

[DrewAPicture]

@jrf I'm gonna leave the additions in class-wp-customizer-widgets.php for 0002-calls-missing-replacement-functions.patch​ out right now as I've asked for a second opinion on whether the quotes should be encoded or not.

They'll go in either way, just want to get a consensus – hard-escaping quotes is mostly a no-no in any kind of string that core passes around, in my experience. :-)

[jrf]

@DrewAPicture I've based my usage of escaped quotes on the precedent for this as set in numerous other function calls to _deprecated_function(), though there is no real consensus as current usage is inconsistent.

Escaped quotes:

• https://core.trac.wordpress.org/browser/trunk/src/wp-includes/deprecated.php#L1306 - line 1306 up to line 1620 (25 functions)
• https://core.trac.wordpress.org/browser/trunk/src/wp-includes/deprecated.php#L2976 - line 2976 up to line 3033 (4 functions)

Non-escaped quotes:

• https://core.trac.wordpress.org/browser/trunk/src/wp-includes/deprecated.php#L2386 (1 function)
• https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/deprecated.php#L921 - line 921 to line 960 ( 4 functions)
• https://core.trac.wordpress.org/browser/trunk/src/wp-includes/pluggable-deprecated.php#L67 - line 67 up to line 84 (2 functions)

[DrewAPicture]

I talked to a few people about it, and @SergeyBiryukov pointed out that if we just reference the hook name as the replacement, it'll output to the log as "Use 'customize_dynamic_setting_args' instead", which I agree is clear enough. I'll fix it on commit.

[jrf]

@DrewAPicture I changed it to explicitely mention the fact it is a filter to distinguish it from the expected/most common alternative being an alternative function.

The _deprecated_function() documentation even explicitly mentions that a function is expected as the replacement: ​https://developer.wordpress.org/reference/functions/_deprecated_function/

There are a few more exceptions where something else is used (not a function) and those are generally made more explicit as well.

See:

• https://core.trac.wordpress.org/browser/tags/4.8/src/wp-includes/deprecated.php#L2514
• https://core.trac.wordpress.org/browser/tags/4.8/src/wp-admin/includes/deprecated.php#L804
• https://core.trac.wordpress.org/browser/tags/4.8/src/wp-admin/includes/deprecated.php#L1141

[SergeyBiryukov]

Replying to jrf:

I've based my usage of escaped quotes on the precedent for this as set in numerous other function calls to _deprecated_function(), though there is no real consensus as current usage is inconsistent.

All of those cases are pieces of code, not strings. In this case, we're trying to slot some hardcoded English text into a translatable string, which seems less than ideal and doesn't have a precedent AFAIK.

With _deprecated_function( __METHOD__, '4.2.0', 'customize_dynamic_setting_args' ), the resulting message would include Use 'customize_dynamic_setting_args' instead, which seems clear enough. It's a developer notice, and developers should be able to figure out that it's a filter, not a function.

[SergeyBiryukov]

In this case, we're trying to slot some hardcoded English text into a translatable string, which seems less than ideal and doesn't have a precedent AFAIK.

We could probably do something like this, but it still seems unnecessary complicated to me:

_deprecated_function( __METHOD__, '4.2.0', sprintf(
	/* translators: %s: 'customize_dynamic_setting_args' */
	__( 'the %s filter' ),
	'customize_dynamic_setting_args'
) );

[DrewAPicture]

In 40929:

Improve the usefulness of several _deprecated_function() calls by passing known replacement functions, methods, or hooks.

Props jrf.
See #41121.

[DrewAPicture]

On 0003-missing-_deprecated_function-calls.patch​:

• Opened #41153 to address formally deprecating screen_icon() and get_screen_icon().
• Followed up on hard-deprecation of wp_ajax_wp_fullscreen_save_post() in 16:ticket:30949.
• Followed up on hard-deprecation of funky_javascript_callback() in 5:ticket:12520.
• Followed up on hard-deprecation of _save_post_hook() in 27:ticket:11399
• Followed up on hard-deprecation default_topic_count_text() in 3:ticket:27262.
• Followed up on hard-deprecation of all of the dashboard widget callbacks in 42:ticket:25824
• Followed up on hard-deprecation of ms_deprecated_blogs_file() in 67:ticket:19235

[DrewAPicture]

@westonruter Looking for feedback on "hard-deprecating" the prepare_controls(), add_tab(), remove_tab(), and print_tab_image() methods in `WP_Customize_Image_Control. See 0004-missing-_deprecated_function-calls-methods.patch​. They were deprecated in 4.1.

[DrewAPicture]

@boonebgorges Seeking feedback on hard-deprecation of individual methods within WP_User_Query, which was itself deprecated in 3.1.

Separately, #41125 has been opened, which would allow for drawing a distinction between the class itself being deprecated from its methods (though, realistically, if the class is deprecated, do we really need to also mark the individual methods as deprecated?). Interested in your perspective here. See 0004-missing-_deprecated_function-calls-methods.patch​.

[DrewAPicture]

@nacin Beside the individual functions I asked for follow up on in 0003, I'd be interested in your overall take on 0005-missing-_deprecated_function-calls-pluggable.patch​. I think ultimately the goal here is to introduce a sense of consistency in how we actually deprecate stuff.

[jrf]

@DrewAPicture

(though, realistically, if the class is deprecated, do we really need to also mark the individual methods as deprecated?).

IMHO, we should.

As the (deprecated) class is not marked as final, it could be extended and the parent methods could be used without calling the parent _construct().
A dev will in that case not be informed of the class deprecation as things stand at this moment.

<?php
class My_User_Search extends WP_User_Search {
        public function __construct() {
                // Set things up differently from the parent.
        }

        public function prepare_query() {
                parent::prepare_query();
                $this->query_where = 'something else';
        }
}

[westonruter]

Replying to DrewAPicture:

@westonruter Looking for feedback on "hard-deprecating" the prepare_controls(), add_tab(), remove_tab(), and print_tab_image() methods in `WP_Customize_Image_Control. See 0004-missing-_deprecated_function-calls-methods.patch​. They were deprecated in 4.1.

It makes sense to me. Note that the prepare_controls method is being called in \WP_Customize_Header_Image_Control::enqueue(), so that would need to be removed. It probably wouldn't cause issues for any plugins that extend WP_Customize_Header_Image_Control, which I hope there are none.

[DrewAPicture]

In 40938:

Scripts: Correctly reference WP_Scripts::print_extra_script() as the replacement for the deprecated WP_Scripts::print_scripts_l10n() method, rather than a nonexistent global print_extra_script() function.

Props fergbrain.
See #41121.

[DrewAPicture]

In 40949:

Docs: Notate format_for_editor() as the replacement function in the DocBlock for the deprecated wp_richedit_pre().

Props rabmalin.
See #41121. Fixes #40709.

[boonebgorges]

Replying to DrewAPicture:

@boonebgorges Seeking feedback on hard-deprecation of individual methods within WP_User_Query, which was itself deprecated in 3.1.

Separately, #41125 has been opened, which would allow for drawing a distinction between the class itself being deprecated from its methods (though, realistically, if the class is deprecated, do we really need to also mark the individual methods as deprecated?). Interested in your perspective here. See 0004-missing-_deprecated_function-calls-methods.patch​.

"Hard" (maybe better to say "explicit" or "complete"?) deprecation seems fine to me, and has the advantage of being consistently explicit. Can you say a bit more about why this feels redundant to you? Is it because of the additional lines of code
(the _deprecated_function() calls themselves) and the maintenance burden they may represent? Or is it because of some redundancy it'd introduce in the autogenerated documentation?

[DrewAPicture]

@boonebgorges I've changed my opinion on whether to also explicitly deprecate the methods in WP_User_Search. In terms of the _deprecated_function() calls, do you think we should try to match up method:method where possible or is it enough to just point to WP_User_Query as in 0004-missing-_deprecated_function-calls-methods.patch​?

[DrewAPicture]

In 41273:

Docs: Explicitly deprecate the add_tab(), remove_tab(), and print_tab_image() methods for WP_Customize_Image_Control, originally soft-deprecated in 4.1.

Props jrf.
See #41121.

[DrewAPicture]

@jrf Looking at 0005-missing-_deprecated_function-calls-pluggable.patch​, I think I'm actually going to push back on this one due to the nature of certain functions being considered "pluggable".

It's quite possible that we could introduce unintended consequences by globally deprecating these pluggable functions – as the core versions are only in play if not already defined.

I don't think it's unreasonable to believe that extensions have created versions of these functions in the wild that may possibly have nothing to do with the purposes they were originally intended to serve.

[jrf]

@DrewAPicture In that case, I believe the documentation at the top of the file should be adjusted to reflect that.

It currently states:

Deprecated warnings are also thrown if one of these functions is being defined by a plugin.

https://core.trac.wordpress.org/browser/trunk/src/wp-includes/pluggable-deprecated.php#L7

I would also like to raise a vote for consistency. Either have all of the deprecated pluggable functions throw a deprecation warning - even when redefined -. Or have none of them throw the deprecation warning when redefined. Not half/half like now.

[boonebgorges]

Replying to DrewAPicture:

@boonebgorges I've changed my opinion on whether to also explicitly deprecate the methods in WP_User_Search. In terms of the _deprecated_function() calls, do you think we should try to match up method:method where possible or is it enough to just point to WP_User_Query as in 0004-missing-_deprecated_function-calls-methods.patch​?

This looks perfect to me.

[westonruter]

This isn't getting traction during 4.9 beta, so punting for now.

[DrewAPicture]

Thanks @westonruter. Hopefully we can finish it up for 5.0.

[peterwilsoncc]

Switching milestone due to the focus on the new editor (Gutenberg) for WordPress 5.0.

[jrf]

Related #45657

[ocean90]

What's missing here?

[JeffPaul]

Per input from @jrf ​in Slack, we're going to punt this to 5.3 to work through remaining updates then.

[davidbaumwald]

@jrf @DrewAPicture Is this still being considered for 5.3? Anything you need to make that happen?

[jrf]

@davidbaumwald Let's punt this to 5.4, but also let's get it done in 5.4.

[davidbaumwald]

With 5.4 Beta 3 landing tomorrow, this is being moved to Future Release. If any maintainer or committer feels this can be resolved in time or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

[helen]

No updates in 3 months and we're into 5.6 beta 3 now, punting again :(

[desrosj]

#56589 was marked as a duplicate.

[Hareesh Pillai]

What's needed to take this forward?

Adds missing _deprecated_function() calls.

Trac ticket: https://core.trac.wordpress.org/ticket/41121

Looks like the tests are failing due to the new deprecation notices. The tests involved will need to be checked. If possible, the test shouldn't be using a deprecated function, but for those tests which are _about_ the deprecated functions, we may need a solution similar to what I'm suggesting here: ​https://github.com/WordPress/wordpress-develop/pull/4779#pullrequestreview-1509655091

Looks like the tests are failing due to the new deprecation notices. The tests involved will need to be checked. If possible, the test shouldn't be using a deprecated function, but for those tests which are _about_ the deprecated functions, we may need a solution similar to what I'm suggesting here: ​https://github.com/WordPress/wordpress-develop/pull/4779#pullrequestreview-1509655091