Add better support for Twig coding conventions

[elnur]

As far as I understand, it's idiomatic to use underscored variables and functions in Twig:

{{ foo_bar }}
{{ foo|number_format }}

If that's the case, Twig should also support accessing object properties with this style:

{{ that_user.full_name }}

But currently, it fails and forces me to use camel case:

{{ that_user.fullName }}

So I'm forced to mix both PHP/Symfony and Twig conventions in my templates and I don't like it.

[stof]

The drawback here is that the logic reading the attributes for this system is already the major performance bottleneck in Twig (which is why it has been reimplemented in C in an extension, which provides only this logic). I'm not sure adding even more logic to each of these calls is a good idea.

[linaori]

It might be overkill, but what about allowing typehints in twig? Sure, compilation might be slower, but that happens only once anyway.

[stof]

I don't understand what you mean

[linaori]

It's already possible for phpstorm to do type inspections based on the action and I think this is also possible: {# @var user Some\User #} to provide auto-complete.

What if we could do that following to optimize the code twig compiles to?

{% type user Vendor\Some\User %}
{{ user.username }}

// would compile to because we know the class+methods
echo $user->getUsername();

Not sure what the compiled code looks like, I haven't really checked. I kinda hope that it will cache away how it "get"-s the value.

[elnur]

It's sad that performance concerns don't let us make Twig more consistent. Is there no way to make this work and somehow optimize the performance? Why can't we make a newer version of the C extension to support this too?

[stof]

@iltar you can write a Twig extension achieving this actually kind of optimized output when knowing the type, by adding this {% type %} and then replacing the GetAttrNode by its own node when it can be 100% sure about the variable type (but this is hard as any block defines a new scope which can be called from a child template after replacing the variable so the usage of the strictly typed variables becomes cumbersome)

But things go harder when doing things like foo.bar.baz. Twig won't know the type of foo.bar here, so you cannot optimize it at compile time. And such strict typing would still be optional.

@elnur if we add the feature, updating the C extension will be a requirement anyway (otherwise you won't be able to get a green build on Travis for tests covering your new feature). But this does not change the fact that it would require some extra string manipulation for any call (for the first usage of this name for a given class at least), be it in C or PHP. And not anyone is running the C extension.

[hason]

See #1299.

[lyrixx]

@elnur I totally understand your feeling, but if we do that, it means it's going to be harder to refactor an object property. Lets say you want to replace User::firstName to User::foobar (stupid example), you will need to grep in you code firstName, getFirstName and now first_name. More over it implies lot's of magic.

So even if the current solution is not perfect, I'm 👎 with the RFC.

[elnur]

@lyrixx, by that logic, why make underscored names a Twig idiom then? I wouldn't mind if it was camel case everywhere. What I don't like is inconsistency.

[stof]

@elnur the Symfony convention for array keys is to use underscores though. So you would still have mismatches even if Twig variables themselves were camelCased.

[elnur]

It'd be best to support both in Twig then. Would that be a huge performance hit?

[linaori]

Is there a performance reason it's not using the property accessor?

[lyrixx]

@iltar At the begining, twig supported PHP 5.2, and property accessor did not exist. Then, property accessor land into symfony, but it's PHP 5.3+ code ; so it could not be used.

[linaori]

Would it be an idea to start using it now?

[stof]

@iltar what are you talking about when saying he property accessor ?

@lyrixx understood it as the Symfony PropertyAccess component AFAICT. Here are the reason why it cannot be used:

• using array access uses a different syntax in PropertyAccess. it requires to enclose the key in brackets
• this would probably decrease performance because the PropertyAccess component is about accessing deep path in an object graph and so it has to parse the string path each time to its internal format. But Twig always accesses things one level deep only with its . operator (and trying to combine chained GetAttrNode would be a pain to manage, especially when combining them with things like the default filter, the sandbox feature or with some other kind of expressions, and I'm not sure it would be worth the additional complexity)
• this discussion was about compiling the accessor to a faster solution. I don't see how replacing $this->getAttribute($foo, 'bar') with $this->propertyAccessor->getValue($foo, 'bar') will be better here. This is not what the discussion was about.

So I don't see the benefit of using them. It solves a different issue than the issue faced by the GetAttrNode in Twig (even though there are some common points)