Repository: aurora Updated Branches: refs/heads/master bd90d768e -> 0dd096dc5
Remove support for positional command line arguments, Reviewed at https://reviews.apache.org/r/45939/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/0dd096dc Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/0dd096dc Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/0dd096dc Branch: refs/heads/master Commit: 0dd096dc51beac2f83c4523bfe170536d489e956 Parents: bd90d76 Author: Bill Farner <[email protected]> Authored: Fri Apr 8 12:28:43 2016 -0700 Committer: Bill Farner <[email protected]> Committed: Fri Apr 8 12:28:43 2016 -0700 ---------------------------------------------------------------------- .../apache/aurora/common/args/Positional.java | 40 ------- .../common/args/apt/CmdLineProcessor.java | 35 +----- .../aurora/common/args/apt/Configuration.java | 60 ++++------ .../apache/aurora/common/args/ArgScanner.java | 31 ------ .../org/apache/aurora/common/args/Args.java | 58 +--------- .../aurora/common/args/PositionalInfo.java | 109 ------------------- .../aurora/common/args/ArgScannerTest.java | 71 ------------ .../org/apache/aurora/common/args/ArgsTest.java | 5 - 8 files changed, 25 insertions(+), 384 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/0dd096dc/commons-args/src/main/java/org/apache/aurora/common/args/Positional.java ---------------------------------------------------------------------- diff --git a/commons-args/src/main/java/org/apache/aurora/common/args/Positional.java b/commons-args/src/main/java/org/apache/aurora/common/args/Positional.java deleted file mode 100644 index 1c6f86f..0000000 --- a/commons-args/src/main/java/org/apache/aurora/common/args/Positional.java +++ /dev/null @@ -1,40 +0,0 @@ -/** - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.aurora.common.args; - -import java.lang.annotation.Retention; -import java.lang.annotation.Target; - -import static java.lang.annotation.ElementType.FIELD; -import static java.lang.annotation.RetentionPolicy.RUNTIME; - -/** - * Annotation to mark an {@link Arg} for gathering the positional command line arguments. - */ -@Retention(RUNTIME) -@Target(FIELD) -public @interface Positional { - /** - * The help string to display on the command line in a usage message. - */ - String help(); - - /** - * The parser class to use for parsing the positional arguments. The parser must return the same - * type as the field being annotated. - */ - Class<? extends Parser<?>> parser() default NoParser.class; - - // TODO: https://github.com/twitter/commons/issues/353, Consider to add argFile for positional. -} http://git-wip-us.apache.org/repos/asf/aurora/blob/0dd096dc/commons-args/src/main/java/org/apache/aurora/common/args/apt/CmdLineProcessor.java ---------------------------------------------------------------------- diff --git a/commons-args/src/main/java/org/apache/aurora/common/args/apt/CmdLineProcessor.java b/commons-args/src/main/java/org/apache/aurora/common/args/apt/CmdLineProcessor.java index d531ba0..5fda5dc 100644 --- a/commons-args/src/main/java/org/apache/aurora/common/args/apt/CmdLineProcessor.java +++ b/commons-args/src/main/java/org/apache/aurora/common/args/apt/CmdLineProcessor.java @@ -57,13 +57,11 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; -import com.google.common.collect.Sets; import org.apache.aurora.common.args.Arg; import org.apache.aurora.common.args.ArgParser; import org.apache.aurora.common.args.CmdLine; import org.apache.aurora.common.args.Parser; -import org.apache.aurora.common.args.Positional; import org.apache.aurora.common.args.Verifier; import org.apache.aurora.common.args.VerifierFor; import org.apache.aurora.common.args.apt.Configuration.ParserInfo; @@ -103,9 +101,6 @@ public class CmdLineProcessor extends AbstractProcessor { @Override public Configuration get() { try { Configuration configuration = Configuration.load(); - for (ArgInfo argInfo : configuration.positionalInfo()) { - configBuilder.addPositionalInfo(argInfo); - } for (ArgInfo argInfo : configuration.optionInfo()) { configBuilder.addCmdLineArg(argInfo); } @@ -169,7 +164,7 @@ public class CmdLineProcessor extends AbstractProcessor { @Override public Set<String> getSupportedAnnotationTypes() { return ImmutableSet.copyOf(Iterables.transform( - ImmutableList.of(Positional.class, CmdLine.class, ArgParser.class, VerifierFor.class), + ImmutableList.of(CmdLine.class, ArgParser.class, VerifierFor.class), GET_NAME)); } @@ -184,27 +179,11 @@ public class CmdLineProcessor extends AbstractProcessor { Set<? extends Element> cmdlineArgs = getAnnotatedElements(roundEnv, CmdLine.class); contributingClassNamesBuilder.addAll(extractEnclosingClassNames(cmdlineArgs)); - Set<? extends Element> positionalArgs = getAnnotatedElements(roundEnv, Positional.class); - contributingClassNamesBuilder.addAll(extractEnclosingClassNames(positionalArgs)); - - ImmutableSet<? extends Element> invalidArgs = - Sets.intersection(cmdlineArgs, positionalArgs).immutableCopy(); - if (!invalidArgs.isEmpty()) { - error("An Arg cannot be annotated with both @CmdLine and @Positional, found bad Arg " - + "fields: %s", invalidArgs); - } for (ArgInfo cmdLineInfo : processAnnotatedArgs(parsedTypes, cmdlineArgs, CmdLine.class)) { configBuilder.addCmdLineArg(cmdLineInfo); } - for (ArgInfo positionalInfo - : processAnnotatedArgs(parsedTypes, positionalArgs, Positional.class)) { - - configBuilder.addPositionalInfo(positionalInfo); - } - checkPositionalArgsAreLists(roundEnv); - processParsers(parsers); Set<? extends Element> verifiers = getAnnotatedElements(roundEnv, VerifierFor.class); @@ -292,18 +271,6 @@ public class CmdLineProcessor extends AbstractProcessor { } } - private void checkPositionalArgsAreLists(RoundEnvironment roundEnv) { - for (Element positionalArg : getAnnotatedElements(roundEnv, Positional.class)) { - @Nullable TypeMirror typeArgument = - getTypeArgument(positionalArg.asType(), typeElement(Arg.class)); - if ((typeArgument == null) - || !typeUtils.isSubtype(typeElement(List.class).asType(), typeArgument)) { - error("Found @Positional %s %s.%s that is not a List", - positionalArg.asType(), positionalArg.getEnclosingElement(), positionalArg); - } - } - } - @Nullable private Set<String> getParsedTypes(@Nullable Configuration configuration, Set<? extends Element> parsers) { http://git-wip-us.apache.org/repos/asf/aurora/blob/0dd096dc/commons-args/src/main/java/org/apache/aurora/common/args/apt/Configuration.java ---------------------------------------------------------------------- diff --git a/commons-args/src/main/java/org/apache/aurora/common/args/apt/Configuration.java b/commons-args/src/main/java/org/apache/aurora/common/args/apt/Configuration.java index b526dd0..1832d41 100644 --- a/commons-args/src/main/java/org/apache/aurora/common/args/apt/Configuration.java +++ b/commons-args/src/main/java/org/apache/aurora/common/args/apt/Configuration.java @@ -82,16 +82,17 @@ public final class Configuration { private static final String DEFAULT_RESOURCE_NAME = "cmdline.arg.info.txt"; private int nextResourceIndex; - private final ImmutableSet<ArgInfo> positionalInfos; private final ImmutableSet<ArgInfo> cmdLineInfos; private final ImmutableSet<ParserInfo> parserInfos; private final ImmutableSet<VerifierInfo> verifierInfos; - private Configuration(int nextResourceIndex, - Iterable<ArgInfo> positionalInfos, Iterable<ArgInfo> cmdLineInfos, - Iterable<ParserInfo> parserInfos, Iterable<VerifierInfo> verifierInfos) { + private Configuration( + int nextResourceIndex, + Iterable<ArgInfo> cmdLineInfos, + Iterable<ParserInfo> parserInfos, + Iterable<VerifierInfo> verifierInfos) { + this.nextResourceIndex = nextResourceIndex; - this.positionalInfos = ImmutableSet.copyOf(positionalInfos); this.cmdLineInfos = ImmutableSet.copyOf(cmdLineInfos); this.parserInfos = ImmutableSet.copyOf(parserInfos); this.verifierInfos = ImmutableSet.copyOf(verifierInfos); @@ -248,22 +249,16 @@ public final class Configuration { } static class Builder { - private final Set<ArgInfo> positionalInfos = Sets.newHashSet(); private final Set<ArgInfo> argInfos = Sets.newHashSet(); private final Set<ParserInfo> parserInfos = Sets.newHashSet(); private final Set<VerifierInfo> verifierInfos = Sets.newHashSet(); public boolean isEmpty() { - return positionalInfos.isEmpty() - && argInfos.isEmpty() + return argInfos.isEmpty() && parserInfos.isEmpty() && verifierInfos.isEmpty(); } - void addPositionalInfo(ArgInfo positionalInfo) { - positionalInfos.add(positionalInfo); - } - void addCmdLineArg(ArgInfo argInfo) { argInfos.add(argInfo); } @@ -285,8 +280,11 @@ public final class Configuration { } public Configuration build(Configuration configuration) { - return new Configuration(configuration.nextResourceIndex + 1, - positionalInfos, argInfos, parserInfos, verifierInfos); + return new Configuration( + configuration.nextResourceIndex + 1, + argInfos, + parserInfos, + verifierInfos); } } @@ -352,7 +350,6 @@ public final class Configuration { private final int nextIndex; private int lineNumber = 0; - private final ImmutableList.Builder<ArgInfo> positionalInfo = ImmutableList.builder(); private final ImmutableList.Builder<ArgInfo> fieldInfoBuilder = ImmutableList.builder(); private final ImmutableList.Builder<ParserInfo> parserInfoBuilder = ImmutableList.builder(); private final ImmutableList.Builder<VerifierInfo> verifierInfoBuilder = ImmutableList.builder(); @@ -372,13 +369,7 @@ public final class Configuration { } String type = parts.remove(0); - if ("positional".equals(type)) { - if (parts.size() != 2) { - throw new ConfigurationException( - "Invalid positional line: %s @%d", trimmed, lineNumber); - } - positionalInfo.add(new ArgInfo(parts.get(0), parts.get(1))); - } else if ("field".equals(type)) { + if ("field".equals(type)) { if (parts.size() != 2) { throw new ConfigurationException("Invalid field line: %s @%d", trimmed, lineNumber); } @@ -403,8 +394,11 @@ public final class Configuration { @Override public Configuration getResult() { - return new Configuration(nextIndex, positionalInfo.build(), - fieldInfoBuilder.build(), parserInfoBuilder.build(), verifierInfoBuilder.build()); + return new Configuration( + nextIndex, + fieldInfoBuilder.build(), + parserInfoBuilder.build(), + verifierInfoBuilder.build()); } } @@ -417,23 +411,12 @@ public final class Configuration { } public boolean isEmpty() { - return positionalInfos.isEmpty() - && cmdLineInfos.isEmpty() + return cmdLineInfos.isEmpty() && parserInfos.isEmpty() && verifierInfos.isEmpty(); } /** - * Returns the field info for the sole {@literal @Positional} annotated field on the classpath, - * if any. - * - * @return The field info for the {@literal @Positional} annotated field if any. - */ - public Iterable<ArgInfo> positionalInfo() { - return positionalInfos; - } - - /** * Returns the field info for all the {@literal @CmdLine} annotated fields on the classpath. * * @return The field info for all the {@literal @CmdLine} annotated fields. @@ -475,11 +458,6 @@ public final class Configuration { writer.printf("# %s\n ", message); writer.println(); - for (ArgInfo info : positionalInfos) { - writer.printf("positional %s %s\n", info.className, info.fieldName); - } - - writer.println(); for (ArgInfo info : cmdLineInfos) { writer.printf("field %s %s\n", info.className, info.fieldName); } http://git-wip-us.apache.org/repos/asf/aurora/blob/0dd096dc/commons/src/main/java/org/apache/aurora/common/args/ArgScanner.java ---------------------------------------------------------------------- diff --git a/commons/src/main/java/org/apache/aurora/common/args/ArgScanner.java b/commons/src/main/java/org/apache/aurora/common/args/ArgScanner.java index 306ca4d..6e7f23d 100644 --- a/commons/src/main/java/org/apache/aurora/common/args/ArgScanner.java +++ b/commons/src/main/java/org/apache/aurora/common/args/ArgScanner.java @@ -331,10 +331,6 @@ public final class ArgScanner { return false; } - Optional<? extends PositionalInfo<?>> positionalInfoOptional = argsInfo.getPositionalInfo(); - checkArgument(positionalInfoOptional.isPresent() || positionalArgs.isEmpty(), - "Positional arguments have been supplied but there is no Arg annotated to received them."); - Iterable<? extends OptionInfo<?>> optionInfos = argsInfo.getOptionInfos(); final Set<String> argsFailedToParse = Sets.newHashSet(); @@ -375,20 +371,10 @@ public final class ArgScanner { } } - if (positionalInfoOptional.isPresent()) { - PositionalInfo<?> positionalInfo = positionalInfoOptional.get(); - positionalInfo.load(parserOracle, positionalArgs); - } - Set<String> commandLineArgumentInfos = Sets.newTreeSet(); Iterable<? extends ArgumentInfo<?>> allArguments = argsInfo.getOptionInfos(); - if (positionalInfoOptional.isPresent()) { - PositionalInfo<?> positionalInfo = positionalInfoOptional.get(); - allArguments = Iterables.concat(optionInfos, ImmutableList.of(positionalInfo)); - } - for (ArgumentInfo<?> anArgumentInfo : allArguments) { Arg<?> arg = anArgumentInfo.getArg(); @@ -447,23 +433,6 @@ public final class ArgScanner { infoLog("-------------------------------------------------------------------------"); infoLog(String.format("%s to print this help message", Joiner.on(" or ").join(Iterables.transform(ArgumentInfo.HELP_ARGS, ARG_NAME_TO_FLAG)))); - Optional<? extends PositionalInfo<?>> positionalInfoOptional = argsInfo.getPositionalInfo(); - if (positionalInfoOptional.isPresent()) { - infoLog("\nPositional args:"); - PositionalInfo<?> positionalInfo = positionalInfoOptional.get(); - Arg<?> arg = positionalInfo.getArg(); - Object defaultValue = arg.uncheckedGet(); - ImmutableList<String> constraints = positionalInfo.collectConstraints(verifiers); - infoLog(String.format("%s%s\n\t%s", - defaultValue != null ? "default " + defaultValue : "", - Iterables.isEmpty(constraints) - ? "" - : " [" + Joiner.on(", ").join(constraints) + "]", - positionalInfo.getHelp())); - // TODO: https://github.com/twitter/commons/issues/353, in the future we may - // want to support @argfile format for positional arguments. We should check - // to update firstArgFileArgumentName for them as well. - } ImmutableList<String> required = requiredHelps.build(); if (!required.isEmpty()) { infoLog("\nRequired flags:"); // yes - this should actually throw! http://git-wip-us.apache.org/repos/asf/aurora/blob/0dd096dc/commons/src/main/java/org/apache/aurora/common/args/Args.java ---------------------------------------------------------------------- diff --git a/commons/src/main/java/org/apache/aurora/common/args/Args.java b/commons/src/main/java/org/apache/aurora/common/args/Args.java index 1983fd9..202835d 100644 --- a/commons/src/main/java/org/apache/aurora/common/args/Args.java +++ b/commons/src/main/java/org/apache/aurora/common/args/Args.java @@ -20,7 +20,6 @@ import javax.annotation.Nullable; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Function; -import com.google.common.base.Joiner; import com.google.common.base.Optional; import com.google.common.base.Preconditions; import com.google.common.base.Predicate; @@ -71,30 +70,16 @@ public final class Args { return OptionInfo.createFromField(field); }; - private static final Function<Field, PositionalInfo<?>> TO_POSITIONAL_INFO = - field -> { - @Nullable Positional positional = field.getAnnotation(Positional.class); - if (positional == null) { - throw new ConfigurationException("No @Positional Arg annotation for field " + field); - } - return PositionalInfo.createFromField(field); - }; - /** * An opaque container for all the positional and optional {@link Arg} metadata in-play for a * command line parse. */ public static final class ArgsInfo { private final Configuration configuration; - private final Optional<? extends PositionalInfo<?>> positionalInfo; private final ImmutableList<? extends OptionInfo<?>> optionInfos; - ArgsInfo(Configuration configuration, - Optional<? extends PositionalInfo<?>> positionalInfo, - Iterable<? extends OptionInfo<?>> optionInfos) { - + ArgsInfo(Configuration configuration, Iterable<? extends OptionInfo<?>> optionInfos) { this.configuration = Preconditions.checkNotNull(configuration); - this.positionalInfo = Preconditions.checkNotNull(positionalInfo); this.optionInfos = ImmutableList.copyOf(optionInfos); } @@ -102,10 +87,6 @@ public final class Args { return configuration; } - Optional<? extends PositionalInfo<?>> getPositionalInfo() { - return positionalInfo; - } - ImmutableList<? extends OptionInfo<?>> getOptionInfos() { return optionInfos; } @@ -121,25 +102,10 @@ public final class Args { * arg field. */ static ArgsInfo fromConfiguration(Configuration configuration, Predicate<Field> filter) { - ImmutableSet<Field> positionalFields = - ImmutableSet.copyOf(filterFields(configuration.positionalInfo(), filter)); - - if (positionalFields.size() > 1) { - throw new IllegalArgumentException( - String.format("Found %d fields marked for @Positional Args after applying filter - " - + "only 1 is allowed:\n\t%s", positionalFields.size(), - Joiner.on("\n\t").join(positionalFields))); - } - - Optional<? extends PositionalInfo<?>> positionalInfo = - Optional.fromNullable( - Iterables.getOnlyElement( - Iterables.transform(positionalFields, TO_POSITIONAL_INFO), null)); - Iterable<? extends OptionInfo<?>> optionInfos = Iterables.transform( filterFields(configuration.optionInfo(), filter), TO_OPTION_INFO); - return new ArgsInfo(configuration, positionalInfo, optionInfos); + return new ArgsInfo(configuration, optionInfos); } private static Iterable<Field> filterFields(Iterable<ArgInfo> infos, Predicate<Field> filter) { @@ -184,33 +150,19 @@ public final class Args { Configuration configuration = Configuration.load(); ArgsInfo staticInfo = Args.fromConfiguration(configuration, filter); - final ImmutableSet.Builder<PositionalInfo<?>> positionalInfos = - ImmutableSet.<PositionalInfo<?>>builder().addAll(staticInfo.getPositionalInfo().asSet()); final ImmutableSet.Builder<OptionInfo<?>> optionInfos = ImmutableSet.<OptionInfo<?>>builder().addAll(staticInfo.getOptionInfos()); for (Object source : sources) { Class<?> clazz = source instanceof Class ? (Class) source : source.getClass(); for (Field field : clazz.getDeclaredFields()) { - if (filter.apply(field)) { - boolean cmdLine = field.isAnnotationPresent(CmdLine.class); - boolean positional = field.isAnnotationPresent(Positional.class); - if (cmdLine && positional) { - throw new IllegalArgumentException( - "An Arg cannot be annotated with both @CmdLine and @Positional, found bad Arg " - + "field: " + field); - } else if (cmdLine) { - optionInfos.add(OptionInfo.createFromField(field, source)); - } else if (positional) { - positionalInfos.add(PositionalInfo.createFromField(field, source)); - } + if (filter.apply(field) && field.isAnnotationPresent(CmdLine.class)) { + optionInfos.add(OptionInfo.createFromField(field, source)); } } } - @Nullable PositionalInfo<?> positionalInfo = - Iterables.getOnlyElement(positionalInfos.build(), null); - return new ArgsInfo(configuration, Optional.fromNullable(positionalInfo), optionInfos.build()); + return new ArgsInfo(configuration, optionInfos.build()); } private Args() { http://git-wip-us.apache.org/repos/asf/aurora/blob/0dd096dc/commons/src/main/java/org/apache/aurora/common/args/PositionalInfo.java ---------------------------------------------------------------------- diff --git a/commons/src/main/java/org/apache/aurora/common/args/PositionalInfo.java b/commons/src/main/java/org/apache/aurora/common/args/PositionalInfo.java deleted file mode 100644 index 83d9cd5..0000000 --- a/commons/src/main/java/org/apache/aurora/common/args/PositionalInfo.java +++ /dev/null @@ -1,109 +0,0 @@ -/** - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.aurora.common.args; - -import java.lang.annotation.Annotation; -import java.lang.reflect.Field; -import java.lang.reflect.Type; -import java.util.Arrays; -import java.util.List; - -import javax.annotation.Nullable; - -import com.google.common.base.Optional; -import com.google.common.base.Preconditions; -import com.google.common.collect.Iterables; -import com.google.common.collect.Lists; -import com.google.common.reflect.TypeToken; - -import org.apache.aurora.common.args.apt.Configuration; - -/** - * Description of a positional command line argument. - */ -public final class PositionalInfo<T> extends ArgumentInfo<List<T>> { - /** - * Factory method to create a PositionalInfo from a field. - * - * @param field The field must contain a {@link Arg Arg<List<?>>}. The List<?> - * represents zero or more positional arguments. - * @return a PositionalInfo describing the field. - */ - static PositionalInfo<?> createFromField(Field field) { - return createFromField(field, null); - } - - /** - * Factory method to create a PositionalInfo from a field. - * - * @param field The field must contain a {@link Arg Arg<List<?>>}. The List<?> - * represents zero or more positional arguments. - * @param instance The object containing the non-static Arg instance or else null if the Arg - * field is static. - * @return a PositionalInfo describing the field. - */ - static PositionalInfo<?> createFromField(Field field, @Nullable Object instance) { - Preconditions.checkNotNull(field); - Positional positional = field.getAnnotation(Positional.class); - if (positional == null) { - throw new Configuration.ConfigurationException( - "No @Positional Arg annotation for field " + field); - } - - Preconditions.checkArgument( - TypeUtil.getRawType(TypeUtil.getTypeParam(field)) == List.class, - "Field is annotated for positional parsing but is not of Arg<List<?>> type"); - Type nestedType = TypeUtil.extractTypeToken(TypeUtil.getTypeParam(field)); - - @SuppressWarnings({"unchecked", "rawtypes"}) // we have no way to know the type here - PositionalInfo<?> positionalInfo = new PositionalInfo( - "[positional args]", - positional.help(), - ArgumentInfo.getArgForField(field, Optional.fromNullable(instance)), - TypeUtil.getTypeParamTypeToken(field), - TypeToken.of(nestedType), - Arrays.asList(field.getAnnotations()), - positional.parser()); - - return positionalInfo; - } - - private final TypeToken<T> elementType; - - private PositionalInfo( - String name, - String help, - Arg<List<T>> arg, - TypeToken<List<T>> type, - TypeToken<T> elementType, - List<Annotation> verifierAnnotations, - @Nullable Class<? extends Parser<? extends List<T>>> parser) { - - // TODO: https://github.com/twitter/commons/issues/353, consider future support of - // argFile for Positional arguments. - super(name, help, arg, type, verifierAnnotations, parser); - this.elementType = elementType; - } - - /** - * Parses the positional args and stores the results in the {@link Arg} described by this - * {@code PositionalInfo}. - */ - void load(final ParserOracle parserOracle, List<String> positionalArgs) { - final Parser<? extends T> parser = parserOracle.get(elementType); - List<T> assignmentValue = Lists.newArrayList(Iterables.transform(positionalArgs, - argValue -> parser.parse(parserOracle, elementType.getType(), argValue))); - setValue(assignmentValue); - } -} http://git-wip-us.apache.org/repos/asf/aurora/blob/0dd096dc/commons/src/test/java/org/apache/aurora/common/args/ArgScannerTest.java ---------------------------------------------------------------------- diff --git a/commons/src/test/java/org/apache/aurora/common/args/ArgScannerTest.java b/commons/src/test/java/org/apache/aurora/common/args/ArgScannerTest.java index 6ac518a..06ce914 100644 --- a/commons/src/test/java/org/apache/aurora/common/args/ArgScannerTest.java +++ b/commons/src/test/java/org/apache/aurora/common/args/ArgScannerTest.java @@ -13,7 +13,6 @@ */ package org.apache.aurora.common.args; -import java.io.File; import java.io.PrintStream; import java.io.Serializable; import java.lang.annotation.Annotation; @@ -117,9 +116,6 @@ public class ArgScannerTest { @CmdLine(name = "range", help = "help") static final Arg<com.google.common.collect.Range<Integer>> RANGE = Arg.create(com.google.common.collect.Range.closed(1, 5)); - @Positional(help = "help") - static final Arg<List<Amount<Long, Time>>> POSITIONAL = - Arg.<List<Amount<Long, Time>>>create(ImmutableList.<Amount<Long, Time>>of()); } @Test @@ -174,12 +170,6 @@ public class ArgScannerTest { test(StandardArgs.class, () -> assertThat(StandardArgs.RANGE.get(), is(com.google.common.collect.Range.closed(1, 5))), "range", "1-5"); - - resetArgs(StandardArgs.class); - assertTrue(parse(StandardArgs.class, "1mins", "2secs")); - assertEquals(ImmutableList.builder() - .add(Amount.of(60L, Time.SECONDS)) - .add(Amount.of(2L, Time.SECONDS)).build(), StandardArgs.POSITIONAL.get()); } public static class Name { @@ -565,67 +555,6 @@ public class ArgScannerTest { parse(ImmutableList.of(AmountContainer.class), "-time_amount=1abcd"); } - static class Main1 { - @Positional(help = "halp") - static final Arg<List<String>> NAMES = Arg.create(null); - } - - static class Main2 { - @Positional(help = "halp") - static final Arg<List<List<String>>> ROSTERS = Arg.create(null); - } - - static class Main3 { - @Positional(help = "halp") - static final Arg<List<Double>> PERCENTILES = Arg.create(null); - - @Positional(help = "halp") - static final Arg<List<File>> FILES = Arg.create(null); - } - - private void resetMainArgs() { - resetArgs(Main1.class); - resetArgs(Main2.class); - resetArgs(Main3.class); - } - - @Test - public void testMultiplePositionalsFails() { - // Indivdually these should work. - - resetMainArgs(); - assertTrue(parse(Main1.class, "jack,jill", "laurel,hardy")); - assertEquals(ImmutableList.of("jack,jill", "laurel,hardy"), - ImmutableList.copyOf(Main1.NAMES.get())); - - resetMainArgs(); - assertTrue(parse(Main2.class, "jack,jill", "laurel,hardy")); - assertEquals( - ImmutableList.of( - ImmutableList.of("jack", "jill"), - ImmutableList.of("laurel", "hardy")), - ImmutableList.copyOf(Main2.ROSTERS.get())); - - // But if combined in the same class or across classes the @Positional is ambiguous and we - // should fail fast. - - resetMainArgs(); - try { - parse(ImmutableList.of(Main1.class, Main2.class), "jack,jill", "laurel,hardy"); - fail("Expected more than 1 in-scope @Positional Arg List to trigger a failure."); - } catch (IllegalArgumentException e) { - // expected - } - - resetMainArgs(); - try { - parse(Main3.class, "50", "90", "99", "99.9"); - fail("Expected more than 1 in-scope @Positional Arg List to trigger a failure."); - } catch (IllegalArgumentException e) { - // expected - } - } - // TODO(William Farner): Do we want to support nested parameterized args? If so, need to define a // syntax for that and build it in. // e.g. List<List<Integer>>, List<Pair<String, String>> http://git-wip-us.apache.org/repos/asf/aurora/blob/0dd096dc/commons/src/test/java/org/apache/aurora/common/args/ArgsTest.java ---------------------------------------------------------------------- diff --git a/commons/src/test/java/org/apache/aurora/common/args/ArgsTest.java b/commons/src/test/java/org/apache/aurora/common/args/ArgsTest.java index 4ffc794..7bccaba 100644 --- a/commons/src/test/java/org/apache/aurora/common/args/ArgsTest.java +++ b/commons/src/test/java/org/apache/aurora/common/args/ArgsTest.java @@ -15,7 +15,6 @@ package org.apache.aurora.common.args; import java.io.File; import java.io.IOException; -import java.util.List; import com.google.common.collect.ImmutableList; @@ -33,9 +32,6 @@ public class ArgsTest { @NotEmpty @CmdLine(name = "name", help = "help") private final Arg<String> name = Arg.create(); - - @Positional(help = "help") - private final Arg<List<Integer>> values = Arg.create(); } @Test @@ -47,7 +43,6 @@ public class ArgsTest { assertEquals(new File("fred"), App.DB.get()); assertEquals("bob", app.name.get()); - assertEquals(ImmutableList.of(1, 137), app.values.get()); } @Test
