Replace referrals to options by their name to option definitions.

Now that we have a standard way of referring to an option, remove all of the places that we were referring to them by their name. Since options can have multiple names, this is more clear and provides the additional information needed to understand the option. It also stops the habit of requesting unqualified strings, which was hard to read.

RELNOTES: None.
PiperOrigin-RevId: 168254584

4 files changed, 186 insertions, 182 deletions

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 37753a9955..73c6c5e2bd 100644
--- a/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java
+++ b/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java
@@ -14,6 +14,7 @@
 package com.google.devtools.common.options;
 
 import com.google.common.base.Joiner;
+import com.google.common.base.Preconditions;
 import com.google.common.base.Verify;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.ImmutableList;
@@ -51,7 +52,8 @@ public final class InvocationPolicyEnforcer {
 
   private static final Logger logger = Logger.getLogger(InvocationPolicyEnforcer.class.getName());
 
-  private static final Function<Object, String> INVOCATION_POLICY_SOURCE = o -> "Invocation policy";
+  private static final Function<OptionDefinition, String> INVOCATION_POLICY_SOURCE =
+      o -> "Invocation policy";
 
   @Nullable private final InvocationPolicy invocationPolicy;
 
@@ -121,11 +123,11 @@ public final class InvocationPolicyEnforcer {
 
       switch (flagPolicy.getOperationCase()) {
         case SET_VALUE:
-          applySetValueOperation(parser, flagPolicy, flagName, valueDescription, optionDescription);
+          applySetValueOperation(parser, flagPolicy, valueDescription, optionDescription);
           break;
 
         case USE_DEFAULT:
-          applyUseDefaultOperation(parser, "UseDefault", flagName);
+          applyUseDefaultOperation(parser, "UseDefault", optionDescription.getOptionDefinition());
           break;
 
         case ALLOW_VALUES:
@@ -135,7 +137,6 @@ public final class InvocationPolicyEnforcer {
               allowValues.getAllowedValuesList(),
               allowValues.hasNewValue() ? allowValues.getNewValue() : null,
               allowValues.hasUseDefault(),
-              flagName,
               valueDescription,
               optionDescription);
           break;
@@ -147,7 +148,6 @@ public final class InvocationPolicyEnforcer {
               disallowValues.getDisallowedValuesList(),
               disallowValues.hasNewValue() ? disallowValues.getNewValue() : null,
               disallowValues.hasUseDefault(),
-              flagName,
               valueDescription,
               optionDescription);
           break;
@@ -238,7 +238,14 @@ public final class InvocationPolicyEnforcer {
       return ImmutableList.of();
     }
 
-    String expansionFlagName = expansionPolicy.getFlagName();
+    Preconditions.checkArgument(
+        expansionPolicy
+            .getFlagName()
+            .equals(optionDescription.getOptionDefinition().getOptionName()),
+        String.format(
+            "The optionDescription provided (for flag %s) does not match the policy for flag %s.",
+            optionDescription.getOptionDefinition().getOptionName(),
+            expansionPolicy.getFlagName()));
 
     ImmutableList.Builder<OptionValueDescription> resultsBuilder = ImmutableList.builder();
     switch (expansionPolicy.getOperationCase()) {
@@ -248,16 +255,20 @@ public final class InvocationPolicyEnforcer {
           if (setValue.getFlagValueCount() > 0) {
             for (String value : setValue.getFlagValueList()) {
               resultsBuilder.addAll(
-                  parser.getExpansionOptionValueDescriptions(expansionFlagName, value));
+                  parser.getExpansionOptionValueDescriptions(
+                      optionDescription.getOptionDefinition(), value));
             }
           } else {
             resultsBuilder.addAll(
-                parser.getExpansionOptionValueDescriptions(expansionFlagName, null));
+                parser.getExpansionOptionValueDescriptions(
+                    optionDescription.getOptionDefinition(), null));
           }
         }
         break;
       case USE_DEFAULT:
-        resultsBuilder.addAll(parser.getExpansionOptionValueDescriptions(expansionFlagName, null));
+        resultsBuilder.addAll(
+            parser.getExpansionOptionValueDescriptions(
+                optionDescription.getOptionDefinition(), null));
         break;
       case ALLOW_VALUES:
         // All expansions originally given to the parser have been expanded by now, so these two
@@ -275,7 +286,8 @@ public final class InvocationPolicyEnforcer {
         logger.warning(
             String.format(
                 "Unknown operation '%s' from invocation policy for flag '%s'",
-                expansionPolicy.getOperationCase(), expansionFlagName));
+                expansionPolicy.getOperationCase(),
+                optionDescription.getOptionDefinition().getOptionName()));
         break;
     }
 
@@ -318,7 +330,7 @@ public final class InvocationPolicyEnforcer {
       // invocation policy does not change, so this can be a log level fine.
       List<String> subflagNames = new ArrayList<>(subflags.size());
       for (OptionValueDescription subflag : subflags) {
-        subflagNames.add("--" + subflag.getName());
+        subflagNames.add("--" + subflag.getOptionDefinition().getOptionName());
       }
 
       logger.logp(
@@ -335,16 +347,16 @@ public final class InvocationPolicyEnforcer {
 
     // Repeated flags are special, and could set multiple times in an expansion, with the user
     // expecting both values to be valid. Collect these separately.
-    Multimap<String, OptionValueDescription> repeatableSubflagsInSetValues =
+    Multimap<OptionDefinition, OptionValueDescription> repeatableSubflagsInSetValues =
         ArrayListMultimap.create();
 
     // Create a flag policy for the child that looks like the parent's policy "transferred" to its
     // child. Note that this only makes sense for SetValue, when setting an expansion flag, or
     // UseDefault, when preventing it from being set.
     for (OptionValueDescription currentSubflag : subflags) {
-      if (currentSubflag.getAllowMultiple()
+      if (currentSubflag.getOptionDefinition().allowsMultiple()
           && originalPolicy.getOperationCase().equals(OperationCase.SET_VALUE)) {
-        repeatableSubflagsInSetValues.put(currentSubflag.getName(), currentSubflag);
+        repeatableSubflagsInSetValues.put(currentSubflag.getOptionDefinition(), currentSubflag);
       } else {
         FlagPolicy subflagAsPolicy =
             getSingleValueSubflagAsPolicy(currentSubflag, originalPolicy, isExpansion);
@@ -356,15 +368,13 @@ public final class InvocationPolicyEnforcer {
     // If there are any repeatable flag SetValues, deal with them together now.
     // Note that expansion flags have no value, and so cannot have multiple values either.
     // Skipping the recursion above is fine.
-    for (String repeatableFlag : repeatableSubflagsInSetValues.keySet()) {
+    for (OptionDefinition repeatableFlag : repeatableSubflagsInSetValues.keySet()) {
       int numValues = repeatableSubflagsInSetValues.get(repeatableFlag).size();
       ArrayList<String> newValues = new ArrayList<>(numValues);
       for (OptionValueDescription setValue : repeatableSubflagsInSetValues.get(repeatableFlag)) {
         newValues.add(setValue.getOriginalValueString());
       }
-      expandedPolicies.add(
-          getSetValueSubflagAsPolicy(
-              repeatableFlag, newValues, /* allowMultiple */ true, originalPolicy));
+      expandedPolicies.add(getSetValueSubflagAsPolicy(repeatableFlag, newValues, originalPolicy));
     }
 
     // Don't add the original policy if it was an expansion flag, which have no value, but do add
@@ -381,21 +391,17 @@ public final class InvocationPolicyEnforcer {
    * policies that set the flag, and so interact with repeatable flags, flags that can be set
    * multiple times, in subtle ways.
    *
-   * @param subflagName, the flag the SetValue'd expansion flag expands to.
+   * @param subflag, the definition of the flag the SetValue'd expansion flag expands to.
    * @param subflagValue, the values that the SetValue'd expansion flag expands to for this flag.
-   * @param allowMultiple, whether the flag is multivalued.
    * @param originalPolicy, the original policy on the expansion flag.
    * @return the flag policy for the subflag given, this will be part of the expanded form of the
-   *  SetValue policy on the original flag.
+   *     SetValue policy on the original flag.
    */
   private static FlagPolicy getSetValueSubflagAsPolicy(
-      String subflagName,
-      List<String> subflagValue,
-      boolean allowMultiple,
-      FlagPolicy originalPolicy) {
+      OptionDefinition subflag, List<String> subflagValue, FlagPolicy originalPolicy) {
     // Some sanity checks.
     Verify.verify(originalPolicy.getOperationCase().equals(OperationCase.SET_VALUE));
-    if (!allowMultiple) {
+    if (!subflag.allowsMultiple()) {
       Verify.verify(subflagValue.size() <= 1);
     }
 
@@ -405,7 +411,7 @@ public final class InvocationPolicyEnforcer {
     for (String value : subflagValue) {
       setValueExpansion.addFlagValue(value);
     }
-    if (allowMultiple) {
+    if (subflag.allowsMultiple()) {
       setValueExpansion.setAppend(originalPolicy.getSetValue().getOverridable());
     } else {
       setValueExpansion.setOverridable(originalPolicy.getSetValue().getOverridable());
@@ -413,10 +419,10 @@ public final class InvocationPolicyEnforcer {
 
     // Commands from the original policy, flag name of the expansion
     return FlagPolicy.newBuilder()
-            .addAllCommands(originalPolicy.getCommandsList())
-            .setFlagName(subflagName)
-            .setSetValue(setValueExpansion)
-            .build();
+        .addAllCommands(originalPolicy.getCommandsList())
+        .setFlagName(subflag.getOptionName())
+        .setSetValue(setValueExpansion)
+        .build();
   }
 
   /**
@@ -429,7 +435,7 @@ public final class InvocationPolicyEnforcer {
     FlagPolicy subflagAsPolicy = null;
     switch (originalPolicy.getOperationCase()) {
       case SET_VALUE:
-        if (currentSubflag.getAllowMultiple()) {
+        if (currentSubflag.getOptionDefinition().allowsMultiple()) {
           throw new AssertionError(
               "SetValue subflags with allowMultiple should have been dealt with separately and "
                   + "accumulated into a single FlagPolicy.");
@@ -444,7 +450,7 @@ public final class InvocationPolicyEnforcer {
         }
         subflagAsPolicy =
             getSetValueSubflagAsPolicy(
-                currentSubflag.getName(), subflagValue, /*allowMultiple=*/ false, originalPolicy);
+                currentSubflag.getOptionDefinition(), subflagValue, originalPolicy);
         break;
 
       case USE_DEFAULT:
@@ -452,10 +458,8 @@ public final class InvocationPolicyEnforcer {
         subflagAsPolicy =
             FlagPolicy.newBuilder()
                 .addAllCommands(originalPolicy.getCommandsList())
-                .setFlagName(currentSubflag.getName())
-                .setUseDefault(
-                    UseDefault
-                        .getDefaultInstance())
+                .setFlagName(currentSubflag.getOptionDefinition().getOptionName())
+                .setUseDefault(UseDefault.getDefaultInstance())
                 .build();
         break;
 
@@ -499,19 +503,18 @@ public final class InvocationPolicyEnforcer {
   private static void applySetValueOperation(
       OptionsParser parser,
       FlagPolicy flagPolicy,
-      String flagName,
       OptionValueDescription valueDescription,
       OptionDescription optionDescription)
       throws OptionsParsingException {
-
     SetValue setValue = flagPolicy.getSetValue();
+    OptionDefinition optionDefinition = optionDescription.getOptionDefinition();
 
     // SetValue.flag_value must have at least 1 value.
     if (setValue.getFlagValueCount() == 0) {
       throw new OptionsParsingException(
           String.format(
               "SetValue operation from invocation policy for flag '%s' does not have a value",
-              flagName));
+              optionDefinition.getOptionName()));
     }
 
     // Flag must allow multiple values if multiple values are specified by the policy.
@@ -521,7 +524,7 @@ public final class InvocationPolicyEnforcer {
           String.format(
               "SetValue operation from invocation policy sets multiple values for flag '%s' which "
                   + "does not allow multiple values",
-              flagName));
+              optionDefinition.getOptionName()));
     }
 
     if (setValue.getOverridable() && valueDescription != null) {
@@ -532,13 +535,13 @@ public final class InvocationPolicyEnforcer {
               + "because the invocation policy specifying the value(s) '%s' is overridable",
           valueDescription.getValue(),
           valueDescription.getSource(),
-          flagName,
+          optionDefinition.getOptionName(),
           setValue.getFlagValueList());
     } else {
 
       if (!setValue.getAppend()) {
         // Clear the value in case the flag is a repeated flag so that values don't accumulate.
-        parser.clearValue(flagName);
+        parser.clearValue(optionDescription.getOptionDefinition());
       }
 
       // Set all the flag values from the policy.
@@ -547,24 +550,28 @@ public final class InvocationPolicyEnforcer {
           logInApplySetValueOperation(
               "Setting value for flag '%s' from invocation "
                   + "policy to '%s', overriding the default value '%s'",
-              flagName, flagValue, optionDescription.getOptionDefinition().getDefaultValue());
+              optionDefinition.getOptionName(), flagValue, optionDefinition.getDefaultValue());
         } else {
           logInApplySetValueOperation(
               "Setting value for flag '%s' from invocation "
                   + "policy to '%s', overriding value '%s' from '%s'",
-              flagName, flagValue, valueDescription.getValue(), valueDescription.getSource());
+              optionDefinition.getOptionName(),
+              flagValue,
+              valueDescription.getValue(),
+              valueDescription.getSource());
         }
-        setFlagValue(parser, flagName, flagValue);
+        setFlagValue(parser, optionDefinition, flagValue);
       }
     }
   }
 
   private static void applyUseDefaultOperation(
-      OptionsParser parser, String policyType, String flagName) throws OptionsParsingException {
-    OptionValueDescription clearedValueDescription = parser.clearValue(flagName);
+      OptionsParser parser, String policyType, OptionDefinition option)
+      throws OptionsParsingException {
+    OptionValueDescription clearedValueDescription = parser.clearValue(option);
     if (clearedValueDescription != null) {
       // Log the removed value.
-      String clearedFlagName = clearedValueDescription.getName();
+      String clearedFlagName = clearedValueDescription.getOptionDefinition().getOptionName();
       String originalValue = clearedValueDescription.getValue().toString();
       String source = clearedValueDescription.getSource();
 
@@ -625,11 +632,10 @@ public final class InvocationPolicyEnforcer {
         List<String> policyValues,
         String newValue,
         boolean useDefault,
-        String flagName,
         OptionValueDescription valueDescription,
         OptionDescription optionDescription)
         throws OptionsParsingException {
-
+      OptionDefinition optionDefinition = optionDescription.getOptionDefinition();
       // Convert all the allowed values from strings to real objects using the options'
       // converters so that they can be checked for equality using real .equals() instead
       // of string comparison. For example, "--foo=0", "--foo=false", "--nofoo", and "-f-"
@@ -637,18 +643,15 @@ public final class InvocationPolicyEnforcer {
       // can be arbitrarily complex.
       Set<Object> convertedPolicyValues = new HashSet<>();
       for (String value : policyValues) {
-        Object convertedValue =
-            optionDescription.getOptionDefinition().getConverter().convert(value);
+        Object convertedValue = optionDefinition.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.getOptionDefinition().allowsMultiple()
-            && convertedValue instanceof List<?>) {
+        if (optionDefinition.allowsMultiple() && convertedValue instanceof List<?>) {
           convertedPolicyValues.addAll((List<?>) convertedValue);
         } else {
-          convertedPolicyValues.add(
-              optionDescription.getOptionDefinition().getConverter().convert(value));
+          convertedPolicyValues.add(optionDefinition.getConverter().convert(value));
         }
       }
 
@@ -666,7 +669,9 @@ public final class InvocationPolicyEnforcer {
               String.format(
                   "%sValues policy disallows the default value '%s' for flag '%s' but also "
                       + "specifies to use the default value",
-                  policyType, optionDescription.getOptionDefinition().getDefaultValue(), flagName));
+                  policyType,
+                  optionDefinition.getDefaultValue(),
+                  optionDefinition.getOptionName()));
         }
       }
 
@@ -676,35 +681,28 @@ public final class InvocationPolicyEnforcer {
         // the flag allowing multiple values, however, flags that allow multiple values cannot have
         // default values, and their value is always the empty list if they haven't been specified,
         // which is why new_default_value is not a repeated field.
-        checkDefaultValue(
-            parser,
-            policyValues,
-            newValue,
-            flagName,
-            optionDescription,
-            convertedPolicyValues);
+        checkDefaultValue(parser, optionDescription, policyValues, newValue, convertedPolicyValues);
       } else {
         checkUserValue(
             parser,
+            optionDescription,
+            valueDescription,
             policyValues,
             newValue,
             useDefault,
-            flagName,
-            valueDescription,
-            optionDescription,
             convertedPolicyValues);
       }
     }
 
     void checkDefaultValue(
         OptionsParser parser,
+        OptionDescription optionDescription,
         List<String> policyValues,
         String newValue,
-        String flagName,
-        OptionDescription optionDescription,
         Set<Object> convertedPolicyValues)
         throws OptionsParsingException {
 
+      OptionDefinition optionDefinition = optionDescription.getOptionDefinition();
       if (!isFlagValueAllowed(
           convertedPolicyValues, optionDescription.getOptionDefinition().getDefaultValue())) {
         if (newValue != null) {
@@ -713,13 +711,13 @@ public final class InvocationPolicyEnforcer {
               String.format(
                   "Overriding default value '%s' for flag '%s' with value '%s' "
                       + "specified by invocation policy. %sed values are: %s",
-                  optionDescription.getOptionDefinition().getDefaultValue(),
-                  flagName,
+                  optionDefinition.getDefaultValue(),
+                  optionDefinition.getOptionName(),
                   newValue,
                   policyType,
                   policyValues));
-          parser.clearValue(flagName);
-          setFlagValue(parser, flagName, newValue);
+          parser.clearValue(optionDefinition);
+          setFlagValue(parser, optionDefinition, newValue);
         } else {
           // The operation disallows the default value, but doesn't supply a new value.
           throw new OptionsParsingException(
@@ -728,7 +726,7 @@ public final class InvocationPolicyEnforcer {
                       + "the policy does not provide a new value. "
                       + "%sed values are: %s",
                   optionDescription.getOptionDefinition().getDefaultValue(),
-                  flagName,
+                  optionDefinition.getOptionName(),
                   policyType,
                   policyValues));
         }
@@ -737,15 +735,14 @@ public final class InvocationPolicyEnforcer {
 
     void checkUserValue(
         OptionsParser parser,
+        OptionDescription optionDescription,
+        OptionValueDescription valueDescription,
         List<String> policyValues,
         String newValue,
         boolean useDefault,
-        String flagName,
-        OptionValueDescription valueDescription,
-        OptionDescription optionDescription,
         Set<Object> convertedPolicyValues)
         throws OptionsParsingException {
-
+      OptionDefinition option = optionDescription.getOptionDefinition();
       if (optionDescription.getOptionDefinition().allowsMultiple()) {
         // allowMultiple requires that the type of the option be List<T>, so cast from Object
         // to List<?>.
@@ -753,16 +750,13 @@ public final class InvocationPolicyEnforcer {
         for (Object value : optionValues) {
           if (!isFlagValueAllowed(convertedPolicyValues, value)) {
             if (useDefault) {
-              applyUseDefaultOperation(parser, policyType + "Values", flagName);
+              applyUseDefaultOperation(parser, policyType + "Values", option);
             } else {
               throw new OptionsParsingException(
                   String.format(
                       "Flag value '%s' for flag '%s' is not allowed by invocation policy. "
                           + "%sed values are: %s",
-                      value,
-                      flagName,
-                      policyType,
-                      policyValues));
+                      value, option.getOptionName(), policyType, policyValues));
             }
           }
         }
@@ -775,33 +769,34 @@ public final class InvocationPolicyEnforcer {
                 String.format(
                     "Overriding disallowed value '%s' for flag '%s' with value '%s' "
                         + "specified by invocation policy. %sed values are: %s",
-                    valueDescription.getValue(), flagName, newValue, policyType, policyValues));
-            parser.clearValue(flagName);
-            setFlagValue(parser, flagName, newValue);
+                    valueDescription.getValue(),
+                    option.getOptionName(),
+                    newValue,
+                    policyType,
+                    policyValues));
+            parser.clearValue(option);
+            setFlagValue(parser, option, newValue);
           } else if (useDefault) {
-            applyUseDefaultOperation(parser, policyType + "Values", flagName);
+            applyUseDefaultOperation(parser, policyType + "Values", option);
           } else {
             throw new OptionsParsingException(
                 String.format(
                     "Flag value '%s' for flag '%s' is not allowed by invocation policy and the "
                         + "policy does not specify a new value. %sed values are: %s",
-                    valueDescription.getValue(),
-                    flagName,
-                    policyType,
-                    policyValues));
+                    valueDescription.getValue(), option.getOptionName(), policyType, policyValues));
           }
         }
       }
     }
   }
 
-  private static void setFlagValue(OptionsParser parser, String flagName, String flagValue)
+  private static void setFlagValue(OptionsParser parser, OptionDefinition flag, String flagValue)
       throws OptionsParsingException {
 
     parser.parseWithSourceFunction(
         OptionPriority.INVOCATION_POLICY,
         INVOCATION_POLICY_SOURCE,
-        ImmutableList.of(String.format("--%s=%s", flagName, flagValue)));
+        ImmutableList.of(String.format("--%s=%s", flag.getOptionName(), flagValue)));
   }
 }
 
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 08962bba0d..a42a624cf7 100644
--- a/src/main/java/com/google/devtools/common/options/OptionDefinition.java
+++ b/src/main/java/com/google/devtools/common/options/OptionDefinition.java
@@ -274,6 +274,25 @@ public class OptionDefinition {
     return defaultValue;
   }
 
+  /**
+   * {@link OptionDefinition} is really a wrapper around a {@link Field} that caches information
+   * obtained through reflection. Checking that the fields they represent are equal is sufficient
+   * to check that two {@link OptionDefinition} objects are equal.
+   */
+  @Override
+  public boolean equals(Object object) {
+    if (!(object instanceof OptionDefinition)) {
+      return false;
+    }
+    OptionDefinition otherOption = (OptionDefinition) object;
+    return field.equals(otherOption.field);
+  }
+
+  @Override
+  public int hashCode() {
+    return field.hashCode();
+  }
+
   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 54642a135d..5bf1b4d4ac 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParser.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParser.java
@@ -278,8 +278,8 @@ public class OptionsParser implements OptionsProvider {
     @Nullable private final Object value;
     @Nullable private final OptionPriority priority;
     @Nullable private final String source;
-    @Nullable private final String implicitDependant;
-    @Nullable private final String expandedFrom;
+    @Nullable private final OptionDefinition implicitDependant;
+    @Nullable private final OptionDefinition expandedFrom;
 
     private OptionValueDescription(
         OptionDefinition optionDefinition,
@@ -288,8 +288,8 @@ public class OptionsParser implements OptionsProvider {
         @Nullable Object value,
         @Nullable OptionPriority priority,
         @Nullable String source,
-        @Nullable String implicitDependant,
-        @Nullable String expandedFrom) {
+        @Nullable OptionDefinition implicitDependant,
+        @Nullable OptionDefinition expandedFrom) {
       this.optionDefinition = optionDefinition;
       this.isDefaultValue = isDefaultValue;
       this.originalValueString = originalValueString;
@@ -306,8 +306,8 @@ public class OptionsParser implements OptionsProvider {
         @Nullable Object value,
         @Nullable OptionPriority priority,
         @Nullable String source,
-        @Nullable String implicitDependant,
-        @Nullable String expandedFrom) {
+        @Nullable OptionDefinition implicitDependant,
+        @Nullable OptionDefinition expandedFrom) {
       return new OptionValueDescription(
           optionDefinition,
           false,
@@ -377,7 +377,7 @@ public class OptionsParser implements OptionsProvider {
       return source;
     }
 
-    public String getImplicitDependant() {
+    public OptionDefinition getImplicitDependant() {
       return implicitDependant;
     }
 
@@ -385,7 +385,7 @@ public class OptionsParser implements OptionsProvider {
       return implicitDependant != null;
     }
 
-    public String getExpansionParent() {
+    public OptionDefinition getExpansionParent() {
       return expandedFrom;
     }
 
@@ -443,6 +443,7 @@ public class OptionsParser implements OptionsProvider {
    * <p>Note that the unparsed value and the source parameters can both be null.
    */
   public static class UnparsedOptionValueDescription {
+
     private final OptionDefinition optionDefinition;
     @Nullable private final String unparsedValue;
     private final OptionPriority priority;
@@ -672,15 +673,15 @@ public class OptionsParser implements OptionsProvider {
   }
 
   /**
-   * Returns a description of the options values that get expanded from this flag with the given
-   * flag value.
+   * Returns a description of the options values that get expanded from this option with the given
+   * value.
    *
    * @return The {@link ImmutableList<OptionValueDescription>} for the option, or null if there is
    *     no option by the given name.
    */
   ImmutableList<OptionValueDescription> getExpansionOptionValueDescriptions(
-      String flagName, @Nullable String flagValue) throws OptionsParsingException {
-    return impl.getExpansionOptionValueDescriptions(flagName, flagValue);
+      OptionDefinition option, @Nullable String optionValue) throws OptionsParsingException {
+    return impl.getExpansionOptionValueDescriptions(option, optionValue);
   }
 
   /**
@@ -729,15 +730,14 @@ public class OptionsParser implements OptionsProvider {
   }
 
   /**
-   * Parses {@code args}, using the classes registered with this parser.
-   * {@link #getOptions(Class)} and {@link #getResidue()} return the results. May be called
-   * multiple times; later options override existing ones if they have equal or higher priority.
-   * The source of options is given as a function that maps option names to the source of the
-   * option. Strings that cannot be parsed as options accumulates as* residue, if this parser
-   * allows it.
+   * Parses {@code args}, using the classes registered with this parser. {@link #getOptions(Class)}
+   * and {@link #getResidue()} return the results. May be called multiple times; later options
+   * override existing ones if they have equal or higher priority. The source of options is given as
+   * a function that maps option names to the source of the option. Strings that cannot be parsed as
+   * options accumulates as* residue, if this parser allows it.
    */
-  public void parseWithSourceFunction(OptionPriority priority,
-      Function<? super String, String> sourceFunction, List<String> args)
+  public void parseWithSourceFunction(
+      OptionPriority priority, Function<OptionDefinition, String> sourceFunction, List<String> args)
       throws OptionsParsingException {
     Preconditions.checkNotNull(priority);
     Preconditions.checkArgument(priority != OptionPriority.DEFAULT);
@@ -754,13 +754,12 @@ public class OptionsParser implements OptionsProvider {
    * <p>This will not affect options objects that have already been retrieved from this parser
    * through {@link #getOptions(Class)}.
    *
-   * @param optionName The full name of the option to clear.
-   * @return A map of an option name to the old value of the options that were cleared.
+   * @param option The option to clear.
+   * @return The old value of the option that was cleared.
    * @throws IllegalArgumentException If the flag does not exist.
    */
-  public OptionValueDescription clearValue(String optionName)
-      throws OptionsParsingException {
-    return impl.clearValue(optionName);
+  public OptionValueDescription clearValue(OptionDefinition option) throws OptionsParsingException {
+    return impl.clearValue(option);
   }
 
   @Override
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 026060653e..7478ab8e2d 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
@@ -196,12 +196,11 @@ class OptionsParserImpl {
   // Warnings should not end with a '.' because the internal reporter adds one automatically.
   private void setValue(
       OptionDefinition optionDefinition,
-      String name,
       Object value,
       OptionPriority priority,
       String source,
-      String implicitDependant,
-      String expandedFrom) {
+      OptionDefinition implicitDependant,
+      OptionDefinition expandedFrom) {
     OptionValueDescription entry = parsedValues.get(optionDefinition);
     if (entry != null) {
       // Override existing option if the new value has higher or equal priority.
@@ -211,40 +210,45 @@ class OptionsParserImpl {
           if (!implicitDependant.equals(entry.getImplicitDependant())) {
             warnings.add(
                 "Option '"
-                    + name
+                    + optionDefinition.getOptionName()
                     + "' is implicitly defined by both option '"
-                    + entry.getImplicitDependant()
+                    + entry.getImplicitDependant().getOptionName()
                     + "' and option '"
-                    + implicitDependant
+                    + implicitDependant.getOptionName()
                     + "'");
           }
         } else if ((implicitDependant != null) && priority.equals(entry.getPriority())) {
           warnings.add(
               "Option '"
-                  + name
+                  + optionDefinition.getOptionName()
                   + "' is implicitly defined by option '"
-                  + implicitDependant
+                  + implicitDependant.getOptionName()
                   + "'; the implicitly set value overrides the previous one");
         } else if (entry.getImplicitDependant() != null) {
           warnings.add(
               "A new value for option '"
-                  + name
+                  + optionDefinition.getOptionName()
                   + "' overrides a previous implicit setting of that option by option '"
-                  + entry.getImplicitDependant()
+                  + entry.getImplicitDependant().getOptionName()
                   + "'");
         } else if ((priority == entry.getPriority())
             && ((entry.getExpansionParent() == null) && (expandedFrom != null))) {
           // Create a warning if an expansion option overrides an explicit option:
-          warnings.add("The option '" + expandedFrom + "' was expanded and now overrides a "
-              + "previous explicitly specified option '" + name + "'");
+          warnings.add(
+              "The option '"
+                  + expandedFrom.getOptionName()
+                  + "' was expanded and now overrides a "
+                  + "previous explicitly specified option '"
+                  + optionDefinition.getOptionName()
+                  + "'");
         } else if ((entry.getExpansionParent() != null) && (expandedFrom != null)) {
           warnings.add(
               "The option '"
-                  + name
+                  + optionDefinition.getOptionName()
                   + "' was expanded to from both options '"
-                  + entry.getExpansionParent()
+                  + entry.getExpansionParent().getOptionName()
                   + "' and '"
-                  + expandedFrom
+                  + expandedFrom.getOptionName()
                   + "'");
         }
 
@@ -268,8 +272,8 @@ class OptionsParserImpl {
       Object value,
       OptionPriority priority,
       String source,
-      String implicitDependant,
-      String expandedFrom) {
+      OptionDefinition implicitDependant,
+      OptionDefinition expandedFrom) {
     OptionValueDescription entry = parsedValues.get(optionDefinition);
     if (entry == null) {
       entry =
@@ -287,13 +291,8 @@ class OptionsParserImpl {
     entry.addValue(priority, value);
   }
 
-  OptionValueDescription clearValue(String optionName)
+  OptionValueDescription clearValue(OptionDefinition optionDefinition)
       throws OptionsParsingException {
-    OptionDefinition optionDefinition = optionsData.getOptionDefinitionFromName(optionName);
-    if (optionDefinition == null) {
-      throw new IllegalArgumentException("No such option '" + optionName + "'");
-    }
-
     // Actually remove the value from various lists tracking effective options.
     canonicalizeValues.removeAll(optionDefinition);
     return parsedValues.remove(optionDefinition);
@@ -317,7 +316,7 @@ class OptionsParserImpl {
         optionDefinition,
         optionsData.getExpansionDataForField(optionDefinition),
         getImplicitDependantDescriptions(
-            ImmutableList.copyOf(optionDefinition.getImplicitRequirements()), name));
+            ImmutableList.copyOf(optionDefinition.getImplicitRequirements()), optionDefinition));
   }
 
   /**
@@ -327,7 +326,8 @@ class OptionsParserImpl {
    *     no conversion has taken place.
    */
   private ImmutableList<OptionValueDescription> getImplicitDependantDescriptions(
-      ImmutableList<String> options, String implicitDependant) throws OptionsParsingException {
+      ImmutableList<String> options, OptionDefinition implicitDependant)
+      throws OptionsParsingException {
     ImmutableList.Builder<OptionValueDescription> builder = ImmutableList.builder();
     Iterator<String> optionsIterator = options.iterator();
 
@@ -354,11 +354,10 @@ class OptionsParserImpl {
    *     is also a string, no conversion has taken place.
    */
   ImmutableList<OptionValueDescription> getExpansionOptionValueDescriptions(
-      String flagName, @Nullable String flagValue) throws OptionsParsingException {
+      OptionDefinition expansionFlag, @Nullable String flagValue) throws OptionsParsingException {
     ImmutableList.Builder<OptionValueDescription> builder = ImmutableList.builder();
-    OptionDefinition optionDefinition = optionsData.getOptionDefinitionFromName(flagName);
 
-    ImmutableList<String> options = optionsData.getEvaluatedExpansion(optionDefinition, flagValue);
+    ImmutableList<String> options = optionsData.getEvaluatedExpansion(expansionFlag, flagValue);
     Iterator<String> optionsIterator = options.iterator();
 
     while (optionsIterator.hasNext()) {
@@ -372,7 +371,7 @@ class OptionsParserImpl {
               /* priority */ null,
               /* source */ null,
               /* implicitDependant */ null,
-              flagName));
+              expansionFlag));
     }
     return builder.build();
   }
@@ -386,34 +385,34 @@ class OptionsParserImpl {
   }
 
   /**
-   * Parses the args, and returns what it doesn't parse. May be called multiple
-   * times, and may be called recursively. In each call, there may be no
-   * duplicates, but separate calls may contain intersecting sets of options; in
-   * that case, the arg seen last takes precedence.
+   * Parses the args, and returns what it doesn't parse. May be called multiple times, and may be
+   * called recursively. In each call, there may be no duplicates, but separate calls may contain
+   * intersecting sets of options; in that case, the arg seen last takes precedence.
    */
-  List<String> parse(OptionPriority priority, Function<? super String, String> sourceFunction,
-      List<String> args) throws OptionsParsingException {
+  List<String> parse(
+      OptionPriority priority, Function<OptionDefinition, String> sourceFunction, List<String> args)
+      throws OptionsParsingException {
     return parse(priority, sourceFunction, null, null, args);
   }
 
   /**
-   * Parses the args, and returns what it doesn't parse. May be called multiple
-   * times, and may be called recursively. Calls may contain intersecting sets
-   * of options; in that case, the arg seen last takes precedence.
+   * Parses the args, and returns what it doesn't parse. May be called multiple times, and may be
+   * called recursively. Calls may contain intersecting sets of options; in that case, the arg seen
+   * last takes precedence.
    *
-   * <p>The method uses the invariant that if an option has neither an implicit
-   * dependent nor an expanded from value, then it must have been explicitly
-   * set.
+   * <p>The method uses the invariant that if an option has neither an implicit dependent nor an
+   * expanded from value, then it must have been explicitly set.
    */
   private List<String> parse(
       OptionPriority priority,
-      Function<? super String, String> sourceFunction,
-      String implicitDependent,
-      String expandedFrom,
-      List<String> args) throws OptionsParsingException {
+      Function<OptionDefinition, String> sourceFunction,
+      OptionDefinition implicitDependent,
+      OptionDefinition expandedFrom,
+      List<String> args)
+      throws OptionsParsingException {
 
     List<String> unparsedArgs = new ArrayList<>();
-    LinkedHashMap<String, List<String>> implicitRequirements = new LinkedHashMap<>();
+    LinkedHashMap<OptionDefinition, List<String>> implicitRequirements = new LinkedHashMap<>();
 
     Iterator<String> argsIterator = argsPreProcessor.preProcess(args).iterator();
     while (argsIterator.hasNext()) {
@@ -479,7 +478,7 @@ class OptionsParserImpl {
                 optionDefinition,
                 value,
                 priority,
-                sourceFunction.apply(optionDefinition.getOptionName()),
+                sourceFunction.apply(optionDefinition),
                 expandedFrom == null);
         unparsedValues.add(unparsedOptionValueDescription);
         if (optionDefinition.allowsMultiple()) {
@@ -499,16 +498,11 @@ class OptionsParserImpl {
             "expanded from option --"
                 + optionDefinition.getOptionName()
                 + " from "
-                + sourceFunction.apply(optionDefinition.getOptionName());
-        Function<Object, String> expansionSourceFunction = o -> sourceMessage;
+                + sourceFunction.apply(optionDefinition);
+        Function<OptionDefinition, String> expansionSourceFunction = o -> sourceMessage;
         maybeAddDeprecationWarning(optionDefinition);
         List<String> unparsed =
-            parse(
-                priority,
-                expansionSourceFunction,
-                null,
-                optionDefinition.getOptionName(),
-                expansion);
+            parse(priority, expansionSourceFunction, null, optionDefinition, expansion);
         if (!unparsed.isEmpty()) {
           // Throw an assertion, because this indicates an error in the code that specified the
           // expansion for the current option.
@@ -535,10 +529,9 @@ class OptionsParserImpl {
         if (!optionDefinition.allowsMultiple()) {
           setValue(
               optionDefinition,
-              optionDefinition.getOptionName(),
               convertedValue,
               priority,
-              sourceFunction.apply(optionDefinition.getOptionName()),
+              sourceFunction.apply(optionDefinition),
               implicitDependent,
               expandedFrom);
         } else {
@@ -551,7 +544,7 @@ class OptionsParserImpl {
               optionDefinition,
               convertedValue,
               priority,
-              sourceFunction.apply(optionDefinition.getOptionName()),
+              sourceFunction.apply(optionDefinition),
               implicitDependent,
               expandedFrom);
         }
@@ -560,22 +553,20 @@ class OptionsParserImpl {
       // Collect any implicit requirements.
       if (optionDefinition.getImplicitRequirements().length > 0) {
         implicitRequirements.put(
-            optionDefinition.getOptionName(),
-            Arrays.asList(optionDefinition.getImplicitRequirements()));
+            optionDefinition, Arrays.asList(optionDefinition.getImplicitRequirements()));
       }
     }
 
     // Now parse any implicit requirements that were collected.
     // TODO(bazel-team): this should happen when the option is encountered.
     if (!implicitRequirements.isEmpty()) {
-      for (Map.Entry<String, List<String>> entry : implicitRequirements.entrySet()) {
+      for (Map.Entry<OptionDefinition, List<String>> entry : implicitRequirements.entrySet()) {
         String sourceMessage =
             "implicit requirement of option --"
                 + entry.getKey()
                 + " from "
                 + sourceFunction.apply(entry.getKey());
-        Function<Object, String> requirementSourceFunction =
-            o -> sourceMessage;
+        Function<OptionDefinition, String> requirementSourceFunction = o -> sourceMessage;
 
         List<String> unparsed = parse(priority, requirementSourceFunction, entry.getKey(), null,
             entry.getValue());