Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for closures or at least Closure::__invoke #3848

Open
faizanakram99 opened this issue Jun 7, 2023 · 14 comments · May be fixed by #4501
Open

Support for closures or at least Closure::__invoke #3848

faizanakram99 opened this issue Jun 7, 2023 · 14 comments · May be fixed by #4501

Comments

@faizanakram99
Copy link

Calling a closure is not supported in twig right now, any variable passed as \Closure cannot be called directly as Twig looks up the function in global functions (and extensions), the alternative is to call it via __invoke method on closure.

https://3v4l.org/EBR2Y (__invoke method does exist on Closures and work the same way as $closure() works).

Unfortunately this doesn't work in twig as twig_get_attribute uses get_class_methods to determine available methods on an object and fallbacks to __call if it exists and php being php get_class_methods($closure) doesn't return __invoke https://3v4l.org/VKjAF 🤷‍♂️ even though it exists and is documented.

I think this can be fixed by checking if $object is instance of \Closure and then allow calling __invoke or is there a better way ?

I am happy to contribute a PR if it makes sense.

@fabpot
Copy link
Contributor

fabpot commented Oct 2, 2024

Using __invoke() explicitly is the way to go.

@fabpot fabpot closed this as completed Oct 2, 2024
@faizanakram99
Copy link
Author

Using __invoke() explicitly is the way to go.

Does it work now?

I didn't check, it wasn't working at the time of opening the issue as mentioned in the OP.

@lowwa132
Copy link

lowwa132 commented Dec 6, 2024

Cant' call __invoke() indeed...
Right now the only solution is to use an extension like https://gist.github.com/romainnorberg/a68e5bfa229d974f9b79027be3cbd058

@faizanakram99
Copy link
Author

@fabpot can you please reopen the issue, apparently it's not yet fixed.

@fabpot
Copy link
Contributor

fabpot commented Dec 7, 2024

@fabpot can you please reopen the issue, apparently it's not yet fixed.

Calling __invoke() like in {{ obj.__invoke() }} is what I suggested above.

@lowwa132
Copy link

lowwa132 commented Dec 9, 2024

@fabpot can you please reopen the issue, apparently it's not yet fixed.

Calling __invoke() like in {{ obj.__invoke() }} is what I suggested above.

With Twig V3.15.0 I get this:

Neither the property "__invoke" nor one of the methods "__invoke()", "get__invoke()"/"is__invoke()"/"has__invoke()" or "__call()" exist and have public access in class "Closure".

image

@ericmorand
Copy link
Contributor

@fabpot can you please reopen the issue, apparently it's not yet fixed.

Calling __invoke() like in {{ obj.__invoke() }} is what I suggested above.

So, your recommendation is to call a reserved method?

https://www.php.net/manual/en/language.oop5.magic.php

All methods names starting with __ are reserved by PHP. Therefore, it is not recommended to use such method names unless overriding PHP's behavior.

@jdreesen
Copy link
Contributor

jdreesen commented Dec 9, 2024

It says that you should not create custom methods that start with __ as this is like a reserved "namespace" for PHP, right?

I'm not sure what the problem is with that in the context of this issue?

@ericmorand
Copy link
Contributor

ericmorand commented Dec 9, 2024

My point is that they serve to override certain behaviors of PHP, but they are not supposed to be called. You would override __sleep so that internally PHP executes its implementation whenever prior to serialization. But nobody would ever recommend that one execute __sleep. Or __construct, right?

Magic method are not supposed to be called by userland code. They are expected to be implemented by userland code, only.

TwigPHP enforces users to execute a magic method, instead of honoring PHP's behavior, that is:

The __invoke() method is called when a script tries to call an object as a function.

__invoke was never supposed to be called. It is the method that is called by the PHP engine whenever a script tries to call an object as a function.

@faizanakram99
Copy link
Author

Thing is it doesn't work, I've explained the issue in the original post.

get_class_methods doesn't contain __invoke for closures and hence twig cannot find it.

@fabpot
Copy link
Contributor

fabpot commented Dec 10, 2024

@faizanakram99 You offered to submit a PR in the ticket description, please do so if you think you've something that will work. Maybe I'm missing something, but I don't see how it could work without a major performance hit. If I understand correctly, you want __invoke() to be called automatically when using {{ obj }} when obj is a Closure. If that's the case, twig_get_attribute() is never called in such a situation as it's only called for attributes (when using something like {{ obj.something }}). Checking if a variable is a Closure whenever a variable is accessed would kill the performance as it would be done at runtime for all variable accesses. You can have a look at how we're dealing with __toString(), but it's a bit easier as we know it always returns a string (for __invoke(), you can get back anything). At this point, someone needs to code and see how far we can go.

@faizanakram99
Copy link
Author

If I understand correctly, you want __invoke() to be called automatically when using {{ obj }} when obj is a Closure.

No that's not what I want, I want {{ obj.__invoke() }} to work, it doesn't since twig cannot find invoke method on closure as it relies on get_class_methods internally.

@fabpot
Copy link
Contributor

fabpot commented Dec 10, 2024

If I understand correctly, you want __invoke() to be called automatically when using {{ obj }} when obj is a Closure.

No that's not what I want, I want {{ obj.__invoke() }} to work, it doesn't since twig cannot find invoke method on closure as it relies on get_class_methods internally.

Sorry, my mistake then. I tried an object implementing __invoke() and that worked well. I've just tried a \Closure and you're right. More than happy to review a PR on that!

@fabpot fabpot reopened this Dec 10, 2024
faizanakram99 added a commit to faizanakram99/Twig that referenced this issue Dec 12, 2024
@faizanakram99 faizanakram99 linked a pull request Dec 12, 2024 that will close this issue
@faizanakram99
Copy link
Author

faizanakram99 commented Dec 12, 2024

Sorry, my mistake then. I tried an object implementing __invoke() and that worked well. I've just tried a \Closure and you're right. More than happy to review a PR on that!

Thank you, submitted a PR #4501

faizanakram99 added a commit to faizanakram99/Twig that referenced this issue Dec 12, 2024
faizanakram99 added a commit to faizanakram99/Twig that referenced this issue Dec 12, 2024
- fixes twigphp#3848
- adds changelog entry
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

5 participants