Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#11090 closed defect (bug) (fixed)

Standardize checking for WP_DEBUG

Reported by: nacin Owned by: westi
Milestone: 2.9 Priority: normal
Severity: normal Version: 2.9
Component: Warnings/Notices Keywords:
Focuses: Cc:


The Codex recommends define('WP_DEBUG', true) or define('WP_DEBUG', false). However, WP_DEBUG is inconsistently checked whether should WP_DEBUG be identical or equal to those values.

Thus, define('WP_DEBUG', 1) will not turn all feedback on, and define('WP_DEBUG', 0) will not turn all feedback off.

Two patches attached, depending on whether it should be standardized to true == WP_DEBUG or true === WP_DEBUG.

Attachments (4)

wp_debug_identical.patch (4.9 KB) - added by nacin 6 years ago.
true === WP_DEBUG
wp_debug_equal.patch (3.6 KB) - added by nacin 6 years ago.
true == WP_DEBUG
11090_2.patch (3.4 KB) - added by nacin 6 years ago.
Better patch for true == WP_DEBUG route
11090_3.patch (4.8 KB) - added by nacin 6 years ago.
wp-settings.php will now set {{WP_DEBUG == false}}, making the {{defined(WP_DEBUG)}} check no longer needed in core or in plugins.

Download all attachments as: .zip

Change History (13)

@nacin6 years ago

true === WP_DEBUG

@nacin6 years ago

true == WP_DEBUG

comment:1 @scribu6 years ago

  • Milestone changed from Unassigned to 2.9

comment:2 @hakre6 years ago

the expression of true == WP_DEBUG is completely equal to the expression of WP_DEBUG. To keep things simple, I would prefer the later one. Same applies to false == WP_DEBUG which is equal to !WP_DEBUG.

comment:3 follow-up: @scribu6 years ago

Consider the case where WP_DEBUG is not defined at all:

var_dump(true == WP_DEBUG); // Result: bool(true)

I vote for '==='.

comment:4 @strider726 years ago

Per discussion on wp-hackers: The docs say that the values should be boolean, thus === is the way to go.


comment:5 @azaozz6 years ago

Don't see a good reason to explicitly forbid other values that evaluate to true. Also true == WP_DEBUG doesn't seem to have a purpose. Looking at other boolean constants, most are checked with:

if ( defined('CONSTANT') && CONSTANT )
// true
if ( !defined('CONSTANT') || !CONSTANT )
// false

which seems clear and concise. So if we are standardizing this perhaps that is the way to go.

@nacin6 years ago

Better patch for true == WP_DEBUG route

comment:6 @dd326 years ago

Is there any good reason for code like this:

 if ( ! defined('WP_DEBUG') || ( defined('WP_DEBUG') && !WP_DEBUG ) ) {

Which is exactly the same as:

 if ( ! defined('WP_DEBUG') || !WP_DEBUG ) {

I agree that adding == and === into it complicates things.

We could even add to wp-settings:

if ( ! defined('WP_DEBUG') ) define('WP_DEBUG', false);

if that makes it easier?

comment:7 @westi6 years ago

  • Status changed from new to accepted

I'll look into this and fix it later today :-)

@nacin6 years ago

wp-settings.php will now set {{WP_DEBUG == false}}, making the {{defined(WP_DEBUG)}} check no longer needed in core or in plugins.

comment:8 in reply to: ↑ 3 @nacin6 years ago

(Oops, poor Wiki formatting on my part.)

Thanks, westi.

New patch following dd32's logic, and to prevent an undefined constant from evaluating to true as scribu pointed out.

The only concern I might see with this route is that a plugin might be only checking for define( WP_DEBUG ) and not actually checking if ( WP_DEBUG ) as well.

This reminds me of #10602 as well, regarding WP_CACHE.

comment:9 @automattor6 years ago

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

(In [12207]) Ensure WP_DEBUG is always defined and simplify the checks on it. Fixes #11090 props nacin.

Note: See TracTickets for help on using tickets.