akeeba / fof

Rapid Application Development framework for Joomla!™ 3 and 4
0 stars 0 forks source link

Problem with PHP 7.2 and fof3 #686

Closed gcourault closed 4 years ago

gcourault commented 4 years ago

There is a issue with the call at the controller en the line of case 0 (line 1023 in the code):

libraries/fof30/Controller/Controller.php

switch (count($arguments))
            {
                case 0:
                    $result = $this->{$event}();
                    break;
                case 1:
                    $result = $this->{$event}($arguments[0]);
                    break;
                case 2:
                    $result = $this->{$event}($arguments[0], $arguments[1]);
                    break;
                case 3:
                    $result = $this->{$event}($arguments[0], $arguments[1], $arguments[2]);
                    break;
                case 4:
                    $result = $this->{$event}($arguments[0], $arguments[1], $arguments[2], $arguments[3]);
                    break;
                case 5:
                    $result = $this->{$event}($arguments[0], $arguments[1], $arguments[2], $arguments[3], $arguments[4]);
                    break;
                default:
                    $result = call_user_func_array(array($this, $event), $arguments);
                    break;
            }

I change it with:

case 0:
                    $result = $this->{$event}(null);
                    break;

Because we obtain a error if the call dont have parameters.

nikosdion commented 4 years ago

Thanks! This code is really old and should have been already replaced. It improved performance in PHP 5.3. Since we dropped PHP 5.3 support this code needs be replaced with a simple call_user_fun_array call.

gcourault commented 4 years ago

I am very glad contrib in this small way at your excelent work! We use FOF and Akeeba a lot

nikosdion commented 4 years ago

I decided to revert this change since it's breaking things. call_user_func_array does not allow for pass-by-reference variables which are used throughout FOF, namely the Model events. Since we need to be calling protected and private object methods whenever we are using this pattern we can't use a helper function outside of the class. At best we could use a trait instead of the copied switch-block but that's irrelevant.

Most importantly we have to consider if what you said is true. Calling a method without a parameter is valid syntax for PHP 7.2. In fact it's been valid syntax since PHP 5 and has not been modified in PHP 7. The only that changed back in PHP 7.0 was how $foo->$bar['baz']() without the braces is interpreted versus how PHP 5 used to interpret it. This is why we added the braces.

Your notice has to do with a bug in your code. Namely, your event handler probably has a signature like this: protected function onFooBar($optionalParameter) instead of the correct protected function onFooBar($optionalParameter = null)

What you asked me to do would, in fact, introduce a massive bug in FOF! When we ask FOF to pass no parameters it would disobey us and pass exactly one parameter with the value of null. This WILL cause PHP notices when a handler method with no parameters in its signature is used e.g. protected function onFooBar()

So, FOF 3 was working correctly and the only problem was a bug in your code. Using call_user_func_array or implementing your suggestion introduce major bugs. Therefore the change is reverted and no further action will be taken.