Underscore.php icon indicating copy to clipboard operation
Underscore.php copied to clipboard

Static invocations such as __::each raise E_STRICT warnings

Open jdp opened this issue 14 years ago • 18 comments

Using the static method invocations, like __::each and __::map will raise E_STRICT warnings. For example, calling __::filter will raise this chain of warnings:

Array
(
    [number] => 2048
    [string] => Non-static method __::filter() should not be called statically
    [line] => 55
)
Array
(
    [number] => 2048
    [string] => Non-static method __::select() should not be called statically
    [file] => ../lib/underscore.php
    [line] => 182
)
Array
(
    [number] => 2048
    [string] => Non-static method __::_wrapArgs() should not be called statically
    [file] => ../lib/underscore.php
    [line] => 184
)
Array
(
    [number] => 2048
    [string] => Non-static method __::_collection() should not be called statically
    [file] => ../lib/underscore.php
    [line] => 186
)

As of PHP 5.4, E_STRICT is or'd into the E_ALL constant, so scripts that report all errors will show these.

jdp avatar Dec 10 '11 10:12 jdp

Thanks for putting this on the radar. This issue ties back to supporting both static calls and OO-style calls.

Having looked into a solution before, nothing jumped out as particularly clean or easy. I'll take a look and see what I can do. Having E_ALL report these in 5.4 makes this more important than in previous versions.

brianhaveri avatar Dec 10 '11 19:12 brianhaveri

What about something like this?

<?php

function __() {
    return Underscore::instance();
}

class __ {

    static function __callStatic($name, $args) {
        return call_user_func_array(array(Underscore::instance(), $name), $args);
    }
}

class Underscore {

    static function instance() {
        static $instance;
        if (! $instance) $instance = new self;
        return $instance;
    }

    function foo() {
        return 'foo';
    }
}

__()->foo();
__::foo();

frosas avatar Mar 07 '12 18:03 frosas

Something like that might work. I briefly tried this approach, but only a couple of tests passed. It's probably worth a second look.

function __($item=null) {
  $__ = Underscore::getInstance();
  if(func_num_args() > 0) $__->_wrapped = $item;
  return $__;
}

class __ {
  public static function __callStatic($method, $args) {
    $__ = Underscore::getInstance();
    if(count($args) > 0) $__->_wrapped = $args[0];
    return call_user_func_array(array($__, $method), array_slice($args, 1));
  }
}

class Underscore {
  // All the functions go here
}

brianhaveri avatar Mar 09 '12 05:03 brianhaveri

Any progress on resolving these warnings?

joseym avatar Apr 22 '12 18:04 joseym

@frosas @brianhaveri I've been pondering this issue too. I like the idea of using overloading. I think it'd be best to do it in reverse (using __call rather than __callStatic):

<?php
function __2($item=null) {
    return new Underscore2($item);
}

class __2 {
    public static function foo() {
        return 'foo';
    }
    // Mark all fns as "public static" to avoid the "calling non-static 
    // methods statically generates an E_STRICT" warning described on
    // php.net/manual/en/language.oop5.static.php
}

class Underscore2 {
    private $_wrapped;

    public function __construct($item = null) {
        $this->_wrapped = $item;
    }

    public function __call($name, $args) {
        array_unshift($args, $this->_wrapped);
        $this->_wrapped = call_user_func_array('__2::' . $name, $args);
        return $this->_wrapped; // add chain logic here to determine if chained
    }
}

I was curious about how much overhead the overloading added and did some tests here. @brianhaveri Mad props on this project—it's pretty sick! =]

Edit - I took out the extends. In that case the the overloader __call is not triggered because the inheritance happens and the method are available. That would have been great tho. In any case my guess is that the static __:: syntax is the one that more people use so it makes sense to design for that.

ryanve avatar May 06 '12 18:05 ryanve

@ryanve very elegant solution!

joseym avatar May 06 '12 19:05 joseym

@joseym Thanks - although not quite as good as how I originally thought. See my edit above. It's basically the reverse of what @frosas has.

ryanve avatar May 07 '12 04:05 ryanve

My code above was intended as a quick fix for the current implementation. As procedural style is probably the most used I also prefer @ryanve approach and avoid instantiating objects if possible to avoid performance issues.

frosas avatar May 07 '12 08:05 frosas

A property declared as static can not be accessed with an instantiated class object (though a static method can).

My best suggestion is to make a __static class which has ONLY static methods. This can be used statically like you'd expect. Then you'd have a __ like expected which has a __call and __callStatic which can wrap it as expected. The issue with this is the current library cannot be used statically, and doing the static library with an init and static library has a few side effects. A complete refactor is in the near future of this library.

Concept Draft

bayleedev avatar Jun 29 '12 06:06 bayleedev

@BlaineSch I think we all understand the issue, I have yet to try @ryanve workaround but it looks promising and doesn't require a rewrite of the entire library.

joseym avatar Jun 29 '12 13:06 joseym

@joseym The wrapper class I made is similar to his approach, the problem with making the entire library static was the instance variables will no longer be accessible, and making those static is obviously a bad idea. So the reverse approach is needed, which is how I implemented the draft I linked to above. The wrapper handles static and non-static requests and basically always has an instance of the library.

I realize the issue was known, I was just walking through it in my head.

bayleedev avatar Jun 29 '12 14:06 bayleedev

Why isn't BlaineSch's solution implemented in this lib?

oojacoboo avatar Feb 01 '13 23:02 oojacoboo

@oojacoboo Judging by the activity, the project is dead. The approach is backwards and would probably need a rewrite. There should be an object with a static class backend that contains the logic.

Check out this similar array library I created.

bayleedev avatar Feb 02 '13 00:02 bayleedev

@BlaineSch yea... looks pretty dead. There is another lib I found on github, but it has some pretty nutty Laravel dependencies and such. It also uses a config file for mapping :/

oojacoboo avatar Feb 02 '13 00:02 oojacoboo

I created a fork that eliminates the E_STRICT warnings by removing the object-oriented way of calling the functions, and changing all the functions to static: https://github.com/JonathanAquino/Underscore.php .

JonathanAquino avatar Sep 20 '13 21:09 JonathanAquino

Great Jonathan! Looking forward to trying it out. Can I assume that the functional footprint and signatures are the same? Might also be worth pointing people to Brian's documentation as this is an important reference item: http://brianhaveri.github.io/Underscore.php/.

Ken

On 20 September 2013 22:46, Jonathan Aquino [email protected]:

I created a fork that eliminates the E_STRICT warnings by removing the object-oriented way of calling the functions, and changing all the functions to static: https://github.com/JonathanAquino/Underscore.php .

— Reply to this email directly or view it on GitHubhttps://github.com/brianhaveri/Underscore.php/issues/4#issuecomment-24843315 .

yankeeinlondon avatar Sep 22 '13 15:09 yankeeinlondon

Hi Ken: Yeah, the signatures are the same, although I had to remove the chain() method. Also, good idea about adding a link to Brian's doc - I have done that.

JonathanAquino avatar Sep 23 '13 03:09 JonathanAquino

up! I'm trying to use this lib from https://packagist.org/packages/underscore/underscore.php with php-v5.3.3-7 and caught warning and error:

  1. Warning like Non-static method __::filter() should not be called statically
  2. Error: Call to undefined function __()

dvapelnik avatar Feb 24 '15 09:02 dvapelnik