I occasionally test my projects against dev branches of the libraries I use. It's probably a bit early for this PR, but since I had to patch Symfony anyway in order to test Twig 3 in my projects, I though I might as well share the branch with you.

BC break: I had to add return types to several non-final classes of the Twig bridge. This will break applications that override these classes without adding the return types.

This PR is marked as a draft because Twig 3 is still a moving target.

carsonbot added Status: Needs Review TwigBridge Feature BC Break labels

Aug 8, 2019

derrabus force-pushed theimprovement/twig-3 branch 2 times, most recently from4bd824f to0ea6aabCompare

August 8, 2019 09:14

nicolas-grekas added Status: Needs Work and removed Status: Needs Review labels

Aug 8, 2019

chalasr added this to the5.0 milestone

Aug 9, 2019

derrabus force-pushed theimprovement/twig-3 branch from0ea6aab toaf7d80bCompare

August 13, 2019 15:25

derrabus mentioned this pull request

Aug 19, 2019

Add return-types with help from DebugClassLoader in the CI#33236

Merged

63 tasks

fabpot mentioned this pull request

Aug 21, 2019

[TwigBridge] Mark all classes extending twig as @final#33269

Merged

Copy link

Member

fabpot commentedAug 21, 2019

See#33269

Copy link

Member

fabpot commentedAug 21, 2019•
edited
Loading

Now that#33269 has been merged, I think we can finish this one@derrabus. Twig 3.0 will be releasedbefore Symfony 5, so this is safe and will allow us to spot issues sooner than later. Thank you.

fabpot added a commit that referenced this pull request

Aug 21, 2019

feature#33269[TwigBridge] Mark all classes extending twig as@Final…

451daa0

…(fabpot)This PR was merged into the 4.4 branch.Discussion----------[TwigBridge] Mark all classes extending twig as@Final| Q             | A| ------------- | ---| Branch?       | 4.4| Bug fix?      | yes-ish| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->| BC breaks?    | no     <!-- seehttps://symfony.com/bc -->| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->| Fixed tickets | refs#33039| License       | MIT| Doc PR        | n/aClasses defining extensions/nodes/node visitors/token parsers should not be changed. They should be final.That would also help with Twig 3.0 which introduces type hints (including return types).Commits-------d657459 [TwigBridge] Mark all classes extending twig as@Final

derrabus force-pushed theimprovement/twig-3 branch fromaf7d80b to6545600Compare

August 21, 2019 08:48

derrabus marked this pull request as ready for review

August 21, 2019 08:48

derrabus requested review fromlyrixx andxabbuh ascode owners

August 21, 2019 08:48

carsonbot added Status: Needs Review and removed Status: Needs Work labels

Aug 21, 2019

derrabus force-pushed theimprovement/twig-3 branch from6545600 to6e108b1Compare

August 21, 2019 08:50

derrabus commented

Aug 21, 2019

View reviewed changes

src/Symfony/Bridge/Twig/Form/TwigRendererEngine.phpShow resolvedHide resolved

derrabus force-pushed theimprovement/twig-3 branch from6e108b1 to9447ccfCompare

August 21, 2019 08:57

Copy link

MemberAuthor

derrabus commentedAug 21, 2019

Ready.

Also, I'm wondering if we should backport the composer.json changes (except for the bridge, of course) to 4.4. An Symfony 4.4 application with TwigBridge 5.0 should technically be able to run Twig 3.

Copy link

Member

fabpot commentedAug 21, 2019

Symfony 4.4 still supports 1.x, so it cannot support 1.x/2.x and 3.0 at the same time.

derrabus mentioned this pull request

Aug 21, 2019

Removed calls to Twig\Environment::loadTemplate()#33273

Merged

derrabus force-pushed theimprovement/twig-3 branch 2 times, most recently from81b4cf1 tob3b7570Compare

August 21, 2019 11:46

fabpot added a commit that referenced this pull request

Aug 21, 2019

bug#33273Removed calls to Twig\Environment::loadTemplate() (derrabus)

c9c3d66

This PR was merged into the 4.4 branch.Discussion----------Removed calls to Twig\Environment::loadTemplate()| Q             | A| ------------- | ---| Branch?       | 4.4| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | N/A| License       | MIT| Doc PR        | N/AThis PR prepares#33039. Twig 3 does not have the `loadTemplate()` anymore, so this PR replaces calls to that method.Commits-------ea9e375 Removed calls to Twig\Environment::loadTemplate().

derrabus force-pushed theimprovement/twig-3 branch fromb3b7570 to57f1451Compare

August 21, 2019 11:52

Copy link

Member

fabpot commentedAug 21, 2019

I've just merged 4.4 into master so that this one can be rebased.

derrabus force-pushed theimprovement/twig-3 branch from57f1451 tob050cccCompare

August 21, 2019 12:52

Copy link

Member

nicolas-grekas commentedAug 21, 2019

We should also replace all@final since Symfony 4.4 by@final here.

Copy link

MemberAuthor

derrabus commentedAug 21, 2019

Rebased.

Support for Twig 3.

f30edca

derrabus force-pushed theimprovement/twig-3 branch fromb050ccc tof30edcaCompare

August 21, 2019 12:56

Copy link

MemberAuthor

derrabus commentedAug 21, 2019

We should also replace all@final since Symfony 4.4 by@final here.

Done.

nicolas-grekas reviewed

Aug 21, 2019

View reviewed changes

src/Symfony/Bridge/Twig/Extension/YamlExtension.php OutdatedShow resolvedHide resolved

fabpot reviewed

Aug 21, 2019

View reviewed changes

src/Symfony/Bridge/Twig/Extension/AssetExtension.php OutdatedShow resolvedHide resolved

Copy link

MemberAuthor

derrabus commentedAug 21, 2019

Should I also remove thegetName() implementations from the Twig extensions? IIRC, we only need them for Twig 1 which has been dropped on master. Do we use them for anything else?

Copy link

MemberAuthor

derrabus commentedAug 21, 2019

Also, I will add return type declarations to all@finalclasses where possible.

Copy link

Member

fabpot commentedAug 21, 2019

getName() can be removed

Finalized all@final classes.

44ed90c

Copy link

MemberAuthor

derrabus commentedAug 21, 2019

Ready.

fabpot approved these changes

Aug 21, 2019

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels

Aug 21, 2019

Copy link

Member

fabpot commentedAug 21, 2019

Thank you@derrabus.

fabpot added a commit that referenced this pull request

Aug 21, 2019

minor#33039[TwigBridge] Support for Twig 3 (derrabus)

4fc0504

This PR was squashed before being merged into the 5.0-dev branch (closes#33039).Discussion----------[TwigBridge] Support for Twig 3| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| BC breaks?    | yes| Deprecations? | no| Tests pass?   | yes| Fixed tickets | N/A| License       | MIT| Doc PR        | N/AI occasionally test my projects against dev branches of the libraries I use. It's probably a bit early for this PR, but since I had to patch Symfony anyway in order to test Twig 3 in my projects, I though I might as well share the branch with you.BC break: I had to add return types to several non-final classes of the Twig bridge. This will break applications that override these classes without adding the return types.This PR is marked as a draft because Twig 3 is still a moving target.Commits-------44ed90c Finalized all `@final` classes.f30edca Support for Twig 3.