Make config_feature_flag's default_value optional.

With this change, if the default_value is not set, but the flag
has been given a value by a configuration transition, that value
is used and no error is produced.

If the default_value is not set and the flag has not been given
a value by a configuration transition, the rule produces an
error (as it did before, but now the error is different).

config_feature_flags with default_values behave as they did before.
(use the default value if no configured value is set, otherwise use
the configured value)

Also transition the Optional used in feature flags from Guava to
Java 8.

RELNOTES: config_feature_flag's default_value is optional. It is
only an error to have a config_feature_flag with no default_value
if that config_feature_flag has not been set in the configuration
it is being evaluated in.
PiperOrigin-RevId: 178418907

3 files changed, 54 insertions, 5 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/BUILD b/src/test/java/com/google/devtools/build/lib/rules/config/BUILD
index 4e115e79e0..feedac96ae 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/config/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/rules/config/BUILD
@@ -33,6 +33,7 @@ java_library(
         "//third_party:jsr305",
         "//third_party:junit4",
         "//third_party:truth",
+        "//third_party:truth8",
     ],
 )
 
diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagConfigurationTest.java b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagConfigurationTest.java
index 9f1e0c2d9f..09bf495e2b 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagConfigurationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagConfigurationTest.java
@@ -15,9 +15,9 @@
 package com.google.devtools.build.lib.rules.config;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
 import static org.junit.Assert.fail;
 
-import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 import com.google.common.testing.EqualsTester;
@@ -26,6 +26,7 @@ import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.common.options.OptionsParser;
 import com.google.devtools.common.options.OptionsParsingException;
 import java.util.Map;
+import java.util.Optional;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -152,11 +153,11 @@ public final class ConfigFeatureFlagConfigurationTest {
   }
 
   @Test
-  public void getFeatureFlagValue_returnsAbsentOptionalWhenRequestingUnsetFlag() throws Exception {
+  public void getFeatureFlagValue_returnsEmptyOptionalWhenRequestingUnsetFlag() throws Exception {
     Optional<String> flagValue =
         getConfigurationWithFlags(ImmutableMap.of(Label.parseAbsoluteUnchecked("//a:a"), "valued"))
             .getFeatureFlagValue(new LabelArtifactOwner(Label.parseAbsoluteUnchecked("//b:b")));
-    assertThat(flagValue).isAbsent();
+    assertThat(flagValue).isEmpty();
   }
 
   @Test
diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java
index 9629bbd54e..dcd80382f7 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java
@@ -76,6 +76,25 @@ public final class ConfigFeatureFlagTest extends SkylarkTestCase {
         "config_feature_flag(",
         "    name = 'flag',",
         "    allowed_values = ['default', 'configured', 'other'],",
+        ")");
+    assertThat(ConfigFeatureFlagProvider.fromTarget(getConfiguredTarget("//test:top")).getValue())
+        .isEqualTo("configured");
+  }
+
+  @Test
+  public void configFeatureFlagProvider_usesConfiguredValueOverDefault() throws Exception {
+    scratch.file(
+        "test/BUILD",
+        "feature_flag_setter(",
+        "    name = 'top',",
+        "    exports_flag = ':flag',",
+        "    flag_values = {",
+        "        ':flag': 'configured',",
+        "    },",
+        ")",
+        "config_feature_flag(",
+        "    name = 'flag',",
+        "    allowed_values = ['default', 'configured', 'other'],",
         "    default_value = 'default',",
         ")");
     assertThat(ConfigFeatureFlagProvider.fromTarget(getConfiguredTarget("//test:top")).getValue())
@@ -250,6 +269,34 @@ public final class ConfigFeatureFlagTest extends SkylarkTestCase {
   }
 
   @Test
+  public void configFeatureFlagProvider_throwsErrorIfNeitherDefaultNorConfiguredValueSet()
+      throws Exception {
+    reporter.removeHandler(failFastHandler); // expecting an error
+    scratch.file(
+        "test/BUILD",
+        "feature_flag_setter(",
+        "    name = 'top',",
+        "    exports_flag = ':flag',",
+        "    flag_values = {",
+        "        ':other': 'configured',",
+        "    },",
+        ")",
+        "config_feature_flag(",
+        "    name = 'flag',",
+        "    allowed_values = ['other', 'configured'],",
+        ")",
+        "config_feature_flag(",
+        "    name = 'other',",
+        "    allowed_values = ['default', 'configured', 'other'],",
+        "    default_value = 'default',",
+        ")");
+    assertThat(getConfiguredTarget("//test:flag")).isNull();
+    assertContainsEvent(
+        "in config_feature_flag rule //test:flag: "
+            + "flag has no default and must be set, but was not set");
+  }
+
+  @Test
   public void allowedValuesAttribute_cannotBeEmpty() throws Exception {
     reporter.removeHandler(failFastHandler); // expecting an error
     scratch.file(
@@ -282,7 +329,7 @@ public final class ConfigFeatureFlagTest extends SkylarkTestCase {
   }
 
   @Test
-  public void defaultValueAttribute_mustBeMemberOfAllowedValues() throws Exception {
+  public void defaultValueAttribute_mustBeMemberOfAllowedValuesIfPresent() throws Exception {
     reporter.removeHandler(failFastHandler); // expecting an error
     scratch.file(
         "test/BUILD",
@@ -305,7 +352,7 @@ public final class ConfigFeatureFlagTest extends SkylarkTestCase {
   }
 
   @Test
-  public void configurationValue_mustBeMemberOfAllowedValues() throws Exception {
+  public void configurationValue_mustBeMemberOfAllowedValuesIfPresent() throws Exception {
     reporter.removeHandler(failFastHandler); // expecting an error
     scratch.file(
         "test/BUILD",