[5.x]: Attribute defined test on Component classes always returns true

[bencroker]

Description

The attribute is defined test on objects that extend the Component class (or any class that implements __call()) always returns true.

Possibly caused by the changes introduced by @brandonkelly in 0e31c67.

Steps to reproduce

1. Render a template with the following Twig code:

{{ attribute(craft.app, 'xyz') is defined ? 'Method exists' : 'Method does not exist' }}

Expected behavior

It should output Method does not exist.

Actual behavior

It outputs Method exists.

Craft CMS version

5.1.0

PHP version

No response

Operating system and version

No response

Database type and version

No response

Image driver and version

No response

Installed plugins and versions

No response

[brandonkelly]

My memory is that is defined has always been unreliable for object properties/methods, because twig_get_attribute() basically just throws in the towel and returns true here when it hasn’t been able to find any matching properties or methods on the object.

I just tested on 4.8.9 (last release before 0e31c67) and even 4.7.4 (last release before updating Twig from 3.4 to 3.8 via aa6fe4e), and in both cases I’m getting the same Method exists output.

[bencroker]

Can confirm that. It looks like the magic __call() method trips it up here.

Reason I’m asking is that this is making Craft Pest fail here, but I’ll bring it up to @markhuot. Thanks for taking a look!

[khalwat]

So I did a deep dive on this a while ago to figure out what was going on...

    {% if craft.mattwilcox is defined %}
        <p>Hi, Matt!</p>
    {% endif %}

compiles down to this PHP:

        // line 18
        if (craft\helpers\Template::attribute($this->env, $this->source, ($context["craft"] ?? null), "mattwilcox", [], "any", true, true)) {
            // line 19
            echo "        <p>Hi, Matt!</p>

which ends up calling getAttribute() in Twig's CoreExtension.php class, and because the craft variable object implements the PHP magic method __call https://www.php.net/manual/en/language.oop5.overloading.php#object.call it falls through to here: https://github.com/twigphp/Twig/blob/3.x/src/Extension/CoreExtension.php#L1698 and since $isDefinedTest = true it just returns that it's defined.

TL;DR: because the craft object implements the PHP __call magic method, it could dynamically return anything as being defined, so it just says it's defined. So literally anything will be "defined" as a property on an object that implements __call in Twig

[brandonkelly]

Right… it’s a byproduct of Twig trying to be extremely lenient on method syntax, e.g. if you type obj.foo, that could be referencing a property foo, or a method foo()/getFoo(). And by extension, if __call() exists, maybe __call() supports a magic method foo().

I feel like the only time __call() should be considered is if it’s explicitly a method call though (obj.foo()). If the object wants to allow obj.foo as a magic property, it can already support that syntax via __get().