Skip to content

Commit

Permalink
Add argument name when throwing a ArgParserException. (#283)
Browse files Browse the repository at this point in the history
Add an `argumentName` field which tracks the argument that was being
parse when the exception is thrown.
  • Loading branch information
fa0311 authored Sep 30, 2024
1 parent e623652 commit 9cdc872
Show file tree
Hide file tree
Showing 9 changed files with 143 additions and 54 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
## 2.5.1-wip
## 2.6.0-wip

* Added source argument when throwing a `ArgParserException`.
* Fix inconsistent `FormatException` messages
* Require Dart 3.3

## 2.5.0
Expand Down
10 changes: 9 additions & 1 deletion lib/src/arg_parser_exception.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,14 @@ class ArgParserException extends FormatException {
/// This will be empty if the error was on the root parser.
final List<String> commands;

ArgParserException(super.message, [Iterable<String>? commands])
/// The name of the argument that was being parsed when the error was
/// discovered.
final String? argumentName;

ArgParserException(super.message,
[Iterable<String>? commands,
this.argumentName,
super.source,
super.offset])
: commands = commands == null ? const [] : List.unmodifiable(commands);
}
10 changes: 5 additions & 5 deletions lib/src/arg_results.dart
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class ArgResults {
/// > flags, [option] for options, and [multiOption] for multi-options.
dynamic operator [](String name) {
if (!_parser.options.containsKey(name)) {
throw ArgumentError('Could not find an option named "$name".');
throw ArgumentError('Could not find an option named "--$name".');
}

final option = _parser.options[name]!;
Expand All @@ -83,7 +83,7 @@ class ArgResults {
bool flag(String name) {
var option = _parser.options[name];
if (option == null) {
throw ArgumentError('Could not find an option named "$name".');
throw ArgumentError('Could not find an option named "--$name".');
}
if (!option.isFlag) {
throw ArgumentError('"$name" is not a flag.');
Expand All @@ -97,7 +97,7 @@ class ArgResults {
String? option(String name) {
var option = _parser.options[name];
if (option == null) {
throw ArgumentError('Could not find an option named "$name".');
throw ArgumentError('Could not find an option named "--$name".');
}
if (!option.isSingle) {
throw ArgumentError('"$name" is a multi-option.');
Expand All @@ -111,7 +111,7 @@ class ArgResults {
List<String> multiOption(String name) {
var option = _parser.options[name];
if (option == null) {
throw ArgumentError('Could not find an option named "$name".');
throw ArgumentError('Could not find an option named "--$name".');
}
if (!option.isMultiple) {
throw ArgumentError('"$name" is not a multi-option.');
Expand Down Expand Up @@ -143,7 +143,7 @@ class ArgResults {
/// [name] must be a valid option name in the parser.
bool wasParsed(String name) {
if (!_parser.options.containsKey(name)) {
throw ArgumentError('Could not find an option named "$name".');
throw ArgumentError('Could not find an option named "--$name".');
}

return _parsed.containsKey(name);
Expand Down
78 changes: 46 additions & 32 deletions lib/src/parser.dart
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,20 @@ class Parser {
// options so that commands can have option-like names.
var command = _grammar.commands[_current];
if (command != null) {
_validate(_rest.isEmpty, 'Cannot specify arguments before a command.');
_validate(_rest.isEmpty, 'Cannot specify arguments before a command.',
_current);
var commandName = _args.removeFirst();
var commandParser = Parser(commandName, command, _args, this, _rest);

try {
commandResults = commandParser.parse();
} on ArgParserException catch (error) {
throw ArgParserException(
error.message, [commandName, ...error.commands]);
error.message,
[commandName, ...error.commands],
error.argumentName,
error.source,
error.offset);
}

// All remaining arguments were passed to command so clear them here.
Expand Down Expand Up @@ -101,7 +106,7 @@ class Parser {
// Check if an option is mandatory and was passed; if not, throw an
// exception.
if (option.mandatory && parsedOption == null) {
throw ArgParserException('Option $name is mandatory.');
throw ArgParserException('Option $name is mandatory.', null, name);
}

// ignore: avoid_dynamic_calls
Expand All @@ -118,11 +123,11 @@ class Parser {
/// Pulls the value for [option] from the second argument in [_args].
///
/// Validates that there is a valid value there.
void _readNextArgAsValue(Option option) {
void _readNextArgAsValue(Option option, String arg) {
// Take the option argument from the next command line arg.
_validate(_args.isNotEmpty, 'Missing argument for "${option.name}".');
_validate(_args.isNotEmpty, 'Missing argument for "$arg".', arg);

_setOption(_results, option, _current);
_setOption(_results, option, _current, arg);
_args.removeFirst();
}

Expand All @@ -145,7 +150,8 @@ class Parser {
var option = _grammar.findByAbbreviation(opt);
if (option == null) {
// Walk up to the parent command if possible.
_validate(_parent != null, 'Could not find an option or flag "-$opt".');
_validate(_parent != null, 'Could not find an option or flag "-$opt".',
'-$opt');
return _parent!._handleSoloOption(opt);
}

Expand All @@ -154,7 +160,7 @@ class Parser {
if (option.isFlag) {
_setFlag(_results, option, true);
} else {
_readNextArgAsValue(option);
_readNextArgAsValue(option, '-$opt');
}

return true;
Expand Down Expand Up @@ -193,22 +199,23 @@ class Parser {
var first = _grammar.findByAbbreviation(c);
if (first == null) {
// Walk up to the parent command if possible.
_validate(
_parent != null, 'Could not find an option with short name "-$c".');
_validate(_parent != null,
'Could not find an option with short name "-$c".', '-$c');
return _parent!
._handleAbbreviation(lettersAndDigits, rest, innermostCommand);
} else if (!first.isFlag) {
// The first character is a non-flag option, so the rest must be the
// value.
var value = '${lettersAndDigits.substring(1)}$rest';
_setOption(_results, first, value);
_setOption(_results, first, value, '-$c');
} else {
// If we got some non-flag characters, then it must be a value, but
// if we got here, it's a flag, which is wrong.
_validate(
rest == '',
'Option "-$c" is a flag and cannot handle value '
'"${lettersAndDigits.substring(1)}$rest".');
'"${lettersAndDigits.substring(1)}$rest".',
'-$c');

// Not an option, so all characters should be flags.
// We use "innermostCommand" here so that if a parent command parses the
Expand All @@ -228,16 +235,16 @@ class Parser {
var option = _grammar.findByAbbreviation(c);
if (option == null) {
// Walk up to the parent command if possible.
_validate(
_parent != null, 'Could not find an option with short name "-$c".');
_validate(_parent != null,
'Could not find an option with short name "-$c".', '-$c');
_parent!._parseShortFlag(c);
return;
}

// In a list of short options, only the first can be a non-flag. If
// we get here we've checked that already.
_validate(
option.isFlag, 'Option "-$c" must be a flag to be in a collapsed "-".');
_validate(option.isFlag,
'Option "-$c" must be a flag to be in a collapsed "-".', '-$c');

_setFlag(_results, option, true);
}
Expand Down Expand Up @@ -269,35 +276,39 @@ class Parser {
if (option != null) {
_args.removeFirst();
if (option.isFlag) {
_validate(
value == null, 'Flag option "$name" should not be given a value.');
_validate(value == null,
'Flag option "--$name" should not be given a value.', '--$name');

_setFlag(_results, option, true);
} else if (value != null) {
// We have a value like --foo=bar.
_setOption(_results, option, value);
_setOption(_results, option, value, '--$name');
} else {
// Option like --foo, so look for the value as the next arg.
_readNextArgAsValue(option);
_readNextArgAsValue(option, '--$name');
}
} else if (name.startsWith('no-')) {
// See if it's a negated flag.
var positiveName = name.substring('no-'.length);
option = _grammar.findByNameOrAlias(positiveName);
if (option == null) {
// Walk up to the parent command if possible.
_validate(_parent != null, 'Could not find an option named "$name".');
_validate(_parent != null, 'Could not find an option named "--$name".',
'--$name');
return _parent!._handleLongOption(name, value);
}

_args.removeFirst();
_validate(option.isFlag, 'Cannot negate non-flag option "$name".');
_validate(option.negatable!, 'Cannot negate option "$name".');
_validate(
option.isFlag, 'Cannot negate non-flag option "--$name".', '--$name');
_validate(
option.negatable!, 'Cannot negate option "--$name".', '--$name');

_setFlag(_results, option, false);
} else {
// Walk up to the parent command if possible.
_validate(_parent != null, 'Could not find an option named "$name".');
_validate(_parent != null, 'Could not find an option named "--$name".',
'--$name');
return _parent!._handleLongOption(name, value);
}

Expand All @@ -307,17 +318,20 @@ class Parser {
/// Called during parsing to validate the arguments.
///
/// Throws an [ArgParserException] if [condition] is `false`.
void _validate(bool condition, String message) {
if (!condition) throw ArgParserException(message);
void _validate(bool condition, String message,
[String? args, List<String>? source, int? offset]) {
if (!condition) {
throw ArgParserException(message, null, args, source, offset);
}
}

/// Validates and stores [value] as the value for [option], which must not be
/// a flag.
void _setOption(Map results, Option option, String value) {
void _setOption(Map results, Option option, String value, String arg) {
assert(!option.isFlag);

if (!option.isMultiple) {
_validateAllowed(option, value);
_validateAllowed(option, value, arg);
results[option.name] = value;
return;
}
Expand All @@ -326,11 +340,11 @@ class Parser {

if (option.splitCommas) {
for (var element in value.split(',')) {
_validateAllowed(option, element);
_validateAllowed(option, element, arg);
list.add(element);
}
} else {
_validateAllowed(option, value);
_validateAllowed(option, value, arg);
list.add(value);
}
}
Expand All @@ -343,11 +357,11 @@ class Parser {
}

/// Validates that [value] is allowed as a value of [option].
void _validateAllowed(Option option, String value) {
void _validateAllowed(Option option, String value, String arg) {
if (option.allowed == null) return;

_validate(option.allowed!.contains(value),
'"$value" is not an allowed value for option "${option.name}".');
'"$value" is not an allowed value for option "$arg".', arg);
}
}

Expand Down
2 changes: 1 addition & 1 deletion pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: args
version: 2.5.1-wip
version: 2.6.0-wip
description: >-
Library for defining parsers for parsing raw command-line arguments into a set
of options and values using GNU and POSIX style options.
Expand Down
10 changes: 5 additions & 5 deletions test/command_runner_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -583,15 +583,15 @@ Run "test help" to see global options.
expect(
runner.run(['--asdf']),
throwsUsageException(
'Could not find an option named "asdf".', _defaultUsage));
'Could not find an option named "--asdf".', _defaultUsage));
});

test('for a command throws the command usage', () {
var command = FooCommand();
runner.addCommand(command);

expect(runner.run(['foo', '--asdf']),
throwsUsageException('Could not find an option named "asdf".', '''
throwsUsageException('Could not find an option named "--asdf".', '''
Usage: test foo [arguments]
-h, --help Print this usage information.
Expand All @@ -616,7 +616,7 @@ Also, footer!'''));
test('includes the footer in usage errors', () {
expect(
runner.run(['--bad']),
throwsUsageException('Could not find an option named "bad".',
throwsUsageException('Could not find an option named "--bad".',
'$_defaultUsage\nAlso, footer!'));
});
});
Expand Down Expand Up @@ -652,7 +652,7 @@ newlines properly.'''));

test('includes the footer in usage errors', () {
expect(runner.run(['--bad']),
throwsUsageException('Could not find an option named "bad".', '''
throwsUsageException('Could not find an option named "--bad".', '''
Usage: test <command> [arguments]
Global options:
Expand All @@ -679,7 +679,7 @@ newlines properly.'''));
expect(
runner.run(['--bad']),
throwsUsageException(
'Could not find an option named "bad".', _defaultUsage));
'Could not find an option named "--bad".', _defaultUsage));
});

test("a top-level command doesn't exist", () {
Expand Down
49 changes: 49 additions & 0 deletions test/parse_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -766,5 +766,54 @@ void main() {
expect(results.rest, equals(['stop', '--', 'arg']));
});
});

group('ArgParser Exception Tests', () {
test('throws exception for unknown option', () {
var parser = ArgParser();
throwsArgParserException(parser, ['--verbose'],
'Could not find an option named "--verbose".', [], '--verbose');
throwsArgParserException(
parser, ['-v'], 'Could not find an option or flag "-v".', [], '-v');
});

test('throws exception for flag with value', () {
var parser = ArgParser();
parser.addFlag('flag', abbr: 'f');
throwsArgParserException(parser, ['--flag=1'],
'Flag option "--flag" should not be given a value.', [], '--flag');
throwsArgParserException(parser, ['-f=1'],
'Option "-f" is a flag and cannot handle value "=1".', [], '-f');
});

test('throws exception after parsing multiple options', () {
var parser = ArgParser();
parser.addOption('first');
parser.addOption('second');
throwsArgParserException(
parser,
['--first', '1', '--second', '2', '--verbose', '3'],
'Could not find an option named "--verbose".',
[],
'--verbose');
});

test('throws exception for option with invalid value', () {
var parser = ArgParser();
parser.addOption('first', allowed: ['a', 'b']);
throwsArgParserException(parser, ['--first', 'c'],
'"c" is not an allowed value for option "--first".', [], '--first');
});

test('throws exception after parsing command', () {
var parser = ArgParser();
parser.addCommand('command', ArgParser());
throwsArgParserException(
parser,
['command', '--verbose'],
'Could not find an option named "--verbose".',
['command'],
'--verbose');
});
});
});
}
Loading

0 comments on commit 9cdc872

Please sign in to comment.