Twig icon indicating copy to clipboard operation
Twig copied to clipboard

Add checks for existing callable at compilation

Open ghost opened this issue 5 years ago • 3 comments

Fixes #3450

Currently Twig does not check if the callable passed to CallExpression is actually a callable. This pull request adds does checks.

If it is a function it check if it exists. If it is an array indicating a class method, then it also checks for the presence of __call and __callStatic methods. If the callable does not exist a \LogicException is thrown. (I don't know if this is the appropriate Exception).

I think Twig should check for the existence of callables if it can otherwise it can be quite cumbersome to debug when using caching, since it is possible a wrong path will be compiled. (This happened to me.)

Some unrelated Function/Filter tests broke because they were passing undefined callables so I fixed them. However, the intent was clear to pass an existing callable.

ghost avatar Dec 01 '20 12:12 ghost

@fabpot do you have an opinion on this pull request?

ghost avatar Dec 14 '20 09:12 ghost

Sorry for pinging again. Is there anything else I have to do to get this PR merged? Or are these checks not desired? @stof @fabpot

ghost avatar Dec 20 '20 07:12 ghost

@stof @fabpot do I have to do anything else?

ghost avatar Jan 14 '21 11:01 ghost