From cd9f646528488e437089e9f41bf2441c02cc8f51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Burnichon?= Date: Wed, 4 Nov 2015 16:09:44 +0100 Subject: [PATCH 1/4] Add a way to avoid throwing Exceptions --- .../Extractor/File/TwigFileExtractor.php | 52 ++++++++++++++++--- 1 file changed, 44 insertions(+), 8 deletions(-) diff --git a/Translation/Extractor/File/TwigFileExtractor.php b/Translation/Extractor/File/TwigFileExtractor.php index 2ed77180..066faace 100644 --- a/Translation/Extractor/File/TwigFileExtractor.php +++ b/Translation/Extractor/File/TwigFileExtractor.php @@ -19,13 +19,15 @@ namespace JMS\TranslationBundle\Translation\Extractor\File; use JMS\TranslationBundle\Exception\RuntimeException; -use Symfony\Bridge\Twig\Node\TransNode; +use JMS\TranslationBundle\Logger\LoggerAwareInterface; use JMS\TranslationBundle\Model\FileSource; use JMS\TranslationBundle\Model\Message; use JMS\TranslationBundle\Model\MessageCatalogue; use JMS\TranslationBundle\Translation\Extractor\FileVisitorInterface; +use Psr\Log\LoggerInterface; +use Symfony\Bridge\Twig\Node\TransNode; -class TwigFileExtractor implements FileVisitorInterface, \Twig_NodeVisitorInterface +class TwigFileExtractor implements FileVisitorInterface, \Twig_NodeVisitorInterface, LoggerAwareInterface { /** * @var \SplFileInfo @@ -43,14 +45,15 @@ class TwigFileExtractor implements FileVisitorInterface, \Twig_NodeVisitorInterf private $traverser; /** - * @var array + * @var array|\Twig_Node */ private $stack = array(); /** - * TwigFileExtractor constructor. - * @param \Twig_Environment $env + * @var LoggerInterface */ + private $logger; + public function __construct(\Twig_Environment $env) { $this->traverser = new \Twig_NodeTraverser($env, array($this)); @@ -69,6 +72,9 @@ public function enterNode(\Twig_NodeInterface $node, \Twig_Environment $env) $id = $node->getNode('body')->getAttribute('data'); $domain = 'messages'; if (null !== $domainNode = $node->getNode('domain')) { + if (!$this->checkNodeIsConstant($domainNode)) { + return $node; + } $domain = $domainNode->getAttribute('value'); } @@ -80,10 +86,9 @@ public function enterNode(\Twig_NodeInterface $node, \Twig_Environment $env) if ('trans' === $name || 'transchoice' === $name) { $idNode = $node->getNode('node'); - if (!$idNode instanceof \Twig_Node_Expression_Constant) { + if (!$this->checkNodeIsConstant($idNode)) { return $node; // FIXME: see below -// throw new \RuntimeException(sprintf('Cannot infer translation id from node "%s". Please refactor to only translate constants.', get_class($idNode))); } $id = $idNode->getAttribute('value'); @@ -92,7 +97,7 @@ public function enterNode(\Twig_NodeInterface $node, \Twig_Environment $env) $arguments = $node->getNode('arguments'); if ($arguments->hasNode($index)) { $argument = $arguments->getNode($index); - if (!$argument instanceof \Twig_Node_Expression_Constant) { + if (!$this->checkNodeIsConstant($argument)) { return $node; // FIXME: Throw exception if there is some way for the user to turn this off // on a case-by-case basis, similar to @Ignore in PHP @@ -202,4 +207,35 @@ public function visitFile(\SplFileInfo $file, MessageCatalogue $catalogue) public function visitPhpFile(\SplFileInfo $file, MessageCatalogue $catalogue, array $ast) { } + + public function setLogger(LoggerInterface $logger) + { + $this->logger = $logger; + } + + /** + * @param \Twig_Node $node + * @return bool + */ + private function checkNodeIsConstant(\Twig_Node $node) + { + if ($node instanceof \Twig_Node_Expression_Constant) { + return true; + } + + $message = sprintf( + 'Cannot infer translation id from node "%s". Please refactor to only translate constants in file "%s" on line %d', + get_class($node), + $this->file, + $node->getLine() + ); + + if (!$this->logger) { + throw new RuntimeException($message); + } + + $this->logger->error($message); + + return false; + } } From 4f324d051ac26ddcf291be44955dbb82027022df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Burnichon?= Date: Fri, 25 Mar 2016 01:17:25 +0100 Subject: [PATCH 2/4] Add some tests to TwigFileExtractor --- ...simple_template_with_expressions.html.twig | 7 +++ .../Extractor/File/TwigFileExtractorTest.php | 59 +++++++++++++++---- 2 files changed, 54 insertions(+), 12 deletions(-) create mode 100644 Tests/Translation/Extractor/File/Fixture/simple_template_with_expressions.html.twig diff --git a/Tests/Translation/Extractor/File/Fixture/simple_template_with_expressions.html.twig b/Tests/Translation/Extractor/File/Fixture/simple_template_with_expressions.html.twig new file mode 100644 index 00000000..7e0f8a0e --- /dev/null +++ b/Tests/Translation/Extractor/File/Fixture/simple_template_with_expressions.html.twig @@ -0,0 +1,7 @@ +{% set key="expression" %} + +{{ key|trans|desc("Foo Bar")|meaning("Some Meaning")}} + +{{ "text.foo_bar"|trans({}, key) }} + +{% trans with {'%name%': 'Johannes'} from key %}text.name{% endtrans %} diff --git a/Tests/Translation/Extractor/File/TwigFileExtractorTest.php b/Tests/Translation/Extractor/File/TwigFileExtractorTest.php index d1394b97..63564864 100644 --- a/Tests/Translation/Extractor/File/TwigFileExtractorTest.php +++ b/Tests/Translation/Extractor/File/TwigFileExtractorTest.php @@ -18,6 +18,7 @@ namespace JMS\TranslationBundle\Tests\Translation\Extractor\File; +use Prophecy\Argument; use Symfony\Bridge\Twig\Form\TwigRendererEngine; use Symfony\Bridge\Twig\Form\TwigRenderer; use Symfony\Component\Routing\RequestContext; @@ -35,12 +36,20 @@ use JMS\TranslationBundle\Model\MessageCatalogue; use JMS\TranslationBundle\Twig\TranslationExtension; use JMS\TranslationBundle\Translation\Extractor\File\TwigFileExtractor; -use Symfony\Bundle\FrameworkBundle\Routing\Router; -use Symfony\Component\DependencyInjection\Container; use Symfony\Bridge\Twig\Extension\FormExtension; class TwigFileExtractorTest extends \PHPUnit_Framework_TestCase { + /** + * @var \Twig_Environment + */ + private $env; + + protected function setUp() + { + $this->env = $this->createTwigEnvironment(); + } + public function testExtractSimpleTemplate() { $expected = new MessageCatalogue(); @@ -136,12 +145,47 @@ public function testEmbeddedTemplate() $this->assertEquals($expected, $this->extract('embedded_template.html.twig')); } + public function testSimpleTemplateWithExpressions() + { + $logger = $this->prophesize('Psr\Log\LoggerInterface'); + $logger->error(Argument::any(), Argument::any()) + ->shouldBeCalledTimes(3); + + $extractor = new TwigFileExtractor($this->env); + $extractor->setLogger($logger->reveal()); + + $this->extract('simple_template_with_expressions.html.twig', $extractor); + } + + public function testSimpleTemplateWithExpressionsWithoutLogger() + { + $this->setExpectedException('JMS\TranslationBundle\Exception\RuntimeException'); + $this->extract('simple_template_with_expressions.html.twig'); + } + private function extract($file, TwigFileExtractor $extractor = null) { if (!is_file($file = __DIR__.'/Fixture/'.$file)) { throw new RuntimeException(sprintf('The file "%s" does not exist.', $file)); } + if (null === $extractor) { + $extractor = new TwigFileExtractor($this->env); + } + + $ast = $this->env->parse($this->env->tokenize(file_get_contents($file), $file)); + + $catalogue = new MessageCatalogue(); + $extractor->visitTwigFile(new \SplFileInfo($file), $catalogue, $ast); + + return $catalogue; + } + + /** + * @return \Twig_Environment + */ + private function createTwigEnvironment() + { $env = new \Twig_Environment(); $env->addExtension(new SymfonyTranslationExtension($translator = new IdentityTranslator(new MessageSelector()))); $env->addExtension(new TranslationExtension($translator, true)); @@ -157,15 +201,6 @@ private function extract($file, TwigFileExtractor $extractor = null) } } - if (null === $extractor) { - $extractor = new TwigFileExtractor($env); - } - - $ast = $env->parse($env->tokenize(file_get_contents($file), $file)); - - $catalogue = new MessageCatalogue(); - $extractor->visitTwigFile(new \SplFileInfo($file), $catalogue, $ast); - - return $catalogue; + return $env; } } From 2a21e7a0d22524cb55b1ce65db6cb1493ac6f492 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Burnichon?= Date: Fri, 25 Mar 2016 01:18:02 +0100 Subject: [PATCH 3/4] Reduce number of files of coverage --- phpunit.xml.dist | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/phpunit.xml.dist b/phpunit.xml.dist index ab61930c..6e1a75f3 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -34,4 +34,17 @@ performance + + + + * + + build + Resources + Tests + vendor + + + + From 9710dec09ca4bdac0e1aa36ca4c38cb202044ccb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Burnichon?= Date: Fri, 25 Mar 2016 01:18:33 +0100 Subject: [PATCH 4/4] PHPCS of TwigFileExtractor --- Translation/Extractor/File/TwigFileExtractor.php | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/Translation/Extractor/File/TwigFileExtractor.php b/Translation/Extractor/File/TwigFileExtractor.php index 066faace..3fa894ed 100644 --- a/Translation/Extractor/File/TwigFileExtractor.php +++ b/Translation/Extractor/File/TwigFileExtractor.php @@ -45,7 +45,7 @@ class TwigFileExtractor implements FileVisitorInterface, \Twig_NodeVisitorInterf private $traverser; /** - * @var array|\Twig_Node + * @var \Twig_Node[] */ private $stack = array(); @@ -110,13 +110,15 @@ public function enterNode(\Twig_NodeInterface $node, \Twig_Environment $env) $message->addSource(new FileSource((string) $this->file, $node->getLine())); for ($i=count($this->stack)-2; $i>=0; $i-=1) { - if (!$this->stack[$i] instanceof \Twig_Node_Expression_Filter) { + $currentNode = $this->stack[$i]; + + if (!$currentNode instanceof \Twig_Node_Expression_Filter) { break; } - $name = $this->stack[$i]->getNode('filter')->getAttribute('value'); + $name = $currentNode->getNode('filter')->getAttribute('value'); if ('desc' === $name || 'meaning' === $name) { - $arguments = $this->stack[$i]->getNode('arguments'); + $arguments = $currentNode->getNode('arguments'); if (!$arguments->hasNode(0)) { throw new RuntimeException(sprintf('The "%s" filter requires exactly one argument, the description text.', $name)); } @@ -208,6 +210,11 @@ public function visitPhpFile(\SplFileInfo $file, MessageCatalogue $catalogue, ar { } + /** + * Inject a Logger + * + * @param LoggerInterface $logger + */ public function setLogger(LoggerInterface $logger) { $this->logger = $logger; @@ -216,6 +223,7 @@ public function setLogger(LoggerInterface $logger) /** * @param \Twig_Node $node * @return bool + * @throws RuntimeException */ private function checkNodeIsConstant(\Twig_Node $node) {