flag.inc declares 2 functions non-statically, but the functions are called as static functions. The code comment suggests that at least 1 of the 2 is intended as static:

 /**
   * Creates a flag from a database row. Returns it.
   *
   * This is static method.
   *
   * The reason this isn't a non-static instance method --like Views's init()--
   * is because the class to instantiate changes according to the 'content_type'
   * database column. This design pattern is known as the "Single Table
   * Inheritance".
   *
   * @static
   */
  function factory_by_row($row) {

  /**
   * Create a complete flag (except an FID) from an array definition.
   */
  function factory_by_array($config) {

This breaks E_STRICT compliance with the warning:

Strict warning: Non-static method flag_flag::factory_by_row() should not be called statically in flag_flag::factory_by_row() (line 1357 of /var/www/sites/all/modules/contrib/flag/flag.module).

Strict warning: Non-static method flag_flag::factory_by_array() should not be called statically in flag_flag::factory_by_array() (line 1433 of /var/www/sites/all/modules/contrib/flag/flag.module).

The patch redeclares these 2 functions as static.

CommentFileSizeAuthor
flag_static_E_STRICT.1.patch894 bytesmanarth

Comments

mooffie’s picture

mooffie commented

The reason these functions aren't declared static is because PHP 4 doesn't support this.

Nate, should we part with PHP 4 ?

If so, this patch needs work:
- There are more functions there that should be declared static (probably all of them are marked with a "@static" comment).
- We should do the same for abstract methods.
- The .info file should say that PHP 5 is a dependency.

quicksketch’s picture

quicksketch
he/him
commented

I'm fine with requiring PHP 5 for Flag 2.x, though we should see what the broader Drupal community is doing with its function declarations. We shouldn't be the only ones using static function declarations.

mooffie’s picture

mooffie commented
Status: Needs review » Needs work

(Yes, D7 is using 'static' and 'abstract' and FileTransfer even have a "factory()" method. Views too uses static functions, but for PHP4 compatibility it does what we do: only make a note of it in a comment.)

mooffie’s picture

mooffie commented

I've added a "php = 5" line to our D6's .info file (it's not needed in D7), because there are patches pending that use PHP 5 features (e.g., "&$param = 123", "abstract", "static").

mooffie’s picture

mooffie commented
Category: bug » task
Status: Needs work » Active

I prefixed all the factory method with the "static" keyword.
http://drupal.org/cvs?commit=488528
http://drupal.org/cvs?commit=488526

I'm still leaving this issue open for the time being (but reclassifying it as "task"):

- _load_content() and get_views_info() too could be static, but I suspect #1035410 will make this practically impossible (think: we'll have to have a different PHP class for each entity for this to work... but PHP doesn't let us dynamically generate classes).

- What about declaring certain methods "abstract"?

- And using "public" and "protected"?

joshk’s picture

joshk commented

+1 for requiring PHP 5 (let's help the world move forward).

-1 for moving to protected methods; just gets in the way of development in my experience.

mlncn’s picture

mlncn commented

I'm getting this in 7.x also. And agree with both joshk's points.

dwightaspinwall’s picture

dwightaspinwall commented

subscribing

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Version: 6.x-2.x-dev » 7.x-2.x-dev

This is a problem with D7 too, am getting the following errors on every page load:

Strict warning: Non-static method flag_flag::factory_by_row() should not be called statically in flag_flag::factory_by_row() (line 1553 of flag.module).
Strict warning: Non-static method flag_flag::factory_by_row() should not be called statically in flag_flag::factory_by_row() (line 1553 of flag.module).
Strict warning: Non-static method flag_flag::factory_by_array() should not be called statically in flag_flag::factory_by_array() (line 1635 of flag.module).
damienmckenna’s picture

damienmckenna
Location TN, USA
commented

FYI this has been fixed in the latest 7.x-2.x codebase.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Active » Fixed

This has also been fixed in 6.x-2.x-dev.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Fixed » Active

Sorry for changing the status.

mefisto75’s picture

mefisto75 commented

I get - Strict warning: Non-static method flag_flag::factory_by_row() should not be called statically in flag_flag::factory_by_row() (line 1553. Don't know if it relates to the above.

mefisto75’s picture

mefisto75 commented

PLus another one - Strict warning: Declaration of flag_handler_relationship_counts::ui_name() should be compatible with that of views_handler::ui_name() in require_once() (line 179 of

quicksketch’s picture

quicksketch
he/him
commented
Status: Active » Closed (fixed)

This original issue has been fixed since #5 (way back in January). The other strict warnings mentioned in #13 and #14 have been fixed in #1119842: Strict warnings with Views 3.x.