Fix logic issue of required after optional arguments.

cakephp · Oct 23, 2016 · 14ef75f · 14ef75f
1 parent 752d0fe
commit 14ef75f
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 6 deletions.
diff --git a/src/Console/ConsoleOptionParser.php b/src/Console/ConsoleOptionParser.php
@@ -16,6 +16,7 @@
 
 use Cake\Console\Exception\ConsoleException;
 use Cake\Utility\Inflector;
+use LogicException;
 
 /**
  * Handles parsing the ARGV in the command line and provides support
@@ -92,7 +93,7 @@ class ConsoleOptionParser
      * Option definitions.
      *
      * @see \Cake\Console\ConsoleOptionParser::addOption()
-     * @var array
+     * @var \Cake\Console\ConsoleInputOption[]
      */
     protected $_options = [];
 
@@ -107,15 +108,15 @@ class ConsoleOptionParser
      * Positional argument definitions.
      *
      * @see \Cake\Console\ConsoleOptionParser::addArgument()
-     * @var array
+     * @var \Cake\Console\ConsoleInputArgument[]
      */
     protected $_args = [];
 
     /**
      * Subcommands for this Shell.
      *
      * @see \Cake\Console\ConsoleOptionParser::addSubcommand()
-     * @var array
+     * @var \Cake\Console\ConsoleInputSubcommand[]
      */
     protected $_subcommands = [];
 
@@ -431,6 +432,9 @@ public function addArgument($name, array $params = [])
             if ($a->isEqualTo($arg)) {
                 return $this;
             }
+            if ($options['required'] && !$a->isRequired()) {
+            	throw new LogicException('A required argument cannot follow an optional one');
+			}
         }
         $this->_args[$index] = $arg;
         ksort($this->_args);

diff --git a/tests/TestCase/Console/ConsoleOptionParserTest.php b/tests/TestCase/Console/ConsoleOptionParserTest.php
@@ -456,6 +456,19 @@ public function testPositionalArgNotEnough()
         $parser->parse(['one']);
     }
 
+    /**
+     * test that when there are required arguments after optional ones an exception is raised
+     *
+     * @expectedException \LogicException
+     * @return void
+     */
+    public function testPositionalArgRequiredAfterOptional()
+    {
+        $parser = new ConsoleOptionParser('test');
+        $parser->addArgument('name', ['required' => false])
+            ->addArgument('other', ['required' => true]);
+    }
+
     /**
      * test that arguments with choices enforce them.
      *

diff --git a/tests/TestCase/Console/HelpFormatterTest.php b/tests/TestCase/Console/HelpFormatterTest.php
@@ -277,14 +277,14 @@ public function testHelpWithLotsOfArguments()
     {
         $parser = new ConsoleOptionParser('mycommand', false);
         $parser
-            ->addArgument('test', ['help' => 'A test option.'])
-            ->addArgument('test2', ['help' => 'A test option.'])
+            ->addArgument('test', ['help' => 'A test option.', 'required' => true])
+            ->addArgument('test2', ['help' => 'A test option.', 'required' => true])
             ->addArgument('test3', ['help' => 'A test option.'])
             ->addArgument('test4', ['help' => 'A test option.'])
             ->addArgument('test5', ['help' => 'A test option.'])
             ->addArgument('test6', ['help' => 'A test option.'])
             ->addArgument('test7', ['help' => 'A test option.'])
-            ->addArgument('model', ['help' => 'The model to make.', 'required' => true])
+            ->addArgument('model', ['help' => 'The model to make.'])
             ->addArgument('other_longer', ['help' => 'Another argument.']);
 
         $formatter = new HelpFormatter($parser);