remkop / picocli

Picocli is a modern framework for building powerful, user-friendly, GraalVM-enabled command line apps with ease. It supports colors, autocompletion, subcommands, and more. In 1 source file so apps can include as source & avoid adding a dependency. Written in Java, usable from Groovy, Kotlin, Scala, etc.
https://picocli.info
Apache License 2.0
4.93k stars 424 forks source link

DuplicateOptionAnnotationsException on using negatable option in ArgGroup #2292

Open bhavikp19 opened 5 months ago

bhavikp19 commented 5 months ago

Summary: I believe there is regression issue introduced in latest 4.7.6 release of picocli and most probably it is introduced in this change : https://github.com/remkop/picocli/commit/fa33be1b57f2fff8955b223a64e4bbd0bd2b8b05

Description:

DuplicateOptionAnnotationsException is thrown when a negatable option is used within an ArgGroup as per below code.

public class HashCodeBugTest {

  static class ResponsePathMixin {

    @CommandLine.ArgGroup(exclusive = true)
    PropertiesSelection selection;

    static class PropertiesSelection {
      @CommandLine.Option(
          names = {"--all-columns", "-A"},
          description = "all columns option",
          negatable = true)
      boolean printAllColumns;

      @CommandLine.Option(
          names = {"--columns", "-c"},
          description = "list of columns option",
          split = ",")
      String[] columnsList;
    }
  }

  @Test
  public void testOptionSpecHashCodeBug() {
    var runnable =
        new Runnable() {
          @Override
          public void run() {
            System.out.println("Hello World!");
          }
        };
    CommandLine.Model.CommandSpec commandSpec =
        CommandLine.Model.CommandSpec.wrapWithoutInspection(runnable);
    commandSpec.addMixin(
        "cols", CommandLine.Model.CommandSpec.forAnnotatedObject(new ResponsePathMixin()));
    CommandLine commandLine = new CommandLine(commandSpec);
    commandLine.execute();
  }
}

StackTrace :

Option name '--no-all-columns' is used by both field boolean dct.toolkit.HashCodeBugTest$ResponsePathMixin$PropertiesSelection.printAllColumns and field boolean dct.toolkit.HashCodeBugTest$ResponsePathMixin$PropertiesSelection.printAllColumns
picocli.CommandLine$DuplicateOptionAnnotationsException: Option name '--no-all-columns' is used by both field boolean dct.toolkit.HashCodeBugTest$ResponsePathMixin$PropertiesSelection.printAllColumns and field boolean dct.toolkit.HashCodeBugTest$ResponsePathMixin$PropertiesSelection.printAllColumns
    at picocli.CommandLine$DuplicateOptionAnnotationsException.create(CommandLine.java:18698)
    at picocli.CommandLine$DuplicateOptionAnnotationsException.access$2900(CommandLine.java:18692)
    at picocli.CommandLine$Model$CommandSpec.addOptionNegative(CommandLine.java:6828)
    at picocli.CommandLine$Model$CommandSpec.addOption(CommandLine.java:6797)
    at picocli.CommandLine$Model$CommandSpec.addMixin(CommandLine.java:7036)
    at dct.toolkit.HashCodeBugTest.testOptionSpecHashCodeBug(HashCodeBugTest.java:41)
    at java.base/java.lang.reflect.Method.invoke(Method.java:568)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

Root Cause:

This is happening because mixin options when added to LinkedHashSet (highlighted by --------> below) have different hashcode as compared to when we are trying to remove them after having them added while adding the ArgGroup (inside the for loop highlighted by --------->).

   ------> Set<OptionSpec> options = new LinkedHashSet<OptionSpec>(mixin.options());
                Set<PositionalParamSpec> positionals = new LinkedHashSet<PositionalParamSpec>(mixin.positionalParameters());
                for (ArgGroupSpec argGroupSpec : mixin.argGroups()) {
                    Set<OptionSpec> groupOptions = new HashSet<OptionSpec>();
                    Set<PositionalParamSpec> groupPositionals = new HashSet<PositionalParamSpec>();
                    addArgGroup(argGroupSpec, groupOptions, groupPositionals);
   --------> options.removeAll(groupOptions);
                    positionals.removeAll(groupPositionals);
                }

Expectation:

It should work as it was working in 4.7.5 version.

remkop commented 5 months ago

Thank you for raising this.

Yes that's a problem. I'll take a look.