twigphp / Twig

Twig, the flexible, fast, and secure template language for PHP
https://twig.symfony.com/
BSD 3-Clause "New" or "Revised" License
8.18k stars 1.25k forks source link

Add types tag #4235

Closed drjayvee closed 2 months ago

drjayvee commented 2 months ago

The is a draft implementation for #4165.

drjayvee commented 2 months ago

I just force-pushed a fix to the docs

drjayvee commented 2 months ago

This PR isn't complete yet. I forgot to add a TypesTokenParser (and tests).

drjayvee commented 2 months ago

To quote my last commit message:

Move parsing of {name: 'string'} mappings to ExpressionParser

This also adds support for optional keys as discussed in https://github.com/twigphp/Twig/issues/4165.

I really don't like using a static method, but I couldn't figure out how else to call the method in tests. See the notes on the method itself.

Any help is appreciated!

drjayvee commented 2 months ago

I don't understand the CI error:

Remaining self deprecation notices (2)

  2x: Since twig/twig 3.12: The "tag" constructor argument of the "Twig\Node\TypesNode" class is deprecated and ignored (check which TokenParser class set it to "null"), the tag is now automatically set by the Parser when needed.
    1x in TypesTest::getTests from Twig\Tests\Node
    1x in TypesTest::testConstructor from Twig\Tests\Node

That test does not pass the tag argument.

Update: I also can't reproduce this on my local dev stack.

I think the problem is that the TypesNode constructor passes tag to parent::__construct(). I'll remove that and push again.

fabpot commented 2 months ago

Thank you @drjayvee.

fabpot commented 2 months ago

Not sure why Github didn't work as expected here. I'm closing but it's merged.

VincentLanglet commented 1 week ago

Hi @drjayvee,

Is there a reason to not support

{% types {
        'foo': 'bool',
        'bar'?: 'int',
    } %}

too ? Like it's done for classic objects {% set bar = {'foo': 42} %}.

ruudk commented 1 week ago

I guess we did not consider this as it looks a bit ugly for optionally. But sure, we could still accept this. Especially as it's better in line with mappings.

I do hope we can turn of the quoting for the types tag in Twig-CS-Fixer though 😉

stof commented 1 week ago

@VincentLanglet variable names need to be valid identifiers anyway to be usable, so I don't see the benefit of supporting string literals as keys there instead of only identifiers. This syntax is not an actual hash anyway (the parser does not allow arbitrary expressions)