commit 432cfe330a363576eec24fc842b49741314f5536
author gregce <gregce@google.com> Wed Nov 21 14:57:05 2018 -0800
committer Copybara-Service <copybara-piper@google.com> Wed Nov 21 14:58:22 2018 -0800
tree 3608335372bef5a298e43373cd09006500f46b31
parent b83c5f800699a1bbcac11baeab1155d85880d9e4

Make multi-value BuildOptions fields immutable.

Addresses #6723

PiperOrigin-RevId: 222464467

src/main/java/com/google/devtools/common/options/OptionValueDescription.java

diff --git a/src/main/java/com/google/devtools/common/options/OptionValueDescription.java b/src/main/java/com/google/devtools/common/options/OptionValueDescription.java
index a54893b..541712e 100644
--- a/src/main/java/com/google/devtools/common/options/OptionValueDescription.java
+++ b/src/main/java/com/google/devtools/common/options/OptionValueDescription.java
@@ -290,17 +290,14 @@
     }
 
     @Override
-    public List<Object> getValue() {
+    public ImmutableList<Object> getValue() {
       // Sort the results by option priority and return them in a new list. The generic type of
       // the list is not known at runtime, so we can't use it here.
-      return optionValues
-          .asMap()
-          .entrySet()
-          .stream()
+      return optionValues.asMap().entrySet().stream()
           .sorted(Comparator.comparing(Map.Entry::getKey))
           .map(Map.Entry::getValue)
           .flatMap(Collection::stream)
-          .collect(Collectors.toList());
+          .collect(ImmutableList.toImmutableList());
     }
 
     @Override

src/test/java/com/google/devtools/build/lib/analysis/config/BuildOptionsTest.java

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/config/BuildOptionsTest.java b/src/test/java/com/google/devtools/build/lib/analysis/config/BuildOptionsTest.java
index e897c11..c5bcd36 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/config/BuildOptionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/config/BuildOptionsTest.java
@@ -19,6 +19,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
+import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Options;
 import com.google.devtools.build.lib.analysis.config.BuildOptions.OptionsDiff;
 import com.google.devtools.build.lib.analysis.config.BuildOptions.OptionsDiffForReconstruction;
 import com.google.devtools.build.lib.rules.android.AndroidConfiguration;
@@ -28,6 +29,7 @@
 import com.google.devtools.build.lib.rules.python.PythonOptions;
 import com.google.devtools.build.lib.skyframe.serialization.testutils.TestUtils;
 import com.google.devtools.common.options.OptionsParser;
+import java.util.AbstractMap;
 import java.util.stream.Collectors;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -327,4 +329,15 @@
         .isEqualTo(
             TestUtils.toBytes(diff, ImmutableMap.of(BuildOptions.OptionsDiffCache.class, cache)));
   }
+
+  @Test
+  public void testMultiValueOptionImmutability() throws Exception {
+    BuildOptions options =
+        BuildOptions.of(TEST_OPTIONS, OptionsParser.newOptionsParser(TEST_OPTIONS));
+    BuildConfiguration.Options coreOptions = options.get(Options.class);
+    assertThrows(
+        UnsupportedOperationException.class,
+        () ->
+            coreOptions.commandLineBuildVariables.add(new AbstractMap.SimpleEntry<>("foo", "bar")));
+  }
 }