[#1054] Fixed arggroup bug where input with missing mandatory element…

…s was accepted when an option was specified multiple times

RELEASE-NOTES.md

@@ -8,6 +8,7 @@ This release contains bugfixes and enhancements.
 From this release, picocli supports abbreviated options and subcommands. When abbreviations are enabled, users can specify the initial letter(s) of the first component and optionally of one or more subsequent components of an option or subcommand name.
 "Components" are separated by `-` dash characters or by case, so for example, both `--CamelCase` and `--kebab-case` have two components.
 
+Fixed a bug in argument group parsing where incorrect input with missing mandatory elements was accepted when an option was specified multiple times.
 
 This is the seventy-first public release.
 Picocli follows [semantic versioning](http://semver.org/).
@@ -83,6 +84,7 @@ When abbreviated options are enabled, user input `-AB` will match the long `-Aaa
 * [#1069] Enhancement: Debug output should show `optionsCaseInsensitive` and `subcommandsCaseInsensitive` settings.
 * [#1065] Bugfix: With a `List<>` option in `@ArgGroup`, group incorrectly appears twice in the synopsis. Thanks to [kap4lin](https://github.com/kap4lin) for raising this.
 * [#1067] Bugfix: `ParserSpec::initFrom` was not copying `useSimplifiedAtFiles`.
+* [#1054] Bugfix: option-parameter gets lost in Argument Groups. Thanks to [waacc-gh](https://github.com/waacc-gh) for raising this.
 * [#1058][#1059] DOC: Man page generator: fix incorrect asciidoctor call in synopsis. Thanks to [Andreas Deininger](https://github.com/deining) for the pull request.
 * [#1058][#1060] DOC: Man page generator: add documentation about creating language variants. Thanks to [Andreas Deininger](https://github.com/deining) for the pull request.
 

src/main/java/picocli/CommandLine.java

@@ -11564,19 +11564,23 @@ public void addError(PicocliException ex) {
  void beforeMatchingGroupElement(ArgSpec argSpec) throws Exception {
  ArgGroupSpec group = argSpec.group();
  if (group == null || isInitializingDefaultValues) { return; }
+ Tracer tracer = commandSpec.commandLine.tracer;
  GroupMatchContainer foundGroupMatchContainer = this.groupMatchContainer.findOrCreateMatchingGroup(argSpec, commandSpec.commandLine);
  GroupMatch match = foundGroupMatchContainer.lastMatch();
  boolean greedy = true; // commandSpec.parser().greedyMatchMultiValueArgsInGroup(); // or @Option(multiplicity=0..*) to control min/max matches
  boolean allowMultipleMatchesInGroup = greedy && argSpec.isMultiValue(); // https://github.com/remkop/picocli/issues/815
+ String elementDescription = ArgSpec.describe(argSpec, "=");
  if (match.matchedMinElements() &&
  (argSpec.required() || match.matchCount(argSpec) > 0) && !allowMultipleMatchesInGroup) {
  // we need to create a new match; if maxMultiplicity has been reached, we need to add a new GroupMatchContainer.
  String previousMatch = argSpec.required() ? "is required" : "has already been matched";
- String elementDescription = ArgSpec.describe(argSpec, "=");
- Tracer tracer = commandSpec.commandLine.tracer;
  tracer.info("GroupMatch %s is complete: its mandatory elements are all matched. (User object: %s.) %s %s in the group, so it starts a new GroupMatch.%n", foundGroupMatchContainer.lastMatch(), foundGroupMatchContainer.group.userObject(), elementDescription, previousMatch);
  foundGroupMatchContainer.addMatch(commandSpec.commandLine);
  this.groupMatchContainer.findOrCreateMatchingGroup(argSpec, commandSpec.commandLine);
+ } else if (match.matchCount(argSpec) > 0 && !allowMultipleMatchesInGroup) {
+ tracer.info("GroupMatch %s is incomplete: its mandatory elements are not all matched. (User object: %s.) However, %s has already been matched in the group, so it starts a new GroupMatch.%n", foundGroupMatchContainer.lastMatch(), foundGroupMatchContainer.group.userObject(), elementDescription);
+ foundGroupMatchContainer.addMatch(commandSpec.commandLine);
+ this.groupMatchContainer.findOrCreateMatchingGroup(argSpec, commandSpec.commandLine);
  }
  }
  }

src/test/java/picocli/ArgGroupTest.java

@@ -2063,18 +2063,28 @@ private static class Change {
  public String replacement = null;
  }
  }
- @Ignore
+ //@Ignore
  @Test // https://github.com/remkop/picocli/issues/1054
  public void testIssue1054() {
  //-f pattern1 -f pattern2 -d --> accepted --> wrong: findPattern = "pattern2", "pattern1" is lost/ignored
  try {
- TestUtil.setTraceLevel("DEBUG");
+ //TestUtil.setTraceLevel("DEBUG");
  Issue1054 bean3 = new Issue1054();
  new CommandLine(bean3).parseArgs("-f pattern1 -f pattern2 -d".split(" "));
- System.out.println(bean3);
+ //System.out.println(bean3);
+ fail("Expected exception");
+ } catch (MissingParameterException ex) {
+ assertEquals("Error: Missing required argument(s): (-d | -w=<replacement>)", ex.getMessage());
+ }
+ }
+ @Test
+ public void testIssue1054Variation() {
+ try {
+ Issue1054 bean3 = new Issue1054();
+ new CommandLine(bean3).parseArgs("-f pattern1 -f pattern2".split(" "));
  fail("Expected exception");
  } catch (MissingParameterException ex) {
- assertEquals("Error: (...)", ex.getMessage()); // TODO exact error message
+ assertEquals("Error: Missing required argument(s): (-d | -w=<replacement>)", ex.getMessage());
  }
  }
  @Test // https://github.com/remkop/picocli/issues/1054