Add a way of constructing OptionsBase subclass instances from maps
Added toMap()/fromMap() to OptionsParser, and moved the implementation of OptionsBase#asMap away from OptionsParserImpl.
RELNOTES: None
PiperOrigin-RevId: 153602479
diff --git a/src/main/java/com/google/devtools/common/options/OptionsBase.java b/src/main/java/com/google/devtools/common/options/OptionsBase.java
index 0ca9e44..48658f7 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsBase.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsBase.java
@@ -16,15 +16,15 @@
import com.google.common.escape.CharEscaperBuilder;
import com.google.common.escape.Escaper;
-
+import java.lang.reflect.Field;
+import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
/**
- * Base class for all options classes. Extend this class, adding public
- * instance fields annotated with @Option. Then you can create instances
- * either programmatically:
+ * Base class for all options classes. Extend this class, adding public instance fields annotated
+ * with {@link Option}. Then you can create instances either programmatically:
*
* <pre>
* X x = Options.getDefaults(X.class);
@@ -40,11 +40,10 @@
* X x = parser.getOptions(X.class);
* </pre>
*
- * <p>Subclasses of OptionsBase <b>must</b> be constructed reflectively,
- * i.e. using not {@code new MyOptions}, but one of the two methods above
- * instead. (Direct construction creates an empty instance, not containing
- * default values. This leads to surprising behavior and often
- * NullPointerExceptions, etc.)
+ * <p>Subclasses of {@code OptionsBase} <i>must</i> be constructed reflectively, i.e. using not
+ * {@code new MyOptions()}, but one of the above methods instead. (Direct construction creates an
+ * empty instance, not containing default values. This leads to surprising behavior and often {@code
+ * NullPointerExceptions}, etc.)
*/
public abstract class OptionsBase {
@@ -61,13 +60,24 @@
}
/**
- * Returns this options object in the form of a (new) mapping from option
- * names, including inherited ones, to option values. If the public fields
- * are mutated, this will be reflected in subsequent calls to {@code asMap}.
- * Mutation of this map by the caller does not affect this options object.
+ * Returns a mapping from option names to values, for each option on this object, including
+ * inherited ones. The mapping is a copy, so subsequent mutations to it or to this object are
+ * independent. Entries are sorted alphabetically.
*/
- public final Map<String, Object> asMap() {
- return OptionsParserImpl.optionsAsMap(this);
+ public final <O extends OptionsBase> Map<String, Object> asMap() {
+ // Generic O is needed to tell the type system that the toMap() call is safe.
+ // The casts are safe because this <= O <= OptionsBase.
+ @SuppressWarnings("unchecked")
+ O castThis = (O) this;
+ @SuppressWarnings("unchecked")
+ Class<O> castClass = (Class<O>) getClass();
+
+ Map<String, Object> map = new LinkedHashMap<>();
+ for (Map.Entry<Field, Object> entry : OptionsParser.toMap(castClass, castThis).entrySet()) {
+ String name = entry.getKey().getAnnotation(Option.class).name();
+ map.put(name, entry.getValue());
+ }
+ return map;
}
@Override
diff --git a/src/main/java/com/google/devtools/common/options/OptionsParser.java b/src/main/java/com/google/devtools/common/options/OptionsParser.java
index a871fd1..355deea 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParser.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParser.java
@@ -21,6 +21,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ListMultimap;
import com.google.common.escape.Escaper;
+import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.nio.file.FileSystem;
import java.util.ArrayList;
@@ -28,6 +29,8 @@
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import javax.annotation.Nullable;
@@ -781,4 +784,109 @@
public List<String> canonicalize() {
return impl.asCanonicalizedList();
}
+
+ /**
+ * Returns a mapping from each option {@link Field} in {@code optionsClass} (including inherited
+ * ones) to its value in {@code options}.
+ *
+ * <p>The map is a mutable copy; changing the map won't affect {@code options} and vice versa.
+ * The map entries appear sorted alphabetically by option name.
+ *
+ * If {@code options} is an instance of a subclass of {@code optionsClass}, any options defined
+ * by the subclass are not included in the map.
+ *
+ * @throws IllegalArgumentException if {@code options} is not an instance of {@code optionsClass}
+ */
+ static <O extends OptionsBase> Map<Field, Object> toMap(Class<O> optionsClass, O options) {
+ OptionsData data = getOptionsDataInternal(optionsClass);
+ // Alphabetized due to getFieldsForClass()'s order.
+ Map<Field, Object> map = new LinkedHashMap<>();
+ for (Field field : data.getFieldsForClass(optionsClass)) {
+ try {
+ map.put(field, field.get(options));
+ } catch (IllegalAccessException e) {
+ // All options fields of options classes should be public.
+ throw new IllegalStateException(e);
+ } catch (IllegalArgumentException e) {
+ // This would indicate an inconsistency in the cached OptionsData.
+ throw new IllegalStateException(e);
+ }
+ }
+ return map;
+ }
+
+ /**
+ * Given a mapping as returned by {@link #toMap}, and the options class it that its entries
+ * correspond to, this constructs the corresponding instance of the options class.
+ *
+ * @throws IllegalArgumentException if {@code map} does not contain exactly the fields of {@code
+ * optionsClass}, with values of the appropriate type
+ */
+ static <O extends OptionsBase> O fromMap(Class<O> optionsClass, Map<Field, Object> map) {
+ // Instantiate the options class.
+ OptionsData data = getOptionsDataInternal(optionsClass);
+ O optionsInstance;
+ try {
+ Constructor<O> constructor = data.getConstructor(optionsClass);
+ Preconditions.checkNotNull(constructor, "No options class constructor available");
+ optionsInstance = constructor.newInstance();
+ } catch (ReflectiveOperationException e) {
+ throw new IllegalStateException("Error while instantiating options class", e);
+ }
+
+ List<Field> fields = data.getFieldsForClass(optionsClass);
+ // Ensure all fields are covered, no extraneous fields.
+ validateFieldsSets(new LinkedHashSet<>(fields), new LinkedHashSet<>(map.keySet()));
+ // Populate the instance.
+ for (Field field : fields) {
+ // Non-null as per above check.
+ Object value = map.get(field);
+ try {
+ field.set(optionsInstance, value);
+ } catch (IllegalAccessException e) {
+ throw new IllegalStateException(e);
+ }
+ // May also throw IllegalArgumentException if map value is ill typed.
+ }
+ return optionsInstance;
+ }
+
+ /**
+ * Raises a pretty {@link IllegalArgumentException} if the two sets of fields are not equal.
+ *
+ * <p>The entries in {@code fieldsFromMap} may be ill formed by being null or lacking an {@link
+ * Option} annotation. (This isn't done for {@code fieldsFromClass} because they come from an
+ * {@link OptionsData} object.)
+ */
+ private static void validateFieldsSets(
+ LinkedHashSet<Field> fieldsFromClass, LinkedHashSet<Field> fieldsFromMap) {
+ if (!fieldsFromClass.equals(fieldsFromMap)) {
+ List<String> extraNamesFromClass = new ArrayList<>();
+ List<String> extraNamesFromMap = new ArrayList<>();
+ for (Field field : fieldsFromClass) {
+ if (!fieldsFromMap.contains(field)) {
+ extraNamesFromClass.add("'" + field.getAnnotation(Option.class).name() + "'");
+ }
+ }
+ for (Field field : fieldsFromMap) {
+ // Extra validation on the map keys since they don't come from OptionsData.
+ if (!fieldsFromClass.contains(field)) {
+ if (field == null) {
+ extraNamesFromMap.add("<null field>");
+ } else {
+ Option annotation = field.getAnnotation(Option.class);
+ if (annotation == null) {
+ extraNamesFromMap.add("<non-Option field>");
+ } else {
+ extraNamesFromMap.add("'" + annotation.name() + "'");
+ }
+ }
+ }
+ }
+ throw new IllegalArgumentException(
+ "Map keys do not match fields of options class; extra map keys: {"
+ + Joiner.on(", ").join(extraNamesFromMap) + "}; extra options class options: {"
+ + Joiner.on(", ").join(extraNamesFromClass) + "}");
+ }
+ }
}
diff --git a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
index 2d442a1..c5f31c4 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
@@ -85,7 +85,7 @@
private final List<String> warnings = new ArrayList<>();
private boolean allowSingleDashLongOptions = false;
-
+
private ArgsPreProcessor argsPreProcessor =
new ArgsPreProcessor() {
@Override
@@ -93,7 +93,7 @@
return args;
}
};
-
+
/**
* Create a new parser object
*/
@@ -112,32 +112,13 @@
void setAllowSingleDashLongOptions(boolean allowSingleDashLongOptions) {
this.allowSingleDashLongOptions = allowSingleDashLongOptions;
}
-
+
/** Sets the ArgsPreProcessor for manipulations of the options before parsing. */
void setArgsPreProcessor(ArgsPreProcessor preProcessor) {
this.argsPreProcessor = Preconditions.checkNotNull(preProcessor);
}
/**
- * The implementation of {@link OptionsBase#asMap}.
- */
- static Map<String, Object> optionsAsMap(OptionsBase optionsInstance) {
- Class<? extends OptionsBase> optionsClass = optionsInstance.getClass();
- OptionsData data = OptionsParser.getOptionsDataInternal(optionsClass);
- Map<String, Object> map = new HashMap<>();
- for (Field field : data.getFieldsForClass(optionsClass)) {
- try {
- String name = field.getAnnotation(Option.class).name();
- Object value = field.get(optionsInstance);
- map.put(name, value);
- } catch (IllegalAccessException e) {
- throw new IllegalStateException(e); // unreachable
- }
- }
- return map;
- }
-
- /**
* Implements {@link OptionsParser#asListOfUnparsedOptions()}.
*/
List<UnparsedOptionValueDescription> asListOfUnparsedOptions() {
@@ -707,8 +688,8 @@
return null;
}
optionsInstance = constructor.newInstance();
- } catch (Exception e) {
- throw new IllegalStateException(e);
+ } catch (ReflectiveOperationException e) {
+ throw new IllegalStateException("Error while instantiating options class", e);
}
// Set the fields
