feature #20093 Twig extensions refatoring to decouple definitions fro…

…m implementations (fabpot)

This PR was merged into the 3.2-dev branch.

Discussion
----------

Twig extensions refatoring to decouple definitions from implementations

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | see twigphp/Twig#1913
| License       | MIT
| Doc PR        | not yet

This PR tries to use the new Twig runtime loader to the Twig bridge extensions.

Commits
-------

b515702 fixed circular reference in Twig Form integration
4b5e412 removed on indirection
c07fc58 [TwigBridge] decoupled Form extension definitions from its runtime

src/Symfony/Bridge/Twig/CHANGELOG.md

@@ -1,6 +1,12 @@
 CHANGELOG
 =========
 
+3.2.0
+-----
+
+ * Deprecated the possibility to inject the Form Twig Renderer into the form
+   extension. Inject it on TwigRendererEngine instead.
+
 2.7.0
 -----
 

src/Symfony/Bridge/Twig/Extension/FormExtension.php

@@ -23,25 +23,26 @@
  */
 class FormExtension extends \Twig_Extension implements \Twig_Extension_InitRuntimeInterface
 {
-    /**
-     * This property is public so that it can be accessed directly from compiled
-     * templates without having to call a getter, which slightly decreases performance.
-     *
-     * @var TwigRendererInterface
-     */
-    public $renderer;
+    private $renderer;
 
-    public function __construct(TwigRendererInterface $renderer)
+    public function __construct(TwigRendererInterface $renderer = null)
     {
+        if (null !== $this->renderer) {
+            @trigger_error(sprintf('Passing a Twig Form Renderer to the "%s" constructor is deprecated since version 3.2 and won\'t be possible in 4.0. Pass the Twig_Environment to the TwigRendererEngine constructor instead.', static::class), E_USER_DEPRECATED);
+        }
         $this->renderer = $renderer;
     }
 
     /**
      * {@inheritdoc}
+     *
+     * To be removed in 4.0
      */
     public function initRuntime(\Twig_Environment $environment)
     {
-        $this->renderer->setEnvironment($environment);
+        if (null !== $this->renderer) {
+            $this->renderer->setEnvironment($environment);
+        }
     }
 
     /**
@@ -69,7 +70,7 @@ public function getFunctions()
             new \Twig_SimpleFunction('form', null, array('node_class' => 'Symfony\Bridge\Twig\Node\RenderBlockNode', 'is_safe' => array('html'))),
             new \Twig_SimpleFunction('form_start', null, array('node_class' => 'Symfony\Bridge\Twig\Node\RenderBlockNode', 'is_safe' => array('html'))),
             new \Twig_SimpleFunction('form_end', null, array('node_class' => 'Symfony\Bridge\Twig\Node\RenderBlockNode', 'is_safe' => array('html'))),
-            new \Twig_SimpleFunction('csrf_token', array($this, 'renderCsrfToken')),
+            new \Twig_SimpleFunction('csrf_token', array('Symfony\Bridge\Twig\Form\TwigRenderer', 'renderCsrfToken')),
         );
     }
 
@@ -79,7 +80,7 @@ public function getFunctions()
     public function getFilters()
     {
         return array(
-            new \Twig_SimpleFilter('humanize', array($this, 'humanize')),
+            new \Twig_SimpleFilter('humanize', array('Symfony\Bridge\Twig\Form\TwigRenderer', 'humanize')),
         );
     }
 
@@ -89,67 +90,35 @@ public function getFilters()
     public function getTests()
     {
         return array(
-            new \Twig_SimpleTest('selectedchoice', array($this, 'isSelectedChoice')),
+            new \Twig_SimpleTest('selectedchoice', 'Symfony\Bridge\Twig\Extension\twig_is_selected_choice'),
         );
     }
 
     /**
      * {@inheritdoc}
      */
-    public function renderCsrfToken($tokenId)
-    {
-        return $this->renderer->renderCsrfToken($tokenId);
-    }
-
-    /**
-     * Makes a technical name human readable.
-     *
-     * @param string $text The text to humanize
-     *
-     * @return string The humanized text
-     */
-    public function humanize($text)
+    public function getName()
     {
-        return $this->renderer->humanize($text);
+        return 'form';
     }
+}
 
-    /**
-     * Returns whether a choice is selected for a given form value.
-     *
-     * Unfortunately Twig does not support an efficient way to execute the
-     * "is_selected" closure passed to the template by ChoiceType. It is faster
-     * to implement the logic here (around 65ms for a specific form).
-     *
-     * Directly implementing the logic here is also faster than doing so in
-     * ChoiceView (around 30ms).
-     *
-     * The worst option tested so far is to implement the logic in ChoiceView
-     * and access the ChoiceView method directly in the template. Doing so is
-     * around 220ms slower than doing the method call here in the filter. Twig
-     * seems to be much more efficient at executing filters than at executing
-     * methods of an object.
-     *
-     * @param ChoiceView   $choice        The choice to check
-     * @param string|array $selectedValue The selected value to compare
-     *
-     * @return bool Whether the choice is selected
-     *
-     * @see ChoiceView::isSelected()
-     */
-    public function isSelectedChoice(ChoiceView $choice, $selectedValue)
-    {
-        if (is_array($selectedValue)) {
-            return in_array($choice->value, $selectedValue, true);
-        }
-
-        return $choice->value === $selectedValue;
+/**
+ * Returns whether a choice is selected for a given form value.
+ *
+ * This is a function and not callable due to performance reasons.
+ *
+ * @param string|array $selectedValue The selected value to compare
+ *
+ * @return bool Whether the choice is selected
+ *
+ * @see ChoiceView::isSelected()
+ */
+function twig_is_selected_choice(ChoiceView $choice, $selectedValue)
+{
+    if (is_array($selectedValue)) {
+        return in_array($choice->value, $selectedValue, true);
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function getName()
-    {
-        return 'form';
-    }
+    return $choice->value === $selectedValue;
 }

src/Symfony/Bridge/Twig/Form/TwigRendererEngine.php

@@ -29,6 +29,16 @@ class TwigRendererEngine extends AbstractRendererEngine implements TwigRendererE
      */
     private $template;
 
+    public function __construct(array $defaultThemes = array(), \Twig_Environment $environment = null)
+    {
+        if (null === $environment) {
+            @trigger_error(sprintf('Not passing a Twig Environment as the second argument for "%s" constructor is deprecated since version 3.2 and won\'t be possible in 4.0.', static::class), E_USER_DEPRECATED);
+        }
+
+        parent::__construct($defaultThemes);
+        $this->environment = $environment;
+    }
+
     /**
      * {@inheritdoc}
      */

src/Symfony/Bridge/Twig/Form/TwigRendererEngineInterface.php

@@ -15,6 +15,8 @@
 
 /**
  * @author Bernhard Schussek <bschussek@gmail.com>
+ *
+ * @deprecated Deprecated since version 3.2, to be removed in 4.0.
  */
 interface TwigRendererEngineInterface extends FormRendererEngineInterface
 {

src/Symfony/Bridge/Twig/Form/TwigRendererInterface.php

@@ -15,6 +15,8 @@
 
 /**
  * @author Bernhard Schussek <bschussek@gmail.com>
+ *
+ * @deprecated Deprecated since version 3.2, to be removed in 4.0.
  */
 interface TwigRendererInterface extends FormRendererInterface
 {

src/Symfony/Bridge/Twig/Node/FormThemeNode.php

@@ -30,7 +30,7 @@ public function compile(\Twig_Compiler $compiler)
     {
         $compiler
             ->addDebugInfo($this)
-            ->write('$this->env->getExtension(\'Symfony\Bridge\Twig\Extension\FormExtension\')->renderer->setTheme(')
+            ->write('$this->env->getRuntime(\'Symfony\Bridge\Twig\Form\TwigRenderer\')->setTheme(')
             ->subcompile($this->getNode('form'))
             ->raw(', ')
             ->subcompile($this->getNode('resources'))

src/Symfony/Bridge/Twig/Node/RenderBlockNode.php

@@ -25,7 +25,7 @@ public function compile(\Twig_Compiler $compiler)
     {
         $compiler->addDebugInfo($this);
         $arguments = iterator_to_array($this->getNode('arguments'));
-        $compiler->write('$this->env->getExtension(\'Symfony\Bridge\Twig\Extension\FormExtension\')->renderer->renderBlock(');
+        $compiler->write('$this->env->getRuntime(\'Symfony\Bridge\Twig\Form\TwigRenderer\')->renderBlock(');
 
         if (isset($arguments[0])) {
             $compiler->subcompile($arguments[0]);

src/Symfony/Bridge/Twig/Node/SearchAndRenderBlockNode.php

@@ -19,7 +19,7 @@ class SearchAndRenderBlockNode extends \Twig_Node_Expression_Function
     public function compile(\Twig_Compiler $compiler)
     {
         $compiler->addDebugInfo($this);
-        $compiler->raw('$this->env->getExtension(\'Symfony\Bridge\Twig\Extension\FormExtension\')->renderer->searchAndRenderBlock(');
+        $compiler->raw('$this->env->getRuntime(\'Symfony\Bridge\Twig\Form\TwigRenderer\')->searchAndRenderBlock(');
 
         preg_match('/_([^_]+)$/', $this->getAttribute('name'), $matches);
 

src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionBootstrap3HorizontalLayoutTest.php

@@ -22,49 +22,38 @@
 
 class FormExtensionBootstrap3HorizontalLayoutTest extends AbstractBootstrap3HorizontalLayoutTest
 {
-    /**
-     * @var FormExtension
-     */
-    protected $extension;
+    use RuntimeLoaderProvider;
 
     protected $testableFeatures = array(
         'choice_attr',
     );
 
+    private $renderer;
+
     protected function setUp()
     {
         parent::setUp();
 
-        $rendererEngine = new TwigRendererEngine(array(
-            'bootstrap_3_horizontal_layout.html.twig',
-            'custom_widgets.html.twig',
-        ));
-        $renderer = new TwigRenderer($rendererEngine, $this->getMock('Symfony\Component\Security\Csrf\CsrfTokenManagerInterface'));
-
-        $this->extension = new FormExtension($renderer);
-
         $loader = new StubFilesystemLoader(array(
             __DIR__.'/../../Resources/views/Form',
             __DIR__.'/Fixtures/templates/form',
         ));
 
         $environment = new \Twig_Environment($loader, array('strict_variables' => true));
         $environment->addExtension(new TranslationExtension(new StubTranslator()));
-        $environment->addExtension($this->extension);
+        $environment->addExtension(new FormExtension());
 
-        $this->extension->initRuntime($environment);
-    }
-
-    protected function tearDown()
-    {
-        parent::tearDown();
-
-        $this->extension = null;
+        $rendererEngine = new TwigRendererEngine(array(
+            'bootstrap_3_horizontal_layout.html.twig',
+            'custom_widgets.html.twig',
+        ), $environment);
+        $this->renderer = new TwigRenderer($rendererEngine, $this->getMock('Symfony\Component\Security\Csrf\CsrfTokenManagerInterface'));
+        $this->registerTwigRuntimeLoader($environment, $this->renderer);
     }
 
     protected function renderForm(FormView $view, array $vars = array())
     {
-        return (string) $this->extension->renderer->renderBlock($view, 'form', $vars);
+        return (string) $this->renderer->renderBlock($view, 'form', $vars);
     }
 
     protected function renderLabel(FormView $view, $label = null, array $vars = array())
@@ -73,41 +62,41 @@ protected function renderLabel(FormView $view, $label = null, array $vars = arra
             $vars += array('label' => $label);
         }
 
-        return (string) $this->extension->renderer->searchAndRenderBlock($view, 'label', $vars);
+        return (string) $this->renderer->searchAndRenderBlock($view, 'label', $vars);
     }
 
     protected function renderErrors(FormView $view)
     {
-        return (string) $this->extension->renderer->searchAndRenderBlock($view, 'errors');
+        return (string) $this->renderer->searchAndRenderBlock($view, 'errors');
     }
 
     protected function renderWidget(FormView $view, array $vars = array())
     {
-        return (string) $this->extension->renderer->searchAndRenderBlock($view, 'widget', $vars);
+        return (string) $this->renderer->searchAndRenderBlock($view, 'widget', $vars);
     }
 
     protected function renderRow(FormView $view, array $vars = array())
     {
-        return (string) $this->extension->renderer->searchAndRenderBlock($view, 'row', $vars);
+        return (string) $this->renderer->searchAndRenderBlock($view, 'row', $vars);
     }
 
     protected function renderRest(FormView $view, array $vars = array())
     {
-        return (string) $this->extension->renderer->searchAndRenderBlock($view, 'rest', $vars);
+        return (string) $this->renderer->searchAndRenderBlock($view, 'rest', $vars);
     }
 
     protected function renderStart(FormView $view, array $vars = array())
     {
-        return (string) $this->extension->renderer->renderBlock($view, 'form_start', $vars);
+        return (string) $this->renderer->renderBlock($view, 'form_start', $vars);
     }
 
     protected function renderEnd(FormView $view, array $vars = array())
     {
-        return (string) $this->extension->renderer->renderBlock($view, 'form_end', $vars);
+        return (string) $this->renderer->renderBlock($view, 'form_end', $vars);
     }
 
     protected function setTheme(FormView $view, array $themes)
     {
-        $this->extension->renderer->setTheme($view, $themes);
+        $this->renderer->setTheme($view, $themes);
     }
 }