[remkop#1001] Support required inherited options

RELEASE-NOTES.md

@@ -182,7 +182,8 @@ class AdvancedMixin {
 
 ## <a name="4.3.0-fixes"></a> Fixed issues
 * [#649][#948] Provide convenience API for inherited/global options (was: Feature request: inheriting mixins in subcommands). Thanks to [Garret Wilson](https://github.com/garretwilson) for the request and subsequent discussion (and patience!).
-* [#996] Default values should not be applied to inherited options. 
+* [#996] Default values should not be applied to inherited options.
+* [#1001] Support required inherited options.
 * [#958] API: Add `@Spec(Spec.Target.MIXEE)` annotation element to allow mixins to get a reference to the command they are mixed into.
 * [#960] API: Add method `CommandSpec::root` to return the `CommandSpec` of the top-level command.
 * [#845][#1008] API: Error handlers now use ANSI colors and styles. Added methods `errors` and `stackTraces` to `Help.ColorScheme`. Thanks to [Neko Null](https://github.com/jerrylususu) for the pull request.

src/main/java/picocli/CommandLine.java

@@ -11673,7 +11673,7 @@ List<CommandLine> parse(String... args) {
  Stack<String> arguments = new Stack<String>();
  arguments.addAll(reverseList(expanded));
  List<CommandLine> result = new ArrayList<CommandLine>();
- parse(result, arguments, args, new ArrayList<Object>());
+ parse(result, arguments, args, new ArrayList<Object>(), new HashSet<ArgSpec>());
  return result;
  }
 
@@ -11771,7 +11771,7 @@ void maybeThrow(PicocliException ex) throws PicocliException {
  }
  }
 
- private void parse(List<CommandLine> parsedCommands, Stack<String> argumentStack, String[] originalArgs, List<Object> nowProcessing) {
+ private void parse(List<CommandLine> parsedCommands, Stack<String> argumentStack, String[] originalArgs, List<Object> nowProcessing, Set<ArgSpec> inheritedRequired) {
  if (tracer.isDebug()) {
  tracer.debug("Initializing %s: %d options, %d positional parameters, %d required, %d groups, %d subcommands.%n",
  commandSpec.toString(), new HashSet<ArgSpec>(commandSpec.optionsMap().values()).size(),
@@ -11781,6 +11781,7 @@ private void parse(List<CommandLine> parsedCommands, Stack<String> argumentStack
  clear(); // first reset any state in case this CommandLine instance is being reused
  parsedCommands.add(CommandLine.this);
  List<ArgSpec> required = new ArrayList<ArgSpec>(commandSpec.requiredArgs());
+ addPostponedRequiredArgs(inheritedRequired, required);
  Set<ArgSpec> initialized = new LinkedHashSet<ArgSpec>();
  Collections.sort(required, new PositionalParametersSorter());
  boolean continueOnError = commandSpec.parser().collectErrors();
@@ -11808,6 +11809,34 @@ private void parse(List<CommandLine> parsedCommands, Stack<String> argumentStack
  }
  }
 
+ private void addPostponedRequiredArgs(Set<ArgSpec> inheritedRequired, List<ArgSpec> required) {
+ for (ArgSpec postponed : inheritedRequired) {
+ if (postponed.isOption()) {
+ OptionSpec inherited = commandSpec.findOption(((OptionSpec) postponed).longestName());
+ Assert.notNull(inherited, "inherited option " + postponed);
+ required.add(inherited);
+ } else {
+ PositionalParamSpec positional = (PositionalParamSpec) postponed;
+ for (PositionalParamSpec existing : commandSpec.positionalParameters()) {
+ if (existing.inherited()
+ && existing.index().equals(positional.index())
+ && existing.arity().equals(positional.arity())
+ && existing.typeInfo().equals(positional.typeInfo())
+ && Assert.equals(existing.paramLabel(), positional.paramLabel())
+ && Assert.equals(existing.hideParamSyntax(), positional.hideParamSyntax())
+ && Assert.equals(existing.required(), positional.required())
+ && Assert.equals(existing.splitRegex(), positional.splitRegex())
+ && Arrays.equals(existing.description(), positional.description())
+ && Assert.equals(existing.descriptionKey(), positional.descriptionKey())
+ && Assert.equals(existing.parameterConsumer(), positional.parameterConsumer())
+ ) {
+ required.add(existing);
+ }
+ }
+ }
+ }
+ }
+
  private void validateConstraints(Stack<String> argumentStack, List<ArgSpec> required, Set<ArgSpec> matched) {
  if (!required.isEmpty()) {
  for (ArgSpec missing : required) {
@@ -11986,13 +12015,24 @@ else if (config().posixClusteredShortOptionsAllowed() && arg.length() > 2 && arg
  }
 
  private void processSubcommand(CommandLine subcommand, ParseResult.Builder builder, List<CommandLine> parsedCommands, Stack<String> args, Collection<ArgSpec> required, String[] originalArgs, List<Object> nowProcessing, String separator, String arg) {
+ if (tracer.isDebug()) {tracer.debug("Found subcommand '%s' (%s)%n", arg, subcommand.commandSpec.toString());}
  nowProcessing.add(subcommand.commandSpec);
  updateHelpRequested(subcommand.commandSpec);
+ Set<ArgSpec> inheritedRequired = new HashSet<ArgSpec>();
+ if (tracer.isDebug()) {tracer.debug("Checking required args for parent %s...%n", subcommand.commandSpec.parent());}
+ Iterator<ArgSpec> requiredIter = required.iterator();
+ while (requiredIter.hasNext()) {
+ ArgSpec requiredArg = requiredIter.next();
+ if (requiredArg.scopeType() == ScopeType.INHERIT || requiredArg.inherited()) {
+ if (tracer.isDebug()) {tracer.debug("Postponing validation for required %s: scopeType=%s, inherited=%s%n", optionDescription("", requiredArg, -1), requiredArg.scopeType(), requiredArg.inherited());}
+ inheritedRequired.add(requiredArg);
+ requiredIter.remove();
+ }
+ }
  if (!isAnyHelpRequested() && !required.isEmpty()) { // ensure current command portion is valid
  throw MissingParameterException.create(CommandLine.this, required, separator);
  }
- if (tracer.isDebug()) {tracer.debug("Found subcommand '%s' (%s)%n", arg, subcommand.commandSpec.toString());}
- subcommand.interpreter.parse(parsedCommands, args, originalArgs, nowProcessing);
+ subcommand.interpreter.parse(parsedCommands, args, originalArgs, nowProcessing, inheritedRequired);
  builder.subcommand(subcommand.interpreter.parseResultBuilder.build());
  }
 

src/test/java/picocli/InheritedOptionTest.java

@@ -333,10 +333,10 @@ public void testInheritedOptionsWithDefault() {
  assertEquals("yyy", bean.y);
  }
 
- @Ignore("Requires https://github.com/remkop/picocli/issues/1001")
+ //@Ignore("Requires https://github.com/remkop/picocli/issues/1001")
  @Test // https://github.com/remkop/picocli/issues/1001
  public void testInheritedRequiredArgs() {
- System.setProperty("picocli.trace", "DEBUG");
+ //System.setProperty("picocli.trace", "DEBUG");
  @Command
  class App {
  @Option(names = "-x", required = true, scope = INHERIT) int x;
@@ -353,15 +353,15 @@ class App {
  cmd2.parseArgs();
  fail("Expected exception");
  } catch (ParameterException ex) {
- assertEquals("Missing required option -x", ex.getMessage());
+ assertEquals("Missing required option '-x=<x>'", ex.getMessage());
  }
 
  CommandLine cmd3 = new CommandLine(new App());
  try {
  cmd3.parseArgs("sub");
  fail("Expected exception");
  } catch (ParameterException ex) {
- assertEquals("Missing required option -x", ex.getMessage());
+ assertEquals("Missing required option '-x=<x>'", ex.getMessage());
  }
  }
 }