Deprecated PHP4 style constructor warnings make debugging hard

[McGuive7]

At present, the PHP 4 style constructor deprecation warning messages read as follows for widgets:

The called constructor method for WP_Widget is deprecated

This indicates that a widget is causing the issue, however gives no indication as to which widget is using the PHP4 style constructor. What might be more helpful is indicate the specific widget that is throwing the notice, like so:

The called constructor method for {Widget class that is extending WP_Widget} is deprecated

This can be easily accomplished by subbing out the static text used in the _deprecated_constructor() call (currently "WP_Widget"), for the actual name of the widget class that is extending WP_Widget.

A quick search for _deprecated_constructor across all files reveals that it is only used in one other instance, in the WP_Widget_Factory class. This class is likely far less extended, however it can't hurt to make the same change here as well.

[McGuive7]

Switch out static text in _deprecated_constructor() calls to dynamically generated class name of actual widget generating warning

[McGuive7]

Patch is in for widgets. Question: are there other non-widget implementations of PHP4 deprecation that need looking at as well?

[dd32]

Arguably switching out WP_Widget for get_class( $this ) is incorrect, as the constructor for the current class isn't deprecated at all, it's that that class is calling parent::WP_Widget() instead.

That being said, if we can include the actual widget in question here somehow that'd be great for debugging.. but it's also worth noting that for all other cases of deprecated notices, we don't/can't detect what is causing the deprecated notice in a performant manner.

​The Log Deprecated Notices plugin hasn't been updated, but one of it's features is to do a back-trace to locate what plugin/class/function/method/arguement the notice is actually refering to.

[McGuive7]

What about something like this:

public function WP_Widget( $id_base, $name, $widget_options = array(), $control_options = array() ) {
	$class = 'WP_Widget';
	if ( $class != get_class( $this ) ) {
		$class .= ' (' . get_class( $this ) . ')';
	}

	_deprecated_constructor( $class, '4.3.0' );
	WP_Widget::__construct( $id_base, $name, $widget_options, $control_options );
}

We could even prepend some text so the whole thing looks like "... WP_Widget (extended by {widget_class)", however I figure it might be better to keep it simple and avoid creating new translations (no idea if that's a big deal or not).

Thoughts?

[jorbin]

I've submitted a patch via slack to @nacin for Log Deprecated Notices to include deprecated constructor support.

Are we able to guarantee that WP_Widget will never be called staticly and thus without $this being set? If get_class() is called with anything other than an object, an E_WARNING level error is raised.

New translatable strings are fine for 4.4. I'm sure there will be plenty of them.

[wonderboymusic]

33440.2.diff​ refreshes after [33746]

[McGuive7]

@jorbin: good question. No way to guarantee that WP_Widget() won't be called statically, but it seems like this same issue would arise for the vast majority of the WP_Widget class's methods, most of which also include calls to $this. So can you think of a reason why WP_Widget() might be particularly likely to be called statically?

A quick search across all core files for the following strings doesn't yield any results that would indicate this method is getting called statically in core:
::WP_Widget
WP_Widget

That said, I could easily imagine a scenario in which a plugin might call the method statically for any number of reasons, well-coded or not :)

Not to overcomplicate things, but it'd be easy enough to detect if the call was made statically, and only proceed when that's not the case with something like:
isset( $this ) && get_class($this) == __CLASS__
(​http://stackoverflow.com/questions/1858538/how-do-i-check-in-php-that-im-in-a-static-context-or-not)

Seems like possible overkill for what may be an unlikely scenario, but does this seem worth doing to others? Any other considerations I'm missing?

[jorbin]

It's unlikely, but I'd rather be defensive, especially since this is for enhancing a deprecation warning that will have been around a release cycle.

I would support an updated version of attachment:33440.2.diff​ that checked if $this is set and if so, used get_class( $this ) instead of WP_Widget.

Any other enhancement ideas for the string.

[McGuive7]

Great, happy to implement this. The patch is ready to go, I'd just like to optimize the message to be as clear as possible. Anyone feel strongly about one of the following options, or another - assuming Example_Widget is the name of the child class?

1. ... method for WP_Widget (Example_Widget)
2. ... method for WP_Widget (called by Example_Widget)
3. ... method for WP_Widget (called via Example_Widget)

I'm inclined to go with option 2.

[ocean90]

#35470 was marked as a duplicate.

[sebastian.pisula]

I added my patch for 4.5-alpha-36312 :)

[ocean90]

In 36541:

Introduce a $parent_class parameter for _deprecated_constructor().

Use the parameter for the deprecated constructor warning in WP_Widget to provide an indication to which widget is using the PHP4 style constructor.

Props sebastian.pisula.
Fixes #33440.