picocli icon indicating copy to clipboard operation
picocli copied to clipboard

NPE on OptionSpec.getValue()

Open rsenden opened this issue 2 years ago • 0 comments

For options defined in an ArgGroup, a call to OptionSpec.getValue() will fail due to a NullPointerException if none of the options contained in the ArgGroup have been specified on the command line.

The following command class demonstrates this issue:

@Command(name = "testGetValue")
public class TestGetValueCommand implements Runnable {
	@Spec CommandSpec spec;
	@Option(names="--arg1") private String arg1;
	@ArgGroup private MyArgGroup argGroup;
	
	private static final class MyArgGroup {
		@Option(names="--arg2") private String arg2;
		@Option(names="--arg3") private String arg3;
	}
	
	@Override
	public void run() {
		spec.options().stream().map(OptionSpec::getValue).forEach(this::printOptionValue);
	}
	
	private void printOptionValue(Object value) {
		System.out.println("Option value: "+value);
	}
}

This command will fail with an exception on OptionSpec::getValue if neither --arg2 nor --arg3 have been specified on the command line.

A simple null-check in the FieldBinding#get() method fixes this issue; we will be submitting a PR for this:

@SuppressWarnings("unchecked") T result = obj==null ? null : (T) field.get(obj);

Note that the FieldBinding#set() method likely exhibits the same problem, however this may be more difficult to fix; if an ArgGroup is null, then there's no object to set the option value on. As such, we won't be including any fix for the set() method in the PR.

It might make sense to always initialize ArgGroups with a non-null value, as that would avoid these kind of issues, and would also reduce the number of null checks in command implementations when trying to access options defined in an ArgGroup. Not sure though whether there are any command implementations that rely on the fact that an ArgGroup is null if none of the options in that ArgGroup have been specified on the command line, so this would potentially be a breaking change.

rsenden avatar Aug 05 '22 11:08 rsenden