package:args shouldn't throw ArgumentError on missing argument

[eseidel]

ArgumentError is an Error and thus supposed to be used for programmer errors.

ArgResults.operator[] throws ArgumentError on what should be a runtime error:

core/pkgs/args/lib/src/arg_results.dart

Line 69 in 61e6771

throw ArgumentError('Could not find an option named "--$name".');

Am I wrong?

[RandalSchwartz]

I could argue that Error subclasses are for things that the programmer should have prevented through proper coding but failed to do so. But this error seems to be correct code and a problem with the data. I could easily argue that this should be an API Exception rather than a programmer Error.

[lrhn]

You're wrong 😉
At least on that line. Now, line 74 on the other hand ...

In line 69 it checks if the name the programmer passes in is one that the same programmer has defined in the parser options for the parser. If there is no defined option with that name, then it's not a name of an option at all.
The programmer can be expected to know the names of all options that the argument parser was configured with.

In line 74, an ArgumentError is thrown for a data issue

core/pkgs/args/lib/src/arg_results.dart

Lines 73 to 74 in 61e6771

	if (option.mandatory && !_parsed.containsKey(name)) {
	throw ArgumentError('Option $name is mandatory.');

The option was defined, as required even, but the parsed result contains no data for it.
There are two possibilities here:

• This can't happen, parsing would have thrown instead (which would be The Way!), but then it shouldn't be throwing an ArgumentError, just have an assert or throw an AssertionError.
• It can happen, and then it should throw an Exception (and be documented as doing so) or have a different return value (also documented).

Issue title checks out, example off by 5 lines. I think that's close enough!

[eseidel]

Confirmed:

Line 74 is hit:

core/pkgs/args/lib/src/arg_results.dart

Line 74 in 61e6771

throw ArgumentError('Option $name is mandatory.');

import 'package:args/args.dart';
import 'package:test/test.dart';

void main() {
  test('test', () {
    final argParser =
        ArgParser()
          ..addFlag('help', help: 'Show this help message.')
          ..addFlag('verbose', help: 'Show verbose output.')
          ..addOption('bar', help: 'The bar option.')
          ..addOption('baz', help: 'The baz option.', mandatory: true);

    final arguments = <String>[];

    final results = argParser.parse(arguments);

    // 'foo' is never defined, so this is a programmer error, resulting
    // in an ArgumentError.
    expect(() => results['foo'], throwsA(isA<ArgumentError>()));
    // 'bar' is defined, but optional, should return String? and thus
    // this line will throw a TypeError.
    expect(() => results['bar'] as String, throwsA(isA<TypeError>()));
    // 'baz' is mandatory but not provided, so this should throw an
    // Exception, not an ArgumentError.
    expect(() => results['baz'] as String, throwsA(isA<Exception>()));
  });
}

% dart run bin/test.dart
00:00 +0: test

00:00 +0 -1: test [E]

  Expected: throws <Instance of 'Exception'>
    Actual: <Closure: () => String>
     Which: threw ArgumentError:<Invalid argument(s): Option baz is mandatory.>
            stack package:args/src/arg_results.dart 74:7  ArgResults.[]
                  bin/test.dart 25:25                     main.<fn>.<fn>
                  package:matcher                         expect
                  bin/test.dart 25:5                      main.<fn>
                  
            which is not an instance of 'Exception'
  

  package:matcher/src/expect/expect.dart 149:31      fail
  package:matcher/src/expect/expect.dart 120:7       _expect
  package:matcher/src/expect/expect.dart 56:3        expect
  bin/test.dart 25:5                                 main.<fn>
  package:test_api/src/backend/declarer.dart 229:19  Declarer.test.<fn>.<fn>
  ===== asynchronous gap ===========================
  package:test_api/src/backend/declarer.dart 227:7   Declarer.test.<fn>
  ===== asynchronous gap ===========================
  package:test_api/src/backend/invoker.dart 258:9    Invoker._waitForOutstandingCallbacks.<fn>
  

00:00 +0 -1: Some tests failed.



Consider enabling the flag chain-stack-traces to receive more detailed exceptions.
For example, 'dart test --chain-stack-traces'.


Unhandled exception:
Dummy exception to set exit code.

[eseidel]

I'm also not sure why we're not hitting:

core/pkgs/args/lib/src/parser.dart

Line 108 in 61e6771

if (option.mandatory && parsedOption == null) {

So presumably there are two bugs here.