Throw during parsing for missing mandatory args (#947)

Fixes #872
Closes #888

The exception for missing mandatory arguments was moved from parse time
to the time when the options are read in order to work around an issue
where trying to run the `help` command would still raise the warning
about missing mandatory arguments. Remove the errors attempting to read
the fields and restore the checking at parse time. Use the presence of
the `help` key or command as a signal to skip validation.

Update the tests to expect the exception during parse time. Add a test
that the default `help` command still prints exactly the intended usage.

Author: natebosch

pkgs/args/CHANGELOG.md

@@ -9,6 +9,8 @@
 * Remove sorting of the subcommands in usage output. Ordering will depend on the
   order that `addSubCommand` is called.
 * Remove extra newlines following separators when using flags without help text.
+* Throw an exception when parsing argument lists with missing mandatory
+  arguments instead of waiting until they are read.
 
 ## 2.7.0
 

pkgs/args/lib/src/arg_results.dart

@@ -70,10 +70,6 @@ class ArgResults {
     }
 
     final option = _parser.options[name]!;
-    if (option.mandatory && !_parsed.containsKey(name)) {
-      throw ArgumentError('Option $name is mandatory.');
-    }
-
     return option.valueOrDefault(_parsed[name]);
   }
 
@@ -102,9 +98,6 @@ class ArgResults {
     if (!option.isSingle) {
       throw ArgumentError('"$name" is a multi-option.');
     }
-    if (option.mandatory && !_parsed.containsKey(name)) {
-      throw ArgumentError('Option $name is mandatory.');
-    }
     return option.valueOrDefault(_parsed[name]) as String?;
   }
 

pkgs/args/lib/src/parser.dart

@@ -122,18 +122,20 @@ class Parser {
     }
 
     // Check if mandatory and invoke existing callbacks.
+    final ignoreMandatoryOptions =
+        _results.containsKey('help') || command?.name == 'help';
     _grammar.options.forEach((name, option) {
       var parsedOption = _results[name];
 
-      var callback = option.callback;
-      if (callback == null) return;
-
       // Check if an option is mandatory and was passed; if not, throw an
       // exception.
-      if (option.mandatory && parsedOption == null) {
+      if (!ignoreMandatoryOptions && option.mandatory && parsedOption == null) {
         throw ArgParserException('Option $name is mandatory.', null, name);
       }
 
+      var callback = option.callback;
+      if (callback == null) return;
+
       // ignore: avoid_dynamic_calls
       callback(option.valueOrDefault(parsedOption));
     });

pkgs/args/test/command_runner_test.dart

@@ -779,12 +779,49 @@ Run "test help" to see global options.'''));
     runner.addCommand(subcommand);
     expect(
         () => runner.run([subcommand.name]),
-        throwsA(isA<ArgumentError>().having((e) => e.message, 'message',
+        throwsA(isA<UsageException>().having((e) => e.message, 'message',
             contains('Option mandatory-option is mandatory'))));
     expect(await runner.run([subcommand.name, '--mandatory-option', 'foo']),
         'foo');
   });
 
+  test('mandatory options in subcommand does not interfere with help command',
+      () async {
+    var subcommand = _MandatoryOptionCommand();
+    runner.addCommand(subcommand);
+    expect(() => runner.run(['help']), prints('''
+A test command runner.
+
+Usage: test <command> [arguments]
+
+Global options:
+-h, --help    Print this usage information.
+
+Available commands:
+  mandatory-option-command   A command with a mandatory option
+
+Run "test help <command>" for more information about a command.
+'''));
+  });
+
+  test('mandatory global options do not interfere with help command', () {
+    runner.argParser.addOption('mandatory', mandatory: true);
+    expect(() => runner.run(['help']), prints('''
+A test command runner.
+
+Usage: test <command> [arguments]
+
+Global options:
+-h, --help                     Print this usage information.
+    --mandatory (mandatory)    
+
+Available commands:
+  help   Display help information for test.
+
+Run "test help <command>" for more information about a command.
+'''));
+  });
+
   test('default command runs', () {
     final defaultSubcommand = FooCommand();
     runner.addCommand(defaultSubcommand, isDefault: true);

pkgs/args/test/parse_test.dart

@@ -594,34 +594,26 @@ void main() {
         test('throw if no args', () {
           var parser = ArgParser();
           parser.addOption('username', mandatory: true);
-          var results = parser.parse([]);
-          expect(() => results['username'], throwsA(isA<ArgumentError>()));
+          throwsArgParserException(
+              parser, [], 'Option username is mandatory.', [], 'username');
         });
 
         test('throw if no mandatory args', () {
           var parser = ArgParser();
           parser.addOption('test');
           parser.addOption('username', mandatory: true);
-          var results = parser.parse(['--test', 'test']);
-          expect(results['test'], equals('test'));
-          expect(() => results['username'], throwsA(isA<ArgumentError>()));
-        });
-
-        test('parse successfully', () {
-          var parser = ArgParser();
-          parser.addOption('test', mandatory: true);
-          var results = parser.parse(['--test', 'test']);
-          expect(results['test'], equals('test'));
+          throwsArgParserException(parser, ['--test', 'test'],
+              'Option username is mandatory.', [], 'username');
         });
 
-        test('throws when value retrieved', () {
+        test('does not interfere with help flag', () {
           var parser = ArgParser();
           parser.addFlag('help', abbr: 'h', negatable: false);
           parser.addOption('test', mandatory: true);
           var results = parser.parse(['-h']);
           expect(results['help'], true);
-          expect(() => results['test'], throwsA(isA<ArgumentError>()));
-          expect(() => results.option('test'), throwsA(isA<ArgumentError>()));
+          expect(results['test'], null);
+          expect(results.option('test'), null);
         });
       });
     });