From 9cdc8726b082b838f6a2f55f42584aa49d6425c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=B5=E3=81=81?= <34892635+fa0311@users.noreply.github.com> Date: Tue, 1 Oct 2024 05:48:43 +0900 Subject: [PATCH 1/2] Add argument name when throwing a `ArgParserException`. (#283) Add an `argumentName` field which tracks the argument that was being parse when the exception is thrown. --- CHANGELOG.md | 4 +- lib/src/arg_parser_exception.dart | 10 +++- lib/src/arg_results.dart | 10 ++-- lib/src/parser.dart | 78 ++++++++++++++++++------------- pubspec.yaml | 2 +- test/command_runner_test.dart | 10 ++-- test/parse_test.dart | 49 +++++++++++++++++++ test/test_utils.dart | 19 +++++++- test/trailing_options_test.dart | 15 +++--- 9 files changed, 143 insertions(+), 54 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e572ec..a205299 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/lib/src/arg_parser_exception.dart b/lib/src/arg_parser_exception.dart index d727d70..fbee82b 100644 --- a/lib/src/arg_parser_exception.dart +++ b/lib/src/arg_parser_exception.dart @@ -9,6 +9,14 @@ class ArgParserException extends FormatException { /// This will be empty if the error was on the root parser. final List commands; - ArgParserException(super.message, [Iterable? commands]) + /// The name of the argument that was being parsed when the error was + /// discovered. + final String? argumentName; + + ArgParserException(super.message, + [Iterable? commands, + this.argumentName, + super.source, + super.offset]) : commands = commands == null ? const [] : List.unmodifiable(commands); } diff --git a/lib/src/arg_results.dart b/lib/src/arg_results.dart index 9fa87cd..72c4410 100644 --- a/lib/src/arg_results.dart +++ b/lib/src/arg_results.dart @@ -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]!; @@ -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.'); @@ -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.'); @@ -111,7 +111,7 @@ class ArgResults { List 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.'); @@ -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); diff --git a/lib/src/parser.dart b/lib/src/parser.dart index 3c5dfed..660e56d 100644 --- a/lib/src/parser.dart +++ b/lib/src/parser.dart @@ -63,7 +63,8 @@ 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); @@ -71,7 +72,11 @@ class Parser { 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. @@ -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 @@ -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(); } @@ -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); } @@ -154,7 +160,7 @@ class Parser { if (option.isFlag) { _setFlag(_results, option, true); } else { - _readNextArgAsValue(option); + _readNextArgAsValue(option, '-$opt'); } return true; @@ -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 @@ -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); } @@ -269,16 +276,16 @@ 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. @@ -286,18 +293,22 @@ class Parser { 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); } @@ -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? 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; } @@ -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); } } @@ -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); } } diff --git a/pubspec.yaml b/pubspec.yaml index 2b70992..8e59181 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -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. diff --git a/test/command_runner_test.dart b/test/command_runner_test.dart index cc80c6b..b9fde8a 100644 --- a/test/command_runner_test.dart +++ b/test/command_runner_test.dart @@ -583,7 +583,7 @@ 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', () { @@ -591,7 +591,7 @@ Run "test help" to see global options. 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. @@ -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!')); }); }); @@ -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 [arguments] Global options: @@ -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", () { diff --git a/test/parse_test.dart b/test/parse_test.dart index b2dda44..9501b5d 100644 --- a/test/parse_test.dart +++ b/test/parse_test.dart @@ -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'); + }); + }); }); } diff --git a/test/test_utils.dart b/test/test_utils.dart index f19da6d..f7d8b8a 100644 --- a/test/test_utils.dart +++ b/test/test_utils.dart @@ -343,8 +343,23 @@ void throwsIllegalArg(void Function() function, {String? reason}) { expect(function, throwsArgumentError, reason: reason); } -void throwsFormat(ArgParser parser, List args) { - expect(() => parser.parse(args), throwsFormatException); +void throwsFormat(ArgParser parser, List args, {String? reason}) { + expect(() => parser.parse(args), throwsA(isA()), + reason: reason); +} + +void throwsArgParserException(ArgParser parser, List args, + String message, List commands, String arg) { + try { + parser.parse(args); + fail('Expected an ArgParserException'); + } on ArgParserException catch (e) { + expect(e.message, message); + expect(e.commands, commands); + expect(e.argumentName, arg); + } catch (e) { + fail('Expected an ArgParserException, but got $e'); + } } Matcher throwsUsageException(Object? message, Object? usage) => diff --git a/test/trailing_options_test.dart b/test/trailing_options_test.dart index 505b6b1..db502d1 100644 --- a/test/trailing_options_test.dart +++ b/test/trailing_options_test.dart @@ -5,6 +5,8 @@ import 'package:args/args.dart'; import 'package:test/test.dart'; +import 'test_utils.dart'; + void main() { test('allowTrailingOptions defaults to true', () { var parser = ArgParser(); @@ -17,9 +19,8 @@ void main() { parser = ArgParser(allowTrailingOptions: true); }); - void expectThrows(List args) { - expect(() => parser.parse(args), throwsFormatException, - reason: 'with allowTrailingOptions: true'); + void expectThrows(List args, String arg) { + throwsFormat(parser, args, reason: 'with allowTrailingOptions: true'); } test('collects non-options in rest', () { @@ -56,7 +57,7 @@ void main() { test('throws on a trailing option missing its value', () { parser.addOption('opt'); - expectThrows(['arg', '--opt']); + expectThrows(['arg', '--opt'], '--opt'); }); test('parses a trailing option', () { @@ -67,16 +68,16 @@ void main() { }); test('throws on a trailing unknown flag', () { - expectThrows(['arg', '--xflag']); + expectThrows(['arg', '--xflag'], '--xflag'); }); test('throws on a trailing unknown option and value', () { - expectThrows(['arg', '--xopt', 'v']); + expectThrows(['arg', '--xopt', 'v'], '--xopt'); }); test('throws on a command', () { parser.addCommand('com'); - expectThrows(['arg', 'com']); + expectThrows(['arg', 'com'], 'com'); }); }); From 09c0fca1785c9df39288a48f767994eed80bed40 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 1 Oct 2024 23:54:08 +0000 Subject: [PATCH 2/2] Bump actions/checkout from 4.1.7 to 4.2.0 in the github-actions group (#286) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps the github-actions group with 1 update: [actions/checkout](https://github.com/actions/checkout). Updates `actions/checkout` from 4.1.7 to 4.2.0
Release notes

Sourced from actions/checkout's releases.

v4.2.0

What's Changed

New Contributors

Full Changelog: https://github.com/actions/checkout/compare/v4.1.7...v4.2.0

Changelog

Sourced from actions/checkout's changelog.

Changelog

v4.2.0

v4.1.7

v4.1.6

v4.1.5

v4.1.4

v4.1.3

v4.1.2

v4.1.1

v4.1.0

v4.0.0

v3.6.0

... (truncated)

Commits

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=actions/checkout&package-manager=github_actions&previous-version=4.1.7&new-version=4.2.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore major version` will close this group update PR and stop Dependabot creating any more for the specific dependency's major version (unless you unignore this specific dependency's major version or upgrade to it yourself) - `@dependabot ignore minor version` will close this group update PR and stop Dependabot creating any more for the specific dependency's minor version (unless you unignore this specific dependency's minor version or upgrade to it yourself) - `@dependabot ignore ` will close this group update PR and stop Dependabot creating any more for the specific dependency (unless you unignore this specific dependency or upgrade to it yourself) - `@dependabot unignore ` will remove all of the ignore conditions of the specified dependency - `@dependabot unignore ` will remove the ignore condition of the specified dependency and ignore conditions
--- .github/workflows/test-package.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test-package.yml b/.github/workflows/test-package.yml index 5afa636..5039169 100644 --- a/.github/workflows/test-package.yml +++ b/.github/workflows/test-package.yml @@ -21,7 +21,7 @@ jobs: matrix: sdk: [dev] steps: - - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 + - uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 - uses: dart-lang/setup-dart@0a8a0fc875eb934c15d08629302413c671d3f672 with: sdk: ${{ matrix.sdk }} @@ -47,7 +47,7 @@ jobs: os: [ubuntu-latest] sdk: ['3.3', dev] steps: - - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 + - uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 - uses: dart-lang/setup-dart@0a8a0fc875eb934c15d08629302413c671d3f672 with: sdk: ${{ matrix.sdk }}