php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Request #50029 __invoke() can not work as class property
Submitted: 2009-10-29 01:15 UTC Modified: 2021-09-30 11:32 UTC
Votes:79
Avg. Score:4.5 ± 0.8
Reproduced:53 of 62 (85.5%)
Same Version:40 (75.5%)
Same OS:22 (41.5%)
From: marc dot gray at gmail dot com Assigned: cmb (profile)
Status: Closed Package: Class/Object related
PHP Version: master OS: *
Private report: No CVE-ID: None
 [2009-10-29 01:15 UTC] marc dot gray at gmail dot com
Description:
------------
Placing a class with an __invoke method as a property inside another 
class seems to nullify the invokeability of the original class.

Tested on:
Ubuntu 9.04, PHP 5.3.0
CentOS 5.3, PHP 5.2.11 ionCube / Suhosin

Reproduce code:
---------------
class a { 
  function __construct() { } 
  function __invoke() { echo("Invoked\n"); } 
} 

$a = new a(); 
$a(); 
// Prints: Invoked 

class b { 
  private $x; 

  function __construct() { 
    $this->x = new a(); 
    $this->x(); 
  } 
} 

$b = new b(); 
// Issues error: undefined method b::x  

Expected result:
----------------
I expect "new b()" construct to call the class a invoke

Actual result:
--------------
Undefined method - it doesn't seem to recognise the invokeable class 
property as actually invokeable.

Patches

csxigxvg (last revision 2015-06-19 06:26 UTC by sample at email dot tst)

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2009-11-02 15:58 UTC] kalle@php.net
There was lots of discussion about this, because it could override class methods like:

class Test { 
  private $closure;

  public function __construct() {
    $this->closure = function() {
      echo 'Hello World';
    };
  }

  public function closure() {
    echo 'Hello PHP';
  }

  public function call() {
    $this->closure();
  }
}

$test = new Test;

// Call Test::$closure or Test::closure() now?
$test->call();


What you need to do is to copy the instance into a variable like:
$closoure = $this->closure;
$closure();
 [2010-04-07 13:41 UTC] weierophinney@php.net
I can understand wanting to ensure that collisions between existing methods and 
invokable properties don't occur, but when there aren't any collisions, it 
doesn't make sense.

I'd argue that the following behavior should be implemented:
* If no matching method exists, simply allow invoking.
* If a matching method exists, call the method, and raise either an E_NOTICE or 
E_WARNING indicating the collision.

Right now, it's a fairly big WTF moment -- you expect it to work, and instead 
get an E_FATAL. Copying to a temporary variable is code clutter, particularly 
when you know the object is invokable.
 [2010-04-07 14:52 UTC] ballouc at gmail dot com
I'm in agreement with the Matt's last suggestion.  I believe that errors should only be raised in the event of a collision.  The current implementation of the __invoke method, IMO, would not perform as a third party developer would have anticipated.  My personal choice would be to throw an E_WARNING for collisions as I have seen ~E_NOTICE far too often.  

Personally, I believe that an __invoke collision occurring would be more indicative of a developer error than intentional.  If this is not the case, and you find many people readily anticipate having both foo() and __invoke called in succession, this would need to be discussed further as that is also a viable option.
 [2010-04-07 20:22 UTC] bkarwin@php.net
How would Matthew's suggestion work if a magic __call() method is present?

class b {
  private $x;

  public function __call($method, $args) { echo "Called\n"; }

  function __construct() { 
    $this->x = new a(); 
    $this->x(); 
  } 

}

Should this execute $this->__call("x") and output "Called" or should it execute $this->x->__invoke() and output "Invoked"?
 [2010-12-01 16:19 UTC] jani@php.net
-Package: Feature/Change Request +Package: Class/Object related
 [2011-02-07 22:36 UTC] dhaarbrink at gmail dot com
@bkarwin: The control would be passed to __call(), since there is no way to disambiguate. __call() would then be responsible for deciding what to do.

I have to agree with Matthew, it's a huge WTF. It just doesn't work as it should (that's beyond what is expected).
 [2011-08-24 18:49 UTC] marc dot gray at gmail dot com
I've been thinking about this since the same issue appears to exist with assigning a closure to a static 
variable too (tested in 5.3.5, 5.3.8 & 5.4alpha3.

//----------------------------------
// Create a very basic static class
class closureProperty {
	static public $myClosure;
}

// Assign a closure to a class property
closureProperty::$myClosure = function() { echo('Hi'); };

// Fatal error: Function name must be a string
closureProperty::$myClosure();

// Works as expected
$safeCopy = closureProperty::$myClosure;
$safeCopy();
//----------------------------------

I can understand why it happens with dynamic properties, apparently you can have a variable and function named 
identically? I admit I've never tried.

I would propose making identically named variables and functions as deprecated (does anyone who's any good 
actually do that? Talk about poor readability...) and implement a collision warning in the mean time. I would 
also suggest this makes less sense in a static case (due to $ variable prefix) than it did in a dynamic case, 
and should be changed.

If nothing else, some discussion on the matter would be lovely.

Thoughts?
 [2011-12-06 16:34 UTC] karsten at typo3 dot org
I would go for simple rules to solve this. When having a class and calling

$this->prop() PHP should

* use __invoke on $this->prop if there is no method prop() in the class
* if there is an actual method, the method takes precedence

In the latter one can still use $this->prop->__invoke(), but should probably 
rather clean up the code. :)
 [2012-06-08 12:24 UTC] programmierung at pfink dot de
The worst thing on this bug is that is_callable() returns true in this case even though it's not callable:

The lines

if(is_callable($this->objectWithInvokeMethod))
return $this->objectWithInvokeMethod(0);

produce the fatal error

Fatal error: Call to undefined method MyClass::objectWithInvokeMethod()
 [2013-08-29 10:44 UTC] worldoffame at hotmail dot com
Any updates on this? I hope it gets fixed asap, cant believe this bug has been around for 3+ years and still left untouched. *sigh*
 [2013-08-29 10:50 UTC] worldoffame at hotmail dot com
Also the argument that invoking class property could override class methods itself is just nonsense. Guess what? Its a poor design if your method shares the same name as your property anyway. If collision occurs it looks more like an improper OOP practice to me.
 [2013-11-02 23:29 UTC] worldoffame at hotmail dot com
Has this bug been fixed yet? Its one of the most annoying glitches in PHP, and funny it has existed for more than 4 years.
 [2013-11-03 04:41 UTC] yohgaki@php.net
-Type: Feature/Change Request +Type: Bug -Operating System: Ubuntu 9.04 +Operating System: * -PHP Version: 5.3.0 +PHP Version: master
 [2013-11-03 04:41 UTC] yohgaki@php.net
I think this is bug, not FR.
 [2013-11-03 04:45 UTC] yohgaki@php.net
-Summary: Weird invoke issue on class as property +Summary: __invoke() can not work as class property.
 [2013-11-03 14:08 UTC] nikic@php.net
-Type: Bug +Type: Feature/Change Request
 [2013-11-03 14:08 UTC] nikic@php.net
This is definitely an FR. Current behavior is intended as is, but somewhat controversial. Some reading on this topic:

 * https://wiki.php.net/rfc/closures/object-extension#warnings_on_direct-method_calls
 * http://markmail.org/message/3zi53txadno6xrqa
 * http://markmail.org/message/adr6ejlr4xewzo5a
 * http://markmail.org/message/5mhjr472jfzmkhd4
 [2013-11-03 14:09 UTC] nikic@php.net
Link to RFC should be https://wiki.php.net/rfc/closures/object-extension.
 [2013-11-03 21:12 UTC] yohgaki@php.net
Are we going to fix(implement) this?
If not, all we have to do is document this behavior.
It seems it's not documented, yet.
 [2013-11-03 21:36 UTC] yohgaki@php.net
I mean if this FR is not feasible, then we may document this and close bug.
If this FR is feasible, then we may document current behavior and keep open this bug.

I didn't follow closure implementation details closely, just curious about feasibility of the fix.
 [2015-01-27 22:27 UTC] worldoffame at hotmail dot com
And whats the current status of this bug? Will this be fixed in PHP 7? I must admit this is one of the unbearable glitches that I find almost impossible to withstand. There are some bad behaviors in PHP that I can live with, but this one annoys me so much. It's not just a bad design, its just a bug. Please make sure to fix this in PHP 7, if it hasnt taken place in the current PHP 5.6.
 [2015-07-18 16:05 UTC] cmb@php.net
The behavior of PHP 7 is still the same as PHP 5[1], and is
unlikely to change for PHP 7.0 which is in feature freeze.

However, due to the "uniform variable syntax"[2] one can use
parens to make it clear that an instance property is meant to be
called[3] what partly solves this issue.

[1] <http://3v4l.org/3Pu60>
[2] <https://wiki.php.net/rfc/uniform_variable_syntax>
[3] <http://3v4l.org/NGXF2>
 [2021-09-30 11:32 UTC] cmb@php.net
-Summary: __invoke() can not work as class property. +Summary: __invoke() can not work as class property -Status: Analyzed +Status: Closed -Assigned To: +Assigned To: cmb
 [2021-09-30 11:32 UTC] cmb@php.net
> […] what partly solves this issue.

Well, actually completely solves this issue – be explicit[1]!

[1] <https://3v4l.org/NGXF2>
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Sat Dec 21 16:01:28 2024 UTC