commit aa2434d301c49d100f50af544333886a6767ce9d
author Henri Yandell <bayard@apache.org> Mon Mar 16 07:26:44 2009 +0000
committer Henri Yandell <bayard@apache.org> Mon Mar 16 07:26:44 2009 +0000
tree 08922154bb54387aadc09ac2dbf371836c4a635a
parent 879c762baac4a9d7aade43d302e730e239a54c21

Applying Jorg Schaible's patch from CLI-177 to fix the OptionBuilder's not resetting when an Exception is thrown

git-svn-id: https://svn.apache.org/repos/asf/commons/proper/cli/branches/cli-1.x@754830 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/commons/cli/OptionBuilder.java

diff --git a/src/java/org/apache/commons/cli/OptionBuilder.java b/src/java/org/apache/commons/cli/OptionBuilder.java
index 83e0299..60b4967 100644
--- a/src/java/org/apache/commons/cli/OptionBuilder.java
+++ b/src/java/org/apache/commons/cli/OptionBuilder.java
@@ -326,6 +326,7 @@
     {
         if (longopt == null)
         {
+            OptionBuilder.reset();
             throw new IllegalArgumentException("must specify longopt");
         }
 
@@ -344,21 +345,23 @@
      */
     public static Option create(String opt) throws IllegalArgumentException
     {
-        // create the option
-        Option option = new Option(opt, description);
+        Option option = null;
+        try {
+            // create the option
+            option = new Option(opt, description);
 
-        // set the option properties
-        option.setLongOpt(longopt);
-        option.setRequired(required);
-        option.setOptionalArg(optionalArg);
-        option.setArgs(numberOfArgs);
-        option.setType(type);
-        option.setValueSeparator(valuesep);
-        option.setArgName(argName);
-
-
-        // reset the OptionBuilder properties
-        OptionBuilder.reset();
+            // set the option properties
+            option.setLongOpt(longopt);
+            option.setRequired(required);
+            option.setOptionalArg(optionalArg);
+            option.setArgs(numberOfArgs);
+            option.setType(type);
+            option.setValueSeparator(valuesep);
+            option.setArgName(argName);
+        } finally {
+            // reset the OptionBuilder properties
+            OptionBuilder.reset();
+        }
 
         // return the Option instance
         return option;

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

diff --git a/src/test/org/apache/commons/cli/OptionBuilderTest.java b/src/test/org/apache/commons/cli/OptionBuilderTest.java
index 868d263..b445a67 100644
--- a/src/test/org/apache/commons/cli/OptionBuilderTest.java
+++ b/src/test/org/apache/commons/cli/OptionBuilderTest.java
@@ -145,6 +145,33 @@
         catch (IllegalArgumentException e)
         {
             // expected
+            
+            // implicitly reset the builder
+            OptionBuilder.create( "opt" );
         }
     }
+
+    public void testBuilderIsResettedAlways() {
+        try
+        {
+            OptionBuilder.withDescription("JUnit").create('"');
+            fail("IllegalArgumentException expected");
+        }
+        catch (IllegalArgumentException e)
+        {
+            // expected
+        }
+        assertNull("we inherited a description", OptionBuilder.create('x').getDescription());
+
+        try
+        {
+            OptionBuilder.withDescription("JUnit").create();
+            fail("IllegalArgumentException expected");
+        }
+        catch (IllegalArgumentException e)
+        {
+            // expected
+        }
+        assertNull("we inherited a description", OptionBuilder.create('x').getDescription());
+    }
 }