Skip to content

Commit 272f175

Browse files
authored
Add argument name when throwing a ArgParserException. (dart-archive/args#283)
Add an `argumentName` field which tracks the argument that was being parse when the exception is thrown.
1 parent feec91d commit 272f175

File tree

9 files changed

+143
-54
lines changed

9 files changed

+143
-54
lines changed

pkgs/args/CHANGELOG.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1-
## 2.5.1-wip
1+
## 2.6.0-wip
22

3+
* Added source argument when throwing a `ArgParserException`.
4+
* Fix inconsistent `FormatException` messages
35
* Require Dart 3.3
46

57
## 2.5.0

pkgs/args/lib/src/arg_parser_exception.dart

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,14 @@ class ArgParserException extends FormatException {
99
/// This will be empty if the error was on the root parser.
1010
final List<String> commands;
1111

12-
ArgParserException(super.message, [Iterable<String>? commands])
12+
/// The name of the argument that was being parsed when the error was
13+
/// discovered.
14+
final String? argumentName;
15+
16+
ArgParserException(super.message,
17+
[Iterable<String>? commands,
18+
this.argumentName,
19+
super.source,
20+
super.offset])
1321
: commands = commands == null ? const [] : List.unmodifiable(commands);
1422
}

pkgs/args/lib/src/arg_results.dart

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ class ArgResults {
6666
/// > flags, [option] for options, and [multiOption] for multi-options.
6767
dynamic operator [](String name) {
6868
if (!_parser.options.containsKey(name)) {
69-
throw ArgumentError('Could not find an option named "$name".');
69+
throw ArgumentError('Could not find an option named "--$name".');
7070
}
7171

7272
final option = _parser.options[name]!;
@@ -83,7 +83,7 @@ class ArgResults {
8383
bool flag(String name) {
8484
var option = _parser.options[name];
8585
if (option == null) {
86-
throw ArgumentError('Could not find an option named "$name".');
86+
throw ArgumentError('Could not find an option named "--$name".');
8787
}
8888
if (!option.isFlag) {
8989
throw ArgumentError('"$name" is not a flag.');
@@ -97,7 +97,7 @@ class ArgResults {
9797
String? option(String name) {
9898
var option = _parser.options[name];
9999
if (option == null) {
100-
throw ArgumentError('Could not find an option named "$name".');
100+
throw ArgumentError('Could not find an option named "--$name".');
101101
}
102102
if (!option.isSingle) {
103103
throw ArgumentError('"$name" is a multi-option.');
@@ -111,7 +111,7 @@ class ArgResults {
111111
List<String> multiOption(String name) {
112112
var option = _parser.options[name];
113113
if (option == null) {
114-
throw ArgumentError('Could not find an option named "$name".');
114+
throw ArgumentError('Could not find an option named "--$name".');
115115
}
116116
if (!option.isMultiple) {
117117
throw ArgumentError('"$name" is not a multi-option.');
@@ -143,7 +143,7 @@ class ArgResults {
143143
/// [name] must be a valid option name in the parser.
144144
bool wasParsed(String name) {
145145
if (!_parser.options.containsKey(name)) {
146-
throw ArgumentError('Could not find an option named "$name".');
146+
throw ArgumentError('Could not find an option named "--$name".');
147147
}
148148

149149
return _parsed.containsKey(name);

pkgs/args/lib/src/parser.dart

Lines changed: 46 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -63,15 +63,20 @@ class Parser {
6363
// options so that commands can have option-like names.
6464
var command = _grammar.commands[_current];
6565
if (command != null) {
66-
_validate(_rest.isEmpty, 'Cannot specify arguments before a command.');
66+
_validate(_rest.isEmpty, 'Cannot specify arguments before a command.',
67+
_current);
6768
var commandName = _args.removeFirst();
6869
var commandParser = Parser(commandName, command, _args, this, _rest);
6970

7071
try {
7172
commandResults = commandParser.parse();
7273
} on ArgParserException catch (error) {
7374
throw ArgParserException(
74-
error.message, [commandName, ...error.commands]);
75+
error.message,
76+
[commandName, ...error.commands],
77+
error.argumentName,
78+
error.source,
79+
error.offset);
7580
}
7681

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

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

125-
_setOption(_results, option, _current);
130+
_setOption(_results, option, _current, arg);
126131
_args.removeFirst();
127132
}
128133

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

@@ -154,7 +160,7 @@ class Parser {
154160
if (option.isFlag) {
155161
_setFlag(_results, option, true);
156162
} else {
157-
_readNextArgAsValue(option);
163+
_readNextArgAsValue(option, '-$opt');
158164
}
159165

160166
return true;
@@ -193,22 +199,23 @@ class Parser {
193199
var first = _grammar.findByAbbreviation(c);
194200
if (first == null) {
195201
// Walk up to the parent command if possible.
196-
_validate(
197-
_parent != null, 'Could not find an option with short name "-$c".');
202+
_validate(_parent != null,
203+
'Could not find an option with short name "-$c".', '-$c');
198204
return _parent!
199205
._handleAbbreviation(lettersAndDigits, rest, innermostCommand);
200206
} else if (!first.isFlag) {
201207
// The first character is a non-flag option, so the rest must be the
202208
// value.
203209
var value = '${lettersAndDigits.substring(1)}$rest';
204-
_setOption(_results, first, value);
210+
_setOption(_results, first, value, '-$c');
205211
} else {
206212
// If we got some non-flag characters, then it must be a value, but
207213
// if we got here, it's a flag, which is wrong.
208214
_validate(
209215
rest == '',
210216
'Option "-$c" is a flag and cannot handle value '
211-
'"${lettersAndDigits.substring(1)}$rest".');
217+
'"${lettersAndDigits.substring(1)}$rest".',
218+
'-$c');
212219

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

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

242249
_setFlag(_results, option, true);
243250
}
@@ -269,35 +276,39 @@ class Parser {
269276
if (option != null) {
270277
_args.removeFirst();
271278
if (option.isFlag) {
272-
_validate(
273-
value == null, 'Flag option "$name" should not be given a value.');
279+
_validate(value == null,
280+
'Flag option "--$name" should not be given a value.', '--$name');
274281

275282
_setFlag(_results, option, true);
276283
} else if (value != null) {
277284
// We have a value like --foo=bar.
278-
_setOption(_results, option, value);
285+
_setOption(_results, option, value, '--$name');
279286
} else {
280287
// Option like --foo, so look for the value as the next arg.
281-
_readNextArgAsValue(option);
288+
_readNextArgAsValue(option, '--$name');
282289
}
283290
} else if (name.startsWith('no-')) {
284291
// See if it's a negated flag.
285292
var positiveName = name.substring('no-'.length);
286293
option = _grammar.findByNameOrAlias(positiveName);
287294
if (option == null) {
288295
// Walk up to the parent command if possible.
289-
_validate(_parent != null, 'Could not find an option named "$name".');
296+
_validate(_parent != null, 'Could not find an option named "--$name".',
297+
'--$name');
290298
return _parent!._handleLongOption(name, value);
291299
}
292300

293301
_args.removeFirst();
294-
_validate(option.isFlag, 'Cannot negate non-flag option "$name".');
295-
_validate(option.negatable!, 'Cannot negate option "$name".');
302+
_validate(
303+
option.isFlag, 'Cannot negate non-flag option "--$name".', '--$name');
304+
_validate(
305+
option.negatable!, 'Cannot negate option "--$name".', '--$name');
296306

297307
_setFlag(_results, option, false);
298308
} else {
299309
// Walk up to the parent command if possible.
300-
_validate(_parent != null, 'Could not find an option named "$name".');
310+
_validate(_parent != null, 'Could not find an option named "--$name".',
311+
'--$name');
301312
return _parent!._handleLongOption(name, value);
302313
}
303314

@@ -307,17 +318,20 @@ class Parser {
307318
/// Called during parsing to validate the arguments.
308319
///
309320
/// Throws an [ArgParserException] if [condition] is `false`.
310-
void _validate(bool condition, String message) {
311-
if (!condition) throw ArgParserException(message);
321+
void _validate(bool condition, String message,
322+
[String? args, List<String>? source, int? offset]) {
323+
if (!condition) {
324+
throw ArgParserException(message, null, args, source, offset);
325+
}
312326
}
313327

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

319333
if (!option.isMultiple) {
320-
_validateAllowed(option, value);
334+
_validateAllowed(option, value, arg);
321335
results[option.name] = value;
322336
return;
323337
}
@@ -326,11 +340,11 @@ class Parser {
326340

327341
if (option.splitCommas) {
328342
for (var element in value.split(',')) {
329-
_validateAllowed(option, element);
343+
_validateAllowed(option, element, arg);
330344
list.add(element);
331345
}
332346
} else {
333-
_validateAllowed(option, value);
347+
_validateAllowed(option, value, arg);
334348
list.add(value);
335349
}
336350
}
@@ -343,11 +357,11 @@ class Parser {
343357
}
344358

345359
/// Validates that [value] is allowed as a value of [option].
346-
void _validateAllowed(Option option, String value) {
360+
void _validateAllowed(Option option, String value, String arg) {
347361
if (option.allowed == null) return;
348362

349363
_validate(option.allowed!.contains(value),
350-
'"$value" is not an allowed value for option "${option.name}".');
364+
'"$value" is not an allowed value for option "$arg".', arg);
351365
}
352366
}
353367

pkgs/args/pubspec.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
name: args
2-
version: 2.5.1-wip
2+
version: 2.6.0-wip
33
description: >-
44
Library for defining parsers for parsing raw command-line arguments into a set
55
of options and values using GNU and POSIX style options.

pkgs/args/test/command_runner_test.dart

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -583,15 +583,15 @@ Run "test help" to see global options.
583583
expect(
584584
runner.run(['--asdf']),
585585
throwsUsageException(
586-
'Could not find an option named "asdf".', _defaultUsage));
586+
'Could not find an option named "--asdf".', _defaultUsage));
587587
});
588588

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

593593
expect(runner.run(['foo', '--asdf']),
594-
throwsUsageException('Could not find an option named "asdf".', '''
594+
throwsUsageException('Could not find an option named "--asdf".', '''
595595
Usage: test foo [arguments]
596596
-h, --help Print this usage information.
597597
@@ -616,7 +616,7 @@ Also, footer!'''));
616616
test('includes the footer in usage errors', () {
617617
expect(
618618
runner.run(['--bad']),
619-
throwsUsageException('Could not find an option named "bad".',
619+
throwsUsageException('Could not find an option named "--bad".',
620620
'$_defaultUsage\nAlso, footer!'));
621621
});
622622
});
@@ -652,7 +652,7 @@ newlines properly.'''));
652652

653653
test('includes the footer in usage errors', () {
654654
expect(runner.run(['--bad']),
655-
throwsUsageException('Could not find an option named "bad".', '''
655+
throwsUsageException('Could not find an option named "--bad".', '''
656656
Usage: test <command> [arguments]
657657
658658
Global options:
@@ -679,7 +679,7 @@ newlines properly.'''));
679679
expect(
680680
runner.run(['--bad']),
681681
throwsUsageException(
682-
'Could not find an option named "bad".', _defaultUsage));
682+
'Could not find an option named "--bad".', _defaultUsage));
683683
});
684684

685685
test("a top-level command doesn't exist", () {

pkgs/args/test/parse_test.dart

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -766,5 +766,54 @@ void main() {
766766
expect(results.rest, equals(['stop', '--', 'arg']));
767767
});
768768
});
769+
770+
group('ArgParser Exception Tests', () {
771+
test('throws exception for unknown option', () {
772+
var parser = ArgParser();
773+
throwsArgParserException(parser, ['--verbose'],
774+
'Could not find an option named "--verbose".', [], '--verbose');
775+
throwsArgParserException(
776+
parser, ['-v'], 'Could not find an option or flag "-v".', [], '-v');
777+
});
778+
779+
test('throws exception for flag with value', () {
780+
var parser = ArgParser();
781+
parser.addFlag('flag', abbr: 'f');
782+
throwsArgParserException(parser, ['--flag=1'],
783+
'Flag option "--flag" should not be given a value.', [], '--flag');
784+
throwsArgParserException(parser, ['-f=1'],
785+
'Option "-f" is a flag and cannot handle value "=1".', [], '-f');
786+
});
787+
788+
test('throws exception after parsing multiple options', () {
789+
var parser = ArgParser();
790+
parser.addOption('first');
791+
parser.addOption('second');
792+
throwsArgParserException(
793+
parser,
794+
['--first', '1', '--second', '2', '--verbose', '3'],
795+
'Could not find an option named "--verbose".',
796+
[],
797+
'--verbose');
798+
});
799+
800+
test('throws exception for option with invalid value', () {
801+
var parser = ArgParser();
802+
parser.addOption('first', allowed: ['a', 'b']);
803+
throwsArgParserException(parser, ['--first', 'c'],
804+
'"c" is not an allowed value for option "--first".', [], '--first');
805+
});
806+
807+
test('throws exception after parsing command', () {
808+
var parser = ArgParser();
809+
parser.addCommand('command', ArgParser());
810+
throwsArgParserException(
811+
parser,
812+
['command', '--verbose'],
813+
'Could not find an option named "--verbose".',
814+
['command'],
815+
'--verbose');
816+
});
817+
});
769818
});
770819
}

0 commit comments

Comments
 (0)