Skip to content

Commit

Permalink
bug #2701 Ensure that syntax errors are triggered with the right line…
Browse files Browse the repository at this point in the history
… (stof)

This PR was merged into the 1.x branch.

Discussion
----------

Ensure that syntax errors are triggered with the right line

When throwing the syntax error without any line and source in these places, the guessing logic enters into action. For the main template, it won't find anything. But for included templates (or any other template loaded during the rendering of another one, even as main one), the guessing will find a template (the caller one) and set the source and line based on it. The source will then be replaced by the proper template by `\Twig_Environment::compileSource`, but the guessed line number will make no sense then.

I searched for all places triggering a syntax error in Twig, to ensure that they always set the actual line number or set the source directly (so that the guessing logic knows that the template it found is the wrong one and so does not try to use it for guessing). There were only a few missing ones.

Commits
-------

6fab6b0 Ensure that syntax errors are triggered with the right line
  • Loading branch information
fabpot committed Jun 7, 2018
2 parents b9c6034 + 6fab6b0 commit 3b71efb
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 7 deletions.
12 changes: 6 additions & 6 deletions lib/Twig/Node/Expression/Call.php
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ protected function getArguments($callable, $arguments)
$named = true;
$name = $this->normalizeName($name);
} elseif ($named) {
throw new Twig_Error_Syntax(sprintf('Positional arguments cannot be used after named arguments for %s "%s".', $callType, $callName));
throw new Twig_Error_Syntax(sprintf('Positional arguments cannot be used after named arguments for %s "%s".', $callType, $callName), $this->getTemplateLine());
}

$parameters[$name] = $node;
Expand Down Expand Up @@ -143,14 +143,14 @@ protected function getArguments($callable, $arguments)

if (array_key_exists($name, $parameters)) {
if (array_key_exists($pos, $parameters)) {
throw new Twig_Error_Syntax(sprintf('Argument "%s" is defined twice for %s "%s".', $name, $callType, $callName));
throw new Twig_Error_Syntax(sprintf('Argument "%s" is defined twice for %s "%s".', $name, $callType, $callName), $this->getTemplateLine());
}

if (count($missingArguments)) {
throw new Twig_Error_Syntax(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))
);
$name, $callType, $callName, implode(', ', $names), count($missingArguments) > 1 ? 's' : '', implode('", "', $missingArguments)
), $this->getTemplateLine());
}

$arguments = array_merge($arguments, $optionalArguments);
Expand All @@ -172,7 +172,7 @@ protected function getArguments($callable, $arguments)
$missingArguments[] = $name;
}
} else {
throw new Twig_Error_Syntax(sprintf('Value for argument "%s" is required for %s "%s".', $name, $callType, $callName));
throw new Twig_Error_Syntax(sprintf('Value for argument "%s" is required for %s "%s".', $name, $callType, $callName), $this->getTemplateLine());
}
}

Expand Down Expand Up @@ -205,7 +205,7 @@ protected function getArguments($callable, $arguments)
throw new Twig_Error_Syntax(sprintf(
'Unknown argument%s "%s" for %s "%s(%s)".',
count($parameters) > 1 ? 's' : '', implode('", "', array_keys($parameters)), $callType, $callName, implode(', ', $names)
), $unknownParameter ? $unknownParameter->getTemplateLine() : -1);
), $unknownParameter ? $unknownParameter->getTemplateLine() : $this->getTemplateLine());
}

return $arguments;
Expand Down
2 changes: 1 addition & 1 deletion test/Twig/Tests/Node/Expression/FilterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ public function testCompileWithWrongNamedArgumentName()

/**
* @expectedException Twig_Error_Syntax
* @expectedExceptionMessage Value for argument "from" is required for filter "replace".
* @expectedExceptionMessage Value for argument "from" is required for filter "replace" at line 1.
*/
public function testCompileWithMissingNamedArgument()
{
Expand Down

0 comments on commit 3b71efb

Please sign in to comment.