pkg/args Option should be more strict about names and abbreviations

pkg/args/test/args_test.dart

Index: pkg/args/test/args_test.dart
diff --git a/pkg/args/test/args_test.dart b/pkg/args/test/args_test.dart
index d807291e04d840e5e3f36f85463462deba04e1b9..3110c4d12a547e700ad60e6df4ad86c70d358b26 100644
--- a/pkg/args/test/args_test.dart
+++ b/pkg/args/test/args_test.dart
@@ -32,6 +32,24 @@ main() {
       var parser = new ArgParser();
       throwsIllegalArg(() => parser.addFlag('flummox', abbr: 'flu'));
     });
+
+    test('throws ArgumentError if a flag name is invalid', () {
+      var parser = new ArgParser();
+
+      for(var name in _invalidOptions) {
+        var reason = '${Error.safeToString(name)} is not valid';
+        throwsIllegalArg(() => parser.addFlag(name), reason: reason);
+      }
+    });
+
+    test('accepts valid flag names', () {
+      var parser = new ArgParser();
+
+      for(var name in _validOptions) {
+        var reason = '${Error.safeToString(name)} is valid';
+        expect(() => parser.addFlag(name), returnsNormally, reason: reason);
+      }
+    });
   });
 
   group('ArgParser.addOption()', () {
@@ -58,6 +76,46 @@ main() {
       var parser = new ArgParser();
       throwsIllegalArg(() => parser.addOption('flummox', abbr: 'flu'));
     });
+
+    test('throws ArgumentError if the abbreviation is empty', () {
+      var parser = new ArgParser();
+      throwsIllegalArg(() => parser.addOption('flummox', abbr: ''));
+    });
+
+    test('throws ArgumentError if the abbreviation is an invalid value', () {
+      var parser = new ArgParser();
+      for(var name in _invalidOptions.where((v) => v != null)) {
+        throwsIllegalArg(() => parser.addOption('flummox', abbr: name));
+      }
+    });
+
+    test('throws ArgumentError if the abbreviation is a dash', () {
+      var parser = new ArgParser();
+      throwsIllegalArg(() => parser.addOption('flummox', abbr: '-'));
+    });
+
+    test('allows explict null value for "abbr"', () {
+      var parser = new ArgParser();
+      expect(() => parser.addOption('flummox', abbr: null), returnsNormally);
+    });
+
+    test('throws ArgumentError if an option name is invalid', () {
+      var parser = new ArgParser();
+
+      for(var name in _invalidOptions) {
+        var reason = '${Error.safeToString(name)} is not valid';
+        throwsIllegalArg(() => parser.addOption(name), reason: reason);
+      }
+    });
+
+    test('accepts valid option names', () {
+      var parser = new ArgParser();
+
+      for(var name in _validOptions) {
+        var reason = '${Error.safeToString(name)} is valid';
+        expect(() => parser.addOption(name), returnsNormally, reason: reason);
+      }
+    });
   });
 
   group('ArgParser.getDefault()', () {
@@ -155,10 +213,33 @@ main() {
   });
 }
 
-throwsIllegalArg(function) {
-  expect(function, throwsArgumentError);
+throwsIllegalArg(function, {String reason: null}) {
+  expect(function, throwsArgumentError, reason: reason);
 }
 
 throwsFormat(ArgParser parser, List<String> args) {
   expect(() => parser.parse(args), throwsFormatException);
 }
+
+const _invalidOptions = const
+[

[Bob Nystrom, 2013/03/14 17:37:11]

Put the "[" on the preceding line, and the "]" on its own, like:

const _invalidOptions = const [
  ...
];

Also, since it's const, make the name ALL_CAPS (or just don't use const).

+ ' ', '', '-', '--', '--foo',
+ ' with space',
+ 'with\ttab',
+ 'with\rcarriage\rreturn',
+ 'with\nline\nfeed',
+ "'singlequotes'",
+ '"doublequotes"',
+ 'back\\slash',
+ 'forward/slash'];
+
+const _validOptions = const
+[
+ 'a' // one char
+ 'contains-dash',
+ 'contains_underscore',
+ 'ends-with-dash-',
+ 'contains--doubledash--',
+ '1starts-with-number',
+ 'contains-a-1number',
+ 'ends-with-a-number8'];