From fa33be1b57f2fff8955b223a64e4bbd0bd2b8b05 Mon Sep 17 00:00:00 2001 From: Remko Popma Date: Mon, 6 May 2024 11:11:42 +0900 Subject: [PATCH] [#2149] bugfix: ArgSpecs are not equal if their enclosing command is different --- RELEASE-NOTES.md | 1 + src/main/java/picocli/CommandLine.java | 6 +- src/test/java/picocli/CommandMethodTest.java | 2 + src/test/java/picocli/Issue2149.java | 101 +++++++++++++++++++ 4 files changed, 108 insertions(+), 2 deletions(-) create mode 100644 src/test/java/picocli/Issue2149.java diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index 32d6b7034..84883aa08 100644 --- a/RELEASE-NOTES.md +++ b/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` 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. diff --git a/src/main/java/picocli/CommandLine.java b/src/main/java/picocli/CommandLine.java index 1b440895f..46953013a 100644 --- a/src/main/java/picocli/CommandLine.java +++ b/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 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) diff --git a/src/test/java/picocli/CommandMethodTest.java b/src/test/java/picocli/CommandMethodTest.java index d8f979343..d283f8553 100644 --- a/src/test/java/picocli/CommandMethodTest.java +++ b/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); diff --git a/src/test/java/picocli/Issue2149.java b/src/test/java/picocli/Issue2149.java new file mode 100644 index 000000000..0f88182d5 --- /dev/null +++ b/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); + } +}