From 8952155f9249473d49ce8d14aef8a8fcc1d90e47 Mon Sep 17 00:00:00 2001 From: Robin Chalas Date: Thu, 15 Sep 2016 22:28:04 +0200 Subject: [PATCH 1/2] [Console] Fix empty optionnal options with = separator in argv --- .../Component/Console/Input/ArgvInput.php | 7 +++- .../Console/Tests/Input/ArgvInputTest.php | 38 +++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Console/Input/ArgvInput.php b/src/Symfony/Component/Console/Input/ArgvInput.php index f18d6e1899..119be49170 100644 --- a/src/Symfony/Component/Console/Input/ArgvInput.php +++ b/src/Symfony/Component/Console/Input/ArgvInput.php @@ -145,7 +145,10 @@ class ArgvInput extends Input $name = substr($token, 2); if (false !== $pos = strpos($name, '=')) { - $this->addLongOption(substr($name, 0, $pos), substr($name, $pos + 1)); + if (0 === strlen($value = substr($name, $pos + 1))) { + array_unshift($this->parsed, null); + } + $this->addLongOption(substr($name, 0, $pos), $value); } else { $this->addLongOption($name, null); } @@ -232,7 +235,7 @@ class ArgvInput extends Input if (isset($next[0]) && '-' !== $next[0]) { $value = $next; } elseif (empty($next)) { - $value = ''; + $value = null; } else { array_unshift($this->parsed, $next); } diff --git a/src/Symfony/Component/Console/Tests/Input/ArgvInputTest.php b/src/Symfony/Component/Console/Tests/Input/ArgvInputTest.php index 3b0dc3f231..06347c87ba 100644 --- a/src/Symfony/Component/Console/Tests/Input/ArgvInputTest.php +++ b/src/Symfony/Component/Console/Tests/Input/ArgvInputTest.php @@ -71,6 +71,18 @@ class ArgvInputTest extends \PHPUnit_Framework_TestCase array('foo' => 'bar'), '->parse() parses long options with a required value (with a space separator)', ), + array( + array('cli.php', '--foo='), + array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL)), + array('foo' => null), + '->parse() parses long options with optional value which is empty (with a = separator) as null', + ), + array( + array('cli.php', '--foo=', 'bar'), + array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL), new InputArgument('name', InputArgument::REQUIRED)), + array('foo' => null), + '->parse() parses long options with optional value which is empty (with a = separator) followed by an argument', + ), array( array('cli.php', '-f'), array(new InputOption('foo', 'f')), @@ -324,4 +336,30 @@ class ArgvInputTest extends \PHPUnit_Framework_TestCase $input->bind(new InputDefinition(array(new InputArgument('file')))); $this->assertEquals(array('file' => '-'), $input->getArguments(), '->parse() parses single dash as an argument'); } + + public function testParseOptionWithValueOptionalGivenEmptyAndRequiredArgument() + { + $input = new ArgvInput(array('cli.php', '--foo=', 'bar')); + $input->bind(new InputDefinition(array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL), new InputArgument('name', InputArgument::REQUIRED)))); + $this->assertEquals(array('foo' => null), $input->getOptions(), '->parse() parses optional options with empty value as null'); + $this->assertEquals(array('name' => 'bar'), $input->getArguments(), '->parse() parses required arguments'); + + $input = new ArgvInput(array('cli.php', '--foo=0', 'bar')); + $input->bind(new InputDefinition(array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL), new InputArgument('name', InputArgument::REQUIRED)))); + $this->assertEquals(array('foo' => '0'), $input->getOptions(), '->parse() parses optional options with empty value as null'); + $this->assertEquals(array('name' => 'bar'), $input->getArguments(), '->parse() parses required arguments'); + } + + public function testParseOptionWithValueOptionalGivenEmptyAndOptionalArgument() + { + $input = new ArgvInput(array('cli.php', '--foo=', 'bar')); + $input->bind(new InputDefinition(array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL), new InputArgument('name', InputArgument::OPTIONAL)))); + $this->assertEquals(array('foo' => null), $input->getOptions(), '->parse() parses optional options with empty value as null'); + $this->assertEquals(array('name' => 'bar'), $input->getArguments(), '->parse() parses optional arguments'); + + $input = new ArgvInput(array('cli.php', '--foo=0', 'bar')); + $input->bind(new InputDefinition(array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL), new InputArgument('name', InputArgument::OPTIONAL)))); + $this->assertEquals(array('foo' => '0'), $input->getOptions(), '->parse() parses optional options with empty value as null'); + $this->assertEquals(array('name' => 'bar'), $input->getArguments(), '->parse() parses optional arguments'); + } } From 12350c2eb1e6785edb28b4b938a3c5cc73222711 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Mon, 19 Sep 2016 12:59:08 -0700 Subject: [PATCH 2/2] [TwigBridge] removed Twig null nodes (deprecated as of Twig 1.25) --- src/Symfony/Bridge/Twig/Node/DumpNode.php | 13 +++++---- .../Bridge/Twig/Node/StopwatchNode.php | 2 +- src/Symfony/Bridge/Twig/Node/TransNode.php | 29 ++++++++++++++----- .../TranslationDefaultDomainNodeVisitor.php | 2 +- .../NodeVisitor/TranslationNodeVisitor.php | 8 ++--- 5 files changed, 33 insertions(+), 21 deletions(-) diff --git a/src/Symfony/Bridge/Twig/Node/DumpNode.php b/src/Symfony/Bridge/Twig/Node/DumpNode.php index 522497ba65..a7c1529446 100644 --- a/src/Symfony/Bridge/Twig/Node/DumpNode.php +++ b/src/Symfony/Bridge/Twig/Node/DumpNode.php @@ -20,7 +20,12 @@ class DumpNode extends \Twig_Node public function __construct($varPrefix, \Twig_Node $values = null, $lineno, $tag = null) { - parent::__construct(array('values' => $values), array(), $lineno, $tag); + $nodes = array(); + if (null !== $values) { + $nodes['values'] = $values; + } + + parent::__construct($nodes, array(), $lineno, $tag); $this->varPrefix = $varPrefix; } @@ -33,9 +38,7 @@ class DumpNode extends \Twig_Node ->write("if (\$this->env->isDebug()) {\n") ->indent(); - $values = $this->getNode('values'); - - if (null === $values) { + if (!$this->hasNode('values')) { // remove embedded templates (macros) from the context $compiler ->write(sprintf('$%svars = array();'."\n", $this->varPrefix)) @@ -50,7 +53,7 @@ class DumpNode extends \Twig_Node ->write("}\n") ->addDebugInfo($this) ->write(sprintf('\Symfony\Component\VarDumper\VarDumper::dump($%svars);'."\n", $this->varPrefix)); - } elseif (1 === $values->count()) { + } elseif (($values = $this->getNode('values')) && 1 === $values->count()) { $compiler ->addDebugInfo($this) ->write('\Symfony\Component\VarDumper\VarDumper::dump(') diff --git a/src/Symfony/Bridge/Twig/Node/StopwatchNode.php b/src/Symfony/Bridge/Twig/Node/StopwatchNode.php index 06eeb49272..839e3f7698 100644 --- a/src/Symfony/Bridge/Twig/Node/StopwatchNode.php +++ b/src/Symfony/Bridge/Twig/Node/StopwatchNode.php @@ -18,7 +18,7 @@ namespace Symfony\Bridge\Twig\Node; */ class StopwatchNode extends \Twig_Node { - public function __construct(\Twig_Node $name, $body, \Twig_Node_Expression_AssignName $var, $lineno = 0, $tag = null) + public function __construct(\Twig_Node $name, \Twig_Node $body, \Twig_Node_Expression_AssignName $var, $lineno = 0, $tag = null) { parent::__construct(array('body' => $body, 'name' => $name, 'var' => $var), array(), $lineno, $tag); } diff --git a/src/Symfony/Bridge/Twig/Node/TransNode.php b/src/Symfony/Bridge/Twig/Node/TransNode.php index af85d8f185..5f1915d05e 100644 --- a/src/Symfony/Bridge/Twig/Node/TransNode.php +++ b/src/Symfony/Bridge/Twig/Node/TransNode.php @@ -18,7 +18,21 @@ class TransNode extends \Twig_Node { public function __construct(\Twig_Node $body, \Twig_Node $domain = null, \Twig_Node_Expression $count = null, \Twig_Node_Expression $vars = null, \Twig_Node_Expression $locale = null, $lineno = 0, $tag = null) { - parent::__construct(array('count' => $count, 'body' => $body, 'domain' => $domain, 'vars' => $vars, 'locale' => $locale), array(), $lineno, $tag); + $nodes = array('body' => $body); + if (null !== $domain) { + $nodes['domain'] = $domain; + } + if (null !== $count) { + $nodes['count'] = $count; + } + if (null !== $vars) { + $nodes['vars'] = $vars; + } + if (null !== $locale) { + $nodes['locale'] = $locale; + } + + parent::__construct($nodes, array(), $lineno, $tag); } /** @@ -30,15 +44,14 @@ class TransNode extends \Twig_Node { $compiler->addDebugInfo($this); - $vars = $this->getNode('vars'); $defaults = new \Twig_Node_Expression_Array(array(), -1); - if ($vars instanceof \Twig_Node_Expression_Array) { + if ($this->hasNode('vars') && ($vars = $this->getNode('vars')) instanceof \Twig_Node_Expression_Array) { $defaults = $this->getNode('vars'); $vars = null; } list($msg, $defaults) = $this->compileString($this->getNode('body'), $defaults, (bool) $vars); - $method = null === $this->getNode('count') ? 'trans' : 'transChoice'; + $method = !$this->hasNode('count') ? 'trans' : 'transChoice'; $compiler ->write('echo $this->env->getExtension(\'translator\')->getTranslator()->'.$method.'(') @@ -47,7 +60,7 @@ class TransNode extends \Twig_Node $compiler->raw(', '); - if (null !== $this->getNode('count')) { + if ($this->hasNode('count')) { $compiler ->subcompile($this->getNode('count')) ->raw(', ') @@ -68,13 +81,13 @@ class TransNode extends \Twig_Node $compiler->raw(', '); - if (null === $this->getNode('domain')) { + if (!$this->hasNode('domain')) { $compiler->repr('messages'); } else { $compiler->subcompile($this->getNode('domain')); } - if (null !== $this->getNode('locale')) { + if ($this->hasNode('locale')) { $compiler ->raw(', ') ->subcompile($this->getNode('locale')) @@ -98,7 +111,7 @@ class TransNode extends \Twig_Node foreach ($matches[1] as $var) { $key = new \Twig_Node_Expression_Constant('%'.$var.'%', $body->getLine()); if (!$vars->hasElement($key)) { - if ('count' === $var && null !== $this->getNode('count')) { + if ('count' === $var && $this->hasNode('count')) { $vars->addElement($this->getNode('count'), $key); } else { $varExpr = new \Twig_Node_Expression_Name($var, $body->getLine()); diff --git a/src/Symfony/Bridge/Twig/NodeVisitor/TranslationDefaultDomainNodeVisitor.php b/src/Symfony/Bridge/Twig/NodeVisitor/TranslationDefaultDomainNodeVisitor.php index c923df9944..3e850350fd 100644 --- a/src/Symfony/Bridge/Twig/NodeVisitor/TranslationDefaultDomainNodeVisitor.php +++ b/src/Symfony/Bridge/Twig/NodeVisitor/TranslationDefaultDomainNodeVisitor.php @@ -78,7 +78,7 @@ class TranslationDefaultDomainNodeVisitor extends \Twig_BaseNodeVisitor } } } elseif ($node instanceof TransNode) { - if (null === $node->getNode('domain')) { + if (!$node->hasNode('domain')) { $node->setNode('domain', $this->scope->get('domain')); } } diff --git a/src/Symfony/Bridge/Twig/NodeVisitor/TranslationNodeVisitor.php b/src/Symfony/Bridge/Twig/NodeVisitor/TranslationNodeVisitor.php index 4426a9d3b6..6e3880d09e 100644 --- a/src/Symfony/Bridge/Twig/NodeVisitor/TranslationNodeVisitor.php +++ b/src/Symfony/Bridge/Twig/NodeVisitor/TranslationNodeVisitor.php @@ -75,7 +75,7 @@ class TranslationNodeVisitor extends \Twig_BaseNodeVisitor // extract trans nodes $this->messages[] = array( $node->getNode('body')->getAttribute('data'), - $this->getReadDomainFromNode($node->getNode('domain')), + $node->hasNode('domain') ? $this->getReadDomainFromNode($node->getNode('domain')) : null, ); } @@ -122,12 +122,8 @@ class TranslationNodeVisitor extends \Twig_BaseNodeVisitor * * @return string|null */ - private function getReadDomainFromNode(\Twig_Node $node = null) + private function getReadDomainFromNode(\Twig_Node $node) { - if (null === $node) { - return; - } - if ($node instanceof \Twig_Node_Expression_Constant) { return $node->getAttribute('value'); }