Make WordPress Core

Opened 5 years ago

Closed 5 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 5 years ago.
true === WP_DEBUG
wp_debug_equal.patch (3.6 KB) - added by nacin 5 years ago.
true == WP_DEBUG
11090_2.patch (3.4 KB) - added by nacin 5 years ago.
Better patch for true == WP_DEBUG route
11090_3.patch (4.8 KB) - added by nacin 5 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)

@nacin5 years ago

true === WP_DEBUG

@nacin5 years ago

true == WP_DEBUG

comment:1 @scribu5 years ago

  • Milestone changed from Unassigned to 2.9

comment:2 @hakre5 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: @scribu5 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 @strider725 years ago

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


comment:5 @azaozz5 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.

@nacin5 years ago

Better patch for true == WP_DEBUG route

comment:6 @dd325 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 @westi5 years ago

  • Status changed from new to accepted

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

@nacin5 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 @nacin5 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 @automattor5 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.