Skip to:

Opened 2 years ago

Closed 21 months ago

Last modified 20 months ago

#5108 closed defect (bug) (fixed)

PHP 5.4+ strict standards warnings

Reported by: boonebgorges Owned by:
Milestone: 1.9 Priority: normal
Severity: normal Version:
Component: Component - Core Keywords:
Cc: slava.abakumov@…


In [WP24288], WP_DEBUG was changed so that it always shows E_STRICT warnings. This has uncovered a number of strict standards violations in BP. Let's fix them for 1.9.

See also r7277

Change History (21)

comment:1 @boonebgorges2 years ago

In 7288:

Remove pass-by-reference for objects in selected places in BP

Since PHP 5.0, objects have been passed by reference by default. In more recent
versions of PHP, standards notices have been introduced to increasingly
deprecate the practice of passing objects by reference using an ampersand.

This changeset removes several of these strict standard notices, by removing
the explicit pass-by-reference for the objects in question.

See #5108

comment:2 @boonebgorges2 years ago

In 7289:

Improve syntax of class method definitions, to avoid E_STRICT warnings

Continues the work started in r7277

See #5108

comment:3 @boonebgorges2 years ago

In 7290:

Moves PHP4 constructor in BP_Groups_Widget below construct()

This help to avoid a "constructor defined twice" strict standards warning in
PHP 5.4+.

See #5108

comment:4 @boonebgorges2 years ago

The biggest remaining offender in BP has to do with our invoking methods statically without using the static keyword. There's an existing ticket for this issue in #3886. Before a sweep can be done, I would like to have some testing to make sure that adding the keyword doesn't break cases where BP or other plugins are calling the methods *non*-statically.

comment:5 @slaFFik2 years ago

  • Cc slava.abakumov@… added
We don't need them, but it's possible that existing plugins are calling BP_Groups_Widget::bp_groups_widget()

I didn't see such direct calling, but ok, perhaps someone does this...

Backward compatibility is the WordPress way.

Even when users are suffering because more server resources needed to handle exact the same amount of traffic? Lets write even more unused code.
Perhaps we should make even the very first unupdated for years plugins work properly on the latest WP version? Everything should have its end.

WordPress defined minimum requirements. It won't work on earlier PHP versions. So in this case one individual plugin's working or not working state is nothing - the whole engine will produce errors.

So, supporting BuddyPress-dependent plugins that relies on BuddyPress that relies on WordPress that relies on PHP 5.2.4. I don't see the logic to support PHP 4 for those plugins.

I absolutely understand your position and can even understand Automattic position (hey, I'm a developer too!), but I strongly disagree that developers should write legacy code that *might* be used by some developers. Old plugins will anyway break, because of lots of other things, but new one will 100% use the right logic and way to init that same BP_Groups_Widget class.

P.S. And I do not want offend you or whatever negative feeling you might get after my words, I do appreciate your work and code. Sorry if in some words I was not very polite. I just disagree with some particular things.

comment:6 @johnjamesjacoby2 years ago

In 7307:

Fix PHP 5.4 static method call warnings in bp-messages-classes.php. See #5108.

comment:7 @johnjamesjacoby2 years ago

In 7308:

Fix some PHP 5.4 static method E_STRICT warnings across several components. More to do here. See #5108.

comment:8 @johnjamesjacoby2 years ago

In 7310:

Revert some private method scope breaking changes accidentally introduced in r7308. See #5108.

comment:9 @johnjamesjacoby2 years ago

In 7311:

Revert more private method scope breaking changes from r7308. See #5108.

comment:10 @boonebgorges2 years ago

In 7316:

Explicit PHP5 visibility and static declarations for bp-groups-classes.php

See #5108, #3886

comment:11 @boonebgorges2 years ago

In 7317:

Explicit visibility and static declarations in bp-blogs-classes.php

See #3886, #5108

comment:12 follow-up: @slaFFik2 years ago

And you are using this: buddypress()->activity

This will break in PHP 4, so this just doesn't make any sense.

comment:13 in reply to: ↑ 12 @boonebgorges2 years ago

Replying to slaFFik:

And you are using this: buddypress()->activity

This will break in PHP 4, so this just doesn't make any sense.

Again, we are *not* trying to maintain compatibility with PHP 4. Instead, we are trying to ensure compatibility with existing plugins by not removing existing methods that may be called explicitly. This has nothing to do with PHP versions. (It's true that, in the case of a class constructor, what I'm talking about is an edge case, because few people call them explicitly. But it does happen, and I remember BP plugins breaking for this reason before.)

comment:14 @johnjamesjacoby23 months ago

In 7350:

Fix E_STRICT warnings in group creation code from using functions like array_shift() and array_pop() that expect references rather than results of array_keys(). See #5108.

comment:15 @johnjamesjacoby23 months ago

In 7351:

Cleaner E_STRICT dodging approach than what was introduced in r7350. See #5108.

comment:16 @johnjamesjacoby22 months ago

In 7354:

BP_XProfile_Group::render_admin_form() is not a static method. See #5108.

comment:17 @DJPaul22 months ago

Groups directory, non-admin user:

PHP Strict standards: Non-static method BP_Groups_Member::check_is_banned() should not be called statically in /plugins/buddypress/bp-groups/bp-groups-functions.php on line 572

comment:18 @r-a-y21 months ago

In 7460:

Mark BP_Groups_Group::convert_type_to_order_orderby() as static.

See #5108.

comment:19 @boonebgorges21 months ago

I think we've done a solid job with this - thanks all. Marking resolved to clear the milestone, though feel free to commit further patches against it.

comment:20 @r-a-y21 months ago

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

comment:21 @johnjamesjacoby20 months ago

In 7512:

Mark BP_XProfile_Field::admin_validate() as a static method. Fixes debug notice when saving XProfile field settings in wp-admin. See #5108.

Note: See TracTickets for help on using tickets.