[TwigBridge] Added compile-time issues checking in twig:lint command #10843

maxromanovsky · 2014-05-02T18:26:17Z

Q	A
Bug fix?	yes
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#10019
License	MIT
Doc PR	none

maxromanovsky · 2014-05-02T18:27:38Z

I've also changed the behavior of this command when template content is passed using STDIN. However, I don't know how to properly cover this functionality with unit tests. IMHO input stream should be a part of the InputInterface. In this case it could be used as a call to an interface method instead of direct manipulation with STDIN.

stof · 2014-05-02T18:30:40Z

src/Symfony/Bridge/Twig/Command/LintCommand.php

-            $twig->parse($twig->tokenize($template, $file ? (string) $file : null));
+            $loader = $twig->getLoader();
+            if ($loader instanceof \Twig_Loader_Filesystem) {
+                $loader->setPaths(dirname($file));


This is wrong. You should not alter the state of the existing loader instance as it might have side effects (for instance when using the console shell to run the command as the same process will continue running, and so the same loader)

Thus, do you really need to use the loader ?

Yes, I need to use the loader. Otherwise the following exception is being thrown: https://github.com/fabpot/Twig/blob/master/lib/Twig/Loader/Filesystem.php#L191

As a workaround I could try to alter \Symfony\Bridge\Twig\Tests\Command\LintCommandTest::createCommandTester() method and pass a filename as an argument. Then I'll be able to set a path only in unit tests.

However it would not help with the STDIN case. In this case I could invoke $loader->getPaths(), then add a current path, but restore path configuration directly after an invocation.

where is the call to findTemplate done ?

Exception message: There are no registered paths for namespace "__main__".

Stack trace:

#0 /vagrant/symfony/vendor/twig/twig/lib/Twig/Loader/Filesystem.php(138): Twig_Loader_Filesystem->findTemplate('/tmp/sf-501ERx') #1 /vagrant/symfony/vendor/twig/twig/lib/Twig/Environment.php(265): Twig_Loader_Filesystem->getCacheKey('/tmp/sf-501ERx') #2 /vagrant/symfony/vendor/twig/twig/lib/Twig/Node/Module.php(127): Twig_Environment->getTemplateClass('/tmp/sf-501ERx', NULL) #3 /vagrant/symfony/vendor/twig/twig/lib/Twig/Node/Module.php(51): Twig_Node_Module->compileClassHeader(Object(Twig_Compiler)) #4 /vagrant/symfony/vendor/twig/twig/lib/Twig/Node/Module.php(38): Twig_Node_Module->compileTemplate(Object(Twig_Compiler)) #5 /vagrant/symfony/vendor/twig/twig/lib/Twig/Compiler.php(86): Twig_Node_Module->compile(Object(Twig_Compiler)) #6 /vagrant/symfony/vendor/twig/twig/lib/Twig/Environment.php(543): Twig_Compiler->compile(Object(Twig_Node_Module)) #7 /vagrant/symfony/src/Symfony/Bridge/Twig/Command/LintCommand.php(136): Twig_Environment->compile(Object(Twig_Node_Module)) #8 /vagrant/symfony/src/Symfony/Bridge/Twig/Command/LintCommand.php(110): Symfony\Bridge\Twig\Command\LintCommand->validate(Object(Twig_Environment), '{{ foo }}', '/tmp/sf-501ERx') #9 /vagrant/symfony/src/Symfony/Component/Console/Command/Command.php(252): Symfony\Bridge\Twig\Command\LintCommand->execute(Object(Symfony\Component\Console\Input\ArrayInput), Object(Symfony\Component\Console\Output\StreamOutput)) #10 /vagrant/symfony/src/Symfony/Component/Console/Tester/CommandTester.php(80): Symfony\Component\Console\Command\Command->run(Object(Symfony\Component\Console\Input\ArrayInput), Object(Symfony\Component\Console\Output\StreamOutput)) #11 /vagrant/symfony/src/Symfony/Bridge/Twig/Tests/Command/LintCommandTest.php(31): Symfony\Component\Console\Tester\CommandTester->execute(Array, Array) #12 [internal function]: Symfony\Bridge\Twig\Tests\Command\LintCommandTest->testLintCorrectFile() [...]

ah, this is to get the cache key.

I suggest doing it in a different way: create an ArrayLoader with the template in it (so that getting the cache key works) and set it in Twig. And at the end of the method, reset the original loader in Twig. This way, the original loader is not altered, and you don't need a temporary file

@stof Just to clarify, you suggest to use it in two cases:

LintCommandTest for all templates used in unit tests

LintCommand just for the case with piped template

Is it correct?
Because it won't work for the most common use case with real templates app/console twig:lint /path/to/files if templates use inheritance.

it should work. the template inheritance is not resolved at compile time as the parent can be dynamic

maxromanovsky · 2014-05-03T09:56:18Z

@stof Thank you for your advice. I've changed the implementation to use Twig_Loader_Array

fabpot · 2014-05-04T17:07:02Z

Thank you @maxromanovsky.

…g:lint command (maxromanovsky) This PR was merged into the 2.5-dev branch. Discussion ---------- [TwigBridge] Added compile-time issues checking in twig:lint command | Q | A | ------------- | --- | Bug fix? | yes | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #10019 | License | MIT |Doc PR | none Commits ------- 418cea1 [TwigBridge] Added compile-time issues checking in twig:lint command

…put in case of error (GromNaN) This PR was merged into the 7.2 branch. Discussion ---------- [TwigBridge] Remove random file name for `lint:twig` output in case of error | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | no | New feature? | no | Deprecations? | no | Issues | Part of #57588 | License | MIT In the `lint:twig`, `uniqid` is used to generate a file name when linting a template from STDIN. - I don't find any reason to use a unique file name. The file name is never stored when the template is tokenized and compiled. The cache is not used. - This meaningless file name is displayed in the output in case of error, which is better replaced by the text "Standard Input". Before: ``` $ bin/console lint:twig - < vendor/symfony/twig-bridge/Resources/views/Email/zurb_2/notification/body.html.twig ERROR in sf_6681e2dd64a2e2.02015003 (line -1) >> The "inky_to_html" filter is part of the InkyExtension, which is not installed/enabled; try running "composer require twig/inky-extra". [WARNING] 0 Twig files have valid syntax and 1 contain errors. ``` After: ``` $ bin/console lint:twig - < vendor/symfony/twig-bridge/Resources/views/Email/zurb_2/notification/body.html.twig ERROR in Standard Input (line -1) >> The "inky_to_html" filter is part of the InkyExtension, which is not installed/enabled; try running "composer require twig/inky-extra". [WARNING] 0 Twig files have valid syntax and 1 contain errors. ``` Note: the `line -1` is due to the lack of context provided to `Twig\Extra\TwigExtraBundle\MissingExtensionSuggestor::suggestFilter()` For reference, this was introduced by #10843 Commits ------- a97acbd Remove random file name for lint:twig output in case of error

stof reviewed May 2, 2014
View reviewed changes

maxromanovsky mentioned this pull request May 2, 2014

[Console] Add method to InputInterface that would allow to operate with input stream #10844

Closed

[TwigBridge] Added compile-time issues checking in twig:lint command

418cea1

fabpot merged commit 418cea1 into symfony:master May 4, 2014

GromNaN mentioned this pull request Jun 30, 2024

[TwigBridge] Remove random file name for lint:twig output in case of error #57608

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[TwigBridge] Added compile-time issues checking in twig:lint command #10843

[TwigBridge] Added compile-time issues checking in twig:lint command #10843

Uh oh!

maxromanovsky commented May 2, 2014

Uh oh!

maxromanovsky commented May 2, 2014

Uh oh!

stof May 2, 2014

Uh oh!

maxromanovsky May 2, 2014

Uh oh!

stof May 2, 2014

Uh oh!

maxromanovsky May 2, 2014

Uh oh!

stof May 2, 2014

Uh oh!

maxromanovsky May 3, 2014

Uh oh!

stof May 3, 2014

Uh oh!

maxromanovsky commented May 3, 2014

Uh oh!

fabpot commented May 4, 2014

Uh oh!

Uh oh!

Uh oh!

[TwigBridge] Added compile-time issues checking in twig:lint command #10843

[TwigBridge] Added compile-time issues checking in twig:lint command #10843

Uh oh!

Conversation

maxromanovsky commented May 2, 2014

Uh oh!

maxromanovsky commented May 2, 2014

Uh oh!

stof May 2, 2014

Choose a reason for hiding this comment

Uh oh!

maxromanovsky May 2, 2014

Choose a reason for hiding this comment

Uh oh!

stof May 2, 2014

Choose a reason for hiding this comment

Uh oh!

maxromanovsky May 2, 2014

Choose a reason for hiding this comment

Uh oh!

stof May 2, 2014

Choose a reason for hiding this comment

Uh oh!

maxromanovsky May 3, 2014

Choose a reason for hiding this comment

Uh oh!

stof May 3, 2014

Choose a reason for hiding this comment

Uh oh!

maxromanovsky commented May 3, 2014

Uh oh!

fabpot commented May 4, 2014

Uh oh!

Uh oh!