Skip to:
Content

Opened 9 months ago

Closed 6 months ago

Last modified 5 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: Core Keywords:
Cc: slava.abakumov@…

Description

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 boonebgorges9 months 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 boonebgorges9 months ago

In 7289:

Improve syntax of class method definitions, to avoid E_STRICT warnings

Continues the work started in r7277

See #5108

comment:3 boonebgorges9 months 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 boonebgorges9 months 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 slaFFik9 months 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 johnjamesjacoby9 months ago

In 7307:

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

comment:7 johnjamesjacoby9 months ago

In 7308:

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

comment:8 johnjamesjacoby9 months ago

In 7310:

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

comment:9 johnjamesjacoby9 months ago

In 7311:

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

comment:10 boonebgorges9 months ago

In 7316:

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

See #5108, #3886

comment:11 boonebgorges9 months ago

In 7317:

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

See #3886, #5108

comment:12 follow-up: slaFFik9 months ago

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

This will break in PHP 4, so this http://buddypress.trac.wordpress.org/changeset/7290 just doesn't make any sense.

comment:13 in reply to: ↑ 12 boonebgorges9 months ago

Replying to slaFFik:

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

This will break in PHP 4, so this http://buddypress.trac.wordpress.org/changeset/7290 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 johnjamesjacoby8 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 johnjamesjacoby8 months ago

In 7351:

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

comment:16 johnjamesjacoby8 months ago

In 7354:

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

comment:17 DJPaul8 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-y6 months ago

In 7460:

Mark BP_Groups_Group::convert_type_to_order_orderby() as static.

See #5108.

comment:19 boonebgorges6 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-y6 months ago

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

comment:21 johnjamesjacoby5 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.