Fix common .bazelrc behavior for flag expansions

When a flag specified with `common` in a `.bazelrc` file expanded to a flag unsupported by the current command, it resulted in an error rather than the flag being ignored. Fixes bazelbuild#18130 (comment) Closes bazelbuild#20720. PiperOrigin-RevId: 597271727 Change-Id: Ieaba4dc00e13495a859e1eedf802759ad7dbf774
bazel-io · Jan 10, 2024 · 1ca47eb · 1ca47eb
1 parent 71cca16
commit 1ca47eb
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 3 deletions.
diff --git a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
@@ -545,7 +545,7 @@ private OptionsParserImplResult parse(
         parsedOption = result.parsedOptionDescription;
       }
       if (parsedOption.isPresent()) {
-        handleNewParsedOption(parsedOption.get());
+        handleNewParsedOption(parsedOption.get(), fallbackData);
       }
       priority = OptionPriority.nextOptionPriority(priority);
     }
@@ -645,7 +645,7 @@ void setOptionValueAtSpecificPriorityWithoutExpansion(
   }
 
   /** Takes care of tracking the parsed option's value in relation to other options. */
-  private void handleNewParsedOption(ParsedOptionDescription parsedOption)
+  private void handleNewParsedOption(ParsedOptionDescription parsedOption, OptionsData fallbackData)
       throws OptionsParsingException {
     OptionDefinition optionDefinition = parsedOption.getOptionDefinition();
     ExpansionBundle expansionBundle = setOptionValue(parsedOption);
@@ -659,7 +659,7 @@ private void handleNewParsedOption(ParsedOptionDescription parsedOption)
               optionDefinition.hasImplicitRequirements() ? parsedOption : null,
               optionDefinition.isExpansionOption() ? parsedOption : null,
               expansionBundle.expansionArgs,
-              /* fallbackData= */ null);
+              fallbackData);
       if (!optionsParserImplResult.getResidue().isEmpty()) {
 
         // Throw an assertion here, because this indicates an error in the definition of this

diff --git a/src/test/java/com/google/devtools/common/options/OptionsParserTest.java b/src/test/java/com/google/devtools/common/options/OptionsParserTest.java
@@ -2480,6 +2480,40 @@ public void fallbackOptions_optionsParsingDifferently() {
     assertThat(e).hasCauseThat().isInstanceOf(DuplicateOptionDeclarationException.class);
   }
 
+  public static class ExpandingOptions extends OptionsBase {
+    @Option(
+        name = "foo",
+        category = "one",
+        documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+        effectTags = {OptionEffectTag.NO_OP},
+        expansion = {"--nobar"},
+        defaultValue = "null")
+    public Void foo;
+  }
+
+  public static class ExpandingOptionsFallback extends OptionsBase {
+    @Option(
+        name = "bar",
+        category = "one",
+        documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+        effectTags = {OptionEffectTag.NO_OP},
+        defaultValue = "true")
+    public boolean bar;
+  }
+
+  @Test
+  public void fallbackOptions_expansionToNegativeBooleanFlag() throws OptionsParsingException {
+    OpaqueOptionsData fallbackData =
+        OptionsParser.getFallbackOptionsData(
+            ImmutableList.of(ExpandingOptions.class, ExpandingOptionsFallback.class));
+    OptionsParser parser = OptionsParser.builder().optionsClasses(ExpandingOptions.class).build();
+    parser.parseWithSourceFunction(
+        PriorityCategory.RC_FILE, o -> ".bazelrc", ImmutableList.of("--foo"), fallbackData);
+
+    assertThat(parser.getOptions(ExpandingOptions.class)).isNotNull();
+    assertThat(parser.getOptions(ExpandingOptionsFallback.class)).isNull();
+  }
+
   private static OptionInstanceOrigin createInvocationPolicyOrigin() {
     return createInvocationPolicyOrigin(/*implicitDependent=*/ null, /*expandedFrom=*/ null);
   }