Clean up InvocationPolicy's use of OptionDescription.

OptionDescription is basically a hack to get the expansion data for options from outside the options parser, but it was being used at various points of invocation policy enforcement. In order to correctly track option origin, we only want to get this information once. Do it during the invocation policy expansion stage, not at enforcement, so that we track the information of the option's origin in the original invocation policy passed to the enforcer, not the expanded one.

PiperOrigin-RevId: 171661669

4 files changed, 159 insertions, 111 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/CanonicalizeCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/CanonicalizeCommand.java
index f06df2bbe3..c2951f1d28 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/CanonicalizeCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/CanonicalizeCommand.java
@@ -21,7 +21,6 @@ import com.google.devtools.build.lib.runtime.BlazeCommandUtils;
 import com.google.devtools.build.lib.runtime.BlazeRuntime;
 import com.google.devtools.build.lib.runtime.Command;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
-import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.FlagPolicy;
 import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy;
 import com.google.devtools.build.lib.util.ExitCode;
 import com.google.devtools.common.options.InvocationPolicyEnforcer;
@@ -162,10 +161,9 @@ public final class CanonicalizeCommand implements BlazeCommand {
 
       // Print out the canonical invocation policy if requested.
       if (canonicalizeOptions.canonicalizePolicy) {
-        ImmutableList<FlagPolicy> effectiveFlagPolicies =
-            InvocationPolicyEnforcer.getEffectivePolicies(policy, parser, commandName, Level.INFO);
         InvocationPolicy effectivePolicy =
-            InvocationPolicy.newBuilder().addAllFlagPolicies(effectiveFlagPolicies).build();
+            InvocationPolicyEnforcer.getEffectiveInvocationPolicy(
+                policy, parser, commandName, Level.INFO);
         env.getReporter().getOutErr().printOutLn(effectivePolicy.toString());
 
       } else {
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 87fb5770a2..ee0e4878c7 100644
--- a/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java
+++ b/src/main/java/com/google/devtools/common/options/InvocationPolicyEnforcer.java
@@ -79,6 +79,16 @@ public final class InvocationPolicyEnforcer {
     this.loglevel = loglevel;
   }
 
+  private static final class FlagPolicyWithContext {
+    private final FlagPolicy policy;
+    private final OptionDescription description;
+
+    public FlagPolicyWithContext(FlagPolicy policy, OptionDescription description) {
+      this.policy = policy;
+      this.description = description;
+    }
+  }
+
   public InvocationPolicy getInvocationPolicy() {
     return invocationPolicy;
   }
@@ -110,11 +120,11 @@ public final class InvocationPolicyEnforcer {
 
     // The effective policy returned is expanded, filtered for applicable commands, and cleaned of
     // redundancies and conflicts.
-    List<FlagPolicy> effectivePolicies =
+    List<FlagPolicyWithContext> effectivePolicies =
         getEffectivePolicies(invocationPolicy, parser, command, loglevel);
 
-    for (FlagPolicy flagPolicy : effectivePolicies) {
-      String flagName = flagPolicy.getFlagName();
+    for (FlagPolicyWithContext flagPolicy : effectivePolicies) {
+      String flagName = flagPolicy.policy.getFlagName();
 
       OptionValueDescription valueDescription;
       try {
@@ -129,26 +139,23 @@ public final class InvocationPolicyEnforcer {
         continue;
       }
 
-      OptionDescription optionDescription =
-          parser.getOptionDescription(
-              flagName, OptionPriority.INVOCATION_POLICY, INVOCATION_POLICY_SOURCE);
       // getOptionDescription() will return null if the option does not exist, however
       // getOptionValueDescription() above would have thrown an IllegalArgumentException if that
       // were the case.
-      Verify.verifyNotNull(optionDescription);
+      Verify.verifyNotNull(flagPolicy.description);
 
-      switch (flagPolicy.getOperationCase()) {
+      switch (flagPolicy.policy.getOperationCase()) {
         case SET_VALUE:
-          applySetValueOperation(parser, flagPolicy, valueDescription, optionDescription, loglevel);
+          applySetValueOperation(parser, flagPolicy, valueDescription, loglevel);
           break;
 
         case USE_DEFAULT:
           applyUseDefaultOperation(
-              parser, "UseDefault", optionDescription.getOptionDefinition(), loglevel);
+              parser, "UseDefault", flagPolicy.description.getOptionDefinition(), loglevel);
           break;
 
         case ALLOW_VALUES:
-          AllowValues allowValues = flagPolicy.getAllowValues();
+          AllowValues allowValues = flagPolicy.policy.getAllowValues();
           FilterValueOperation.AllowValueOperation allowValueOperation =
               new FilterValueOperation.AllowValueOperation(loglevel);
           allowValueOperation.apply(
@@ -157,11 +164,11 @@ public final class InvocationPolicyEnforcer {
               allowValues.hasNewValue() ? allowValues.getNewValue() : null,
               allowValues.hasUseDefault(),
               valueDescription,
-              optionDescription);
+              flagPolicy.description);
           break;
 
         case DISALLOW_VALUES:
-          DisallowValues disallowValues = flagPolicy.getDisallowValues();
+          DisallowValues disallowValues = flagPolicy.policy.getDisallowValues();
           FilterValueOperation.DisallowValueOperation disallowValueOperation =
               new FilterValueOperation.DisallowValueOperation(loglevel);
           disallowValueOperation.apply(
@@ -170,7 +177,7 @@ public final class InvocationPolicyEnforcer {
               disallowValues.hasNewValue() ? disallowValues.getNewValue() : null,
               disallowValues.hasUseDefault(),
               valueDescription,
-              optionDescription);
+              flagPolicy.description);
           break;
 
         case OPERATION_NOT_SET:
@@ -180,7 +187,7 @@ public final class InvocationPolicyEnforcer {
           logger.warning(
               String.format(
                   "Unknown operation '%s' from invocation policy for flag '%s'",
-                  flagPolicy.getOperationCase(), flagName));
+                  flagPolicy.policy.getOperationCase(), flagName));
           break;
       }
     }
@@ -202,12 +209,26 @@ public final class InvocationPolicyEnforcer {
     return !Collections.disjoint(policy.getCommandsList(), applicableCommands);
   }
 
+  /** Returns the expanded and filtered policy that would be enforced for the given command. */
+  public static InvocationPolicy getEffectiveInvocationPolicy(
+      InvocationPolicy invocationPolicy, OptionsParser parser, String command, Level loglevel)
+      throws OptionsParsingException {
+    ImmutableList<FlagPolicyWithContext> effectivePolicies =
+        getEffectivePolicies(invocationPolicy, parser, command, loglevel);
+
+    InvocationPolicy.Builder builder = InvocationPolicy.newBuilder();
+    for (FlagPolicyWithContext policyWithContext : effectivePolicies) {
+      builder.addFlagPolicies(policyWithContext.policy);
+    }
+    return builder.build();
+  }
+
   /**
    * Takes the provided policy and processes it to the form that can be used on the user options.
    *
    * <p>Expands any policies on expansion flags.
    */
-  public static ImmutableList<FlagPolicy> getEffectivePolicies(
+  private static ImmutableList<FlagPolicyWithContext> getEffectivePolicies(
       InvocationPolicy invocationPolicy, OptionsParser parser, String command, Level loglevel)
       throws OptionsParsingException {
     if (invocationPolicy == null) {
@@ -220,20 +241,35 @@ public final class InvocationPolicyEnforcer {
             : CommandNameCache.CommandNameCacheInstance.INSTANCE.get(command);
 
     // Expand all policies to transfer policies on expansion flags to policies on the child flags.
-    List<FlagPolicy> expandedPolicies = new ArrayList<>();
+    List<FlagPolicyWithContext> expandedPolicies = new ArrayList<>();
     for (FlagPolicy policy : invocationPolicy.getFlagPoliciesList()) {
       if (!policyApplies(policy, commandAndParentCommands)) {
         // Only keep and expand policies that are applicable to the current command.
         continue;
       }
-      List<FlagPolicy> policies = expandPolicy(policy, parser, loglevel);
+      OptionDescription optionDescription =
+          parser.getOptionDescription(
+              policy.getFlagName(), OptionPriority.INVOCATION_POLICY, INVOCATION_POLICY_SOURCE);
+      if (optionDescription == null) {
+        // InvocationPolicy ignores policy on non-existing flags by design, for version
+        // compatibility.
+        logger.log(
+            loglevel,
+            String.format(
+                "Flag '%s' specified by invocation policy does not exist, and will be ignored",
+                policy.getFlagName()));
+        continue;
+      }
+      FlagPolicyWithContext policyWithContext =
+          new FlagPolicyWithContext(policy, optionDescription);
+      List<FlagPolicyWithContext> policies = expandPolicy(policyWithContext, parser, loglevel);
       expandedPolicies.addAll(policies);
     }
 
     // Only keep that last policy for each flag.
-    Map<String, FlagPolicy> effectivePolicy = new HashMap<>();
-    for (FlagPolicy expandedPolicy : expandedPolicies) {
-      String flagName = expandedPolicy.getFlagName();
+    Map<String, FlagPolicyWithContext> effectivePolicy = new HashMap<>();
+    for (FlagPolicyWithContext expandedPolicy : expandedPolicies) {
+      String flagName = expandedPolicy.policy.getFlagName();
       effectivePolicy.put(flagName, expandedPolicy);
     }
 
@@ -253,31 +289,27 @@ public final class InvocationPolicyEnforcer {
   }
 
   private static ImmutableList<ParsedOptionDescription> getExpansionsFromFlagPolicy(
-      FlagPolicy expansionPolicy, OptionDescription optionDescription, OptionsParser parser)
-      throws OptionsParsingException {
-    if (!optionDescription.isExpansion()) {
+      FlagPolicyWithContext expansionPolicy, OptionsParser parser) throws OptionsParsingException {
+    if (!expansionPolicy.description.isExpansion()) {
       return ImmutableList.of();
     }
-
+    String policyFlagName = expansionPolicy.policy.getFlagName();
+    String optionName = expansionPolicy.description.getOptionDefinition().getOptionName();
     Preconditions.checkArgument(
-        expansionPolicy
-            .getFlagName()
-            .equals(optionDescription.getOptionDefinition().getOptionName()),
-        String.format(
+        policyFlagName.equals(optionName),
             "The optionDescription provided (for flag %s) does not match the policy for flag %s.",
-            optionDescription.getOptionDefinition().getOptionName(),
-            expansionPolicy.getFlagName()));
+            optionName, policyFlagName);
 
     ImmutableList.Builder<ParsedOptionDescription> resultsBuilder = ImmutableList.builder();
-    switch (expansionPolicy.getOperationCase()) {
+    switch (expansionPolicy.policy.getOperationCase()) {
       case SET_VALUE:
         {
-          SetValue setValue = expansionPolicy.getSetValue();
+          SetValue setValue = expansionPolicy.policy.getSetValue();
           if (setValue.getFlagValueCount() > 0) {
             for (String value : setValue.getFlagValueList()) {
               resultsBuilder.addAll(
                   parser.getExpansionOptionValueDescriptions(
-                      optionDescription.getOptionDefinition(),
+                      expansionPolicy.description.getOptionDefinition(),
                       value,
                       OptionPriority.INVOCATION_POLICY,
                       INVOCATION_POLICY_SOURCE));
@@ -285,7 +317,7 @@ public final class InvocationPolicyEnforcer {
           } else {
             resultsBuilder.addAll(
                 parser.getExpansionOptionValueDescriptions(
-                    optionDescription.getOptionDefinition(),
+                    expansionPolicy.description.getOptionDefinition(),
                     null,
                     OptionPriority.INVOCATION_POLICY,
                     INVOCATION_POLICY_SOURCE));
@@ -295,7 +327,7 @@ public final class InvocationPolicyEnforcer {
       case USE_DEFAULT:
         resultsBuilder.addAll(
             parser.getExpansionOptionValueDescriptions(
-                optionDescription.getOptionDefinition(),
+                expansionPolicy.description.getOptionDefinition(),
                 null,
                 OptionPriority.INVOCATION_POLICY,
                 INVOCATION_POLICY_SOURCE));
@@ -305,19 +337,19 @@ public final class InvocationPolicyEnforcer {
         // cases aren't necessary (the values given in the flag policy shouldn't need to be
         // checked). If you care about blocking specific flag values you should block the behavior
         // on the specific ones, not the expansion that contains them.
-        throwAllowValuesOnExpansionFlagException(expansionPolicy.getFlagName());
+        throwAllowValuesOnExpansionFlagException(optionName);
         break;
       case DISALLOW_VALUES:
-        throwDisallowValuesOnExpansionFlagException(expansionPolicy.getFlagName());
+        throwDisallowValuesOnExpansionFlagException(optionName);
         break;
       case OPERATION_NOT_SET:
-        throw new PolicyOperationNotSetException(expansionPolicy.getFlagName());
+        throw new PolicyOperationNotSetException(optionName);
       default:
         logger.warning(
             String.format(
                 "Unknown operation '%s' from invocation policy for flag '%s'",
-                expansionPolicy.getOperationCase(),
-                optionDescription.getOptionDefinition().getOptionName()));
+                expansionPolicy.policy.getOperationCase(),
+                optionName));
         break;
     }
 
@@ -331,30 +363,20 @@ public final class InvocationPolicyEnforcer {
    *
    * <p>None of the flagPolicies returned should be on expansion flags.
    */
-  private static List<FlagPolicy> expandPolicy(
-      FlagPolicy originalPolicy, OptionsParser parser, Level loglevel)
+  private static List<FlagPolicyWithContext> expandPolicy(
+      FlagPolicyWithContext originalPolicy, OptionsParser parser, Level loglevel)
       throws OptionsParsingException {
-    List<FlagPolicy> expandedPolicies = new ArrayList<>();
-
-    OptionDescription originalOptionDescription =
-        parser.getOptionDescription(
-            originalPolicy.getFlagName(),
-            OptionPriority.INVOCATION_POLICY,
-            INVOCATION_POLICY_SOURCE);
-    if (originalOptionDescription == null) {
-      // InvocationPolicy ignores policy on non-existing flags by design, for version compatibility.
-      return expandedPolicies;
-    }
+    List<FlagPolicyWithContext> expandedPolicies = new ArrayList<>();
 
     ImmutableList<ParsedOptionDescription> expansions =
-        getExpansionsFromFlagPolicy(originalPolicy, originalOptionDescription, parser);
+        getExpansionsFromFlagPolicy(originalPolicy, parser);
     ImmutableList.Builder<ParsedOptionDescription> subflagBuilder = ImmutableList.builder();
     ImmutableList<ParsedOptionDescription> subflags =
         subflagBuilder
-            .addAll(originalOptionDescription.getImplicitRequirements())
+            .addAll(originalPolicy.description.getImplicitRequirements())
             .addAll(expansions)
             .build();
-    boolean isExpansion = originalOptionDescription.isExpansion();
+    boolean isExpansion = originalPolicy.description.isExpansion();
 
     if (!subflags.isEmpty() && logger.isLoggable(loglevel)) {
       // Log the expansion. This is only really useful for understanding the invocation policy
@@ -370,27 +392,34 @@ public final class InvocationPolicyEnforcer {
           "expandPolicy",
           String.format(
               "Expanding %s on option %s to its %s: %s.",
-              originalPolicy.getOperationCase(),
-              originalPolicy.getFlagName(),
+              originalPolicy.policy.getOperationCase(),
+              originalPolicy.policy.getFlagName(),
               isExpansion ? "expansions" : "implied flags",
               Joiner.on("; ").join(subflagNames)));
     }
 
     // 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<OptionDefinition, ParsedOptionDescription> repeatableSubflagsInSetValues =
+    Multimap<OptionDescription, ParsedOptionDescription> 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 (ParsedOptionDescription currentSubflag : subflags) {
+      OptionDescription subflagOptionDescription =
+          parser.getOptionDescription(
+              currentSubflag.getOptionDefinition().getOptionName(),
+              OptionPriority.INVOCATION_POLICY,
+              INVOCATION_POLICY_SOURCE);
+
       if (currentSubflag.getOptionDefinition().allowsMultiple()
-          && originalPolicy.getOperationCase().equals(OperationCase.SET_VALUE)) {
-        repeatableSubflagsInSetValues.put(currentSubflag.getOptionDefinition(), currentSubflag);
+          && originalPolicy.policy.getOperationCase().equals(OperationCase.SET_VALUE)) {
+        repeatableSubflagsInSetValues.put(subflagOptionDescription, currentSubflag);
       } else {
-        FlagPolicy subflagAsPolicy =
-            getSingleValueSubflagAsPolicy(currentSubflag, originalPolicy, isExpansion);
+        FlagPolicyWithContext subflagAsPolicy =
+            getSingleValueSubflagAsPolicy(
+                subflagOptionDescription, currentSubflag, originalPolicy, isExpansion);
         // In case any of the expanded flags are themselves expansions, recurse.
         expandedPolicies.addAll(expandPolicy(subflagAsPolicy, parser, loglevel));
       }
@@ -399,7 +428,7 @@ 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 (OptionDefinition repeatableFlag : repeatableSubflagsInSetValues.keySet()) {
+    for (OptionDescription repeatableFlag : repeatableSubflagsInSetValues.keySet()) {
       int numValues = repeatableSubflagsInSetValues.get(repeatableFlag).size();
       ArrayList<String> newValues = new ArrayList<>(numValues);
       for (ParsedOptionDescription setValue : repeatableSubflagsInSetValues.get(repeatableFlag)) {
@@ -422,16 +451,19 @@ 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 subflag, the definition of the flag the SetValue'd expansion flag expands to.
+   * @param subflagDesc, the description 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 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.
    */
-  private static FlagPolicy getSetValueSubflagAsPolicy(
-      OptionDefinition subflag, List<String> subflagValue, FlagPolicy originalPolicy) {
+  private static FlagPolicyWithContext getSetValueSubflagAsPolicy(
+      OptionDescription subflagDesc,
+      List<String> subflagValue,
+      FlagPolicyWithContext originalPolicy) {
     // Some sanity checks.
-    Verify.verify(originalPolicy.getOperationCase().equals(OperationCase.SET_VALUE));
+    OptionDefinition subflag = subflagDesc.getOptionDefinition();
+    Verify.verify(originalPolicy.policy.getOperationCase().equals(OperationCase.SET_VALUE));
     if (!subflag.allowsMultiple()) {
       Verify.verify(subflagValue.size() <= 1);
     }
@@ -443,28 +475,33 @@ public final class InvocationPolicyEnforcer {
       setValueExpansion.addFlagValue(value);
     }
     if (subflag.allowsMultiple()) {
-      setValueExpansion.setAppend(originalPolicy.getSetValue().getOverridable());
+      setValueExpansion.setAppend(originalPolicy.policy.getSetValue().getOverridable());
     } else {
-      setValueExpansion.setOverridable(originalPolicy.getSetValue().getOverridable());
+      setValueExpansion.setOverridable(originalPolicy.policy.getSetValue().getOverridable());
     }
 
     // Commands from the original policy, flag name of the expansion
-    return FlagPolicy.newBuilder()
-        .addAllCommands(originalPolicy.getCommandsList())
-        .setFlagName(subflag.getOptionName())
-        .setSetValue(setValueExpansion)
-        .build();
+    return new FlagPolicyWithContext(
+        FlagPolicy.newBuilder()
+            .addAllCommands(originalPolicy.policy.getCommandsList())
+            .setFlagName(subflag.getOptionName())
+            .setSetValue(setValueExpansion)
+            .build(),
+        subflagDesc);
   }
 
   /**
    * For an expansion flag in an invocation policy, each flag it expands to must be given a
    * corresponding policy.
    */
-  private static FlagPolicy getSingleValueSubflagAsPolicy(
-      ParsedOptionDescription currentSubflag, FlagPolicy originalPolicy, boolean isExpansion)
+  private static FlagPolicyWithContext getSingleValueSubflagAsPolicy(
+      OptionDescription subflagContext,
+      ParsedOptionDescription currentSubflag,
+      FlagPolicyWithContext originalPolicy,
+      boolean isExpansion)
       throws OptionsParsingException {
-    FlagPolicy subflagAsPolicy = null;
-    switch (originalPolicy.getOperationCase()) {
+    FlagPolicyWithContext subflagAsPolicy = null;
+    switch (originalPolicy.policy.getOperationCase()) {
       case SET_VALUE:
         if (currentSubflag.getOptionDefinition().allowsMultiple()) {
           throw new AssertionError(
@@ -479,24 +516,24 @@ public final class InvocationPolicyEnforcer {
         } else {
           subflagValue = ImmutableList.of(currentSubflag.getUnconvertedValue());
         }
-        subflagAsPolicy =
-            getSetValueSubflagAsPolicy(
-                currentSubflag.getOptionDefinition(), subflagValue, originalPolicy);
+        subflagAsPolicy = getSetValueSubflagAsPolicy(subflagContext, subflagValue, originalPolicy);
         break;
 
       case USE_DEFAULT:
         // Commands from the original policy, flag name of the expansion
         subflagAsPolicy =
-            FlagPolicy.newBuilder()
-                .addAllCommands(originalPolicy.getCommandsList())
-                .setFlagName(currentSubflag.getOptionDefinition().getOptionName())
-                .setUseDefault(UseDefault.getDefaultInstance())
-                .build();
+            new FlagPolicyWithContext(
+                FlagPolicy.newBuilder()
+                    .addAllCommands(originalPolicy.policy.getCommandsList())
+                    .setFlagName(currentSubflag.getOptionDefinition().getOptionName())
+                    .setUseDefault(UseDefault.getDefaultInstance())
+                    .build(),
+                subflagContext);
         break;
 
       case ALLOW_VALUES:
         if (isExpansion) {
-          throwAllowValuesOnExpansionFlagException(originalPolicy.getFlagName());
+          throwAllowValuesOnExpansionFlagException(originalPolicy.policy.getFlagName());
         }
         // If this flag is an implicitRequirement, and some values for the parent flag are
         // allowed, nothing needs to happen on the implicitRequirement that is set for all
@@ -505,7 +542,7 @@ public final class InvocationPolicyEnforcer {
 
       case DISALLOW_VALUES:
         if (isExpansion) {
-          throwDisallowValuesOnExpansionFlagException(originalPolicy.getFlagName());
+          throwDisallowValuesOnExpansionFlagException(originalPolicy.policy.getFlagName());
         }
         // If this flag is an implicitRequirement, and some values for the parent flag are
         // disallowed, that implies that all others are allowed, so nothing needs to happen
@@ -513,7 +550,7 @@ public final class InvocationPolicyEnforcer {
         break;
 
       case OPERATION_NOT_SET:
-        throw new PolicyOperationNotSetException(originalPolicy.getFlagName());
+        throw new PolicyOperationNotSetException(originalPolicy.policy.getFlagName());
 
       default:
         return null;
@@ -534,13 +571,12 @@ public final class InvocationPolicyEnforcer {
 
   private static void applySetValueOperation(
       OptionsParser parser,
-      FlagPolicy flagPolicy,
+      FlagPolicyWithContext flagPolicy,
       OptionValueDescription valueDescription,
-      OptionDescription optionDescription,
       Level loglevel)
       throws OptionsParsingException {
-    SetValue setValue = flagPolicy.getSetValue();
-    OptionDefinition optionDefinition = optionDescription.getOptionDefinition();
+    SetValue setValue = flagPolicy.policy.getSetValue();
+    OptionDefinition optionDefinition = flagPolicy.description.getOptionDefinition();
 
     // SetValue.flag_value must have at least 1 value.
     if (setValue.getFlagValueCount() == 0) {
@@ -552,7 +588,7 @@ public final class InvocationPolicyEnforcer {
 
     // Flag must allow multiple values if multiple values are specified by the policy.
     if (setValue.getFlagValueCount() > 1
-        && !optionDescription.getOptionDefinition().allowsMultiple()) {
+        && !flagPolicy.description.getOptionDefinition().allowsMultiple()) {
       throw new OptionsParsingException(
           String.format(
               "SetValue operation from invocation policy sets multiple values for flag '%s' which "
@@ -575,7 +611,7 @@ public final class InvocationPolicyEnforcer {
 
       if (!setValue.getAppend()) {
         // Clear the value in case the flag is a repeated flag so that values don't accumulate.
-        parser.clearValue(optionDescription.getOptionDefinition());
+        parser.clearValue(flagPolicy.description.getOptionDefinition());
       }
 
       // Set all the flag values from the policy.
@@ -585,7 +621,9 @@ public final class InvocationPolicyEnforcer {
               loglevel,
               "Setting value for flag '%s' from invocation policy to '%s', overriding the "
                   + "default value '%s'",
-              optionDefinition.getOptionName(), flagValue, optionDefinition.getDefaultValue());
+              optionDefinition.getOptionName(),
+              flagValue,
+              optionDefinition.getDefaultValue());
         } else {
           logInApplySetValueOperation(
               loglevel,
@@ -608,14 +646,8 @@ public final class InvocationPolicyEnforcer {
     if (clearedValueDescription != null) {
       // Log the removed value.
       String clearedFlagName = clearedValueDescription.getOptionDefinition().getOptionName();
-
-      OptionDescription desc =
-          parser.getOptionDescription(
-              clearedFlagName, OptionPriority.INVOCATION_POLICY, INVOCATION_POLICY_SOURCE);
-      Object clearedFlagDefaultValue = null;
-      if (desc != null) {
-        clearedFlagDefaultValue = desc.getOptionDefinition().getDefaultValue();
-      }
+      Object clearedFlagDefaultValue =
+          clearedValueDescription.getOptionDefinition().getDefaultValue();
       logger.log(
           loglevel,
           String.format(
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 2b4bd2a3e4..25733c7179 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParser.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParser.java
@@ -275,6 +275,22 @@ public class OptionsParser implements OptionsProvider {
         throws OptionsParsingException {
       return expansionData.getExpansion(context);
     }
+
+    @Override
+    public boolean equals(Object obj) {
+      if (obj instanceof OptionDescription) {
+        OptionDescription other = (OptionDescription) obj;
+        // Check that the option is the same and that it is in the same context (expansionData)
+        return other.optionDefinition.equals(optionDefinition)
+            && other.expansionData.equals(expansionData);
+      }
+      return false;
+    }
+
+    @Override
+    public int hashCode() {
+      return optionDefinition.hashCode() + expansionData.hashCode();
+    }
   }
 
   /**
diff --git a/src/test/java/com/google/devtools/common/options/InvocationPolicyEnforcerTestBase.java b/src/test/java/com/google/devtools/common/options/InvocationPolicyEnforcerTestBase.java
index a217c5083a..86f0112d59 100644
--- a/src/test/java/com/google/devtools/common/options/InvocationPolicyEnforcerTestBase.java
+++ b/src/test/java/com/google/devtools/common/options/InvocationPolicyEnforcerTestBase.java
@@ -21,6 +21,7 @@ import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.In
 import java.io.ByteArrayOutputStream;
 import java.util.Arrays;
 import java.util.List;
+import java.util.logging.Level;
 import org.junit.Before;
 import org.junit.BeforeClass;
 
@@ -62,7 +63,8 @@ public class InvocationPolicyEnforcerTestBase {
 
     return new InvocationPolicyEnforcer(
         InvocationPolicyParser.parsePolicy(
-            startupOptionsParser.getOptions(BlazeServerStartupOptions.class).invocationPolicy));
+            startupOptionsParser.getOptions(BlazeServerStartupOptions.class).invocationPolicy),
+        Level.INFO);
   }
 
   OptionsParser parser;