[#354] Bug fix: Interpreter should reset options and positional param…

…eters to their initial value before parsing new input. Closes #354
remkop · Apr 12, 2018 · 0607c98 · 0607c98
1 parent 113d990
commit 0607c98
Show file tree

Hide file tree

Showing 4 changed files with 125 additions and 3 deletions.
diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md
@@ -31,6 +31,7 @@ No features have been promoted in this picocli release.
 - [#350] Enhancement: Improve error message for `usageHelp` and `versionHelp` validation.
 - [#344] Enhancement: Don't show WARN message for unmatched args or overwritten options.
 - [#351] Documentation: Improve javadoc for OptionSpec.usageHelp and versionHelp.
+- [#354] Bug fix: Interpreter should reset options and positional parameters to their initial value before parsing new input.
 
 ## <a name="3.0.0-alpha-6-deprecated"></a> Deprecations
 - The `Help.Ansi.Text::append` method is now deprecated in favour of the new `concat` method.

diff --git a/src/main/java/picocli/CommandLine.java b/src/main/java/picocli/CommandLine.java
@@ -4742,11 +4742,21 @@ private void clear() {
                 option.resetStringValues();
                 option.resetOriginalStringValues();
                 option.typedValues.clear();
+                try {
+                    option.setter().set(option.initialValue());
+                } catch (Exception ex) {
+                    tracer.warn("Could not clear value for %s: %s", option, ex);
+                }
             }
             for (PositionalParamSpec positional : getCommandSpec().positionalParameters()) {
                 positional.resetStringValues();
                 positional.resetOriginalStringValues();
                 positional.typedValues.clear();
+                try {
+                    positional.setter().set(positional.initialValue());
+                } catch (Exception ex) {
+                    tracer.warn("Could not clear value for %s: %s", positional, ex);
+                }
             }
         }
 

diff --git a/src/test/java/picocli/CommandLineModelTest.java b/src/test/java/picocli/CommandLineModelTest.java
@@ -1397,4 +1397,115 @@ public void testOptionLongestName_returnsLongest() {
         assertEquals("-aaa", OptionSpec.builder("-x", "-xx", "-aaa").build().longestName());
         assertEquals("-abcd", OptionSpec.builder("-x", "-abcd", "-aaa").build().longestName());
     }
+
+    @Test
+    public void testClearArrayOptionOldValueBeforeParse() {
+        CommandSpec cmd = CommandSpec.create();
+        cmd.addOption(OptionSpec.builder("-x").arity("2..3").build());
+
+        CommandLine cl = new CommandLine(cmd);
+        cl.parseArgs("-x", "1", "2", "3");
+        assertArrayEquals(new String[] {"1", "2", "3"}, (String[]) cmd.findOption("x").getValue());
+
+        cl.parseArgs("-x", "4", "5");
+        assertArrayEquals(new String[] {"4", "5"}, (String[]) cmd.findOption("x").getValue());
+    }
+
+    @Test
+    public void testClearListOptionOldValueBeforeParse() {
+        CommandSpec cmd = CommandSpec.create();
+        cmd.addOption(OptionSpec.builder("-x").type(List.class).build());
+
+        CommandLine cl = new CommandLine(cmd);
+        cl.parseArgs("-x", "1", "-x", "2", "-x", "3");
+        assertEquals(Arrays.asList("1", "2", "3"), cmd.findOption("x").getValue());
+
+        cl.parseArgs("-x", "4", "-x", "5");
+        assertEquals(Arrays.asList("4", "5"), cmd.findOption("x").getValue());
+    }
+
+    @Test
+    public void testClearMapOptionOldValueBeforeParse() {
+        CommandSpec cmd = CommandSpec.create();
+        cmd.addOption(OptionSpec.builder("-x").type(Map.class).build());
+
+        CommandLine cl = new CommandLine(cmd);
+        cl.parseArgs("-x", "A=1", "-x", "B=2", "-x", "C=3");
+        Map<String, String> expected = new LinkedHashMap<String, String>();
+        expected.put("A", "1");
+        expected.put("B", "2");
+        expected.put("C", "3");
+        assertEquals(expected, cmd.findOption("x").getValue());
+
+        cl.parseArgs("-x", "D=4", "-x", "E=5");
+        expected = new LinkedHashMap<String, String>();
+        expected.put("D", "4");
+        expected.put("E", "5");
+        assertEquals(expected, cmd.findOption("x").getValue());
+    }
+
+    @Test
+    public void testClearSimpleOptionOldValueBeforeParse() {
+        CommandSpec cmd = CommandSpec.create();
+        cmd.addOption(OptionSpec.builder("-x").type(String.class).build());
+
+        CommandLine cl = new CommandLine(cmd);
+        cl.parseArgs("-x", "1");
+        assertEquals("1", cmd.findOption("x").getValue());
+
+        cl.parseArgs("-x", "2");
+        assertEquals("2", cmd.findOption("x").getValue());
+    }
+
+    @Test
+    public void testOptionClearCustomSetterBeforeParse() {
+        CommandSpec cmd = CommandSpec.create();
+        final List<Object> values = new ArrayList<Object>();
+        ISetter setter = new ISetter() {
+            public <T> T set(T value) throws Exception {
+                values.add(value);
+                return null;
+            }
+        };
+        cmd.addOption(OptionSpec.builder("-x").type(String.class).setter(setter).build());
+
+        CommandLine cl = new CommandLine(cmd);
+        assertTrue(values.isEmpty());
+        cl.parseArgs("-x", "1");
+        assertEquals(2, values.size());
+        assertEquals(null, values.get(0));
+        assertEquals("1", values.get(1));
+
+        values.clear();
+        cl.parseArgs("-x", "2");
+        assertEquals(2, values.size());
+        assertEquals(null, values.get(0));
+        assertEquals("2", values.get(1));
+    }
+
+    @Test
+    public void testPositionalClearCustomSetterBeforeParse() {
+        CommandSpec cmd = CommandSpec.create();
+        final List<Object> values = new ArrayList<Object>();
+        ISetter setter = new ISetter() {
+            public <T> T set(T value) throws Exception {
+                values.add(value);
+                return null;
+            }
+        };
+        cmd.add(PositionalParamSpec.builder().type(String.class).setter(setter).build());
+
+        CommandLine cl = new CommandLine(cmd);
+        assertTrue(values.isEmpty());
+        cl.parseArgs("1");
+        assertEquals(2, values.size());
+        assertEquals(null, values.get(0));
+        assertEquals("1", values.get(1));
+
+        values.clear();
+        cl.parseArgs("2");
+        assertEquals(2, values.size());
+        assertEquals(null, values.get(0));
+        assertEquals("2", values.get(1));
+    }
 }
diff --git a/src/test/java/picocli/CommandLineTest.java b/src/test/java/picocli/CommandLineTest.java
@@ -3927,9 +3927,9 @@ class Flags {
         assertFalse(!flags.p0);
         assertTrue (!flags.p1);
         commandLine.parse("-a", "-b", "true", "false");
-        assertFalse(flags.a);
-        assertTrue (flags.b);
-        assertFalse(!flags.p0); // as set
+        assertFalse(!flags.a);
+        assertTrue (!flags.b);
+        assertFalse(!flags.p0);
         assertTrue (!flags.p1);
     }