[#2149] bugfix: ArgSpecs are not equal if their enclosing command is …

…different

RELEASE-NOTES.md

@@ -31,6 +31,7 @@ Artifacts in this release are signed by Remko Popma (6601 E5C0 8DCC BB96).
 * [#2058] Bugfix: `defaultValue` should not be applied in addition to user-specified value for options with a custom `IParameterConsumer`. Thanks to [Staffan Arvidsson McShane](https://github.com/StaffanArvidsson) for raising this.
 * [#2148] Bugfix: Fix NPE in jline3 `Example.jar` as `ConfigurationPath` cannot be `null` anymore. Thanks to [llzen44](https://github.com/llzen44) for the pull request.
 * [#2232] Bugfix: fix bug for `Optional<T>` arguments with initial value. Thanks to [hq6](https://github.com/hq6) for raising this.
+* [#2149] Bugfix: `@Option`-annotated setter method not invoked with default value when used in mixin for both command and subcommand. Thanks to [Zhonghao Wang](https://github.com/JBWKZsf) for raising this.
 * [#2170] Enhancement: Use `...` vararg instead of array parameter to match overridden method signature. Thanks to [Michael Vorburger](https://github.com/vorburger) for the pull request.
 * [#2234] BUILD: fix errorprone `TruthSelfEquals` warnings
 * [#2172] BUILD: Fix broken build. Thanks to [Michael Vorburger](https://github.com/vorburger) for the pull request.

src/main/java/picocli/CommandLine.java

@@ -6782,7 +6782,7 @@ public CommandSpec addOption(OptionSpec option) {
                 for (String name : interpolator.interpolate(option.names())) { // cannot be null or empty
                     String existingName = optionsByNameMap.getCaseSensitiveKey(name);
                     OptionSpec existing = optionsByNameMap.put(name, option);
-                    if (existing != null) { /* was: && !existing.equals(option)) {*/ // since 4.0 ArgGroups: an option cannot be in multiple groups
+                    if (existing != null && !existing.equals(option)/* equals check needed after fix for #2149 */) { // since 4.0 ArgGroups: an option cannot be in multiple groups
                         throw DuplicateOptionAnnotationsException.create(existingName, option, existing);
                     }
                     // #1022 checks if negated options exist with the same name
@@ -9383,7 +9383,8 @@ private static String restoreQuotedValues(String part, Queue<String> quotedValue
             }
 
             protected boolean equalsImpl(ArgSpec other) {
-                return Assert.equals(this.defaultValue, other.defaultValue)
+                return this.commandSpec == other.commandSpec
+                        && Assert.equals(this.defaultValue, other.defaultValue)
                         && Assert.equals(this.mapFallbackValue, other.mapFallbackValue)
                         && Assert.equals(this.arity, other.arity)
                         && Assert.equals(this.hidden, other.hidden)
@@ -9402,6 +9403,7 @@ protected boolean equalsImpl(ArgSpec other) {
             }
             protected int hashCodeImpl() {
                 return 17
+                        + 37 * Assert.hashCode(commandSpec)
                         + 37 * Assert.hashCode(defaultValue)
                         + 37 * Assert.hashCode(mapFallbackValue)
                         + 37 * Assert.hashCode(arity)

src/test/java/picocli/CommandMethodTest.java

@@ -449,6 +449,8 @@ public CompactFields run(
             return ret;
         }
     }
+
+    @Ignore("This is no longer true after the fix for #2149") // TODO DELETME?
     @Test
     public void testAnnotateMethod_matchesAnnotatedClass() throws Exception {
         setTraceLevel(CommandLine.TraceLevel.OFF);

src/test/java/picocli/Issue2149.java

@@ -0,0 +1,101 @@
+package picocli;
+
+import org.junit.Test;
+import picocli.CommandLine.Command;
+import picocli.CommandLine.Mixin;
+import picocli.CommandLine.Model.CommandSpec;
+import picocli.CommandLine.Option;
+import picocli.CommandLine.ParseResult;
+
+import static org.junit.Assert.*;
+
+import static picocli.CommandLine.Spec.Target.MIXEE;
+
+public class Issue2149 {
+    static class InputOptions {
+        final static String DEFAULT_ENV = "env";
+        final static String DEFAULT_REGION = "region";
+
+        private String env;
+        private String region;
+
+        @CommandLine.Spec(MIXEE)
+        private CommandSpec mixee;
+
+        @Option(
+            names = {"-e", "--env"},
+            defaultValue = DEFAULT_ENV)
+        public void setEnv(String env) {
+            this.env = env;
+            if (!DEFAULT_ENV.equals(env) && !this.equals(getRoot().inputOptions)) {
+                getRoot().inputOptions.setEnv(env);
+            }
+        }
+
+        @Option(
+            names = {"-r", "--region"},
+            defaultValue = DEFAULT_REGION)
+        public void setRegion(String region) {
+            this.region = region;
+            if (!DEFAULT_REGION.equals(region) && !this.equals(getRoot().inputOptions)) {
+                getRoot().inputOptions.setRegion(region);
+            }
+        }
+
+
+        public String getEnv() {
+            if (this.equals(getRoot().inputOptions)) return env;
+            return getRoot().inputOptions.getEnv();
+        }
+
+        public String getRegion() {
+            if (this.equals(getRoot().inputOptions)) return region;
+            return getRoot().inputOptions.getRegion();
+        }
+
+        private A getRoot() {
+            return (A) mixee.root().userObject();
+        }
+    }
+
+    @Command(name = "A", subcommands = B.class)
+    static class A {
+        @Mixin InputOptions inputOptions;
+    }
+
+    @Command(name = "B", subcommands = C.class)
+    static class B {
+        @Mixin InputOptions inputOptions;
+    }
+
+    @Command(name = "C")
+    static class C {
+        @Mixin InputOptions inputOptions;
+    }
+
+    @Test
+    public void testDefaultValueInvoked() {
+        A a = new A();
+        ParseResult parseResult = new CommandLine(a).parseArgs("B -e XX C".split(" "));
+        assertEquals(InputOptions.DEFAULT_ENV, a.inputOptions.env);
+
+        B b = (B) parseResult.subcommand().commandSpec().userObject();
+        assertEquals("XX", b.inputOptions.env);
+
+        C c = (C) parseResult.subcommand().subcommand().commandSpec().userObject();
+        assertEquals(InputOptions.DEFAULT_ENV, c.inputOptions.env);
+    }
+
+    @Test
+    public void testDefaultValueInvoked2() {
+        A a = new A();
+        ParseResult parseResult = new CommandLine(a).parseArgs("B C -e XX".split(" "));
+        assertEquals(InputOptions.DEFAULT_ENV, a.inputOptions.env);
+
+        B b = (B) parseResult.subcommand().commandSpec().userObject();
+        assertEquals(InputOptions.DEFAULT_ENV, b.inputOptions.env);
+
+        C c = (C) parseResult.subcommand().subcommand().commandSpec().userObject();
+        assertEquals("XX", c.inputOptions.env);
+    }
+}