diff --git a/CHANGELOG b/CHANGELOG index 667418b86..a4e5c403a 100644 --- a/CHANGELOG +++ b/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) diff --git a/doc/deprecated.rst b/doc/deprecated.rst index 4c0e2b9a8..5386cb71e 100644 --- a/doc/deprecated.rst +++ b/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 ------------- diff --git a/lib/Twig/Parser.php b/lib/Twig/Parser.php index b7b9d17cb..4b8682e06 100644 --- a/lib/Twig/Parser.php +++ b/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); } } diff --git a/lib/Twig/Test/IntegrationTestCase.php b/lib/Twig/Test/IntegrationTestCase.php index ad1fb4c36..2f0323090 100644 --- a/lib/Twig/Test/IntegrationTestCase.php +++ b/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) { diff --git a/test/Twig/Tests/Fixtures/tags/block/capturing_block.test b/test/Twig/Tests/Fixtures/tags/block/capturing_block.test new file mode 100644 index 000000000..a2eba7a63 --- /dev/null +++ b/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 %} +

+ Title +

+{% endblock %} +{% endspaceless %} + +{{ block('bar') }} +--DATA-- +return array() +--EXPECT-- +FOO +

Title

+

+ Title +

diff --git a/test/Twig/Tests/Fixtures/tags/block/conditional_block.test b/test/Twig/Tests/Fixtures/tags/block/conditional_block.test new file mode 100644 index 000000000..d4e2ae009 --- /dev/null +++ b/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 diff --git a/test/Twig/Tests/Fixtures/tags/inheritance/capturing_block.test b/test/Twig/Tests/Fixtures/tags/inheritance/capturing_block.test new file mode 100644 index 000000000..703e33fd0 --- /dev/null +++ b/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 diff --git a/test/Twig/Tests/Fixtures/tags/inheritance/conditional_block.legacy.test b/test/Twig/Tests/Fixtures/tags/inheritance/conditional_block.legacy.test new file mode 100644 index 000000000..137526956 --- /dev/null +++ b/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