Option.Builder.option("") should throw IllegalArgumentException instead

of ArrayIndexOutOfBoundsException

Author: garydgregory

src/main/java/org/apache/commons/cli/OptionValidator.java

@@ -91,7 +91,7 @@ private static boolean search(final char[] chars, final char c) {
      * is valid are:
      *
      * <ul>
-     * <li>a single character {@code opt} that is either ' '(special case), '?', '@' or a letter</li>
+     * <li>a single character {@code opt} that is either Chars.SP(special case), '?', '@' or a letter</li>
      * <li>a multi character {@code opt} that only contains valid characters</li>
      * </ul>
      * </p><p>
@@ -118,17 +118,20 @@ static String validate(final String option) throws IllegalArgumentException {
         if (option == null) {
             return null;
         }
-
+        if (option.isEmpty()) {
+            throw new IllegalArgumentException("Empty option name.");
+        }
         final char[] chars = option.toCharArray();
-
-        if (!isValidOpt(chars[0])) {
-            throw new IllegalArgumentException("Illegal option name '" + chars[0] + "'");
+        final char ch0 = chars[0];
+        if (!isValidOpt(ch0)) {
+            throw new IllegalArgumentException(String.format("Illegal option name '%s'.", ch0));
         }
         // handle the multi-character opt
         if (option.length() > 1) {
             for (int i = 1; i < chars.length; i++) {
-                if (!isValidChar(chars[i])) {
-                    throw new IllegalArgumentException("The option '" + option + "' contains an illegal " + "character : '" + chars[i] + "'");
+                final char ch = chars[i];
+                if (!isValidChar(ch)) {
+                    throw new IllegalArgumentException(String.format("The option '%s' contains an illegal " + "character : '%s'.", option, ch));
                 }
             }
         }

src/test/java/org/apache/commons/cli/OptionTest.java

@@ -121,6 +121,13 @@ public void testBuilderInsufficientParams2() {
         assertThrows(IllegalArgumentException.class, () -> Option.builder(null).desc("desc").build());
     }
 
+    @Test
+    public void testBuilderInvalidOptionName0() {
+        assertNull(Option.builder().option(null));
+        assertThrows(IllegalArgumentException.class, () -> Option.builder().option(""));
+        assertThrows(IllegalArgumentException.class, () -> Option.builder().option(" "));
+    }
+
     @Test
     public void testBuilderInvalidOptionName1() {
         assertThrows(IllegalArgumentException.class, () -> Option.builder().option("invalid?"));