Prevent object scope resolution in View::process_file()

As of PHP 5.4, anonymous functions may be declared statically. This prevents them from having the current class automatically bound to them. Objects may also not be bound to them at runtime.

core/classes/view.php

Line 248 in 52d939d

$clean_room = function($__file_name, array $__data)

Sets as static to prevent object scope resolution. Current View::process_file() unusefull because $this available in anonymous function.

Not sure what you are trying to say here, but having $this available in a view is intentional.

@WanWizard what is this "wrapper" for?

It makes sure view variables are created in the local scope of the closure, and not "bleed" into the View instance.

@WanWizard

local scope of the closure

The parent scope of a closure is the function in which the closure was declared (not necessarily the function it was called from).

You misunderstood me here too.

In the closuse, local variables are created using extract(), so the data passed to the view is available as PHP variables in the view file. Without the closure, those variables would be created in the process_file() local scope, which leads to a mess on reuse of the View object.

@WanWizard

which leads to a mess on reuse of the View object

I can't repeat this problem, can u show example?
Compare current version with:

public function process_file(array $data)
{
    extract($data, EXTR_REFS);
    // Capture the view output
    ob_start();
    try
    {
        // Load the view within the current scope
        include $this->file_name;
    }
    catch (\Exception $e)
    {
        // Delete the output buffer
        ob_end_clean();
        // Re-throw the exception
        throw $e;
    }
    // return the result
    return ob_get_clean();
}

Sorry for this long thread, Fuel is a mature framework, and my memory isn't all that anymore. ;-)

I've did some digging.

Fuel, and this code, was written for PHP 5.3.3, and this is still the minimum version. PHP 5.3.3 does not support the static keyword on closures, and it does not expose $this inside the closure. As of PHP 5.4, $this is exposed in the closure, and the static keyword is needed to prevent that.

This was never noticed (I presume) at the time, and never implemented. It would also have meant a PHP version check and a definition of both, depending on the PHP version used.

It has been like this for years, and changing/fixing it now would mean breaking existing code out there, that uses $this in view files.

I've done a quick grep on Fuel apps we have in maintenance (some we've made, some we didn't), and several use $this->variable in a view, instead of simply $variable

So, yes, you are correct. And no, I don't think this should be changed as that would create a regression and a potentionally massive check and change operation when users do an upgrade.

@WanWizard https://github.com/fuel/fuel/tree/1.9/develop

FuelPHP is a fast, lightweight PHP 5.4+ framework.

I know that, it was updated in the past to "persuade" users to move on from 5.3.

The minimum requirements are in the docs (https://fuelphp.com/docs/requirements.html) and still mention 5.3.3.

And I am pretty sure there are still apps running on this version, we've recently took an app in for maintenance that was still running on Fuel v1.0.0.

@WanWizard u persuade me... ok, I don't touch that dead cow anymore