From 49ee8d6e41bf94c1b8532181878182fcbc3a7471 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Sat, 17 Mar 2012 18:12:19 +0100 Subject: [PATCH] made the strategy used to guess the real template file name and line number in exception messages much faster and more accurate (refs #647) --- CHANGELOG | 2 +- lib/Twig/Compiler.php | 9 ++++ lib/Twig/Error.php | 63 +++++++++----------------- lib/Twig/Node/Module.php | 13 +++++ test/Twig/Tests/ErrorTest.php | 32 +++++++++++-- test/Twig/Tests/Extension/SandboxTest.php | 2 +- test/Twig/Tests/Node/ModuleTest.php | 14 ++++++ test/Twig/Tests/Node/SandboxedModuleTest.php | 9 ++++ test/Twig/Tests/TemplateTest.php | 5 ++ 9 files changed, 100 insertions(+), 49 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index baed766..da3e3fb 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,6 @@ * 1.7.0 (2012-XX-XX) - * n/a + * made the strategy used to guess the real template file name and line number in exception messages much faster and more accurate * 1.6.2 (2012-03-18) diff --git a/lib/Twig/Compiler.php b/lib/Twig/Compiler.php index 0cce6a4..149a5a0 100644 --- a/lib/Twig/Compiler.php +++ b/lib/Twig/Compiler.php @@ -22,6 +22,7 @@ class Twig_Compiler implements Twig_CompilerInterface protected $source; protected $indentation; protected $env; + protected $debugInfo; /** * Constructor. @@ -31,6 +32,7 @@ class Twig_Compiler implements Twig_CompilerInterface public function __construct(Twig_Environment $env) { $this->env = $env; + $this->debugInfo = array(); } /** @@ -178,6 +180,8 @@ class Twig_Compiler implements Twig_CompilerInterface public function addDebugInfo(Twig_NodeInterface $node) { if ($node->getLine() != $this->lastLine) { + $this->debugInfo[substr_count($this->source, "\n")] = $node->getLine(); + $this->lastLine = $node->getLine(); $this->write("// line {$node->getLine()}\n"); } @@ -185,6 +189,11 @@ class Twig_Compiler implements Twig_CompilerInterface return $this; } + public function getDebugInfo() + { + return $this->debugInfo; + } + /** * Indents the generated code. * diff --git a/lib/Twig/Error.php b/lib/Twig/Error.php index 8c1c54b..77418f4 100644 --- a/lib/Twig/Error.php +++ b/lib/Twig/Error.php @@ -33,7 +33,15 @@ class Twig_Error extends Exception public function __construct($message, $lineno = -1, $filename = null, Exception $previous = null) { if (-1 === $lineno || null === $filename) { - list($lineno, $filename) = $this->findTemplateInfo(null !== $previous ? $previous : $this, $lineno, $filename); + if ($trace = $this->getTemplateTrace()) { + if (-1 === $lineno) { + $lineno = $this->guessTemplateLine($trace); + } + + if (null === $filename) { + $filename = $trace['object']->getTemplateName(); + } + } } $this->lineno = $lineno; @@ -144,52 +152,23 @@ class Twig_Error extends Exception } } - protected function findTemplateInfo(Exception $e, $currentLine, $currentFile) + protected function getTemplateTrace() { - if (!function_exists('token_get_all')) { - return array($currentLine, $currentFile); - } - - $traces = $e->getTrace(); - foreach ($traces as $i => $trace) { - if (!isset($trace['class']) || 'Twig_Template' === $trace['class']) { - continue; - } - - $r = new ReflectionClass($trace['class']); - if (!$r->implementsInterface('Twig_TemplateInterface')) { - continue; - } - - if (!is_file($r->getFilename())) { - // probably an eval()'d code - return array($currentLine, $currentFile); + foreach (debug_backtrace() as $trace) { + if (isset($trace['object']) && $trace['object'] instanceof Twig_Template) { + return $trace; } + } + } - if (0 === $i) { - $line = $e->getLine(); - } else { - $line = isset($traces[$i - 1]['line']) ? $traces[$i - 1]['line'] : -log(0); - } - - $tokens = token_get_all(file_get_contents($r->getFilename())); - $templateline = -1; - $template = null; - foreach ($tokens as $token) { - if (isset($token[2]) && $token[2] >= $line) { - return array($templateline, $template); - } - - if (T_COMMENT === $token[0] && null === $template && preg_match('#/\* +(.+) +\*/#', $token[1], $match)) { - $template = $match[1]; - } elseif (T_COMMENT === $token[0] && preg_match('#^//\s*line (\d+)\s*$#', $token[1], $match)) { - $templateline = $match[1]; - } + protected function guessTemplateLine($trace) + { + foreach ($trace['object']->getDebugInfo() as $codeLine => $templateLine) { + if ($codeLine <= $trace['line']) { + return $templateLine; } - - return array($currentLine, $template); } - return array($currentLine, $currentFile); + return -1; } } diff --git a/lib/Twig/Node/Module.php b/lib/Twig/Node/Module.php index 349ba68..8585935 100644 --- a/lib/Twig/Node/Module.php +++ b/lib/Twig/Node/Module.php @@ -57,6 +57,8 @@ class Twig_Node_Module extends Twig_Node $this->compileIsTraitable($compiler); + $this->compileDebugInfo($compiler); + $this->compileClassFooter($compiler); } @@ -294,6 +296,17 @@ class Twig_Node_Module extends Twig_Node ->indent() ->write(sprintf("return %s;\n", $traitable ? 'true' : 'false')) ->outdent() + ->write("}\n\n") + ; + } + + public function compileDebugInfo(Twig_Compiler $compiler) + { + $compiler + ->write("public function getDebugInfo()\n", "{\n") + ->indent() + ->write(sprintf("return %s;\n", str_replace("\n", '', var_export(array_reverse($compiler->getDebugInfo(), true), true)))) + ->outdent() ->write("}\n") ; } diff --git a/test/Twig/Tests/ErrorTest.php b/test/Twig/Tests/ErrorTest.php index 7c065b9..a63b5fa 100644 --- a/test/Twig/Tests/ErrorTest.php +++ b/test/Twig/Tests/ErrorTest.php @@ -1,7 +1,5 @@ "\n\n{{ foo.bar }}")); - $twig = new Twig_Environment($loader, array('strict_variables' => true, 'debug' => true, 'cache' => $this->getTempDir())); + $twig = new Twig_Environment($loader, array('strict_variables' => true, 'debug' => true, 'cache' => false)); $template = $twig->loadTemplate('index'); @@ -34,7 +32,7 @@ class Twig_Tests_ErrorTest extends Twig_Tests_TestCase public function testRenderWrapsExceptions() { $loader = new Twig_Loader_Array(array('index' => "\n\n\n{{ foo.bar }}")); - $twig = new Twig_Environment($loader, array('strict_variables' => true, 'debug' => true, 'cache' => $this->getTempDir())); + $twig = new Twig_Environment($loader, array('strict_variables' => true, 'debug' => true, 'cache' => false)); $template = $twig->loadTemplate('index'); @@ -48,6 +46,30 @@ class Twig_Tests_ErrorTest extends Twig_Tests_TestCase $this->assertEquals('index', $e->getTemplateFile()); } } + + public function testTwigExceptionAddsFileAndLineWhenMissingWithInheritance() + { + $loader = new Twig_Loader_Array(array( + 'index' => "{% extends 'base' %} + {% block content %} + {{ foo.bar }} + {% endblock %}", + 'base' => '{% block content %}{% endblock %}' + )); + $twig = new Twig_Environment($loader, array('strict_variables' => true, 'debug' => true, 'cache' => false)); + + $template = $twig->loadTemplate('index'); + + try { + $template->render(array()); + + $this->fail(); + } catch (Twig_Error_Runtime $e) { + $this->assertEquals('Variable "foo" does not exist in "index" at line 3', $e->getMessage()); + $this->assertEquals(3, $e->getTemplateLine()); + $this->assertEquals('index', $e->getTemplateFile()); + } + } } class Twig_Tests_ErrorTest_Foo diff --git a/test/Twig/Tests/Extension/SandboxTest.php b/test/Twig/Tests/Extension/SandboxTest.php index abb7aa7..0768c65 100644 --- a/test/Twig/Tests/Extension/SandboxTest.php +++ b/test/Twig/Tests/Extension/SandboxTest.php @@ -38,7 +38,7 @@ class Twig_Tests_Extension_SandboxTest extends PHPUnit_Framework_TestCase /** * @expectedException Twig_Sandbox_SecurityError - * @expectedExceptionMessage Filter "json_encode" is not allowed. + * @expectedExceptionMessage Filter "json_encode" is not allowed in "1_child". */ public function testSandboxWithInheritance() { diff --git a/test/Twig/Tests/Node/ModuleTest.php b/test/Twig/Tests/Node/ModuleTest.php index 9ad3b26..6ca4087 100644 --- a/test/Twig/Tests/Node/ModuleTest.php +++ b/test/Twig/Tests/Node/ModuleTest.php @@ -79,6 +79,10 @@ class __TwigTemplate_be925a7b06dda0dfdbd18a1509f7eb34 extends Twig_Template return "foo.twig"; } + public function getDebugInfo() + { + return array (); + } } EOF , $twig); @@ -115,6 +119,11 @@ class __TwigTemplate_be925a7b06dda0dfdbd18a1509f7eb34 extends Twig_Template { return false; } + + public function getDebugInfo() + { + return array (); + } } EOF , $twig); @@ -153,6 +162,11 @@ class __TwigTemplate_be925a7b06dda0dfdbd18a1509f7eb34 extends Twig_Template { return false; } + + public function getDebugInfo() + { + return array (); + } } EOF , $twig); diff --git a/test/Twig/Tests/Node/SandboxedModuleTest.php b/test/Twig/Tests/Node/SandboxedModuleTest.php index f1966be..1ed141b 100644 --- a/test/Twig/Tests/Node/SandboxedModuleTest.php +++ b/test/Twig/Tests/Node/SandboxedModuleTest.php @@ -86,6 +86,10 @@ class __TwigTemplate_be925a7b06dda0dfdbd18a1509f7eb34 extends Twig_Template return "foo.twig"; } + public function getDebugInfo() + { + return array (); + } } EOF , $twig); @@ -134,6 +138,11 @@ class __TwigTemplate_be925a7b06dda0dfdbd18a1509f7eb34 extends Twig_Template { return false; } + + public function getDebugInfo() + { + return array (); + } } EOF , $twig); diff --git a/test/Twig/Tests/TemplateTest.php b/test/Twig/Tests/TemplateTest.php index e9caad9..a4e658f 100644 --- a/test/Twig/Tests/TemplateTest.php +++ b/test/Twig/Tests/TemplateTest.php @@ -211,6 +211,11 @@ class Twig_TemplateTest extends Twig_Template { } + public function getDebugInfo() + { + return array(); + } + protected function doGetParent(array $context) { } -- 1.7.2.5