diff --git a/CHANGELOG b/CHANGELOG index c8de6b82d..c8b59cfa9 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -194,6 +194,8 @@ * 1.39.0 (2019-XX-XX) + * fixed some wrong location in error messages + * made exception creation faster * made escaping on ternary expressions (?: and ??) more fine-grained * added the possibility to give a nice name to string templates (template_from_string function) * fixed the "with" behavior to always include the globals (for consistency with the "include" and "embed" tags) diff --git a/src/Error/Error.php b/src/Error/Error.php index 69b9df5f9..197c003e9 100644 --- a/src/Error/Error.php +++ b/src/Error/Error.php @@ -61,7 +61,7 @@ class Error extends \Exception * @param Source|string|null $source The source context where the error occurred * @param \Exception $previous The previous exception */ - public function __construct(string $message, int $lineno = -1, $source = null, \Exception $previous = null, bool $autoGuess = true) + public function __construct(string $message, int $lineno = -1, $source = null, \Exception $previous = null) { parent::__construct('', 0, $previous); @@ -77,14 +77,8 @@ class Error extends \Exception } $this->lineno = $lineno; - $this->name = $name; - - if ($autoGuess && (-1 === $lineno || null === $name || null === $this->sourcePath)) { - $this->guessTemplateInfo(); - } - + $this->filename = $name; $this->rawMessage = $message; - $this->updateRepr(); } diff --git a/src/Error/LoaderError.php b/src/Error/LoaderError.php index ec688e15d..dc5a9f1af 100644 --- a/src/Error/LoaderError.php +++ b/src/Error/LoaderError.php @@ -14,22 +14,10 @@ namespace Twig\Error; /** * Exception thrown when an error occurs during template loading. * - * Automatic template information guessing is always turned off as - * if a template cannot be loaded, there is nothing to guess. - * However, when a template is loaded from another one, then, we need - * to find the current context and this is automatically done by - * Twig\Template::displayWithErrorHandling(). - * - * This strategy makes Twig\Environment::resolveTemplate() much faster. - * * @author Fabien Potencier */ class LoaderError extends Error { - public function __construct(string $message, int $lineno = -1, $source = null, \Exception $previous = null) - { - parent::__construct($message, $lineno, $source, $previous, false); - } } class_alias('Twig\Error\LoaderError', 'Twig_Error_Loader'); diff --git a/src/Node/Expression/CallExpression.php b/src/Node/Expression/CallExpression.php index 64de6ae4f..4ecd2c111 100644 --- a/src/Node/Expression/CallExpression.php +++ b/src/Node/Expression/CallExpression.php @@ -125,7 +125,7 @@ abstract class CallExpression extends AbstractExpression $named = true; $name = $this->normalizeName($name); } elseif ($named) { - throw new SyntaxError(sprintf('Positional arguments cannot be used after named arguments for %s "%s".', $callType, $callName), $this->getTemplateLine(), null, null, false); + throw new SyntaxError(sprintf('Positional arguments cannot be used after named arguments for %s "%s".', $callType, $callName), $this->getTemplateLine(), $this->getSourceContext()); } $parameters[$name] = $node; @@ -157,14 +157,14 @@ abstract class CallExpression extends AbstractExpression if (\array_key_exists($name, $parameters)) { if (\array_key_exists($pos, $parameters)) { - throw new SyntaxError(sprintf('Argument "%s" is defined twice for %s "%s".', $name, $callType, $callName), $this->getTemplateLine(), null, null, false); + throw new SyntaxError(sprintf('Argument "%s" is defined twice for %s "%s".', $name, $callType, $callName), $this->getTemplateLine(), $this->getSourceContext()); } if (\count($missingArguments)) { throw new SyntaxError(sprintf( 'Argument "%s" could not be assigned for %s "%s(%s)" because it is mapped to an internal PHP function which cannot determine default value for optional argument%s "%s".', $name, $callType, $callName, implode(', ', $names), \count($missingArguments) > 1 ? 's' : '', implode('", "', $missingArguments) - ), $this->getTemplateLine(), null, null, false); + ), $this->getTemplateLine(), $this->getSourceContext()); } $arguments = array_merge($arguments, $optionalArguments); @@ -186,7 +186,7 @@ abstract class CallExpression extends AbstractExpression $missingArguments[] = $name; } } else { - throw new SyntaxError(sprintf('Value for argument "%s" is required for %s "%s".', $name, $callType, $callName), $this->getTemplateLine(), null, null, false); + throw new SyntaxError(sprintf('Value for argument "%s" is required for %s "%s".', $name, $callType, $callName), $this->getTemplateLine(), $this->getSourceContext()); } } @@ -216,10 +216,14 @@ abstract class CallExpression extends AbstractExpression } } - throw new SyntaxError(sprintf( - 'Unknown argument%s "%s" for %s "%s(%s)".', - \count($parameters) > 1 ? 's' : '', implode('", "', array_keys($parameters)), $callType, $callName, implode(', ', $names) - ), $unknownParameter ? $unknownParameter->getTemplateLine() : $this->getTemplateLine(), null, null, false); + throw new SyntaxError( + sprintf( + 'Unknown argument%s "%s" for %s "%s(%s)".', + \count($parameters) > 1 ? 's' : '', implode('", "', array_keys($parameters)), $callType, $callName, implode(', ', $names) + ), + $unknownParameter ? $unknownParameter->getTemplateLine() : $this->getTemplateLine(), + $unknownParameter ? $unknownParameter->getSourceContext() : $this->getSourceContext() + ); } return $arguments; diff --git a/src/Node/Expression/Test/DefinedTest.php b/src/Node/Expression/Test/DefinedTest.php index 03fdb6d60..e87b83577 100644 --- a/src/Node/Expression/Test/DefinedTest.php +++ b/src/Node/Expression/Test/DefinedTest.php @@ -48,7 +48,7 @@ class DefinedTest extends TestExpression } elseif ($node instanceof ConstantExpression || $node instanceof ArrayExpression) { $node = new ConstantExpression(true, $node->getTemplateLine()); } else { - throw new SyntaxError('The "defined" test only works with simple variables.', $this->getTemplateLine(), null, null, false); + throw new SyntaxError('The "defined" test only works with simple variables.', $lineno); } parent::__construct($node, $name, $arguments, $lineno); diff --git a/src/Node/MacroNode.php b/src/Node/MacroNode.php index df1931f8c..dd887e6ee 100644 --- a/src/Node/MacroNode.php +++ b/src/Node/MacroNode.php @@ -27,7 +27,7 @@ class MacroNode extends Node { foreach ($arguments as $argumentName => $argument) { if (self::VARARGS_NAME === $argumentName) { - throw new SyntaxError(sprintf('The argument "%s" in macro "%s" cannot be defined because the variable "%s" is reserved for arbitrary arguments.', self::VARARGS_NAME, $name, self::VARARGS_NAME), $argument->getTemplateLine(), null, null, false); + throw new SyntaxError(sprintf('The argument "%s" in macro "%s" cannot be defined because the variable "%s" is reserved for arbitrary arguments.', self::VARARGS_NAME, $name, self::VARARGS_NAME), $argument->getTemplateLine(), $argument->getSourceContext()); } } diff --git a/src/Node/ModuleNode.php b/src/Node/ModuleNode.php index f32741bbf..cdf993361 100644 --- a/src/Node/ModuleNode.php +++ b/src/Node/ModuleNode.php @@ -30,16 +30,12 @@ use Twig\Source; */ class ModuleNode extends Node { - private $source; - public function __construct(Node $body, AbstractExpression $parent = null, Node $blocks, Node $macros, Node $traits, $embeddedTemplates, Source $source) { if (__CLASS__ !== \get_class($this)) { @trigger_error('Overriding '.__CLASS__.' is deprecated since Twig 2.4.0 and the class will be final in 3.0.', E_USER_DEPRECATED); } - $this->source = $source; - $nodes = [ 'body' => $body, 'blocks' => $blocks, @@ -62,7 +58,8 @@ class ModuleNode extends Node ], 1); // populate the template name of all node children - $this->setTemplateName($this->source->getName()); + $this->setTemplateName($source->getName()); + $this->setSourceContext($source); } public function setIndex($index) @@ -129,7 +126,7 @@ class ModuleNode extends Node ->raw('$this->loadTemplate(') ->subcompile($parent) ->raw(', ') - ->repr($this->source->getName()) + ->repr($this->getSourceContext()->getName()) ->raw(', ') ->repr($parent->getTemplateLine()) ->raw(')') @@ -165,8 +162,8 @@ class ModuleNode extends Node } $compiler // if the template name contains */, add a blank to avoid a PHP parse error - ->write('/* '.str_replace('*/', '* /', $this->source->getName())." */\n") - ->write('class '.$compiler->getEnvironment()->getTemplateClass($this->source->getName(), $this->getAttribute('index'))) + ->write('/* '.str_replace('*/', '* /', $this->getSourceContext()->getName())." */\n") + ->write('class '.$compiler->getEnvironment()->getTemplateClass($this->getSourceContext()->getName(), $this->getAttribute('index'))) ->raw(sprintf(" extends %s\n", $compiler->getEnvironment()->getBaseTemplateClass(false))) ->write("{\n") ->indent() @@ -205,15 +202,16 @@ class ModuleNode extends Node ->raw(");\n") ; + $node = $node = $trait->getNode('template'); $compiler - ->addDebugInfo($trait->getNode('template')) + ->addDebugInfo($node) ->write(sprintf("if (!\$_trait_%s->isTraitable()) {\n", $i)) ->indent() ->write("throw new RuntimeError('Template \"'.") ->subcompile($trait->getNode('template')) ->raw(".'\" cannot be used as a trait.', ") ->repr($node->getTemplateLine()) - ->raw(", \$this->source);\n") + ->raw(", \$this->getSourceContext());\n") ->outdent() ->write("}\n") ->write(sprintf("\$_trait_%s_blocks = \$_trait_%s->getBlocks();\n\n", $i, $i)) @@ -229,9 +227,9 @@ class ModuleNode extends Node ->string($key) ->raw(' is not defined in trait ') ->subcompile($trait->getNode('template')) - ->raw(".', ") + ->raw(".'), ") ->repr($node->getTemplateLine()) - ->raw(", \$this->source);\n") + ->raw(", \$this->getSourceContext());\n") ->outdent() ->write("}\n\n") @@ -330,7 +328,7 @@ class ModuleNode extends Node ->write('$this->parent = $this->loadTemplate(') ->subcompile($parent) ->raw(', ') - ->repr($this->source->getName()) + ->repr($this->getSourceContext()->getName()) ->raw(', ') ->repr($parent->getTemplateLine()) ->raw(");\n") @@ -369,7 +367,7 @@ class ModuleNode extends Node ->write("public function getTemplateName()\n", "{\n") ->indent() ->write('return ') - ->repr($this->source->getName()) + ->repr($this->getSourceContext()->getName()) ->raw(";\n") ->outdent() ->write("}\n\n") @@ -445,11 +443,11 @@ class ModuleNode extends Node ->write("public function getSourceContext()\n", "{\n") ->indent() ->write('return new Source(') - ->string($compiler->getEnvironment()->isDebug() ? $this->source->getCode() : '') + ->string($compiler->getEnvironment()->isDebug() ? $this->getSourceContext()->getCode() : '') ->raw(', ') - ->string($this->source->getName()) + ->string($this->getSourceContext()->getName()) ->raw(', ') - ->string($this->source->getPath()) + ->string($this->getSourceContext()->getPath()) ->raw(");\n") ->outdent() ->write("}\n") diff --git a/src/Node/Node.php b/src/Node/Node.php index fe16861c3..af5d97ef2 100644 --- a/src/Node/Node.php +++ b/src/Node/Node.php @@ -13,6 +13,7 @@ namespace Twig\Node; use Twig\Compiler; +use Twig\Source; /** * Represents a node in the AST. @@ -27,6 +28,7 @@ class Node implements \Countable, \IteratorAggregate protected $tag; private $name; + private $sourceContext; /** * @param array $nodes An array of named nodes @@ -178,6 +180,21 @@ class Node implements \Countable, \IteratorAggregate { return $this->name; } + + public function setSourceContext(Source $source) + { + $this->sourceContext = $source; + foreach ($this->nodes as $node) { + if (null !== $node) { + $node->setSourceContext($source); + } + } + } + + public function getSourceContext() + { + return $this->sourceContext; + } } class_alias('Twig\Node\Node', 'Twig_Node'); diff --git a/src/Node/WithNode.php b/src/Node/WithNode.php index 1a52ff579..74d1ea0a3 100644 --- a/src/Node/WithNode.php +++ b/src/Node/WithNode.php @@ -35,16 +35,17 @@ class WithNode extends Node $compiler->addDebugInfo($this); if ($this->hasNode('variables')) { + $node = $this->getNode('variables'); $varsName = $compiler->getVarName(); $compiler ->write(sprintf('$%s = ', $varsName)) - ->subcompile($this->getNode('variables')) + ->subcompile($node) ->raw(";\n") ->write(sprintf("if (!twig_test_iterable(\$%s)) {\n", $varsName)) ->indent() ->write("throw new RuntimeError('Variables passed to the \"with\" tag must be a hash.', ") - ->repr($this->getTemplateLine()) - ->raw(", \$this->source);\n") + ->repr($node->getTemplateLine()) + ->raw(", \$this->getSourceContext());\n") ->outdent() ->write("}\n") ->write(sprintf("\$%s = twig_to_array(\$%s);\n", $varsName, $varsName)) diff --git a/src/Template.php b/src/Template.php index 9cf96952d..63e54d169 100644 --- a/src/Template.php +++ b/src/Template.php @@ -195,7 +195,10 @@ abstract class Template throw $e; } catch (\Exception $e) { - throw new RuntimeError(sprintf('An exception has been thrown during the rendering of a template ("%s").', $e->getMessage()), -1, $template->getSourceContext(), $e); + $e = new RuntimeError(sprintf('An exception has been thrown during the rendering of a template ("%s").', $e->getMessage()), -1, $template->getSourceContext(), $e); + $e->guess(); + + throw $e; } } elseif (false !== $parent = $this->getParent($context)) { $parent->displayBlock($name, $context, array_merge($this->blocks, $blocks), false, $templateContext ?? $this); @@ -390,7 +393,10 @@ abstract class Template throw $e; } catch (\Exception $e) { - throw new RuntimeError(sprintf('An exception has been thrown during the rendering of a template ("%s").', $e->getMessage()), -1, $this->getSourceContext(), $e); + $e = new RuntimeError(sprintf('An exception has been thrown during the rendering of a template ("%s").', $e->getMessage()), -1, $this->getSourceContext(), $e); + $e->guess(); + + throw $e; } } diff --git a/test/Twig/Tests/ExpressionParserTest.php b/test/Twig/Tests/ExpressionParserTest.php index 4813e94a3..ccf331ba3 100644 --- a/test/Twig/Tests/ExpressionParserTest.php +++ b/test/Twig/Tests/ExpressionParserTest.php @@ -56,8 +56,9 @@ class Twig_Tests_ExpressionParserTest extends \PHPUnit\Framework\TestCase public function testArrayExpression($template, $expected) { $env = new Environment($this->getMockBuilder(LoaderInterface::class)->getMock(), ['cache' => false, 'autoescape' => false]); - $stream = $env->tokenize(new Source($template, '')); + $stream = $env->tokenize($source = new Source($template, '')); $parser = new Parser($env); + $expected->setSourceContext($source); $this->assertEquals($expected, $parser->parse($stream)->getNode('body')->getNode(0)->getNode('expr')); } @@ -176,8 +177,9 @@ class Twig_Tests_ExpressionParserTest extends \PHPUnit\Framework\TestCase public function testStringExpression($template, $expected) { $env = new Environment($this->getMockBuilder(LoaderInterface::class)->getMock(), ['cache' => false, 'autoescape' => false, 'optimizations' => 0]); - $stream = $env->tokenize(new Source($template, '')); + $stream = $env->tokenize($source = new Source($template, '')); $parser = new Parser($env); + $expected->setSourceContext($source); $this->assertEquals($expected, $parser->parse($stream)->getNode('body')->getNode(0)->getNode('expr')); } diff --git a/test/Twig/Tests/Fixtures/exceptions/exception_in_extension_extends.test b/test/Twig/Tests/Fixtures/exceptions/exception_in_extension_extends.test new file mode 100644 index 000000000..2ab298059 --- /dev/null +++ b/test/Twig/Tests/Fixtures/exceptions/exception_in_extension_extends.test @@ -0,0 +1,12 @@ +--TEST-- +Exception thrown from a child for an extension error +--TEMPLATE-- +{% extends 'base.twig' %} +--TEMPLATE(base.twig)-- + + +{{ random([]) }} +--DATA-- +return [] +--EXCEPTION-- +Twig\Error\RuntimeError: The random function cannot pick from an empty array in "base.twig" at line 4. diff --git a/test/Twig/Tests/Fixtures/exceptions/exception_in_extension_include.test b/test/Twig/Tests/Fixtures/exceptions/exception_in_extension_include.test new file mode 100644 index 000000000..e2281b290 --- /dev/null +++ b/test/Twig/Tests/Fixtures/exceptions/exception_in_extension_include.test @@ -0,0 +1,12 @@ +--TEST-- +Exception thrown from an include for an extension error +--TEMPLATE-- +{% include 'content.twig' %} +--TEMPLATE(content.twig)-- + + +{{ random([]) }} +--DATA-- +return [] +--EXCEPTION-- +Twig\Error\RuntimeError: The random function cannot pick from an empty array in "content.twig" at line 4. diff --git a/test/Twig/Tests/Fixtures/expressions/call_argument_defined_twice.test b/test/Twig/Tests/Fixtures/expressions/call_argument_defined_twice.test new file mode 100644 index 000000000..36539a6d1 --- /dev/null +++ b/test/Twig/Tests/Fixtures/expressions/call_argument_defined_twice.test @@ -0,0 +1,8 @@ +--TEST-- +Argument is defined twice in a call +--TEMPLATE-- +{{ date(987654, date = 123456) }} +--DATA-- +return [] +--EXCEPTION-- +Twig\Error\SyntaxError: Argument "date" is defined twice for function "date" in "index.twig" at line 2. diff --git a/test/Twig/Tests/Fixtures/expressions/call_positional_arg_after_named_arg.test b/test/Twig/Tests/Fixtures/expressions/call_positional_arg_after_named_arg.test new file mode 100644 index 000000000..729c67485 --- /dev/null +++ b/test/Twig/Tests/Fixtures/expressions/call_positional_arg_after_named_arg.test @@ -0,0 +1,8 @@ +--TEST-- +Positional arguments after named arguments in a call +--TEMPLATE-- +{{ date(date = 123456, 'Y-m-d') }} +--DATA-- +return [] +--EXCEPTION-- +Twig\Error\SyntaxError: Positional arguments cannot be used after named arguments for function "date" in "index.twig" at line 2. diff --git a/test/Twig/Tests/Fixtures/tests/defined_on_complex_expr.test b/test/Twig/Tests/Fixtures/tests/defined_on_complex_expr.test new file mode 100644 index 000000000..2d0615832 --- /dev/null +++ b/test/Twig/Tests/Fixtures/tests/defined_on_complex_expr.test @@ -0,0 +1,8 @@ +--TEST-- +"defined" support for "complex" expressions +--TEMPLATE-- +{{ (1 + 2) is defined ? 'ok' : 'ko' }} +--DATA-- +return [] +--EXCEPTION-- +Twig\Error\SyntaxError: The "defined" test only works with simple variables in "index.twig" at line 2.