Support @Spec-annotated members in ArgGroup classes (was: Question: Throwing a ParameterException inside an ArgGroup)

[clone1612]

For our application we make heavy use of the ArgGroup feature, which works really well. Due to the amount of parameters present in said groups these classes are stored in separate files, rather than inside the command class where they are used.

One such parameter is a String where we want to limit the possible options. As these depend on the location where the application is deployed & its configuration we cannot use an enum and the build-in validation for that type offered by picocli. We therefore have to perform some manual validation, as in following (stripped-down) example:

import picocli.CommandLine;

import java.util.Arrays;
import java.util.List;

public class CriteriaWithEnvironment {

    private static final List<String> DYNAMIC_LIST = Arrays.asList("FOO", "BAR");

    private String environment;

    @CommandLine.Option(names = {"-e", "--environment"})
    public void setEnvironment(String environment) {
        if (!DYNAMIC_LIST.contains(environment)) {
            // Should throw a ParameterException
            throw new IllegalArgumentException("Should be one of...");
        }
        this.environment = environment;
    }

    public String getEnvironment() {
        return environment;
    }
}

Ideally we want to throw a ParameterException, however for such an exception we need access to the CommandLine object of the command. Injecting the CommandSpec in the ArgGroup sadly leads to NPE's:

• @CommandLine.Spec CommandLine.Model.CommandSpec commandSpec;
  Application does not launch at all, printing following stacktrace:

Exception in thread "main" picocli.CommandLine$InitializationException: Could not inject spec
	at picocli.CommandLine$Model$CommandReflection.initFromAnnotatedTypedMembers(CommandLine.java:10880)
	at picocli.CommandLine$Model$CommandReflection.initFromAnnotatedFields(CommandLine.java:10818)
	at picocli.CommandLine$Model$CommandReflection.extractArgGroupSpec(CommandLine.java:10696)
	at picocli.CommandLine$Model$CommandReflection.buildArgGroupForMember(CommandLine.java:10969)
	at picocli.CommandLine$Model$CommandReflection.initFromAnnotatedTypedMembers(CommandLine.java:10854)
	at picocli.CommandLine$Model$CommandReflection.initFromAnnotatedFields(CommandLine.java:10818)
	at picocli.CommandLine$Model$CommandReflection.extractCommandSpec(CommandLine.java:10749)
	at picocli.CommandLine$Model$CommandSpec.forAnnotatedObject(CommandLine.java:5879)
	at picocli.CommandLine.<init>(CommandLine.java:223)
	at picocli.CommandLine.<init>(CommandLine.java:196)
	at Search.main(Search.java:25)
Caused by: picocli.CommandLine$PicocliException: Could not set value for field picocli.CommandLine$Model$CommandSpec CriteriaWithEnvironment.commandSpec to command 'search' (user object: Search@28ac3dc3)
	at picocli.CommandLine$Model$FieldBinding.set(CommandLine.java:11015)
	... 11 more
Caused by: java.lang.NullPointerException
	at java.base/jdk.internal.reflect.UnsafeFieldAccessorImpl.ensureObj(UnsafeFieldAccessorImpl.java:57)
	at java.base/jdk.internal.reflect.UnsafeObjectFieldAccessorImpl.get(UnsafeObjectFieldAccessorImpl.java:36)
	at java.base/java.lang.reflect.Field.get(Field.java:418)
	at picocli.CommandLine$Model$FieldBinding.set(CommandLine.java:11011)
	... 11 more

• @CommandLine.Spec(CommandLine.Spec.Target.MIXEE) CommandLine.Model.CommandSpec commandSpec;
  Application launches, but field remains null. As the ArgGroup isn't injected in the command as a Mixee this makes sense but it was worth a shot.

The documentation does not mention that it is possible to inject a Spec inside an ArgGroup so I wonder if it's supported in any way? If not, what would be the preferred way of throwing a ParameterException inside an ArgGroup?

Our current implementation lets the ArgGroup define a validate() function, which verifies the arguments inside the ArgGroup and throws an IllegalArgumentException. Before the actual command logic is then run we call said function, and map the IllegalArgumentException to a ParameterException. This is however rather convoluted, and we'd prefer to perform the actual validation where it makes the most sense (i.e. when & where the parameter is set).

[remkop]

Good point.
We should make changes to support injecting @Spec into ArgGroups.

Do you think you’ll be able to provide a pull request for this?

[clone1612]

I've never used Java reflection before (and some call me lucky for that fact), so my ability to contribute on the issue is rather limited.

A quick debug session shows that when I initialize the ArgGroup first the NPE is gone, although the CommandSpec remains null even though member.setter().set(commandSpec); was correctly executed. If the ArgGroup was not initialized, the NPE appears as the instance is null and hence the scope is initialized with a null field which causes T result = (T) field.get(obj) in the .set(commandSpec) to throw a NPE.

That quick session did not immediately provide any ideas for a solution though. I can investigate it a bit further, see if getting a bit more familiar with the codebase provides any leads.

[remkop]

Hi @clone1612, thanks for investigating! That is helpful!

A quick debug session shows that when I initialize the ArgGroup first the NPE is gone, although the CommandSpec remains null even though member.setter().set(commandSpec); was correctly executed.

What I believe is happening is that the ArgGroup instance is replaced by a new separate instance when the first option in that group is recognized during parsing. Like you mention, the @Spec annotated field of the original instance is initialized with member.setter().set(commandSpec) when the model is initially created, but that original instance is later replaced during parsing. When picocli subsequently creates a new instance of the ArgGroup, the @Spec annotated field of the new group instance is not assigned and stays null.

ArgGroup instances are created during parsing in ParseResult.Builder::beforeMatchingGroupElement > ParseResult.GroupmatchContainer::addMatch > ArgGroupSpec::initUserObject. This is probably where we should initialize the @Spec-annotated field(s).

That means the ArgGroupSpec model should track whether it has any @Spec-annotated field(s). The ArgGroupSpec model should probably be extended to have a List<IAnnotatedElement> specElements property, similar to CommandSpec, and these should be initialized in the above call chain.

Object creation (especially for groups) is the most complex part of picocli, so I imagine this is not easy to get familiar with. Thanks for helping out!

[remkop]

I pushed a fix to master that addresses this issue.
The test now passes.

I want to add more tests with a @Spec-annotated getter and setter methods, and add some documentation, before I close this issue.