Deprecated notices should be classified as such.

[jrf]

All errors triggers should be classified with the appropriate error level.

Most notably, the _deprecated_function()/_deprecated_constructor()/_deprecated_file()/_deprecated_argument()/_doing_it_wrong() function do not pass an appropriate error level to trigger_error().

For the deprecated function group, the most appropriate level seems to be E_USER_DEPRECATED which was introduced in PHP 5.3.0.

For _doing_it_wrong() an E_USER_NOTICE (or E_USER_WARNING) seems more appropriate.

Fixed in the accompanying patch.

For backward compatibility with PHP 5.2, a define for E_USER_DEPRECATED has been added to wp-includes/compat.php which follows the same logic as used in SimplePie for consistency:
https://core.trac.wordpress.org/browser/trunk/src/wp-includes/class-simplepie.php#L699

[DrewAPicture]

Interesting. @jrf: Thanks for the patch.

I think we're going to need ​changelog entries added for the change in behavior when calling trigger_error() in these functions.

[jrf]

Added @since changelog tags

[jrf]

I think we're going to need changelog entries added for the change in behavior when calling trigger_error() in these functions.

@DrewAPicture Good point. Added for the deprecated functions. Not for the _doing_it_wrong() function as in effect the behaviour of that function does not change.
To be precise: no classification defaults to E_USER_NOTICE and the new classification is the same.
See: ​http://php.net/manual/en/function.trigger-error.php

For whomever is interested:
I've put in some work on - and will be sending in patches for - the Debug Bar plugin which will fix a number of issues with how the plugin logs and presents the error notices to devs.
This patch will not cause any conflicts in that respect and was in actual fact the result of my debugging efforts on the Debug Bar plugin.

For more info on that:
​https://wordpress.org/support/topic/where-does-development-of-this-plugin-take-place
​https://github.com/dd32/debug-bar/pulls and specifically: ​https://github.com/dd32/debug-bar/pull/17 and even more specifically: ​https://github.com/dd32/debug-bar/pull/17/commits/a4f5c64a742d8e44bd7d200d5f353d396210a3ee

[jrf]

What needs to be done to move this ticket forward ? (aside from rebasing it) @DrewAPicture ?

[DrewAPicture]

@jrf I apologize for the delay. Unfortunately at this point, we have once again missed the boat in getting this in pre-beta. So let's mark it for 4.8-early and be vigilant about jumping on it first thing.

[jrf]

As 4.8 is out, shall we try & get this in early for 4.9 ?

[swissspidy]

I'm not opposed to doing so.

[jrf]

Updated the patch + applied the same logic to the _deprecated_hook() function

[jrf]

@swissspidy Thanks for your response. I've updated the patch. Should be good to go now.

[jrf]

Oh and some love for the hook deprecation path in #10441 might also be nice for 4.9 ;-)

[jrf]

Just realized a minor issue with this patch. There are a couple of places in core where error_reporting() is called like so:

<?php
error_reporting( E_CORE_ERROR | E_CORE_WARNING | E_COMPILE_ERROR | E_ERROR | E_WARNING | E_PARSE | E_USER_ERROR | E_USER_WARNING | E_RECOVERABLE_ERROR );

These calls do not include the PHP 5.3 E_DEPRECATED and E_USER_DEPRECATED and at least two of these calls are made before the wp-includes/compat.php file is loaded.

These are the places where I found the troublesome error_reporting() calls:

• https://core.trac.wordpress.org/browser/trunk/src/wp-load.php#L24 (before compat file is loaded)
• https://core.trac.wordpress.org/browser/trunk/src/wp-includes/load.php#L333 (called from wp-settings.php before compat file is loaded)
• https://core.trac.wordpress.org/browser/trunk/src/wp-admin/plugins.php#L159
• https://core.trac.wordpress.org/browser/trunk/src/wp-admin/update.php#L87

Anyone got any bright ideas how to handle this ? Or should the backfill define(s) for the PHP 5.3 error constants just be moved to the top of the wp-load.php file and the above four calls adjusted to include them ?

[johnbillion]

I believe that the current error levels for deprecated calls, wrongdoings, etc is intentional. Note that the default error level used by error_log() is E_NOTICE.

We can't upgrade anything to an E_WARNING because they can become visible on the front end of a poorly configured server (eg one that has WP_DEBUG correctly set to false but which has display_errors enabled, and it'll also cause logs to fill up if they're configured to only log warnings.

If we downgrade the E_NOTICE triggered by deprecated calls to E_DEPRECATED, I'm wary of this change causing deprecated usage to go unnoticed when a server is configured to log notices but not deprecated notices. Is this a common configuration?

[jrf]

@johnbillion

I believe that the current error levels for deprecated calls, wrongdoings, etc is intentional. Note that the default error level used by error_log() is E_NOTICE.

I don't necessarily agree.
I think it's more a choice of convenience as E_(USER_)DEPRECATED only became available in PHP 5.3.

I also think that being explicit about the error level used would be better than relying on a silent default level.

In the above proposal and the associated patches:

• For doing it wrong, the error level doesn't change, it's just being made explicit.
• For deprecated notices, the error level does change to the more appropriate E_USER_DEPRECATED.

We can't upgrade anything to an E_WARNING ...

I'm not proposing to upgrade to E_WARNING anywhere as that would not be an appropriate choice anyway.

... because they can become visible on the front end of a poorly configured server (eg one that has WP_DEBUG correctly set to false but which has display_errors enabled, and it'll also cause logs to fill up if they're configured to only log warnings.

That has nothing to do with the error level chosen and is actually incorrect.

If display_errors is set to 1 (enabled) and WP_DEBUG is set to false, error will NOT display on the screen. Those messages will also NOT go into the error log either.

Both display_errors as well as log_errors respect the error_reporting setting and will not act on errors not covered by that.

Setting WP_DEBUG to false effectively sets error_reporting to E_CORE_ERROR | E_CORE_WARNING | E_COMPILE_ERROR | E_ERROR | E_WARNING | E_PARSE | E_USER_ERROR | E_USER_WARNING | E_RECOVERABLE_ERROR which doesn't include E_NOTICE or E_DEPRECATED

The PHP defaults are:
display_errors: 1 (i.e. display)
error_reporting: As stated in the PHP manual:

PHP 5.3 or later, the default value is E_ALL & ~E_NOTICE & ~E_STRICT & ~E_DEPRECATED. This setting does not show E_NOTICE, E_STRICT and E_DEPRECATED level errors. You may want to show them during development. Prior to PHP 5.3.0, the default value is E_ALL & ~E_NOTICE & ~E_STRICT. In PHP 4 the default value is E_ALL & ~E_NOTICE.

Ref: ​http://php.net/manual/en/errorfunc.configuration.php#ini.error-reporting

If we downgrade the E_NOTICE triggered by deprecated calls to E_DEPRECATED, I'm wary of this change causing deprecated usage to go unnoticed when a server is configured to log notices but not deprecated notices. Is this a common configuration?

I think the most common configuration is the default as quoted above which doesn't include notices nor deprecation errors, so effectively this change would not make a difference for the default case.

[jrf]

I would like to see if we can move this ticket forward. Probably too late for WP 5.3, but WP 5.4 early would be a good target IMO.

As WP 5.3 is no longer supported, we no longer need to be concerned over whether or not the PHP native E_USER_DEPRECATED (PHP 5.3) is available.

I will be uploading an updated patch momentarily.

[jrf]

Refreshed patch: removed PHP 5.3 compat changes; updated the version mentioned in the @since entries; fixed up the trigger_error() function calls to multi-line for improved readability.

[SergeyBiryukov]

In 46625:

Code Modernization: Pass an appropriate error level to trigger_error() in _doing_it_wrong() and related functions:

• _deprecated_function()
• _deprecated_argument()
• _deprecated_constructor()
• _deprecated_file()

The error level passed is E_USER_DEPRECATED for the deprecated function group and E_USER_NOTICE for _doing_it_wrong().

Props jrf.
Fixes #36561.