From b30314409f16d7491d27d7bad7c050e8b075c9db Mon Sep 17 00:00:00 2001
From: Fabian Meumertzheim <fabian@meumertzhe.im>
Date: Wed, 10 Jan 2024 09:25:11 -0800
Subject: [PATCH] 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 https://github.com/bazelbuild/bazel/pull/18130#issuecomment-1874687869

Closes #20720.

PiperOrigin-RevId: 597271727
Change-Id: Ieaba4dc00e13495a859e1eedf802759ad7dbf774
---
 .../common/options/OptionsParserImpl.java     |  6 ++--
 .../common/options/OptionsParserTest.java     | 34 +++++++++++++++++++
 2 files changed, 37 insertions(+), 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
index 5201fccd08c747..3874069227ad61 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
@@ -544,7 +544,7 @@ private OptionsParserImplResult parse(
         parsedOption = result.parsedOptionDescription;
       }
       if (parsedOption.isPresent()) {
-        handleNewParsedOption(parsedOption.get());
+        handleNewParsedOption(parsedOption.get(), fallbackData);
       }
       priority = OptionPriority.nextOptionPriority(priority);
     }
@@ -644,7 +644,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);
@@ -658,7 +658,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
index 7cb68fc35bb59d..9d031aafe8317c 100644
--- a/src/test/java/com/google/devtools/common/options/OptionsParserTest.java
+++ b/src/test/java/com/google/devtools/common/options/OptionsParserTest.java
@@ -2589,6 +2589,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);
   }