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

pkg/args/test/args_test.dart

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library args_test;
 
 import 'package:unittest/unittest.dart';
 import 'package:args/args.dart';
 
 main() {
@@ skipping 14 matching lines @@
       var parser = new ArgParser();
       parser.addFlag('foo', abbr: 'f');
       throwsIllegalArg(() => parser.addFlag('flummox', abbr: 'f'));
     });
 
     test('throws ArgumentError if the abbreviation is longer '
          'than one character', () {
       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()', () {
     test('throws ArgumentError if the flag already exists', () {
       var parser = new ArgParser();
       parser.addFlag('foo');
       throwsIllegalArg(() => parser.addOption('foo'));
     });
 
     test('throws ArgumentError if the option already exists', () {
       var parser = new ArgParser();
       parser.addOption('foo');
       throwsIllegalArg(() => parser.addOption('foo'));
     });
 
     test('throws ArgumentError if the abbreviation exists', () {
       var parser = new ArgParser();
       parser.addFlag('foo', abbr: 'f');
       throwsIllegalArg(() => parser.addOption('flummox', abbr: 'f'));
     });
 
     test('throws ArgumentError if the abbreviation is longer '
          'than one character', () {
       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()', () {
     test('returns the default value for an option', () {
       var parser = new ArgParser();
       parser.addOption('mode', defaultsTo: 'debug');
       expect(parser.getDefault('mode'), 'debug');
     });
 
     test('throws if the option is unknown', () {
@@ skipping 77 matching lines @@
 
   group('ArgResults[]', () {
     test('throws if the name is not an option', () {
       var parser = new ArgParser();
       var results = parser.parse([]);
       throwsIllegalArg(() => results['unknown']);
     });
   });
 }
 
-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'];