diff --git a/airline-core/src/main/java/com/github/rvesse/airline/model/CommandMetadata.java b/airline-core/src/main/java/com/github/rvesse/airline/model/CommandMetadata.java index a18aedb93..943d763da 100644 --- a/airline-core/src/main/java/com/github/rvesse/airline/model/CommandMetadata.java +++ b/airline-core/src/main/java/com/github/rvesse/airline/model/CommandMetadata.java @@ -62,7 +62,7 @@ public CommandMetadata(String name, throw new IllegalArgumentException("Command name may not be null/empty"); if (StringUtils.containsWhitespace(name)) throw new IllegalArgumentException("Command name may not contain whitespace"); - + this.name = name; this.description = description; this.hidden = hidden; @@ -73,6 +73,27 @@ public CommandMetadata(String name, this.positionalArgs = positionalArguments; this.arguments = arguments; + // Check that we don't have required positional/non-positional arguments + // after optional positional arguments + boolean posArgsRequired = false; + for (int i = 0; i < this.positionalArgs.size(); i++) { + if (i == 0) { + posArgsRequired = this.positionalArgs.get(i).isRequired(); + } else if (!posArgsRequired && this.positionalArgs.get(i).isRequired()) { + throw new IllegalArgumentException(String.format( + "Positional argument %d (%s) is declared as @Required but one/more preceeding positional arguments are optional", + i, this.positionalArgs.get(i).getTitle())); + } else { + posArgsRequired = this.positionalArgs.get(i).isRequired(); + } + } + if (this.positionalArgs.size() > 0 && !posArgsRequired) { + if (this.arguments.isRequired()) { + throw new IllegalArgumentException( + "Non-positional arguments are declared as required but one/more preceding positional arguments are optional"); + } + } + if (this.defaultOption != null && this.arguments != null) { throw new IllegalArgumentException("Command cannot declare both @Arguments and @DefaultOption"); } @@ -129,7 +150,7 @@ public List getCommandOptions() { public OptionMetadata getDefaultOption() { return defaultOption; } - + public List getPositionalArguments() { return positionalArgs; } @@ -153,6 +174,7 @@ public List getGroupNames() { public List getGroups() { return groups; } + @Override public String toString() { final StringBuilder sb = new StringBuilder(); @@ -170,15 +192,17 @@ public String toString() { sb.append('}'); return sb.toString(); } - + @Override public boolean equals(Object other) { - if (this == other) return true; - - if (!(other instanceof CommandMetadata)) return false; - + if (this == other) + return true; + + if (!(other instanceof CommandMetadata)) + return false; + CommandMetadata cmd = (CommandMetadata) other; - + // TODO This should ideally be more robust return StringUtils.equals(this.name, cmd.name) && this.type.equals(cmd.type); } diff --git a/airline-core/src/main/java/com/github/rvesse/airline/model/MetadataLoader.java b/airline-core/src/main/java/com/github/rvesse/airline/model/MetadataLoader.java index 0854cf6b4..ee4c53f0d 100644 --- a/airline-core/src/main/java/com/github/rvesse/airline/model/MetadataLoader.java +++ b/airline-core/src/main/java/com/github/rvesse/airline/model/MetadataLoader.java @@ -733,7 +733,7 @@ public static void loadInjectionMetadata(Class type, InjectionMetadata inject throw new IllegalArgumentException(String.format( "Field %s annotated with @DefaultOption must also have an @Option annotation", field)); } - + // Process positional arguments annotations PositionalArgument positionalArgumentAnnotation = field.getAnnotation(PositionalArgument.class); if (field.isAnnotationPresent(PositionalArgument.class)) { @@ -741,11 +741,14 @@ public static void loadInjectionMetadata(Class type, InjectionMetadata inject if (StringUtils.isBlank(title)) { title = field.getName(); } - - TypeConverterProvider provider = ParserUtil.createInstance(positionalArgumentAnnotation.typeConverterProvider()); - - List restrictions = collectArgumentRestrictions(field); - + + TypeConverterProvider provider = ParserUtil + .createInstance(positionalArgumentAnnotation.typeConverterProvider()); + + // Collect restrictions, note that @Partials/@Partial does + // not make sense for positional arguments + List restrictions = collectArgumentRestrictions(field, false); + //@formatter:off injectionMetadata.positionalArgs.add(new ArgumentMetadata(positionalArgumentAnnotation.position(), title, @@ -779,7 +782,7 @@ public static void loadInjectionMetadata(Class type, InjectionMetadata inject TypeConverterProvider provider = ParserUtil .createInstance(argumentsAnnotation.typeConverterProvider()); - List restrictions = collectArgumentRestrictions(field); + List restrictions = collectArgumentRestrictions(field, true); //@formatter:off injectionMetadata.arguments.add(new ArgumentsMetadata(titles, @@ -793,16 +796,16 @@ public static void loadInjectionMetadata(Class type, InjectionMetadata inject } } - public static List collectArgumentRestrictions(Field field) { - Map, Set> partials = loadPartials(field); + private static List collectArgumentRestrictions(Field field, boolean allowPartials) { + Map, Set> partials = allowPartials ? loadPartials(field) + : Collections., Set> emptyMap(); List restrictions = new ArrayList<>(); for (Class annotationClass : RestrictionRegistry .getArgumentsRestrictionAnnotationClasses()) { Annotation annotation = field.getAnnotation(annotationClass); if (annotation == null) continue; - ArgumentsRestriction restriction = RestrictionRegistry.getArgumentsRestriction(annotationClass, - annotation); + ArgumentsRestriction restriction = RestrictionRegistry.getArgumentsRestriction(annotationClass, annotation); if (restriction != null) { // Adjust for partial if necessary if (partials.containsKey(annotationClass)) @@ -962,7 +965,7 @@ private static List overridePositionalArgumentSet(List positionalArgs = new ArrayList<>(); private List arguments = new ArrayList<>(); private List metadataInjections = new ArrayList<>(); - - private void compact() { globalOptions = overrideOptionSet(globalOptions);