Skip to content

Commit

Permalink
Small cleanups.
Browse files Browse the repository at this point in the history 
Change-Id: I94f54b225bc1dca31493c86eaba91830f44c4378
  • Loading branch information
@rluble
rluble committed Jan 12, 2015
1 parent 78059f7 commit 9b48c84
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 75 deletions.
25 changes: 12 additions & 13 deletions dev/core/src/com/google/gwt/dev/jjs/impl/CompileTimeConstantsReplacer.java
View file
Expand Up @@ -52,23 +52,22 @@ private JExpression resolveFieldValue(JField field) {
JExpression value = resolveValuesByField.get(field); JExpression value = resolveValuesByField.get(field);
if (value != null) { if (value != null) {
return new CloneExpressionVisitor().cloneExpression(value); return new CloneExpressionVisitor().cloneExpression(value);
} else {
// TODO(rluble): Simplify the expression to a literal here after refactoring Simplifier.
// The initializer is guaranteed to be constant but it may be a non literal expression, so
// clone it and recursively remove field references, and finally cache the results.
value = accept(new CloneExpressionVisitor().cloneExpression(field.getInitializer()));
JType fieldType = field.getType().getUnderlyingType();
assert fieldType instanceof JPrimitiveType || fieldType == program.getTypeJavaLangString()
: fieldType.getName() + " is not a primitive nor String ";
if (fieldType != value.getType()) {
value = new JCastOperation(value.getSourceInfo(), fieldType, value);
}
resolveValuesByField.put(field, value);
} }
// TODO(rluble): Simplify the expression to a literal here after refactoring Simplifier.
// The initializer is guaranteed to be constant but it may be a non literal expression, so
// clone it and recursively remove field references, and finally cache the results.
value = accept(new CloneExpressionVisitor().cloneExpression(field.getInitializer()));
JType fieldType = field.getType().getUnderlyingType();
assert fieldType instanceof JPrimitiveType || fieldType == program.getTypeJavaLangString()
: fieldType.getName() + " is not a primitive nor String";
if (fieldType != value.getType()) {
value = new JCastOperation(value.getSourceInfo(), fieldType, value);
}
resolveValuesByField.put(field, value);
return value; return value;
} }


public CompileTimeConstantsReplacingVisitor(JProgram program) { private CompileTimeConstantsReplacingVisitor(JProgram program) {
this.program = program; this.program = program;
} }
} }
Expand Down
106 changes: 57 additions & 49 deletions dev/core/src/com/google/gwt/util/tools/ArgHandlerEnum.java
View file
Expand Up @@ -15,16 +15,19 @@
*/ */
package com.google.gwt.util.tools; package com.google.gwt.util.tools;


import com.google.gwt.thirdparty.guava.common.base.Function; import static com.google.gwt.thirdparty.guava.common.base.Preconditions.checkNotNull;

import com.google.gwt.thirdparty.guava.common.base.Ascii;
import com.google.gwt.thirdparty.guava.common.base.Enums;
import com.google.gwt.thirdparty.guava.common.base.Joiner; import com.google.gwt.thirdparty.guava.common.base.Joiner;
import com.google.gwt.thirdparty.guava.common.base.Preconditions;
import com.google.gwt.thirdparty.guava.common.base.Predicate; import com.google.gwt.thirdparty.guava.common.base.Predicate;
import com.google.gwt.thirdparty.guava.common.collect.Collections2; import com.google.gwt.thirdparty.guava.common.collect.FluentIterable;
import com.google.gwt.thirdparty.guava.common.collect.Lists;


import java.util.Arrays; import java.util.Arrays;
import java.util.Collection;
import java.util.List; import java.util.List;
import java.util.Locale;
import javax.annotation.Nullable;


/** /**
* A generic arg handler for options defined by enums. * A generic arg handler for options defined by enums.
Expand All @@ -39,7 +42,7 @@ public abstract class ArgHandlerEnum<T extends Enum<T>> extends ArgHandler {
private static final int ABBREVIATION_MIN_SIZE = 3; private static final int ABBREVIATION_MIN_SIZE = 3;


/** /**
* Constructor, assumes that the default value for the options is the first enum value. * Constructor, default value must be handled by the user code.
*/ */
public ArgHandlerEnum(Class<T> optionsEnumClass) { public ArgHandlerEnum(Class<T> optionsEnumClass) {
this(optionsEnumClass, null, false); this(optionsEnumClass, null, false);
Expand All @@ -48,9 +51,14 @@ public ArgHandlerEnum(Class<T> optionsEnumClass) {
/** /**
* Constructor, allows to specify the default value for the option and whether to accept or * Constructor, allows to specify the default value for the option and whether to accept or
* prefixes as abbreviations. * prefixes as abbreviations.
* <p>
* When {@code defaultValue} is null, handling of the default for the option is left to be
* handled by the user code.
*/ */
public ArgHandlerEnum(Class<T> optionsEnumClass, T defaultValue, boolean allowAbbreviation) { public ArgHandlerEnum(
this.optionsEnumClass = optionsEnumClass; Class<T> optionsEnumClass, @Nullable T defaultValue, boolean allowAbbreviation) {
Preconditions.checkArgument(optionsEnumClass.getEnumConstants().length > 1);
this.optionsEnumClass = checkNotNull(optionsEnumClass);
this.defaultValue = defaultValue; this.defaultValue = defaultValue;
this.allowAbbreviation = allowAbbreviation; this.allowAbbreviation = allowAbbreviation;
} }
Expand All @@ -70,29 +78,34 @@ public String[] getTagArgs() {


@Override @Override
public final int handle(String[] args, int startIndex) { public final int handle(String[] args, int startIndex) {
if (startIndex + 1 < args.length) { // A command line argument corresponding to an enum option has a parameter
String value = args[startIndex + 1].trim().toUpperCase(Locale.ROOT); // (the desired value), will be at startIndex + 1.
int optionValueIndex = startIndex + 1;
if (optionValueIndex < args.length) {
String value = Ascii.toUpperCase(args[optionValueIndex].trim());
T mode = matchOption(value); T mode = matchOption(value);
if (mode == null) { if (mode == null) {
System.err.println(value + " is not a valid option for " + getTag()); System.err.format("%s is not a valid option for %s\n", value, getTag());
System.err.println( System.err.format("%s value must be one of %s.\n",
getTag() + " value must be one of " + getTag(), getFormattedOptionNames(", ", " or ", optionsEnumClass));
getFormattedOptionNames(", ", " or ", optionsEnumClass) + ".");
return -1; return -1;
} }
setValue(mode); setValue(mode);
return 1; return 1;
} }


System.err.println("Missing argument for " + getTag() + " must be followed by one of " + System.err.format("Missing argument for %s must be followed by one of %s.\n" +
getFormattedOptionNames(", ", " or ", optionsEnumClass) + "."); getTag(), getFormattedOptionNames(", ", " or ", optionsEnumClass));
return -1; return -1;
} }


protected String getPurposeString(String prefix) { protected String getPurposeString(String prefix) {
return (isExperimental() ? "EXPERIMENTAL: " : "") + prefix + " " + String maybeExperimentalString = isExperimental() ? "EXPERIMENTAL: " : "";
getFormattedOptionNames(", ", " or ", optionsEnumClass) + String defaultValueString = defaultValue != null ?
(defaultValue != null ? " (defaults to " + defaultValue.name() + ")" : ""); " (defaults to " + defaultValue.name() + ")" : "";

return String.format("%s%s %s%s", maybeExperimentalString, prefix,
getFormattedOptionNames(", ", " or ", optionsEnumClass), defaultValueString);
} }


/** /**
Expand All @@ -101,46 +114,41 @@ protected String getPurposeString(String prefix) {
public abstract void setValue(T value); public abstract void setValue(T value);


private List<String> getEnumNames(Class<T> optionsEnumClass) { private List<String> getEnumNames(Class<T> optionsEnumClass) {
return Lists.transform(Arrays.asList(optionsEnumClass.getEnumConstants()), return FluentIterable.from(Arrays.asList(optionsEnumClass.getEnumConstants()))
new Function<T, String>() { .transform(Enums.stringConverter(optionsEnumClass).reverse())
@Override .toList();
public String apply(T t) {
return t.name();
}
});
} }


private String getFormattedOptionNames(String separator,Class<T> optionsEnumClass) { private String getFormattedOptionNames(String separator, Class<T> optionsEnumClass) {
return getFormattedOptionNames(separator, separator, optionsEnumClass); return getFormattedOptionNames(separator, separator, optionsEnumClass);
} }


private String getFormattedOptionNames(String separator,String lastSeparator, private String getFormattedOptionNames(
Class<T> optionsEnumClass) { String separator, String lastSeparator, Class<T> optionsEnumClass) {
List<String> enumNames = getEnumNames(optionsEnumClass); List<String> enumNames = getEnumNames(optionsEnumClass);
List<String> allNamesButLast = enumNames.subList(0, enumNames.size() - 1); List<String> allNamesButLast = enumNames.subList(0, enumNames.size() - 1);
String lastName = enumNames.get(enumNames.size() - 1); String lastName = enumNames.get(enumNames.size() - 1);


return Joiner.on(lastSeparator).join(Joiner.on(separator).join(allNamesButLast), lastName); return Joiner.on(separator).join(allNamesButLast) + lastSeparator + lastName;
} }


private T matchOption(final String value) { private Predicate<Enum<?>> buildMatchPredicate(final String value) {
try { return
Collection<T> matches = Collections2.filter( new Predicate<Enum<?>>() {
Arrays.asList(optionsEnumClass.getEnumConstants()), @Override
new Predicate<T>() { public boolean apply(Enum<?> t) {
@Override if (allowAbbreviation && value.length() >= ABBREVIATION_MIN_SIZE) {
public boolean apply(T t) { return t.name().startsWith(value);
if (allowAbbreviation && value.length() >= ABBREVIATION_MIN_SIZE) { }
return t.name().startsWith(value); return t.name().equals(value);
} }
return t.name().equals(value); };
} }
});
if (matches.size() == 1) { private T matchOption(String value) {
return matches.iterator().next(); List<T> matchedOptions = FluentIterable.from(Arrays.asList(optionsEnumClass.getEnumConstants()))
} .filter(buildMatchPredicate(value))
} catch (IllegalArgumentException e) { .toList();
} return matchedOptions.size() == 1 ? matchedOptions.get(0) : null;
return null;
} }
} }
50 changes: 37 additions & 13 deletions dev/core/test/com/google/gwt/util/tools/ArgHandlerEnumTest.java
View file
Expand Up @@ -24,17 +24,21 @@ public class ArgHandlerEnumTest extends TestCase {


enum SomeFlags { THIS_FLAG, THAT_FLAG, THAT_OTHER_FLAG, ANOTHER_FLAG } enum SomeFlags { THIS_FLAG, THAT_FLAG, THAT_OTHER_FLAG, ANOTHER_FLAG }


static public SomeFlags optionValue = null; enum EmptyEnum { }


private abstract class MockArgHandlerEnumBase extends ArgHandlerEnum<SomeFlags> { enum OneOptionEnum { SINGLE_OPTION }


public MockArgHandlerEnumBase() { static public Enum<?> optionValue = null;
super(SomeFlags.class);
private abstract class MockArgHandlerEnumBase<T extends Enum<T>> extends ArgHandlerEnum<T> {

public MockArgHandlerEnumBase(Class<T> enumClass) {
super(enumClass);
} }


public MockArgHandlerEnumBase(SomeFlags defaultValue, public MockArgHandlerEnumBase(
boolean allowAbbraviation) { Class<T> enumClass, T defaultValue, boolean allowAbbraviation) {
super(SomeFlags.class, defaultValue, allowAbbraviation); super(enumClass, defaultValue, allowAbbraviation);
} }


@Override @Override
Expand All @@ -53,13 +57,13 @@ public boolean isExperimental() {
} }


@Override @Override
public void setValue(SomeFlags value) { public void setValue(T value) {
optionValue = value; optionValue = value;
} }
} }


public void testHandle() { public void testHandle() {
ArgHandler handler = new MockArgHandlerEnumBase() { }; ArgHandler handler = new MockArgHandlerEnumBase<SomeFlags>(SomeFlags.class) { };
optionValue = null; optionValue = null;
int consuemdArguments = handler.handle(new String[] {"-Xflag", "THIS_FLAG"}, 0); int consuemdArguments = handler.handle(new String[] {"-Xflag", "THIS_FLAG"}, 0);
assertEquals(1, consuemdArguments); assertEquals(1, consuemdArguments);
Expand All @@ -79,15 +83,16 @@ public void testHandle() {
} }


public void testHandle_default() { public void testHandle_default() {
ArgHandler handler = new MockArgHandlerEnumBase(SomeFlags.THAT_OTHER_FLAG, false) { }; ArgHandler handler = new MockArgHandlerEnumBase<SomeFlags>(
SomeFlags.class, SomeFlags.THAT_OTHER_FLAG, false) { };
optionValue = null; optionValue = null;
int consuemdArguments = handler.handle(handler.getDefaultArgs(), 0); int consuemdArguments = handler.handle(handler.getDefaultArgs(), 0);
assertEquals(1, consuemdArguments); assertEquals(1, consuemdArguments);
assertEquals(SomeFlags.THAT_OTHER_FLAG, optionValue); assertEquals(SomeFlags.THAT_OTHER_FLAG, optionValue);
} }


public void testHandle_Abbreviations() { public void testHandle_Abbreviations() {
ArgHandler handler = new MockArgHandlerEnumBase(null, true) { }; ArgHandler handler = new MockArgHandlerEnumBase<SomeFlags>(SomeFlags.class, null, true) { };
optionValue = null; optionValue = null;
int consuemdArguments = handler.handle(new String[] {"-Xflag", "THIS"}, 0); int consuemdArguments = handler.handle(new String[] {"-Xflag", "THIS"}, 0);
assertEquals(1, consuemdArguments); assertEquals(1, consuemdArguments);
Expand Down Expand Up @@ -115,13 +120,32 @@ public void testHandle_Abbreviations() {
} }


public void testGetDefaultTags() { public void testGetDefaultTags() {
ArgHandler handler = new MockArgHandlerEnumBase() { }; ArgHandler handler = new MockArgHandlerEnumBase<SomeFlags>(SomeFlags.class) { };
assertNull(handler.getDefaultArgs()); assertNull(handler.getDefaultArgs());


handler = new MockArgHandlerEnumBase(SomeFlags.THAT_FLAG, false) { }; handler =
new MockArgHandlerEnumBase<SomeFlags>(SomeFlags.class, SomeFlags.THAT_FLAG, false) { };
assertContentsEquals(new String[]{"-Xflag", "THAT_FLAG"}, handler.getDefaultArgs()); assertContentsEquals(new String[]{"-Xflag", "THAT_FLAG"}, handler.getDefaultArgs());
} }


public void testGetPurposeString() {
assertEquals("EXPERIMENTAL: Set flag: THIS_FLAG, THAT_FLAG, THAT_OTHER_FLAG or ANOTHER_FLAG",
new MockArgHandlerEnumBase<SomeFlags>(SomeFlags.class) { }.getPurpose());
}

public void testBadEnums() {
try {
new MockArgHandlerEnumBase<EmptyEnum>(EmptyEnum.class) { };
fail("Should have thrown IllegalArgumentException");
} catch (IllegalArgumentException expected) {
}

try {
new MockArgHandlerEnumBase<OneOptionEnum>(OneOptionEnum.class) { };
fail("Should have thrown IllegalArgumentException");
} catch (IllegalArgumentException expected) {
}
}
private <T> void assertContentsEquals(T[] expected, T[] actual) { private <T> void assertContentsEquals(T[] expected, T[] actual) {
assertEquals("Different sizes", expected.length, actual.length); assertEquals("Different sizes", expected.length, actual.length);
for (int i = 0; i < expected.length; i++) { for (int i = 0; i < expected.length; i++) {
Expand Down

0 comments on commit 9b48c84

Please sign in to comment.