Move default value & converter finding logic to the OptionDefinition class.

Removes some duplicate computation by memoizing the results. Consolidates caching into a single optionDefinition object, instead of having multiple caches that go from the option name to different parts of what defines an option.

Fly-by cleanup of OptionDescription's contents, all contents that are statically defined as part of an option are in OptionDefintion, while expansion data, which depends on the existence of other options, is more clearly stored separately.

Will move the converter-to-option type matching sanity checks to a compile time check in a later change.

RELNOTES: None.
PiperOrigin-RevId: 166912716

13 files changed, 430 insertions, 291 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/TransitiveOptionDetails.java b/src/main/java/com/google/devtools/build/lib/analysis/config/TransitiveOptionDetails.java
index 72f001fce0..6aa27b22d2 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/TransitiveOptionDetails.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/TransitiveOptionDetails.java
@@ -17,9 +17,9 @@ package com.google.devtools.build.lib.analysis.config;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.common.options.OptionDefinition;
+import com.google.devtools.common.options.OptionDefinition.NotAnOptionException;
 import com.google.devtools.common.options.OptionMetadataTag;
 import com.google.devtools.common.options.OptionsBase;
-import com.google.devtools.common.options.OptionsParser.ConstructionException;
 import java.io.Serializable;
 import java.lang.reflect.Field;
 import java.util.Map;
@@ -47,7 +47,7 @@ public final class TransitiveOptionDetails implements Serializable {
           OptionDefinition optionDefinition;
           try {
             optionDefinition = OptionDefinition.extractOptionDefinition(field);
-          } catch (ConstructionException e) {
+          } catch (NotAnOptionException e) {
             // Skip non @Option fields.
             continue;
           }
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
index 8d9f93caba..a1602342e8 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
@@ -76,11 +76,11 @@ import com.google.devtools.build.lib.windows.WindowsSubprocessFactory;
 import com.google.devtools.common.options.CommandNameCache;
 import com.google.devtools.common.options.InvocationPolicyParser;
 import com.google.devtools.common.options.OptionDefinition;
+import com.google.devtools.common.options.OptionDefinition.NotAnOptionException;
 import com.google.devtools.common.options.OptionPriority;
 import com.google.devtools.common.options.OptionsBase;
 import com.google.devtools.common.options.OptionsClassProvider;
 import com.google.devtools.common.options.OptionsParser;
-import com.google.devtools.common.options.OptionsParser.ConstructionException;
 import com.google.devtools.common.options.OptionsParsingException;
 import com.google.devtools.common.options.OptionsProvider;
 import com.google.devtools.common.options.TriState;
@@ -659,7 +659,7 @@ public final class BlazeRuntime {
         if (field.getType() == boolean.class || field.getType() == TriState.class) {
           prefixes.add("--no" + optionDefinition.getOptionName());
         }
-      } catch (ConstructionException e) {
+      } catch (NotAnOptionException e) {
         // Do nothing, just ignore fields that are not actually options. OptionsBases technically
         // shouldn't have fields that are not @Options, but this is a requirement that isn't yet
         // being enforced, so this should not cause a failure here.
diff --git a/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java b/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java
index 4be2b2327c..e76688c6d0 100644
--- a/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java
+++ b/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java
@@ -515,7 +515,8 @@ public final class InvocationPolicyEnforcer {
     }
 
     // Flag must allow multiple values if multiple values are specified by the policy.
-    if (setValue.getFlagValueCount() > 1 && !optionDescription.getAllowMultiple()) {
+    if (setValue.getFlagValueCount() > 1
+        && !optionDescription.getOptionDefinition().allowsMultiple()) {
       throw new OptionsParsingException(
           String.format(
               "SetValue operation from invocation policy sets multiple values for flag '%s' which "
@@ -546,7 +547,7 @@ public final class InvocationPolicyEnforcer {
           logInApplySetValueOperation(
               "Setting value for flag '%s' from invocation "
                   + "policy to '%s', overriding the default value '%s'",
-              flagName, flagValue, optionDescription.getDefaultValue());
+              flagName, flagValue, optionDescription.getOptionDefinition().getDefaultValue());
         } else {
           logInApplySetValueOperation(
               "Setting value for flag '%s' from invocation "
@@ -570,7 +571,7 @@ public final class InvocationPolicyEnforcer {
       OptionDescription desc = parser.getOptionDescription(clearedFlagName);
       Object clearedFlagDefaultValue = null;
       if (desc != null) {
-        clearedFlagDefaultValue = desc.getDefaultValue();
+        clearedFlagDefaultValue = desc.getOptionDefinition().getDefaultValue();
       }
       log.info(
           String.format(
@@ -640,15 +641,18 @@ public final class InvocationPolicyEnforcer {
       // can be arbitrarily complex.
       Set<Object> convertedPolicyValues = new HashSet<>();
       for (String value : policyValues) {
-        Object convertedValue = optionDescription.getConverter().convert(value);
+        Object convertedValue =
+            optionDescription.getOptionDefinition().getConverter().convert(value);
         // Some converters return lists, and if the flag is a repeatable flag, the items in the
         // list from the converter should be added, and not the list itself. Otherwise the items
         // from invocation policy will be compared to lists, which will never work.
         // See OptionsParserImpl.ParsedOptionEntry.addValue.
-        if (optionDescription.getAllowMultiple() && convertedValue instanceof List<?>) {
+        if (optionDescription.getOptionDefinition().allowsMultiple()
+            && convertedValue instanceof List<?>) {
           convertedPolicyValues.addAll((List<?>) convertedValue);
         } else {
-          convertedPolicyValues.add(optionDescription.getConverter().convert(value));
+          convertedPolicyValues.add(
+              optionDescription.getOptionDefinition().getConverter().convert(value));
         }
       }
 
@@ -656,18 +660,17 @@ public final class InvocationPolicyEnforcer {
       // does not also set use_default. Otherwise the default value would will still be set if the
       // user uses a disallowed value. This doesn't apply to repeatable flags since the default
       // value for repeatable flags is always the empty list.
-      if (!optionDescription.getAllowMultiple()) {
+      if (!optionDescription.getOptionDefinition().allowsMultiple()) {
 
         boolean defaultValueAllowed =
-            isFlagValueAllowed(convertedPolicyValues, optionDescription.getDefaultValue());
+            isFlagValueAllowed(
+                convertedPolicyValues, optionDescription.getOptionDefinition().getDefaultValue());
         if (!defaultValueAllowed && useDefault) {
           throw new OptionsParsingException(
               String.format(
                   "%sValues policy disallows the default value '%s' for flag '%s' but also "
                       + "specifies to use the default value",
-                  policyType,
-                  optionDescription.getDefaultValue(),
-                  flagName));
+                  policyType, optionDescription.getOptionDefinition().getDefaultValue(), flagName));
         }
       }
 
@@ -706,14 +709,15 @@ public final class InvocationPolicyEnforcer {
         Set<Object> convertedPolicyValues)
         throws OptionsParsingException {
 
-      if (!isFlagValueAllowed(convertedPolicyValues, optionDescription.getDefaultValue())) {
+      if (!isFlagValueAllowed(
+          convertedPolicyValues, optionDescription.getOptionDefinition().getDefaultValue())) {
         if (newValue != null) {
           // Use the default value from the policy.
           log.info(
               String.format(
                   "Overriding default value '%s' for flag '%s' with value '%s' "
                       + "specified by invocation policy. %sed values are: %s",
-                  optionDescription.getDefaultValue(),
+                  optionDescription.getOptionDefinition().getDefaultValue(),
                   flagName,
                   newValue,
                   policyType,
@@ -727,7 +731,7 @@ public final class InvocationPolicyEnforcer {
                   "Default flag value '%s' for flag '%s' is not allowed by invocation policy, but "
                       + "the policy does not provide a new value. "
                       + "%sed values are: %s",
-                  optionDescription.getDefaultValue(),
+                  optionDescription.getOptionDefinition().getDefaultValue(),
                   flagName,
                   policyType,
                   policyValues));
@@ -746,7 +750,7 @@ public final class InvocationPolicyEnforcer {
         Set<Object> convertedPolicyValues)
         throws OptionsParsingException {
 
-      if (optionDescription.getAllowMultiple()) {
+      if (optionDescription.getOptionDefinition().allowsMultiple()) {
         // allowMultiple requires that the type of the option be List<T>, so cast from Object
         // to List<?>.
         List<?> optionValues = (List<?>) valueDescription.getValue();
diff --git a/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java b/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java
index 406b6cd94b..ca91b1d83d 100644
--- a/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java
+++ b/src/main/java/com/google/devtools/common/options/IsolatedOptionsData.java
@@ -16,6 +16,7 @@ package com.google.devtools.common.options;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.devtools.common.options.OptionDefinition.NotAnOptionException;
 import com.google.devtools.common.options.OptionsParser.ConstructionException;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
@@ -25,7 +26,6 @@ import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -74,28 +74,6 @@ public class IsolatedOptionsData extends OpaqueOptionsData {
       allOptionsFields;
 
   /**
-   * Mapping from each {@code Option}-annotated field to the default value for that field
-   * (unordered).
-   *
-   * <p>(This is immutable like the others, but uses {@code Collections.unmodifiableMap} to support
-   * null values.)
-   */
-  private final Map<OptionDefinition, Object> optionDefaults;
-
-  /**
-   * Mapping from each {@code Option}-annotated field to the proper converter (unordered).
-   *
-   * @see #findConverter
-   */
-  private final ImmutableMap<OptionDefinition, Converter<?>> converters;
-
-  /**
-   * Mapping from each {@code Option}-annotated field to a boolean for whether that field allows
-   * multiple values (unordered).
-   */
-  private final ImmutableMap<OptionDefinition, Boolean> allowMultiple;
-
-  /**
    * Mapping from each options class to whether or not it has the {@link UsesOnlyCoreTypes}
    * annotation (unordered).
    */
@@ -110,18 +88,11 @@ public class IsolatedOptionsData extends OpaqueOptionsData {
       Map<String, OptionDefinition> nameToField,
       Map<Character, OptionDefinition> abbrevToField,
       Map<Class<? extends OptionsBase>, ImmutableList<OptionDefinition>> allOptionsFields,
-      Map<OptionDefinition, Object> optionDefaults,
-      Map<OptionDefinition, Converter<?>> converters,
-      Map<OptionDefinition, Boolean> allowMultiple,
       Map<Class<? extends OptionsBase>, Boolean> usesOnlyCoreTypes) {
     this.optionsClasses = ImmutableMap.copyOf(optionsClasses);
     this.nameToField = ImmutableMap.copyOf(nameToField);
     this.abbrevToField = ImmutableMap.copyOf(abbrevToField);
     this.allOptionsFields = ImmutableMap.copyOf(allOptionsFields);
-    // Can't use an ImmutableMap here because of null values.
-    this.optionDefaults = Collections.unmodifiableMap(optionDefaults);
-    this.converters = ImmutableMap.copyOf(converters);
-    this.allowMultiple = ImmutableMap.copyOf(allowMultiple);
     this.usesOnlyCoreTypes = ImmutableMap.copyOf(usesOnlyCoreTypes);
   }
 
@@ -131,9 +102,6 @@ public class IsolatedOptionsData extends OpaqueOptionsData {
         other.nameToField,
         other.abbrevToField,
         other.allOptionsFields,
-        other.optionDefaults,
-        other.converters,
-        other.allowMultiple,
         other.usesOnlyCoreTypes);
   }
 
@@ -176,135 +144,29 @@ public class IsolatedOptionsData extends OpaqueOptionsData {
     return allOptionsFields.get(optionsClass);
   }
 
-  public Object getDefaultValue(OptionDefinition optionDefinition) {
-    return optionDefaults.get(optionDefinition);
-  }
-
-  public Converter<?> getConverter(OptionDefinition optionDefinition) {
-    return converters.get(optionDefinition);
-  }
-
-  public boolean getAllowMultiple(OptionDefinition optionDefinition) {
-    return allowMultiple.get(optionDefinition);
-  }
-
   public boolean getUsesOnlyCoreTypes(Class<? extends OptionsBase> optionsClass) {
     return usesOnlyCoreTypes.get(optionsClass);
   }
 
-  /**
-   * For an option that does not use {@link Option#allowMultiple}, returns its type. For an option
-   * that does use it, asserts that the type is a {@code List<T>} and returns its element type
-   * {@code T}.
-   */
-  private static Type getFieldSingularType(OptionDefinition optionDefinition) {
-    Type fieldType = optionDefinition.getField().getGenericType();
-    if (optionDefinition.allowsMultiple()) {
-      // If the type isn't a List<T>, this is an error in the option's declaration.
-      if (!(fieldType instanceof ParameterizedType)) {
-        throw new ConstructionException("Type of multiple occurrence option must be a List<...>");
-      }
-      ParameterizedType pfieldType = (ParameterizedType) fieldType;
-      if (pfieldType.getRawType() != List.class) {
-        throw new ConstructionException("Type of multiple occurrence option must be a List<...>");
-      }
-      fieldType = pfieldType.getActualTypeArguments()[0];
-    }
-    return fieldType;
-  }
-
-  /**
-   * Returns whether a field should be considered as boolean.
-   *
-   * <p>Can be used for usage help and controlling whether the "no" prefix is allowed.
-   */
-  boolean isBooleanField(OptionDefinition optionDefinition) {
-    return isBooleanField(optionDefinition, getConverter(optionDefinition));
-  }
-
-  private static boolean isBooleanField(OptionDefinition optionDefinition, Converter<?> converter) {
-    return optionDefinition.getType().equals(boolean.class)
-        || optionDefinition.getType().equals(TriState.class)
-        || converter instanceof BoolOrEnumConverter;
-  }
-
-  /**
-   * Given an {@code @Option}-annotated field, retrieves the {@link Converter} that will be used,
-   * taking into account the default converters if an explicit one is not specified.
-   */
-  private static Converter<?> findConverter(OptionDefinition optionDefinition) {
-    if (optionDefinition.getProvidedConverter() == Converter.class) {
-      // No converter provided, use the default one.
-      Type type = getFieldSingularType(optionDefinition);
-      Converter<?> converter = Converters.DEFAULT_CONVERTERS.get(type);
-      if (converter == null) {
-        throw new ConstructionException(
-            "No converter found for "
-                + type
-                + "; possible fix: add "
-                + "converter=... to @Option annotation for "
-                + optionDefinition.getField().getName());
-      }
-      return converter;
-    }
-    try {
-      // Instantiate the given Converter class.
-      Class<?> converter = optionDefinition.getProvidedConverter();
-      Constructor<?> constructor = converter.getConstructor();
-      return (Converter<?>) constructor.newInstance();
-    } catch (Exception e) {
-      // This indicates an error in the Converter, and should be discovered the first time it is
-      // used.
-      throw new ConstructionException(e);
-    }
-  }
   /** Returns all {@code optionDefinitions}, ordered by their option name (not their field name). */
   private static ImmutableList<OptionDefinition> getAllOptionDefinitionsSorted(
       Class<? extends OptionsBase> optionsClass) {
     return Arrays.stream(optionsClass.getFields())
-        .map(field -> {
-          try {
-            return OptionDefinition.extractOptionDefinition(field);
-          } catch (ConstructionException e) {
-            // Ignore non-@Option annotated fields. Requiring all fields in the OptionsBase to be
-            // @Option-annotated requires a depot cleanup.
-            return null;
-          }
-        })
+        .map(
+            field -> {
+              try {
+                return OptionDefinition.extractOptionDefinition(field);
+              } catch (NotAnOptionException e) {
+                // Ignore non-@Option annotated fields. Requiring all fields in the OptionsBase to
+                // be @Option-annotated requires a depot cleanup.
+                return null;
+              }
+            })
         .filter(Objects::nonNull)
         .sorted(OptionDefinition.BY_OPTION_NAME)
         .collect(ImmutableList.toImmutableList());
   }
 
-  private static Object retrieveDefaultValue(OptionDefinition optionDefinition) {
-    Converter<?> converter = findConverter(optionDefinition);
-    String defaultValueAsString = optionDefinition.getUnparsedDefaultValue();
-    // Special case for "null"
-    if (optionDefinition.isSpecialNullDefault()) {
-      return null;
-    }
-    boolean allowsMultiple = optionDefinition.allowsMultiple();
-    // If the option allows multiple values then we intentionally return the empty list as
-    // the default value of this option since it is not always the case that an option
-    // that allows multiple values will have a converter that returns a list value.
-    if (allowsMultiple) {
-      return Collections.emptyList();
-    }
-    // Otherwise try to convert the default value using the converter
-    Object convertedValue;
-    try {
-      convertedValue = converter.convert(defaultValueAsString);
-    } catch (OptionsParsingException e) {
-      throw new IllegalStateException(
-          "OptionsParsingException while "
-              + "retrieving default for "
-              + optionDefinition.getField().getName()
-              + ": "
-              + e.getMessage());
-    }
-    return convertedValue;
-  }
-
   private static <A> void checkForCollisions(
       Map<A, OptionDefinition> aFieldMap, A optionName, String description) {
     if (aFieldMap.containsKey(optionName)) {
@@ -351,9 +213,6 @@ public class IsolatedOptionsData extends OpaqueOptionsData {
         new HashMap<>();
     Map<String, OptionDefinition> nameToFieldBuilder = new LinkedHashMap<>();
     Map<Character, OptionDefinition> abbrevToFieldBuilder = new HashMap<>();
-    Map<OptionDefinition, Object> optionDefaultsBuilder = new HashMap<>();
-    Map<OptionDefinition, Converter<?>> convertersBuilder = new HashMap<>();
-    Map<OptionDefinition, Boolean> allowMultipleBuilder = new HashMap<>();
 
     // Maps the negated boolean flag aliases to the original option name.
     Map<String, String> booleanAliasMap = new HashMap<>();
@@ -391,7 +250,7 @@ public class IsolatedOptionsData extends OpaqueOptionsData {
                   + "\" is disallowed.");
         }
 
-        Type fieldType = getFieldSingularType(optionDefinition);
+        Type fieldType = optionDefinition.getFieldSingularType();
         // For simple, static expansions, don't accept non-Void types.
         if (optionDefinition.getOptionExpansion().length != 0 && !optionDefinition.isVoidField()) {
           throw new ConstructionException(
@@ -429,8 +288,6 @@ public class IsolatedOptionsData extends OpaqueOptionsData {
           throw new ConstructionException(
               "A known converter object doesn't implement the convert method");
         }
-        Converter<?> converter = findConverter(optionDefinition);
-        convertersBuilder.put(optionDefinition, converter);
 
         if (optionDefinition.allowsMultiple()) {
           if (GenericTypeHelper.getRawType(converterResultType) == List.class) {
@@ -467,7 +324,7 @@ public class IsolatedOptionsData extends OpaqueOptionsData {
           }
         }
 
-        if (isBooleanField(optionDefinition, converter)) {
+        if (optionDefinition.isBooleanField()) {
           checkAndUpdateBooleanAliases(nameToFieldBuilder, booleanAliasMap, optionName);
         }
 
@@ -482,7 +339,7 @@ public class IsolatedOptionsData extends OpaqueOptionsData {
           nameToFieldBuilder.put(optionDefinition.getOldOptionName(), optionDefinition);
 
           // If boolean, repeat the alias dance for the old name.
-          if (isBooleanField(optionDefinition, converter)) {
+          if (optionDefinition.isBooleanField()) {
             checkAndUpdateBooleanAliases(nameToFieldBuilder, booleanAliasMap, oldName);
           }
         }
@@ -491,9 +348,6 @@ public class IsolatedOptionsData extends OpaqueOptionsData {
               abbrevToFieldBuilder, optionDefinition.getAbbreviation(), "option abbreviation");
           abbrevToFieldBuilder.put(optionDefinition.getAbbreviation(), optionDefinition);
         }
-
-        optionDefaultsBuilder.put(optionDefinition, retrieveDefaultValue(optionDefinition));
-        allowMultipleBuilder.put(optionDefinition, optionDefinition.allowsMultiple());
       }
 
       boolean usesOnlyCoreTypes = parsedOptionsClass.isAnnotationPresent(UsesOnlyCoreTypes.class);
@@ -523,9 +377,6 @@ public class IsolatedOptionsData extends OpaqueOptionsData {
         nameToFieldBuilder,
         abbrevToFieldBuilder,
         allOptionsFieldsBuilder,
-        optionDefaultsBuilder,
-        convertersBuilder,
-        allowMultipleBuilder,
         usesOnlyCoreTypesBuilder);
   }
 
diff --git a/src/main/java/com/google/devtools/common/options/OptionDefinition.java b/src/main/java/com/google/devtools/common/options/OptionDefinition.java
index 589208a4f1..d3f0d34648 100644
--- a/src/main/java/com/google/devtools/common/options/OptionDefinition.java
+++ b/src/main/java/com/google/devtools/common/options/OptionDefinition.java
@@ -15,8 +15,13 @@
 package com.google.devtools.common.options;
 
 import com.google.devtools.common.options.OptionsParser.ConstructionException;
+import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
+import java.lang.reflect.ParameterizedType;
+import java.lang.reflect.Type;
+import java.util.Collections;
 import java.util.Comparator;
+import java.util.List;
 
 /**
  * Everything the {@link OptionsParser} needs to know about how an option is defined.
@@ -25,23 +30,32 @@ import java.util.Comparator;
  * the {@link Field} that is annotated, and should contain all logic about default settings and
  * behavior.
  */
-public final class OptionDefinition {
+public class OptionDefinition {
+
+  // TODO(b/65049598) make ConstructionException checked, which will make this checked as well.
+  public static class NotAnOptionException extends ConstructionException {
+    public NotAnOptionException(Field field) {
+      super(
+          "The field " + field + " does not have the right annotation to be considered an option.");
+    }
+  }
 
   /**
    * If the {@code field} is annotated with the appropriate @{@link Option} annotation, returns the
-   * {@code OptionDefinition} for that option. Otherwise, throws a {@link ConstructionException}.
+   * {@code OptionDefinition} for that option. Otherwise, throws a {@link NotAnOptionException}.
    */
   public static OptionDefinition extractOptionDefinition(Field field) {
     Option annotation = field == null ? null : field.getAnnotation(Option.class);
     if (annotation == null) {
-      throw new ConstructionException(
-          "The field " + field + " does not have the right annotation to be considered an option.");
+      throw new NotAnOptionException(field);
     }
     return new OptionDefinition(field, annotation);
   }
 
   private final Field field;
   private final Option optionAnnotation;
+  private Converter<?> converter = null;
+  private Object defaultValue = null;
 
   private OptionDefinition(Field field, Option optionAnnotation) {
     this.field = field;
@@ -170,6 +184,110 @@ public final class OptionDefinition {
     return getExpansionFunction() != ExpansionFunction.class;
   }
 
+  /**
+   * For an option that does not use {@link Option#allowMultiple}, returns its type. For an option
+   * that does use it, asserts that the type is a {@code List<T>} and returns its element type
+   * {@code T}.
+   */
+  Type getFieldSingularType() {
+    Type fieldType = getField().getGenericType();
+    if (allowsMultiple()) {
+      // If the type isn't a List<T>, this is an error in the option's declaration.
+      if (!(fieldType instanceof ParameterizedType)) {
+        throw new ConstructionException(
+            String.format(
+                "Option %s allows multiple occurrences, so must be of type List<...>",
+                getField().getName()));
+      }
+      ParameterizedType pfieldType = (ParameterizedType) fieldType;
+      if (pfieldType.getRawType() != List.class) {
+        throw new ConstructionException(
+            String.format(
+                "Option %s allows multiple occurrences, so must be of type List<...>",
+                getField().getName()));
+      }
+      fieldType = pfieldType.getActualTypeArguments()[0];
+    }
+    return fieldType;
+  }
+
+  /**
+   * Retrieves the {@link Converter} that will be used for this option, taking into account the
+   * default converters if an explicit one is not specified.
+   *
+   * <p>Memoizes the converter-finding logic to avoid repeating the computation.
+   */
+  Converter<?> getConverter() {
+    if (converter != null) {
+      return converter;
+    }
+    Class<? extends Converter> converterClass = getProvidedConverter();
+    if (converterClass == Converter.class) {
+      // No converter provided, use the default one.
+      Type type = getFieldSingularType();
+      converter = Converters.DEFAULT_CONVERTERS.get(type);
+      if (converter == null) {
+        throw new ConstructionException(
+            String.format(
+                "Option %s expects values of type %s, but no converter was found; possible fix: "
+                    + "add converter=... to its @Option annotation.",
+                getField().getName(), type));
+      }
+    } else {
+      try {
+        // Instantiate the given Converter class.
+        Constructor<?> constructor = converterClass.getConstructor();
+        converter = (Converter<?>) constructor.newInstance();
+      } catch (SecurityException | IllegalArgumentException | ReflectiveOperationException e) {
+        // This indicates an error in the Converter, and should be discovered the first time it is
+        // used.
+        throw new ConstructionException(
+            String.format("Error in the provided converter for option %s", getField().getName()),
+            e);
+      }
+    }
+    return converter;
+  }
+
+  /**
+   * Returns whether a field should be considered as boolean.
+   *
+   * <p>Can be used for usage help and controlling whether the "no" prefix is allowed.
+   */
+  boolean isBooleanField() {
+    return getType().equals(boolean.class)
+        || getType().equals(TriState.class)
+        || getConverter() instanceof BoolOrEnumConverter;
+  }
+
+  /** Returns the evaluated default value for this option & memoizes the result. */
+  public Object getDefaultValue() {
+    if (defaultValue != null || isSpecialNullDefault()) {
+      return defaultValue;
+    }
+    Converter<?> converter = getConverter();
+    String defaultValueAsString = getUnparsedDefaultValue();
+    boolean allowsMultiple = allowsMultiple();
+    // If the option allows multiple values then we intentionally return the empty list as
+    // the default value of this option since it is not always the case that an option
+    // that allows multiple values will have a converter that returns a list value.
+    if (allowsMultiple) {
+      defaultValue = Collections.emptyList();
+    } else {
+      // Otherwise try to convert the default value using the converter
+      try {
+        defaultValue = converter.convert(defaultValueAsString);
+      } catch (OptionsParsingException e) {
+        throw new ConstructionException(
+            String.format(
+                "OptionsParsingException while retrieving the default value for %s: %s",
+                getField().getName(), e.getMessage()),
+            e);
+      }
+    }
+    return defaultValue;
+  }
+
   static final Comparator<OptionDefinition> BY_OPTION_NAME =
       Comparator.comparing(OptionDefinition::getOptionName);
 
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 36d11895b2..9d683208d1 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParser.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParser.java
@@ -20,6 +20,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ListMultimap;
 import com.google.common.escape.Escaper;
+import com.google.devtools.common.options.OptionDefinition.NotAnOptionException;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
 import java.nio.file.FileSystem;
@@ -67,6 +68,7 @@ import javax.annotation.Nullable;
  */
 public class OptionsParser implements OptionsProvider {
 
+  // TODO(b/65049598) make ConstructionException checked.
   /**
    * An unchecked exception thrown when there is a problem constructing a parser, e.g. an error
    * while validating an {@link OptionDefinition} in one of its {@link OptionsBase} subclasses.
@@ -228,50 +230,24 @@ public class OptionsParser implements OptionsProvider {
     }
   }
 
-  // TODO(b/64904491) remove this once the converter and default information is in OptionDefinition
-  // and cached.
-  /** The metadata about an option. */
+  /** The metadata about an option, in the context of this options parser. */
   public static final class OptionDescription {
 
-    private final String name;
-
-    // For valued flags
-    private final Object defaultValue;
-    private final Converter<?> converter;
-    private final boolean allowMultiple;
-
+    private final OptionDefinition optionDefinition;
     private final OptionsData.ExpansionData expansionData;
     private final ImmutableList<OptionValueDescription> implicitRequirements;
 
     OptionDescription(
-        String name,
-        Object defaultValue,
-        Converter<?> converter,
-        boolean allowMultiple,
+        OptionDefinition definition,
         OptionsData.ExpansionData expansionData,
         ImmutableList<OptionValueDescription> implicitRequirements) {
-      this.name = name;
-      this.defaultValue = defaultValue;
-      this.converter = converter;
-      this.allowMultiple = allowMultiple;
+      this.optionDefinition = definition;
       this.expansionData = expansionData;
       this.implicitRequirements = implicitRequirements;
     }
 
-    public String getName() {
-      return name;
-    }
-
-    public Object getDefaultValue() {
-      return defaultValue;
-    }
-
-    public Converter<?> getConverter() {
-      return converter;
-    }
-
-    public boolean getAllowMultiple() {
-      return allowMultiple;
+    public OptionDefinition getOptionDefinition() {
+      return optionDefinition;
     }
 
     public ImmutableList<OptionValueDescription> getImplicitRequirements() {
@@ -926,7 +902,7 @@ public class OptionsParser implements OptionsProvider {
           try {
             optionDefinition = OptionDefinition.extractOptionDefinition(field);
             extraNamesFromMap.add("'" + optionDefinition.getOptionName() + "'");
-          } catch (ConstructionException e) {
+          } catch (NotAnOptionException e) {
             extraNamesFromMap.add("<non-Option field>");
           }
         }
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 f8d67789f2..28aeb22b70 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
@@ -173,7 +173,7 @@ class OptionsParserImpl {
       OptionDefinition optionDefinition = mapEntry.getValue();
       OptionValueDescription entry = parsedValues.get(optionDefinition);
       if (entry == null) {
-        Object value = optionsData.getDefaultValue(optionDefinition);
+        Object value = optionDefinition.getDefaultValue();
         result.add(
             new OptionValueDescription(
                 fieldName,
@@ -327,10 +327,7 @@ class OptionsParserImpl {
     }
 
     return new OptionDescription(
-        name,
-        optionsData.getDefaultValue(optionDefinition),
-        optionsData.getConverter(optionDefinition),
-        optionsData.getAllowMultiple(optionDefinition),
+        optionDefinition,
         optionsData.getExpansionDataForField(optionDefinition),
         getImplicitDependantDescriptions(
             ImmutableList.copyOf(optionDefinition.getImplicitRequirements()), name));
@@ -359,7 +356,7 @@ class OptionsParserImpl {
               /* source */ null,
               implicitDependant,
               /* expendedFrom */ null,
-              optionsData.getAllowMultiple(parseResult.optionDefinition)));
+              parseResult.optionDefinition.allowsMultiple()));
     }
     return builder.build();
   }
@@ -390,7 +387,7 @@ class OptionsParserImpl {
               /* source */ null,
               /* implicitDependant */ null,
               flagName,
-              optionsData.getAllowMultiple(parseResult.optionDefinition)));
+              parseResult.optionDefinition.allowsMultiple()));
     }
     return builder.build();
   }
@@ -527,7 +524,7 @@ class OptionsParserImpl {
                   + Joiner.on(' ').join(unparsed));
         }
       } else {
-        Converter<?> converter = optionsData.getConverter(optionDefinition);
+        Converter<?> converter = optionDefinition.getConverter();
         Object convertedValue;
         try {
           convertedValue = converter.convert(value);
@@ -645,7 +642,7 @@ class OptionsParserImpl {
         booleanValue = false;
         if (optionDefinition != null) {
           // TODO(bazel-team): Add tests for these cases.
-          if (!optionsData.isBooleanField(optionDefinition)) {
+          if (!optionDefinition.isBooleanField()) {
             throw new OptionsParsingException(
                 "Illegal use of 'no' prefix on non-boolean option: " + arg, arg);
           }
@@ -670,7 +667,7 @@ class OptionsParserImpl {
 
     if (value == null) {
       // Special-case boolean to supply value based on presence of "no" prefix.
-      if (optionsData.isBooleanField(optionDefinition)) {
+      if (optionDefinition.isBooleanField()) {
         value = booleanValue ? "1" : "0";
       } else if (optionDefinition.getType().equals(Void.class)
           && !optionDefinition.isWrapperOption()) {
@@ -707,7 +704,7 @@ class OptionsParserImpl {
       Object value;
       OptionValueDescription entry = parsedValues.get(optionDefinition);
       if (entry == null) {
-        value = optionsData.getDefaultValue(optionDefinition);
+        value = optionDefinition.getDefaultValue();
       } else {
         value = entry.getValue();
       }
diff --git a/src/main/java/com/google/devtools/common/options/OptionsUsage.java b/src/main/java/com/google/devtools/common/options/OptionsUsage.java
index 51cc3c624a..0ab30da2b5 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsUsage.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsUsage.java
@@ -101,8 +101,8 @@ class OptionsUsage {
       StringBuilder usage,
       OptionsParser.HelpVerbosity helpVerbosity,
       OptionsData optionsData) {
-    String flagName = getFlagName(optionDefinition, optionsData);
-    String typeDescription = getTypeDescription(optionDefinition, optionsData);
+    String flagName = getFlagName(optionDefinition);
+    String typeDescription = getTypeDescription(optionDefinition);
     usage.append("  --").append(flagName);
     if (helpVerbosity == OptionsParser.HelpVerbosity.SHORT) { // just the name
       usage.append('\n');
@@ -163,12 +163,12 @@ class OptionsUsage {
       Escaper escaper,
       OptionsData optionsData) {
     String plainFlagName = optionDefinition.getOptionName();
-    String flagName = getFlagName(optionDefinition, optionsData);
+    String flagName = getFlagName(optionDefinition);
     String valueDescription = optionDefinition.getValueTypeHelpText();
-    String typeDescription = getTypeDescription(optionDefinition, optionsData);
+    String typeDescription = getTypeDescription(optionDefinition);
     usage.append("<dt><code><a name=\"flag--").append(plainFlagName).append("\"></a>--");
     usage.append(flagName);
-    if (optionsData.isBooleanField(optionDefinition) || optionDefinition.isVoidField()) {
+    if (optionDefinition.isBooleanField() || optionDefinition.isVoidField()) {
       // Nothing for boolean, tristate, boolean_or_enum, or void options.
     } else if (!valueDescription.isEmpty()) {
       usage.append("=").append(escaper.escape(valueDescription));
@@ -279,12 +279,12 @@ class OptionsUsage {
     }
   }
 
-  private static String getTypeDescription(OptionDefinition optionsField, OptionsData optionsData) {
-    return optionsData.getConverter(optionsField).getTypeDescription();
+  private static String getTypeDescription(OptionDefinition optionsDefinition) {
+    return optionsDefinition.getConverter().getTypeDescription();
   }
 
-  static String getFlagName(OptionDefinition optionDefinition, OptionsData optionsData) {
+  static String getFlagName(OptionDefinition optionDefinition) {
     String name = optionDefinition.getOptionName();
-    return optionsData.isBooleanField(optionDefinition) ? "[no]" + name : name;
+    return optionDefinition.isBooleanField() ? "[no]" + name : name;
   }
 }
diff --git a/src/test/java/com/google/devtools/common/options/BUILD b/src/test/java/com/google/devtools/common/options/BUILD
index a0a4e36fd7..84e3178c4f 100644
--- a/src/test/java/com/google/devtools/common/options/BUILD
+++ b/src/test/java/com/google/devtools/common/options/BUILD
@@ -19,6 +19,7 @@ java_test(
         "//third_party:guava-testlib",
         "//third_party:jsr305",
         "//third_party:junit4",
+        "//third_party:mockito",
         "//third_party:truth",
     ],
 )
diff --git a/src/test/java/com/google/devtools/common/options/OptionDefinitionTest.java b/src/test/java/com/google/devtools/common/options/OptionDefinitionTest.java
new file mode 100644
index 0000000000..d369f33639
--- /dev/null
+++ b/src/test/java/com/google/devtools/common/options/OptionDefinitionTest.java
@@ -0,0 +1,231 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// 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 com.google.devtools.common.options;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+import com.google.devtools.common.options.Converters.IntegerConverter;
+import com.google.devtools.common.options.Converters.StringConverter;
+import com.google.devtools.common.options.OptionsParser.ConstructionException;
+import java.util.Map;
+import java.util.Set;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+import org.mockito.Mockito;
+
+/** Tests for {@link OptionDefinition}. */
+@RunWith(JUnit4.class)
+public class OptionDefinitionTest {
+
+  /** Dummy options class, to test various expected failures of the OptionDefinition. */
+  public static class BrokenOptions extends OptionsBase {
+    @Option(
+      name = "missing_its_converter",
+      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+      effectTags = {OptionEffectTag.NO_OP},
+      defaultValue = "1"
+    )
+    public Map<String, String> noConverter;
+
+    @Option(
+      name = "multiple_but_not_a_list",
+      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+      effectTags = {OptionEffectTag.NO_OP},
+      defaultValue = "null",
+      allowMultiple = true
+    )
+    public String multipleWithNoList;
+
+    @Option(
+      name = "multiple_with_wrong_collection",
+      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+      effectTags = {OptionEffectTag.NO_OP},
+      defaultValue = "1",
+      allowMultiple = true
+    )
+    public Set<String> multipleWithSetType;
+
+    @Option(
+      name = "invalid_default",
+      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+      effectTags = {OptionEffectTag.NO_OP},
+      defaultValue = "not a number"
+    )
+    public int invalidDefault;
+  }
+
+  @Test
+  public void errorForMissingOptionConverter() throws Exception {
+    OptionDefinition optionDef =
+        OptionDefinition.extractOptionDefinition(BrokenOptions.class.getField("noConverter"));
+    try {
+      optionDef.getConverter();
+      fail("Missing converter should have caused getConverter to fail.");
+    } catch (ConstructionException e) {
+      assertThat(e)
+          .hasMessageThat()
+          .contains(
+              "Option noConverter expects values of type java.util.Map<java.lang.String, "
+                  + "java.lang.String>, but no converter was found; possible fix: "
+                  + "add converter=... to its @Option annotation.");
+    }
+  }
+
+  @Test
+  public void errorForInvalidOptionTypeForRepeatableOptions() throws Exception {
+    OptionDefinition optionDef =
+        OptionDefinition.extractOptionDefinition(
+            BrokenOptions.class.getField("multipleWithNoList"));
+    try {
+      optionDef.getConverter();
+      fail("Mistyped allowMultiple option did not fail getConverter().");
+    } catch (ConstructionException e) {
+      assertThat(e)
+          .hasMessageThat()
+          .contains
+              ("Option multipleWithNoList allows multiple occurrences, so must be of type "
+                  + "List<...>");
+    }
+  }
+
+  @Test
+  public void errorForInvalidCollectionOptionConverter() throws Exception {
+    OptionDefinition optionDef =
+        OptionDefinition.extractOptionDefinition(
+            BrokenOptions.class.getField("multipleWithSetType"));
+    try {
+      optionDef.getConverter();
+      fail("Mistyped allowMultiple option did not fail getConverter().");
+    } catch (ConstructionException e) {
+      assertThat(e)
+          .hasMessageThat()
+          .contains(
+              "Option multipleWithSetType allows multiple occurrences, so must be of type "
+                  + "List<...>");
+    }
+  }
+
+  @Test
+  public void errorForInvalidDefaultValue() throws Exception {
+    OptionDefinition optionDef =
+        OptionDefinition.extractOptionDefinition(BrokenOptions.class.getField("invalidDefault"));
+    try {
+      optionDef.getDefaultValue();
+      fail("Invalid default value parsed without failure.");
+    } catch (ConstructionException e) {
+      assertThat(e)
+          .hasMessageThat()
+          .contains(
+              "OptionsParsingException while retrieving the default value for invalidDefault: "
+                  + "'not a number' is not an int");
+    }
+  }
+
+  /** The rare valid options. */
+  public static class ValidOptionUsingDefaultConverterForMocking extends OptionsBase {
+    @Option(
+      name = "foo",
+      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+      effectTags = {OptionEffectTag.NO_OP},
+      defaultValue = "42"
+    )
+    public int foo;
+
+    @Option(
+      name = "bar",
+      converter = StringConverter.class,
+      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+      effectTags = {OptionEffectTag.NO_OP},
+      defaultValue = "strings"
+    )
+    public int bar;
+  }
+
+  /**
+   * Test that the converter and option default values are only computed once and then are obtained
+   * from the stored values, in the case where a default converter is used.
+   */
+  @Test
+  public void optionDefinitionMemoizesDefaultConverterValue() throws Exception {
+    OptionDefinition optionDefinition =
+        OptionDefinition.extractOptionDefinition(
+            ValidOptionUsingDefaultConverterForMocking.class.getField("foo"));
+    OptionDefinition mockOptionDef = Mockito.spy(optionDefinition);
+
+    // Do a bunch of potentially repeat operations on this option that need to know information
+    // about the converter and default value. Also verify that the values are as expected.
+    boolean isBoolean = mockOptionDef.isBooleanField();
+    assertThat(isBoolean).isFalse();
+
+    Converter<?> converter = mockOptionDef.getConverter();
+    assertThat(converter).isInstanceOf(IntegerConverter.class);
+
+    int value = (int) mockOptionDef.getDefaultValue();
+    assertThat(value).isEqualTo(42);
+
+    // Expect reference equality, since we didn't recompute the value
+    Converter<?> secondConverter = mockOptionDef.getConverter();
+    assertThat(secondConverter).isSameAs(converter);
+
+    mockOptionDef.getDefaultValue();
+
+    // Verify that we didn't re-calculate the converter from the provided class object.
+    verify(mockOptionDef, times(1)).getProvidedConverter();
+    // The first call to getDefaultValue checks isSpecialNullDefault, which called
+    // getUnparsedValueDefault as well, but expect no more calls to it after the initial call.
+    verify(mockOptionDef, times(1)).isSpecialNullDefault();
+    verify(mockOptionDef, times(2)).getUnparsedDefaultValue();
+  }
+
+  /**
+   * Test that the converter and option default values are only computed once and then are obtained
+   * from the stored values, in the case where a converter was provided.
+   */
+  @Test
+  public void optionDefinitionMemoizesProvidedConverterValue() throws Exception {
+    OptionDefinition optionDefinition =
+        OptionDefinition.extractOptionDefinition(
+            ValidOptionUsingDefaultConverterForMocking.class.getField("bar"));
+    OptionDefinition mockOptionDef = Mockito.spy(optionDefinition);
+
+    // Do a bunch of potentially repeat operations on this option that need to know information
+    // about the converter and default value. Also verify that the values are as expected.
+    boolean isBoolean = mockOptionDef.isBooleanField();
+    assertThat(isBoolean).isFalse();
+
+    Converter<?> converter = mockOptionDef.getConverter();
+    assertThat(converter).isInstanceOf(StringConverter.class);
+
+    String value = (String) mockOptionDef.getDefaultValue();
+    assertThat(value).isEqualTo("strings");
+
+    // Expect reference equality, since we didn't recompute the value
+    Converter<?> secondConverter = mockOptionDef.getConverter();
+    assertThat(secondConverter).isSameAs(converter);
+
+    mockOptionDef.getDefaultValue();
+
+    // Verify that we didn't re-calculate the converter from the provided class object.
+    verify(mockOptionDef, times(1)).getProvidedConverter();
+    // The first call to getDefaultValue checks isSpecialNullDefault, which called
+    // getUnparsedValueDefault as well, but expect no more calls to it after the initial call.
+    verify(mockOptionDef, times(1)).isSpecialNullDefault();
+    verify(mockOptionDef, times(2)).getUnparsedDefaultValue();
+  }
+}
diff --git a/src/test/java/com/google/devtools/common/options/OptionsDataTest.java b/src/test/java/com/google/devtools/common/options/OptionsDataTest.java
index 10e5a527e8..1266d2e25d 100644
--- a/src/test/java/com/google/devtools/common/options/OptionsDataTest.java
+++ b/src/test/java/com/google/devtools/common/options/OptionsDataTest.java
@@ -250,53 +250,6 @@ public class OptionsDataTest {
     }
   }
 
-  /** Dummy options class. */
-  public static class InvalidOptionConverter extends OptionsBase {
-    @Option(
-      name = "foo",
-      converter = StringConverter.class,
-      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
-      effectTags = {OptionEffectTag.NO_OP},
-      defaultValue = "1"
-    )
-    public Integer foo;
-  }
-
-  @Test
-  public void errorForInvalidOptionConverter() throws Exception {
-    try {
-      construct(InvalidOptionConverter.class);
-    } catch (ConstructionException e) {
-      // Expected exception
-      return;
-    }
-    fail();
-  }
-
-  /** Dummy options class. */
-  public static class InvalidListOptionConverter extends OptionsBase {
-    @Option(
-      name = "foo",
-      converter = StringConverter.class,
-      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
-      effectTags = {OptionEffectTag.NO_OP},
-      defaultValue = "1",
-      allowMultiple = true
-    )
-    public List<Integer> foo;
-  }
-
-  @Test
-  public void errorForInvalidListOptionConverter() throws Exception {
-    try {
-      construct(InvalidListOptionConverter.class);
-    } catch (ConstructionException e) {
-      // Expected exception
-      return;
-    }
-    fail();
-  }
-
   /** Dummy options class using deprecated category. */
   public static class InvalidUndocumentedCategory extends OptionsBase {
     @Option(
diff --git a/src/test/java/com/google/devtools/common/options/OptionsTest.java b/src/test/java/com/google/devtools/common/options/OptionsTest.java
index 32f257b3a0..ac2be6af90 100644
--- a/src/test/java/com/google/devtools/common/options/OptionsTest.java
+++ b/src/test/java/com/google/devtools/common/options/OptionsTest.java
@@ -491,12 +491,11 @@ public class OptionsTest {
       Options.parse(K.class, NO_ARGS).getOptions();
       fail();
     } catch (OptionsParser.ConstructionException e) {
-      assertThat(e).hasCauseThat().isInstanceOf(IllegalStateException.class);
+      assertThat(e).hasCauseThat().isInstanceOf(OptionsParsingException.class);
       assertThat(e)
-          .hasCauseThat()
           .hasMessageThat()
           .isEqualTo(
-              "OptionsParsingException while retrieving default for "
+              "OptionsParsingException while retrieving the default value for "
                   + "int1: 'null' is not an int");
     }
   }
diff --git a/src/test/shell/integration/execution_phase_tests.sh b/src/test/shell/integration/execution_phase_tests.sh
index 50369831b1..21d6dc8da1 100755
--- a/src/test/shell/integration/execution_phase_tests.sh
+++ b/src/test/shell/integration/execution_phase_tests.sh
@@ -209,13 +209,22 @@ function test_cache_computed_file_digests_ui() {
 }
 
 function test_jobs_default_auto() {
+  # The default flag value is only read if --jobs is not set explicitly.
+  # Do not use a bazelrc here, this would break the test.
+  # TODO(b/65166983) this should be --bazelrc=/dev/null, since this is a bazel
+  # test and we want to encourage bazel-specific naming, but that would
+  # currently break the test because --bazelrc and --blazerc are treated
+  # separately.
   mkdir -p package || fail "mkdir failed"
   echo "cc_library(name = 'foo', srcs = ['foo.cc'])" >package/BUILD
   echo "int foo(void) { return 0; }" >package/foo.cc
 
-  local java_log="$(bazel info output_base 2>/dev/null)/java.log"
+  local output_base="$(bazel --nomaster_bazelrc --blazerc=/dev/null info \
+      output_base 2>/dev/null)" || fail "bazel info should work"
+  local java_log="${output_base}/java.log"
+  bazel --nomaster_bazelrc --blazerc=/dev/null build package:foo \
+      >>"${TEST_log}" 2>&1 || fail "Should build"
 
-  bazel build package:foo >>"${TEST_log}" 2>&1 || fail "Should build"
   assert_last_log "BuildRequest" 'Flag "jobs" was set to "auto"' "${java_log}" \
       "--jobs was not set to auto by default"
 }