Skip to content

Commit

Permalink
Fixed issue MissingParameterException not thrown for missing mandatory
Browse files Browse the repository at this point in the history
…@parameter when options are specified. Closes #106
  • Loading branch information
remkop committed Apr 19, 2017
1 parent 070632a commit b837081
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 13 deletions.
36 changes: 23 additions & 13 deletions src/main/java/picocli/CommandLine.java
Original file line number Diff line number Diff line change
Expand Up @@ -932,6 +932,10 @@ private static void init(Class<?> cls,
+ field.getName() + "' is both.");
}
positionalParametersFields.add(field);
Arity arity = Arity.forParameters(field);
if (arity.min > 0) {
requiredFields.add(field);
}
}
}
}
Expand Down Expand Up @@ -1017,9 +1021,10 @@ Object parse(String... args) {

private void parse(Stack<String> argumentStack, String[] originalArgs) {
parsedCommands.add(annotatedObject);
Set<Field> required = new HashSet<Field>(requiredFields);
List<Field> required = new ArrayList<Field>(requiredFields);
Collections.sort(required, new PositionalParametersSorter());
try {
processPositionalParameters0(true, argumentStack);
processPositionalParameters0(required, true, argumentStack);
processArguments(argumentStack, required, originalArgs);
} catch (ParameterException ex) {
throw ex;
Expand All @@ -1029,11 +1034,15 @@ private void parse(Stack<String> argumentStack, String[] originalArgs) {
throw ParameterException.create(ex, arg, argumentStack.size(), originalArgs);
}
if (!isHelpRequested && !required.isEmpty()) {
throw MissingParameterException.create(required);
if (required.get(0).isAnnotationPresent(Option.class)) {
throw MissingParameterException.create(required);
} else {
assertNoMissingParameters(required.get(0), 1, new Stack<String>());
}
}
}

private void processArguments(Stack<String> args, Set<Field> required, final String[] originalArgs) throws Exception {
private void processArguments(Stack<String> args, Collection<Field> required, String[] originalArgs) throws Exception {
// arg must be one of:
// 1. the "--" double dash separating options from positional arguments
// 1. a stand-alone flag, like "-v" or "--verbose": no value required, must map to boolean or Boolean field
Expand All @@ -1049,7 +1058,7 @@ private void processArguments(Stack<String> args, Set<Field> required, final Str
// Double-dash separates options from positional arguments.
// If found, then interpret the remaining args as positional parameters.
if ("--".equals(arg)) {
processPositionalParameters(args);
processPositionalParameters(required, args);
return; // we are done
}

Expand Down Expand Up @@ -1090,17 +1099,17 @@ else if (arg.length() > 2 && arg.startsWith("-")) {
// We take this to mean that the remainder are positional arguments
else {
args.push(arg);
processPositionalParameters(args);
processPositionalParameters(required, args);
return;
}
}
}

private void processPositionalParameters(Stack<String> args) throws Exception {
processPositionalParameters0(false, args);
private void processPositionalParameters(Collection<Field> required, Stack<String> args) throws Exception {
processPositionalParameters0(required, false, args);
}

private void processPositionalParameters0(boolean validateOnly, Stack<String> args) throws Exception {
private void processPositionalParameters0(Collection<Field> required, boolean validateOnly, Stack<String> args) throws Exception {
for (Field positionalParam : positionalParametersFields) {
Arity indexRange = Arity.valueOf(positionalParam.getAnnotation(Parameters.class).index());
@SuppressWarnings("unchecked")
Expand All @@ -1117,14 +1126,15 @@ private void processPositionalParameters0(boolean validateOnly, Stack<String> ar
assertNoMissingParameters(positionalParam, arity.min, argsCopy);
if (!validateOnly) {
applyOption(positionalParam, Parameters.class, arity, false, argsCopy);
required.remove(positionalParam);
}
}
if (!validateOnly) {
args.clear(); // clear the stack to prevent processing the elements twice
}
}

private void processStandaloneOption(Set<Field> required,
private void processStandaloneOption(Collection<Field> required,
String arg,
Stack<String> args,
boolean paramAttachedToKey) throws Exception {
Expand All @@ -1137,7 +1147,7 @@ private void processStandaloneOption(Set<Field> required,
applyOption(field, Option.class, arity, paramAttachedToKey, args);
}

private void processClusteredShortOptions(Set<Field> required, String arg, Stack<String> args)
private void processClusteredShortOptions(Collection<Field> required, String arg, Stack<String> args)
throws Exception {
String cluster = arg.substring(1);
do {
Expand Down Expand Up @@ -1169,7 +1179,7 @@ private void processClusteredShortOptions(Set<Field> required, String arg, Stack
// nor a parameter that the last option could consume.
// Consume it and any other remaining parameters as positional parameters.
args.push(cluster);
processPositionalParameters(args);
processPositionalParameters(required, args);
return;
}
} while (true);
Expand Down Expand Up @@ -3171,7 +3181,7 @@ public MissingParameterException(String msg) {
super(msg);
}

private static MissingParameterException create(Set<Field> missing) {
private static MissingParameterException create(Collection<Field> missing) {
if (missing.size() == 1) {
return new MissingParameterException("Missing required option '"
+ missing.iterator().next().getName() + "'");
Expand Down
36 changes: 36 additions & 0 deletions src/test/java/picocli/CommandLineTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,42 @@ class Tricky2 {
}
}
@Test
public void testMissingRequiredParamsWithOptions() {
class Tricky3 {
@Option(names="-v") boolean more;
@Option(names="-t") boolean any;
@Parameters(index = "1") String alsoMandatory;
@Parameters(index = "0") String mandatory;
}
try {
CommandLine.parse(new Tricky3(), new String[] {"-t", "-v", "mandatory"});
fail("Should not accept missing mandatory parameter");
} catch(MissingParameterException ex) {
assertEquals("Missing required parameter: alsoMandatory", ex.getMessage());
}

try {
CommandLine.parse(new Tricky3(), new String[] { "-t", "-v"});
fail("Should not accept missing two mandatory parameters");
} catch(MissingParameterException ex) {
assertEquals("Missing required parameters: mandatory, alsoMandatory", ex.getMessage());
}
}
@Test
public void testMissingRequiredParamWithOption() {
class Tricky3 {
@Option(names="-t") boolean any;
@Parameters(index = "0") String mandatory;
}
try {
CommandLine.parse(new Tricky3(), new String[] {"-t"});
fail("Should not accept missing mandatory parameter");
} catch(MissingParameterException ex) {

assertEquals("Missing required parameter: mandatory", ex.getMessage());
}
}
@Test
public void testHelpRequestedFlagResetWhenParsing_staticMethod() {
RequiredField requiredField = CommandLine.parse(new RequiredField(), "--help");
assertTrue("help requested", requiredField.isHelpRequested);
Expand Down

0 comments on commit b837081

Please sign in to comment.