deprecated the possibility to define a block in a non-capturing block from a child template

CHANGELOG

@@ -1,5 +1,6 @@
 * 2.5.0 (2018-XX-XX)
 
+ * deprecated the possibility to define a block in a non-capturing block in a child template
  * added the Symfony ctype polyfill as a dependency
 
 * 2.4.8 (2018-04-02)

doc/deprecated.rst

@@ -5,6 +5,14 @@ This document lists deprecated features in Twig 2.x. Deprecated features are
 kept for backward compatibility and removed in the next major release (a
 feature that was deprecated in Twig 2.x is removed in Twig 3.0).
 
+Inheritance
+-----------
+
+* Defining a "block" definition in a non-capturing block in a child template is
+  deprecated since Twig 2.5.0. In Twig 3.0, it will throw a
+  ``Twig_Error_Syntax`` exception. It does not work anyway, so most projects
+  won't need to do anything to upgrade.
+
 Final Classes
 -------------
 

lib/Twig/Parser.php

@@ -313,7 +313,7 @@ class Twig_Parser
         return $this->stream->getCurrent();
     }
 
-    private function filterBodyNodes(Twig_Node $node)
+    private function filterBodyNodes(Twig_Node $node, $nested = false)
     {
         // check that the body does not contain non-empty output nodes
         if (
@@ -328,17 +328,30 @@ class Twig_Parser
             throw new Twig_Error_Syntax('A template that extends another one cannot include content outside Twig blocks. Did you forget to put the content inside a {% block %} tag?', $node->getTemplateLine(), $this->stream->getSourceContext());
         }
 
-        // bypass nodes that will "capture" the output
+        // bypass nodes that "capture" the output
         if ($node instanceof Twig_NodeCaptureInterface) {
+            // a "block" tag in such a node will serve as a block definition AND be displayed in place as well
             return $node;
         }
 
+        // "block" tags that are not captured (see above) are only used for defining
+        // the content of the block. In such a case, nesting it does not work as
+        // expected as the definition is not part of the default template code flow.
+        if ($nested && $node instanceof Twig_Node_BlockReference) {
+            //throw new Twig_Error_Syntax('A block definition cannot be nested under non-capturing nodes.', $node->getTemplateLine(), $this->stream->getSourceContext());
+            @trigger_error(sprintf('Nesting a block definition under a non-capturing node in "%s" at line %d is deprecated since version 2.5.0 and will become a syntax error in 3.0.', $this->stream->getSourceContext()->getName(), $node->getTemplateLine()), E_USER_DEPRECATED);
+            return;
+        }
+
         if ($node instanceof Twig_NodeOutputInterface) {
             return;
         }
 
+        // here, $nested means "being at the root level of a child template"
+        // we need to discard the wrapping "Twig_Node" for the "body" node
+        $nested = $nested || get_class($node) !== 'Twig_Node';
         foreach ($node as $k => $n) {
-            if (null !== $n && null === $this->filterBodyNodes($n)) {
+            if (null !== $n && null === $this->filterBodyNodes($n, $nested)) {
                 $node->removeNode($k);
             }
         }

lib/Twig/Test/IntegrationTestCase.php

@@ -67,18 +67,18 @@ abstract class Twig_Test_IntegrationTestCase extends TestCase
     /**
      * @dataProvider getTests
      */
-    public function testIntegration($file, $message, $condition, $templates, $exception, $outputs)
+    public function testIntegration($file, $message, $condition, $templates, $exception, $outputs, $deprecation = '')
     {
-        $this->doIntegrationTest($file, $message, $condition, $templates, $exception, $outputs);
+        $this->doIntegrationTest($file, $message, $condition, $templates, $exception, $outputs, $deprecation);
     }
 
     /**
      * @dataProvider getLegacyTests
      * @group legacy
      */
-    public function testLegacyIntegration($file, $message, $condition, $templates, $exception, $outputs)
+    public function testLegacyIntegration($file, $message, $condition, $templates, $exception, $outputs, $deprecation = '')
     {
-        $this->doIntegrationTest($file, $message, $condition, $templates, $exception, $outputs);
+        $this->doIntegrationTest($file, $message, $condition, $templates, $exception, $outputs, $deprecation);
     }
 
     public function getTests($name, $legacyTests = false)
@@ -97,23 +97,25 @@ abstract class Twig_Test_IntegrationTestCase extends TestCase
 
             $test = file_get_contents($file->getRealpath());
 
-            if (preg_match('/--TEST--\s*(.*?)\s*(?:--CONDITION--\s*(.*))?\s*((?:--TEMPLATE(?:\(.*?\))?--(?:.*?))+)\s*(?:--DATA--\s*(.*))?\s*--EXCEPTION--\s*(.*)/sx', $test, $match)) {
+            if (preg_match('/--TEST--\s*(.*?)\s*(?:--CONDITION--\s*(.*))?\s*(?:--DEPRECATION--\s*(.*?))?\s*((?:--TEMPLATE(?:\(.*?\))?--(?:.*?))+)\s*(?:--DATA--\s*(.*))?\s*--EXCEPTION--\s*(.*)/sx', $test, $match)) {
                 $message = $match[1];
                 $condition = $match[2];
-                $templates = self::parseTemplates($match[3]);
-                $exception = $match[5];
-                $outputs = array(array(null, $match[4], null, ''));
-            } elseif (preg_match('/--TEST--\s*(.*?)\s*(?:--CONDITION--\s*(.*))?\s*((?:--TEMPLATE(?:\(.*?\))?--(?:.*?))+)--DATA--.*?--EXPECT--.*/s', $test, $match)) {
+                $deprecation = $match[3];
+                $templates = self::parseTemplates($match[4]);
+                $exception = $match[6];
+                $outputs = array(array(null, $match[5], null, ''));
+            } elseif (preg_match('/--TEST--\s*(.*?)\s*(?:--CONDITION--\s*(.*))?\s*(?:--DEPRECATION--\s*(.*?))?\s*((?:--TEMPLATE(?:\(.*?\))?--(?:.*?))+)--DATA--.*?--EXPECT--.*/s', $test, $match)) {
                 $message = $match[1];
                 $condition = $match[2];
-                $templates = self::parseTemplates($match[3]);
+                $deprecation = $match[3];
+                $templates = self::parseTemplates($match[4]);
                 $exception = false;
                 preg_match_all('/--DATA--(.*?)(?:--CONFIG--(.*?))?--EXPECT--(.*?)(?=\-\-DATA\-\-|$)/s', $test, $outputs, PREG_SET_ORDER);
             } else {
                 throw new InvalidArgumentException(sprintf('Test "%s" is not valid.', str_replace($fixturesDir.'/', '', $file)));
             }
 
-            $tests[] = array(str_replace($fixturesDir.'/', '', $file), $message, $condition, $templates, $exception, $outputs);
+            $tests[] = array(str_replace($fixturesDir.'/', '', $file), $message, $condition, $templates, $exception, $outputs, $deprecation);
         }
 
         if ($legacyTests && empty($tests)) {
@@ -129,7 +131,7 @@ abstract class Twig_Test_IntegrationTestCase extends TestCase
         return $this->getTests('testLegacyIntegration', true);
     }
 
-    protected function doIntegrationTest($file, $message, $condition, $templates, $exception, $outputs)
+    protected function doIntegrationTest($file, $message, $condition, $templates, $exception, $outputs, $deprecation = '')
     {
         if (!$outputs) {
             $this->markTestSkipped('no tests to run');
@@ -176,7 +178,16 @@ abstract class Twig_Test_IntegrationTestCase extends TestCase
             $p->setAccessible(true);
             $p->setValue($twig, '__TwigTemplate_'.hash('sha256', uniqid(mt_rand(), true), false).'_');
 
+            $deprecations = array();
             try {
+                $prevHandler = set_error_handler(function ($type, $msg, $file, $line, $context = array()) use (&$deprecations, &$prevHandler) {
+                    if (E_USER_DEPRECATED === $type) {
+                        $deprecations[] = $msg;
+                        return true;
+                    }
+                    return $prevHandler ? $prevHandler($type, $msg, $file, $line, $context) : false;
+                });
+
                 $template = $twig->loadTemplate('index.twig');
             } catch (Exception $e) {
                 if (false !== $exception) {
@@ -189,8 +200,12 @@ abstract class Twig_Test_IntegrationTestCase extends TestCase
                 }
 
                 throw new Twig_Error(sprintf('%s: %s', get_class($e), $e->getMessage()), -1, $file, $e);
+            } finally {
+                restore_error_handler();
             }
 
+            $this->assertSame($deprecation, implode("\n", $deprecations));
+
             try {
                 $output = trim($template->render(eval($match[1].';')), "\n ");
             } catch (Exception $e) {

test/Twig/Tests/Fixtures/tags/block/capturing_block.test

@@ -0,0 +1,22 @@
+--TEST--
+capturing "block" tag
+--TEMPLATE--
+{% set foo %}{% block foo %}FOO{% endblock %}{% endset %}
+{{ foo }}
+{% spaceless %}
+{% block bar %}
+<h1>
+    <b>Title</b>
+</h1>
+{% endblock %}
+{% endspaceless %}
+
+{{ block('bar') }}
+--DATA--
+return array()
+--EXPECT--
+FOO
+<h1><b>Title</b></h1>
+<h1>
+    <b>Title</b>
+</h1>

test/Twig/Tests/Fixtures/tags/block/conditional_block.test

@@ -0,0 +1,9 @@
+--TEST--
+conditional "block" tag
+--TEMPLATE--
+{% if false %}{% block foo %}FOO{% endblock %}{% endif %}
+{% if true %}{% block bar %}BAR{% endblock %}{% endif %}
+--DATA--
+return array()
+--EXPECT--
+BAR

test/Twig/Tests/Fixtures/tags/inheritance/capturing_block.test

@@ -0,0 +1,17 @@
+--TEST--
+capturing "block" tag with "extends" tag
+--TEMPLATE--
+{% extends "layout.twig" %}
+
+{% set foo %}
+    {%- block content %}FOO{% endblock %}
+{% endset %}
+
+{% block content1 %}BAR{{ foo }}{% endblock %}
+--TEMPLATE(layout.twig)--
+{% block content %}{% endblock %}
+{% block content1 %}{% endblock %}
+--DATA--
+return array()
+--EXPECT--
+FOOBARFOO

test/Twig/Tests/Fixtures/tags/inheritance/conditional_block.legacy.test

@@ -0,0 +1,16 @@
+--TEST--
+conditional "block" tag with "extends" tag
+--DEPRECATION--
+Nesting a block definition under a non-capturing node in "index.twig" at line 5 is deprecated since version 2.5.0 and will become a syntax error in 3.0.
+--TEMPLATE--
+{% extends "layout.twig" %}
+
+{% if false %}
+    {% block content %}FOO{% endblock %}
+{% endif %}
+--TEMPLATE(layout.twig)--
+{% block content %}{% endblock %}
+--DATA--
+return array()
+--EXPECT--
+FOO