Skip to content

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

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

Merged
merged 1 commit into from
Aug 21, 2019

Conversation

fabpot
Copy link
Member

@fabpot fabpot commented Aug 21, 2019

Q A
Branch? 4.4
Bug fix? yes-ish
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets refs #33039
License MIT
Doc PR n/a

Classes 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).

@yceruto
Copy link
Member

yceruto commented Aug 21, 2019

There are now some remaining deprecation warnings from SecurityBundle, FB and TwigBridge related to Symfony\Bridge\Twig\TokenParser\TransTokenParser.

@fabpot
Copy link
Member Author

fabpot commented Aug 21, 2019

@yceruto "fixed". I don't see why TransChoiceTokenParser extends TransTokenParser (as no code is shared). I made it extend AbstractExtension instead.

@fabpot fabpot force-pushed the twig-bridge-final-extensions branch from 2bb1e91 to 9d37851 Compare August 21, 2019 07:11
@fabpot fabpot force-pushed the twig-bridge-final-extensions branch from 9d37851 to d657459 Compare August 21, 2019 07:28
fabpot added a commit that referenced this pull request Aug 21, 2019
…(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     <!-- see https://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/a

Classes 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
@fabpot fabpot merged commit d657459 into symfony:4.4 Aug 21, 2019
@fabpot fabpot deleted the twig-bridge-final-extensions branch September 7, 2019 12:04
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.4 Oct 27, 2019
This was referenced Nov 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants