From e55b1ba4585ca2d8ee18b4008685dadccbd739ca Mon Sep 17 00:00:00 2001
From: Fabian Meumertzheim <fabian@meumertzhe.im>
Date: Thu, 20 Jul 2023 12:18:46 +0200
Subject: [PATCH] TMP: Test that all Starlark options are loaded

---
 .../devtools/build/lib/runtime/BlazeOptionHandler.java    | 8 +++++---
 .../devtools/build/lib/runtime/StarlarkOptionsParser.java | 3 ---
 .../build/lib/runtime/commands/CanonicalizeCommand.java   | 3 ++-
 .../build/lib/starlark/util/StarlarkOptionsTestCase.java  | 8 +++++---
 4 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java
index bc547303ab4fb5..4756e023ff7387 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java
@@ -17,6 +17,7 @@
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
+import com.google.common.base.Preconditions;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
@@ -418,9 +419,10 @@ DetailedExitCode parseStarlarkOptions(CommandEnvironment env) {
       return DetailedExitCode.success();
     }
     try {
-      StarlarkOptionsParser.newStarlarkOptionsParser(
-              new SkyframeExecutorTargetLoader(env), optionsParser)
-          .parse();
+      Preconditions.checkState(
+          StarlarkOptionsParser.newStarlarkOptionsParser(
+                  new SkyframeExecutorTargetLoader(env), optionsParser)
+              .parse());
     } catch (OptionsParsingException e) {
       String logMessage = "Error parsing Starlark options";
       logger.atInfo().withCause(e).log("%s", logMessage);
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/StarlarkOptionsParser.java b/src/main/java/com/google/devtools/build/lib/runtime/StarlarkOptionsParser.java
index a8ba82092e2f4d..c86b4a3728a1c9 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/StarlarkOptionsParser.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/StarlarkOptionsParser.java
@@ -34,7 +34,6 @@
 import com.google.devtools.common.options.Converter;
 import com.google.devtools.common.options.OptionsParser;
 import com.google.devtools.common.options.OptionsParsingException;
-import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
@@ -103,7 +102,6 @@ public static StarlarkOptionsParser newStarlarkOptionsParser(
   // OptionsParserImpl.identifyOptionAndPossibleArgument. Consider combining. This would probably
   // require multiple rounds of parsing to fit starlark-defined options into native option format.
   @VisibleForTesting
-  @CanIgnoreReturnValue
   public boolean parse() throws OptionsParsingException {
     return parseGivenArgs(nativeOptionsParser.getSkippedArgs());
   }
@@ -115,7 +113,6 @@ public boolean parse() throws OptionsParsingException {
    *     work to retrieve build setting targets (after which it'll call this method again)
    */
   @VisibleForTesting
-  @CanIgnoreReturnValue
   public boolean parseGivenArgs(List<String> args) throws OptionsParsingException {
     // Map of <option name (label), <unparsed option value, loaded option>>.
     Multimap<String, Pair<String, Target>> unparsedOptions = LinkedListMultimap.create();
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 92aaa1b81c6626..ea67bf09d13d7c 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
@@ -16,6 +16,7 @@
 import static com.google.devtools.common.options.Converters.BLAZE_ALIASING_FLAG;
 
 import com.google.common.base.Joiner;
+import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.events.Event;
@@ -200,7 +201,7 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult opti
         StarlarkOptionsParser.newStarlarkOptionsParser(
             new SkyframeExecutorTargetLoader(env), parser);
     try {
-      starlarkOptionsParser.parse();
+      Preconditions.checkState(starlarkOptionsParser.parse());
     } catch (OptionsParsingException e) {
       return reportAndCreateCommandFailure(
           env, e.getMessage(), FailureDetails.Command.Code.STARLARK_OPTIONS_PARSE_FAILURE);
diff --git a/src/test/java/com/google/devtools/build/lib/starlark/util/StarlarkOptionsTestCase.java b/src/test/java/com/google/devtools/build/lib/starlark/util/StarlarkOptionsTestCase.java
index b8f30dbed8816a..8f031407f6031a 100644
--- a/src/test/java/com/google/devtools/build/lib/starlark/util/StarlarkOptionsTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/starlark/util/StarlarkOptionsTestCase.java
@@ -14,6 +14,8 @@
 
 package com.google.devtools.build.lib.starlark.util;
 
+import static com.google.common.truth.Truth.assertThat;
+
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
@@ -76,7 +78,7 @@ protected OptionsParsingResult parseStarlarkOptions(String options, boolean only
     if (!onlyStarlarkParser) {
       optionsParser.parse(asList);
     }
-    starlarkOptionsParser.parseGivenArgs(asList);
+    assertThat(starlarkOptionsParser.parseGivenArgs(asList)).isTrue();
     return starlarkOptionsParser.getNativeOptionsParserFortesting();
   }
 
@@ -86,11 +88,11 @@ protected OptionsParsingResult parseStarlarkOptions(
     List<String> bazelrcOptionsList = Arrays.asList(bazelrcOptions.split(" "));
     optionsParser.parse(PriorityCategory.COMMAND_LINE, /* source= */ null, commandLineOptionsList);
     optionsParser.parse(PriorityCategory.RC_FILE, "fake.bazelrc", bazelrcOptionsList);
-    starlarkOptionsParser.parseGivenArgs(
+    assertThat(starlarkOptionsParser.parseGivenArgs(
         ImmutableList.<String>builder()
             .addAll(commandLineOptionsList)
             .addAll(bazelrcOptionsList)
-            .build());
+            .build())).isTrue();
     return starlarkOptionsParser.getNativeOptionsParserFortesting();
   }