Skip to content

[TwigBundle] Commands as a service #23519

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions UPGRADE-3.4.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,21 @@ TwigBridge
* deprecated the `Symfony\Bridge\Twig\Form\TwigRenderer` class, use the `FormRenderer`
class from the Form component instead

* deprecated `Symfony\Bridge\Twig\Command\DebugCommand::set/getTwigEnvironment` and the ability
to pass a command name as first argument

* deprecated `Symfony\Bridge\Twig\Command\LintCommand::set/getTwigEnvironment` and the ability
to pass a command name as first argument

TwigBundle
----------

* deprecated the `Symfony\Bundle\TwigBundle\Command\DebugCommand` class, use the `DebugCommand`
class from the Twig bridge instead

* deprecated relying on the `ContainerAwareInterface` implementation for
`Symfony\Bundle\TwigBundle\Command\LintCommand`

Validator
---------

Expand Down
10 changes: 10 additions & 0 deletions UPGRADE-4.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,10 @@ TwigBundle
* The `ContainerAwareRuntimeLoader` class has been removed. Use the
Twig `Twig_ContainerRuntimeLoader` class instead.

* Removed `DebugCommand` in favor of `Symfony\Bridge\Twig\Command\DebugCommand`.

* Removed `ContainerAwareInterface` implementation in `Symfony\Bundle\TwigBundle\Command\LintCommand`.

TwigBridge
----------

Expand Down Expand Up @@ -550,6 +554,12 @@ TwigBridge

* The `TwigRendererEngine::setEnvironment()` method has been removed.
Pass the Twig Environment as second argument of the constructor instead.

* Removed `Symfony\Bridge\Twig\Command\DebugCommand::set/getTwigEnvironment` and the ability
to pass a command name as first argument.

* Removed `Symfony\Bridge\Twig\Command\LintCommand::set/getTwigEnvironment` and the ability
to pass a command name as first argument.

Validator
---------
Expand Down
2 changes: 2 additions & 0 deletions src/Symfony/Bridge/Twig/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ CHANGELOG
-----

* deprecated `Symfony\Bridge\Twig\Form\TwigRenderer`
* deprecated `Symfony\Bridge\Twig\Command\DebugCommand::set/getTwigEnvironment` and the ability to pass a command name as first argument
* deprecated `Symfony\Bridge\Twig\Command\LintCommand::set/getTwigEnvironment` and the ability to pass a command name as first argument

3.3.0
-----
Expand Down
37 changes: 30 additions & 7 deletions src/Symfony/Bridge/Twig/Command/DebugCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,27 @@ class DebugCommand extends Command
private $twig;

/**
* {@inheritdoc}
* @param Environment $twig
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Patched.

*/
public function __construct($name = 'debug:twig')
public function __construct($twig = null)
{
parent::__construct($name);
parent::__construct();

if (!$twig instanceof Environment) {
@trigger_error(sprintf('Passing a command name as the first argument of "%s" is deprecated since version 3.4 and will be removed in 4.0. If the command was registered by convention, make it a service instead.', __METHOD__), E_USER_DEPRECATED);

$this->setName(null === $twig ? 'debug:twig' : $twig);
Copy link
Contributor

@ogizanagi ogizanagi Jul 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor, but it looks safe to me to simply use $this->setName($twig ?: 'debug:twig'); here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but who would name this command 0? 😄
It won't even work with the symfony base Application and will juste execute the default command.

Copy link
Contributor Author

@ro0NL ro0NL Jul 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah.. to me that should be null !== $name as well :)

* @return string The value of the first argument or null otherwise
(or null otherwise)

If you insist i change it, just saying im preserving previous behavior :)


return;
}

$this->twig = $twig;
}

public function setTwigEnvironment(Environment $twig)
{
@trigger_error(sprintf('Method "%s" is deprecated since version 3.4 and will be removed in 4.0.', __METHOD__), E_USER_DEPRECATED);

$this->twig = $twig;
}

Expand All @@ -46,12 +58,15 @@ public function setTwigEnvironment(Environment $twig)
*/
protected function getTwigEnvironment()
{
@trigger_error(sprintf('Method "%s" is deprecated since version 3.4 and will be removed in 4.0.', __METHOD__), E_USER_DEPRECATED);

return $this->twig;
}

protected function configure()
{
$this
->setName('debug:twig')
->setDefinition(array(
new InputArgument('filter', InputArgument::OPTIONAL, 'Show details for all entries matching this filter'),
new InputOption('format', null, InputOption::VALUE_REQUIRED, 'The output format (text or json)', 'text'),
Expand Down Expand Up @@ -80,9 +95,17 @@ protected function configure()
protected function execute(InputInterface $input, OutputInterface $output)
{
$io = new SymfonyStyle($input, $output);
$twig = $this->getTwigEnvironment();

if (null === $twig) {
// BC to be removed in 4.0
if (__CLASS__ !== get_class($this)) {
$r = new \ReflectionMethod($this, 'getTwigEnvironment');
if (__CLASS__ !== $r->getDeclaringClass()->getName()) {
@trigger_error(sprintf('Usage of method "%s" is deprecated since version 3.4 and will no longer be supported in 4.0.', get_class($this).'::getTwigEnvironment'), E_USER_DEPRECATED);

$this->twig = $this->getTwigEnvironment();
}
}
if (null === $this->twig) {
throw new \RuntimeException('The Twig environment needs to be set.');
}

Expand All @@ -91,7 +114,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
if ($input->getOption('format') === 'json') {
$data = array();
foreach ($types as $type) {
foreach ($twig->{'get'.ucfirst($type)}() as $name => $entity) {
foreach ($this->twig->{'get'.ucfirst($type)}() as $name => $entity) {
$data[$type][$name] = $this->getMetadata($type, $entity);
}
}
Expand All @@ -105,7 +128,7 @@ protected function execute(InputInterface $input, OutputInterface $output)

foreach ($types as $index => $type) {
$items = array();
foreach ($twig->{'get'.ucfirst($type)}() as $name => $entity) {
foreach ($this->twig->{'get'.ucfirst($type)}() as $name => $entity) {
if (!$filter || false !== strpos($name, $filter)) {
$items[$name] = $name.$this->getPrettyMetadata($type, $entity);
}
Expand Down
54 changes: 39 additions & 15 deletions src/Symfony/Bridge/Twig/Command/LintCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,27 @@ class LintCommand extends Command
private $twig;

/**
* {@inheritdoc}
* @param Environment $twig
*/
public function __construct($name = 'lint:twig')
public function __construct($twig = null)
{
parent::__construct($name);
parent::__construct();

if (!$twig instanceof Environment) {
@trigger_error(sprintf('Passing a command name as the first argument of "%s" is deprecated since version 3.4 and will be removed in 4.0. If the command was registered by convention, make it a service instead.', __METHOD__), E_USER_DEPRECATED);

$this->setName(null === $twig ? 'lint:twig' : $twig);

return;
}

$this->twig = $twig;
}

public function setTwigEnvironment(Environment $twig)
{
@trigger_error(sprintf('Method "%s" is deprecated since version 3.4 and will be removed in 4.0.', __METHOD__), E_USER_DEPRECATED);

$this->twig = $twig;
}

Expand All @@ -51,12 +63,15 @@ public function setTwigEnvironment(Environment $twig)
*/
protected function getTwigEnvironment()
{
@trigger_error(sprintf('Method "%s" is deprecated since version 3.4 and will be removed in 4.0.', __METHOD__), E_USER_DEPRECATED);

return $this->twig;
}

protected function configure()
{
$this
->setName('lint:twig')
->setDescription('Lints a template and outputs encountered errors')
->addOption('format', null, InputOption::VALUE_REQUIRED, 'The output format', 'txt')
->addArgument('filename', InputArgument::IS_ARRAY)
Expand Down Expand Up @@ -86,7 +101,16 @@ protected function execute(InputInterface $input, OutputInterface $output)
{
$io = new SymfonyStyle($input, $output);

if (null === $twig = $this->getTwigEnvironment()) {
// BC to be removed in 4.0
if (__CLASS__ !== get_class($this)) {
$r = new \ReflectionMethod($this, 'getTwigEnvironment');
if (__CLASS__ !== $r->getDeclaringClass()->getName()) {
@trigger_error(sprintf('Usage of method "%s" is deprecated since version 3.4 and will no longer be supported in 4.0.', get_class($this).'::getTwigEnvironment'), E_USER_DEPRECATED);

$this->twig = $this->getTwigEnvironment();
}
}
if (null === $this->twig) {
throw new \RuntimeException('The Twig environment needs to be set.');
}

Expand All @@ -102,20 +126,20 @@ protected function execute(InputInterface $input, OutputInterface $output)
$template .= fread(STDIN, 1024);
}

return $this->display($input, $output, $io, array($this->validate($twig, $template, uniqid('sf_', true))));
return $this->display($input, $output, $io, array($this->validate($template, uniqid('sf_', true))));
}

$filesInfo = $this->getFilesInfo($twig, $filenames);
$filesInfo = $this->getFilesInfo($filenames);

return $this->display($input, $output, $io, $filesInfo);
}

private function getFilesInfo(Environment $twig, array $filenames)
private function getFilesInfo(array $filenames)
{
$filesInfo = array();
foreach ($filenames as $filename) {
foreach ($this->findFiles($filename) as $file) {
$filesInfo[] = $this->validate($twig, file_get_contents($file), $file);
$filesInfo[] = $this->validate(file_get_contents($file), $file);
}
}

Expand All @@ -133,17 +157,17 @@ protected function findFiles($filename)
throw new \RuntimeException(sprintf('File or directory "%s" is not readable', $filename));
}

private function validate(Environment $twig, $template, $file)
private function validate($template, $file)
{
$realLoader = $twig->getLoader();
$realLoader = $this->twig->getLoader();
try {
$temporaryLoader = new ArrayLoader(array((string) $file => $template));
$twig->setLoader($temporaryLoader);
$nodeTree = $twig->parse($twig->tokenize(new Source($template, (string) $file)));
$twig->compile($nodeTree);
$twig->setLoader($realLoader);
$this->twig->setLoader($temporaryLoader);
$nodeTree = $this->twig->parse($this->twig->tokenize(new Source($template, (string) $file)));
$this->twig->compile($nodeTree);
$this->twig->setLoader($realLoader);
} catch (Error $e) {
$twig->setLoader($realLoader);
$this->twig->setLoader($realLoader);

return array('template' => $template, 'file' => $file, 'line' => $e->getTemplateLine(), 'valid' => false, 'exception' => $e);
}
Expand Down
23 changes: 19 additions & 4 deletions src/Symfony/Bridge/Twig/Tests/Command/LintCommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,30 @@ public function testLintFileCompileTimeException()
$this->assertRegExp('/ERROR in \S+ \(line /', trim($tester->getDisplay()));
}

/**
* @group legacy
* @expectedDeprecation Passing a command name as the first argument of "Symfony\Bridge\Twig\Command\LintCommand::__construct" is deprecated since version 3.4 and will be removed in 4.0. If the command was registered by convention, make it a service instead.
* @expectedException \RuntimeException
* @expectedExceptionMessage The Twig environment needs to be set.
*/
public function testLegacyLintCommand()
{
$command = new LintCommand();

$application = new Application();
$application->add($command);
$command = $application->find('lint:twig');

$tester = new CommandTester($command);
$tester->execute(array());
}

/**
* @return CommandTester
*/
private function createCommandTester()
{
$twig = new Environment(new FilesystemLoader());

$command = new LintCommand();
$command->setTwigEnvironment($twig);
$command = new LintCommand(new Environment(new FilesystemLoader()));

$application = new Application();
$application->add($command);
Expand Down
6 changes: 6 additions & 0 deletions src/Symfony/Bundle/TwigBundle/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
CHANGELOG
=========

3.4.0
-----

* deprecated `Symfony\Bundle\TwigBundle\Command\DebugCommand`, use `Symfony\Bridge\Twig\Command\DebugCommand` instead
* deprecated relying on the `ContainerAwareInterface` implementation for `Symfony\Bundle\TwigBundle\Command\LintCommand`

3.3.0
-----

Expand Down
4 changes: 4 additions & 0 deletions src/Symfony/Bundle/TwigBundle/Command/DebugCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

namespace Symfony\Bundle\TwigBundle\Command;

@trigger_error(sprintf('The %s class is deprecated since version 3.4 and will be removed in 4.0. Use Symfony\Bridge\Twig\Command\DebugCommand instead.', DebugCommand::class), E_USER_DEPRECATED);

use Symfony\Bridge\Twig\Command\DebugCommand as BaseDebugCommand;
use Symfony\Component\DependencyInjection\ContainerAwareInterface;
use Symfony\Component\DependencyInjection\ContainerAwareTrait;
Expand All @@ -19,6 +21,8 @@
* Lists twig functions, filters, globals and tests present in the current project.
*
* @author Jordi Boggiano <j.boggiano@seld.be>
*
* @deprecated since version 3.4, to be removed in 4.0.
*/
final class DebugCommand extends BaseDebugCommand implements ContainerAwareInterface
{
Expand Down
9 changes: 1 addition & 8 deletions src/Symfony/Bundle/TwigBundle/Command/LintCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,9 @@
*/
final class LintCommand extends BaseLintCommand implements ContainerAwareInterface
{
// BC to be removed in 4.0
use ContainerAwareTrait;

/**
* {@inheritdoc}
*/
protected function getTwigEnvironment()
{
return $this->container->get('twig');
}

/**
* {@inheritdoc}
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use Symfony\Bridge\Twig\Extension\WebLinkExtension;
use Symfony\Component\Config\FileLocator;
use Symfony\Component\Console\Application;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
Expand Down Expand Up @@ -49,6 +50,10 @@ public function load(array $configs, ContainerBuilder $container)
$loader->load('templating.xml');
}

if (class_exists(Application::class)) {
$loader->load('console.xml');
}

if (!interface_exists('Symfony\Component\Translation\TranslatorInterface')) {
$container->removeDefinition('twig.translation.extractor');
}
Expand Down
25 changes: 25 additions & 0 deletions src/Symfony/Bundle/TwigBundle/Resources/config/console.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?xml version="1.0" ?>

<container xmlns="http://symfony.com/schema/dic/services"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">

<services>
<defaults public="false" />

<service id="twig.command.debug" class="Symfony\Bridge\Twig\Command\DebugCommand">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we not using class name == service id convention for new code?

<argument type="service" id="twig" />
<tag name="console.command" command="debug:twig" />
</service>

<service id="twig.command.lint" class="Symfony\Bundle\TwigBundle\Command\LintCommand">
<argument type="service" id="twig" />
<tag name="console.command" command="lint:twig" />
</service>

<!-- BC to be removed in 4.0 -->
<service id="console.command.symfony_bundle_twigbundle_command_debugcommand" class="Symfony\Bundle\TwigBundle\Command\DebugCommand" public="true">
<deprecated>The "%service_id%" service is deprecated since Symfony 3.4 and will be removed in 4.0. Use "twig.command.debug" instead.</deprecated>
</service>
</services>
</container>