[remkop#1331] Bugfix for scope = INHERIT:

Avoid `IllegalArgumentException` when parent has no standard help options and `scope = INHERIT`, while subcommand does have mixed-in standard help options.
MarkoMackic · Oct 17, 2021 · 01fab04 · 01fab04
1 parent dda2504
commit 01fab04
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 7 deletions.
diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md
@@ -27,6 +27,7 @@ Picocli follows [semantic versioning](http://semver.org/).
 * [#1300] Bugfix: Avoid spurious warning "Could not set initial value for field boolean" when reusing `CommandLine` with ArgGroup. Thanks to [Yashodhan Ghadge](https://github.com/codexetreme) for raising this.
 * [#1316] Bugfix: Avoid `DuplicateOptionAnnotationsException` thrown on `mixinStandardHelpOptions` for subcommands when parent has `scope = INHERIT` by `picocli-codegen` annotation processor. Thanks to [Philippe Charles](https://github.com/charphi) for raising this.
 * [#1319] Bugfix: Avoid `DuplicateOptionAnnotationsException` when parent has inherited mixed-in help options and the built-in `HelpCommand` subcommand. Thanks to [Andreas Deininger](https://github.com/deining) for raising this.
+* [#1331] Bugfix: Avoid `IllegalArgumentException` when parent has no standard help options and `scope = INHERIT`, while subcommand does have mixed-in standard help options. Thanks to [Andreas Deininger](https://github.com/deining) for raising this.
 * [#1320][#1321] Bugfix/Enhancement: Use system properties `sun.stdout.encoding` and `sun.stderr.encoding` when creating the `PrintWriters` returned by  `CommandLine::getOut` and `CommandLine::getErr`. Thanks to [Philippe Charles](https://github.com/charphi) for the investigation and the pull request.
 * [#1296] DOC: add Kotlin code samples to user manual; other user manual improvements. Thanks to [Andreas Deininger](https://github.com/deining) for the pull request.
 * [#1299] DOC: Link to `IParameterPreprocessor` from `IParameterConsumer` javadoc. Thanks to [Andreas Deininger](https://github.com/deining) for the pull request.

diff --git a/src/main/java/picocli/CommandLine.java b/src/main/java/picocli/CommandLine.java
@@ -6425,8 +6425,8 @@ private void inheritAttributesFrom(CommandSpec root) {
                 updatedSubcommandsToInheritFrom(root);
             }
             private void updatedSubcommandsToInheritFrom(CommandSpec root) {
-                if (root != this) {
-                    mixinStandardHelpOptions(root.mixinStandardHelpOptions());
+                if (root != this && root.mixinStandardHelpOptions()) { // #1331 only add, don't remove
+                    mixinStandardHelpOptions(true);
                 }
                 Set<CommandLine> subcommands = new HashSet<CommandLine>(subcommands().values());
                 for (CommandLine sub : subcommands) {
@@ -7165,6 +7165,12 @@ public CommandSpec mixinStandardHelpOptions(boolean newValue) {
                         mixin.inherited = this.inherited();
                         addMixin(AutoHelpMixin.KEY, mixin);
                     }
+                    // #1331 if inherit(ed) we also add to subcommands
+                    if (scopeType() == ScopeType.INHERIT || inherited()) {
+                        for (CommandLine sub : new HashSet<CommandLine>(subcommands().values())) {
+                            sub.getCommandSpec().mixinStandardHelpOptions(newValue);
+                        }
+                    }
                 } else {
                     CommandSpec helpMixin = mixins.remove(AutoHelpMixin.KEY);
                     if (helpMixin != null) {
@@ -7176,11 +7182,7 @@ public CommandSpec mixinStandardHelpOptions(boolean newValue) {
                             }
                         }
                     }
-                }
-                if (scopeType() == ScopeType.INHERIT || inherited()) {
-                    for (CommandLine sub : new HashSet<CommandLine>(subcommands().values())) {
-                        sub.getCommandSpec().mixinStandardHelpOptions(newValue);
-                    }
+                    // #1331 we don't remove StandardHelpOptions from subcommands, even if they inherit from us
                 }
                 return this;
             }

diff --git a/src/test/java/picocli/InheritedOptionTest.java b/src/test/java/picocli/InheritedOptionTest.java
@@ -576,4 +576,21 @@ void sub(@Option(names = "-foo") int foo) {
         new CommandLine(app).execute("sub", "-foo", "42" );
         assertEquals(42, app.subFoo);
     }
+
+    @Test
+    public void testIssue1331() {
+        @Command(scope = CommandLine.ScopeType.INHERIT)
+        class InheritHelpApp  {
+            int subFoo;
+
+            @Command(mixinStandardHelpOptions = true)
+            void sub(@Option(names = "-foo") int foo) {
+                System.out.printf("Foo: %d", foo);
+                subFoo = foo;
+            }
+        }
+        InheritHelpApp app = new InheritHelpApp();
+        new CommandLine(app).execute("sub", "-foo", "42" );
+        assertEquals(42, app.subFoo);
+    }
 }