[Bug]: Variable evaluation fails for objects using magic getters

[tillschander]

What happened?

Given a class like this that uses a magic getter...

class User {
    public function __construct(
        private array $data
    ) {}

    public function __get(string $property): mixed {
        return $this->data[$property] ?? "Property '$property' does not exist.";
    }
}

...with...

$user = new User(["name" => "Foo"]);

...i would expect the following template to output "Foo" and not nothing.

{{ user.name }}

How to reproduce the bug

Try to access any property of any class with a magic getter.

The models in Laravel use magic getters and setters:
https://github.com/illuminate/database/blob/ab85fdb286ba05bb735ea7d2a01bcdb2938ac609/Eloquent/Model.php#L2273-L2282

So here is a basic example using Laravel and keepsuit/laravel-liquid

// routes/web.php

Route::get('/', function () {
    return view('home', [
        'user' => User::first()
    ]);
});

{% # resources/views/home.liquid %}

{{ user.name }}

Package Version

v0.8.0

PHP Version

8.3.0

Which operating systems does with happen with?

No response

Notes

The reason for why this fails lies in internalContextLookup() of the RenderContext class:

php-liquid/src/Render/RenderContext.php

Line 201 in 96a4e87

is_object($scope) && property_exists($scope, (string) $key) => $scope->{$key},

The call to property_exists() only works for actual class properties. For magic / virtual properties it will always return false.

But maybe we can work around this with isset()? Unlike property_exists() it can work in conjunction with magic properties as its behavior in such cases can be defined via __isset().

An alternative lookup using isset() might look something like this:

- is_object($scope) && property_exists($scope, (string) $key) => $scope->{$key}, 
+ is_object($scope) && (isset($scope->$key) || is_null($scope->$key ?? false)) => $scope->{$key}, 

But maybe that line should be broken out into its own method as it is getting a bit complicated 😅

Basically I am checking if the property is set or if it is null (as isset() returns false for null values). The weird null coalescing operator is there to prevent errors in case the property really is undefined.

[cappuc]

A solution for this bug is not simple, I've done some tests and these are the problems I found:

• property_exists returns true even for protected and private properties.
• isset with magic properties works only if __isset method is defined.
• isset returns false for properties with null value.
• I tried accessing the property without checks in a try/catch but it always returns null for undefined properties
• Your check is_null($scope->$key ?? false) doesn't work because it always returns false if $scope->$key is null

[cappuc]

With laravel models I usually map the model to a liquid Drop.
You can also check with properties are exposed with this method.
Here is a simple example:

class ModelDrop extends Drop {
    public __construct(protected Model $model){}

    // add methods for computed properties

    protected function liquidMethodMissing(string $name): mixed {
       // Check if property/relation exists or throw an exception
      //  throw new UndefinedDropMethodException($name);

        return $this->model->$name;
    }
}

[tillschander]

You are right. This is not a simple problem and there are workarounds like Drops. In our case we went a slightly different route by casting the objects to arrays before passing them on to the renderer.

But I still think that support for classes with magic getters would be a win for the developer experience. It might even be beneficial to the performance / resource usage because we can skip all the casting or instantiating of new objects.

I have looked into this problem a bit further and think one option could be to keep the existing case for objects in internalContextLookup() and add another one for objects with magic getters:

$value = match (true) {
    is_array($scope) && array_key_exists($key, $scope) => $scope[$key],
    $scope instanceof Drop => $scope->{$key},
    is_object($scope) && property_exists($scope, (string) $key) => $scope->{$key},
+   is_object($scope) && (isset($scope->{$key}) || is_null($scope->{$key} ?? false)) => $scope->{$key},
    default => new MissingValue,
};

This way we keep the behaviour of property_exists for regular objects while also gaining support for objects with magic getters. You are correct that this only works for objects with __isset but I'd argue that in most cases where you have a magic getter you also have the magic __isset. For all the other cases where that don't define __isset the code would work the same as it did so far.

I have also written some test cases to ensure that the behavior for both regular and magic objects is in fact as expected. You can find the in PR #42

Maybe you can take a look at that PR to see if there are any other cases that should get covered by the tests. Or maybe you have an even better idea for how to handle magic objects.