Move BuildConfiguration option data access to a new class.

ConfigSetting is being moved to rules/config, and that means that it
no longer has its special package-private access to BuildConfiguration.
The solution: a new TransitiveOptionDetails class which cuts down on
clutter in BuildConfiguration and is publicly accessible.

However, we don't really want anyone accessing BuildConfiguration's
TransitiveOptionsDetails - only config-related rules should ever need
it. As a result, BuildConfigurationOptionDetails provides public access
to BuildConfiguration's TransitiveOptionDetails, but is limited via
Blaze visibility to only configuration rules.

RELNOTES: None.

PiperOrigin-RevId: 151828068

5 files changed, 224 insertions, 130 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
index 31ec385229..8b0f36d626 100644
--- a/src/main/java/com/google/devtools/build/lib/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -512,6 +512,17 @@ java_library(
 )
 
 java_library(
+    name = "build-configuration-option-details",
+    srcs = ["analysis/config/BuildConfigurationOptionDetails.java"],
+    visibility = [
+        "//src/main/java/com/google/devtools/build/lib/rules/config:__pkg__",
+    ],
+    deps = [
+        ":build-base",
+    ],
+)
+
+java_library(
     name = "build-base",
     srcs = glob(
         [
@@ -532,6 +543,7 @@ java_library(
         exclude = [
             "analysis/BuildInfo.java",
             "analysis/TransitiveInfoProvider.java",
+            "analysis/config/BuildConfigurationOptionDetails.java",
         ],
     ) + [
         "runtime/BlazeServerStartupOptions.java",
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
index 9561e2324d..5c479ee495 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -75,11 +75,8 @@ import com.google.devtools.common.options.Converter;
 import com.google.devtools.common.options.Converters;
 import com.google.devtools.common.options.EnumConverter;
 import com.google.devtools.common.options.Option;
-import com.google.devtools.common.options.OptionsBase;
 import com.google.devtools.common.options.OptionsParsingException;
 import com.google.devtools.common.options.TriState;
-import java.io.Serializable;
-import java.lang.reflect.Field;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -1159,42 +1156,8 @@ public final class BuildConfiguration {
 
   private final int hashCode; // We can precompute the hash code as all its inputs are immutable.
 
-  /**
-   * Helper container for {@link #transitiveOptionsMap} below.
-   */
-  private static class OptionDetails implements Serializable {
-    private OptionDetails(Class<? extends OptionsBase> optionsClass, Object value,
-        boolean allowsMultiple) {
-      this.optionsClass = optionsClass;
-      this.value = value;
-      this.allowsMultiple = allowsMultiple;
-    }
-
-    /** The {@link FragmentOptions} class that defines this option. */
-    private final Class<? extends OptionsBase> optionsClass;
-
-    /**
-     * The value of the given option (either explicitly defined or default). May be null.
-     */
-    private final Object value;
-
-    /** Whether or not this option supports multiple values. */
-    private final boolean allowsMultiple;
-  }
-
-  /**
-   * Maps option names to the {@link OptionDetails} the option takes for this configuration.
-   *
-   * <p>This can be used to:
-   * <ol>
-   *   <li>Find an option's (parsed) value given its command-line name</li>
-   *   <li>Parse alternative values for the option.</li>
-   * </ol>
-   *
-   * <p>This map is "transitive" in that it includes *all* options recognizable by this
-   * configuration, including those defined in child fragments.
-   */
-  private final Map<String, OptionDetails> transitiveOptionsMap;
+  /** Data for introspecting the options used by this configuration. */
+  private final TransitiveOptionDetails transitiveOptionDetails;
 
   /**
    * Returns true if this configuration is semantically equal to the other, with
@@ -1415,7 +1378,7 @@ public final class BuildConfiguration {
     this.localShellEnvironment = shellEnvironment.getFirst();
     this.envVariables = shellEnvironment.getSecond();
 
-    this.transitiveOptionsMap = computeOptionsMap(buildOptions, fragments.values());
+    this.transitiveOptionDetails = computeOptionsMap(buildOptions, fragments.values());
 
     ImmutableMap.Builder<String, String> globalMakeEnvBuilder = ImmutableMap.builder();
     for (Fragment fragment : fragments.values()) {
@@ -1494,11 +1457,17 @@ public final class BuildConfiguration {
   }
 
   /**
-   * Computes and returns the transitive optionName -> "option info" map for
-   * this configuration.
+   * Retrieves the {@link TransitiveOptionDetails} containing data on this configuration's options.
+   *
+   * @see BuildConfigurationOptionDetails
    */
-  private static Map<String, OptionDetails> computeOptionsMap(BuildOptions buildOptions,
-      Iterable<Fragment> fragments) {
+  TransitiveOptionDetails getTransitiveOptionDetails() {
+    return transitiveOptionDetails;
+  }
+
+  /** Computes and returns the {@link TransitiveOptionDetails} for this configuration. */
+  private static TransitiveOptionDetails computeOptionsMap(
+      BuildOptions buildOptions, Iterable<Fragment> fragments) {
     // Collect from our fragments "alternative defaults" for options where the default
     // should be something other than what's specified in Option.defaultValue.
     Map<String, Object> lateBoundDefaults = Maps.newHashMap();
@@ -1506,35 +1475,8 @@ public final class BuildConfiguration {
       lateBoundDefaults.putAll(fragment.lateBoundOptionDefaults());
     }
 
-    ImmutableMap.Builder<String, OptionDetails> map = ImmutableMap.builder();
-    try {
-      for (FragmentOptions options : buildOptions.getOptions()) {
-        for (Field field : options.getClass().getFields()) {
-          if (field.isAnnotationPresent(Option.class)) {
-            Option option = field.getAnnotation(Option.class);
-            if (option.category().equals("internal")) {
-              // ignore internal options
-              continue;
-            }
-            Object value = field.get(options);
-            if (value == null) {
-              if (lateBoundDefaults.containsKey(option.name())) {
-                value = lateBoundDefaults.get(option.name());
-              } else if (!option.defaultValue().equals("null")) {
-                // See {@link Option#defaultValue} for an explanation of default "null" strings.
-                value = option.defaultValue();
-              }
-            }
-            map.put(option.name(),
-                new OptionDetails(options.getClass(), value, option.allowMultiple()));
-          }
-        }
-      }
-    } catch (IllegalAccessException e) {
-      throw new IllegalStateException(
-          "Unexpected illegal access trying to create this configuration's options map: ", e);
-    }
-    return map.build();
+    return TransitiveOptionDetails.forOptionsWithDefaults(
+        buildOptions.getOptions(), lateBoundDefaults);
   }
 
   private String buildMnemonic() {
@@ -2042,44 +1984,6 @@ public final class BuildConfiguration {
   }
 
   /**
-   * Returns the {@link Option} class the defines the given option, null if the
-   * option isn't recognized.
-   *
-   * <p>optionName is the name of the option as it appears on the command line
-   * e.g. {@link Option#name}).
-   */
-  Class<? extends OptionsBase> getOptionClass(String optionName) {
-    OptionDetails optionData = transitiveOptionsMap.get(optionName);
-    return optionData == null ? null : optionData.optionsClass;
-  }
-
-  /**
-   * Returns the value of the specified option for this configuration or null if the
-   * option isn't recognized. Since an option's legitimate value could be null, use
-   * {@link #getOptionClass} to distinguish between that and an unknown option.
-   *
-   * <p>optionName is the name of the option as it appears on the command line
-   * e.g. {@link Option#name}).
-   */
-  Object getOptionValue(String optionName) {
-    OptionDetails optionData = transitiveOptionsMap.get(optionName);
-    return (optionData == null) ? null : optionData.value;
-  }
-
-  /**
-   * Returns whether or not the given option supports multiple values at the command line (e.g.
-   * "--myoption value1 --myOption value2 ..."). Returns false for unrecognized options. Use
-   * {@link #getOptionClass} to distinguish between those and legitimate single-value options.
-   *
-   * <p>As declared in {@link Option#allowMultiple}, multi-value options are expected to be
-   * of type {@code List<T>}.
-   */
-  boolean allowsMultipleValues(String optionName) {
-    OptionDetails optionData = transitiveOptionsMap.get(optionName);
-    return (optionData == null) ? false : optionData.allowsMultiple;
-  }
-
-  /**
    * The platform string, suitable for use as a key into a MakeEnvironment.
    */
   public String getPlatformName() {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationOptionDetails.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationOptionDetails.java
new file mode 100644
index 0000000000..2a5d4354c1
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationOptionDetails.java
@@ -0,0 +1,33 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.analysis.config;
+
+/**
+ * Retrieves {@link TransitiveOptionDetails} from {@link BuildConfiguration} instances.
+ *
+ * <p>This class's existence allows for the use of Blaze visibility to limit access to option data
+ * to only the configuration-specific rules which need to access or manipulate the configuration in
+ * such a meta way - in most cases, there should be no need to use this class. Instead, access
+ * desired configuration fragments via {@link BuildConfiguration#getFragment(Class)}.
+ */
+public class BuildConfigurationOptionDetails {
+
+  /** Utility class - no need to instantiate. */
+  private BuildConfigurationOptionDetails() {}
+
+  public static TransitiveOptionDetails get(BuildConfiguration configuration) {
+    return configuration.getTransitiveOptionDetails();
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigSetting.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigSetting.java
index ad0e54a2e8..5b097c6cef 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigSetting.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigSetting.java
@@ -31,7 +31,6 @@ import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.common.options.OptionsBase;
 import com.google.devtools.common.options.OptionsParser;
 import com.google.devtools.common.options.OptionsParsingException;
-
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -59,8 +58,11 @@ public class ConfigSetting implements RuleConfiguredTargetFactory {
 
     ConfigMatchingProvider configMatcher;
     try {
-      configMatcher = new ConfigMatchingProvider(ruleContext.getLabel(), settings,
-          matchesConfig(settings, ruleContext.getConfiguration()));
+      configMatcher =
+          new ConfigMatchingProvider(
+              ruleContext.getLabel(),
+              settings,
+              matchesConfig(settings, ruleContext.getConfiguration().getTransitiveOptionDetails()));
     } catch (OptionsParsingException e) {
       ruleContext.attributeError(ConfigRuleClasses.ConfigSettingRule.SETTINGS_ATTRIBUTE,
           "error while parsing configuration settings: " + e.getMessage());
@@ -77,10 +79,11 @@ public class ConfigSetting implements RuleConfiguredTargetFactory {
   }
 
   /**
-   * Given a list of [flagName, flagValue] pairs, returns true if flagName == flagValue for
-   * every item in the list under this configuration, false otherwise.
+   * Given a list of [flagName, flagValue] pairs, returns true if flagName == flagValue for every
+   * item in the list under this configuration, false otherwise.
    */
-  private boolean matchesConfig(Map<String, String> expectedSettings, BuildConfiguration config)
+  private boolean matchesConfig(
+      Map<String, String> expectedSettings, TransitiveOptionDetails options)
       throws OptionsParsingException {
     // Rather than returning fast when we find a mismatch, continue looking at the other flags
     // to check that they're indeed valid flag specifications.
@@ -93,7 +96,7 @@ public class ConfigSetting implements RuleConfiguredTargetFactory {
       String optionName = setting.getKey();
       String expectedRawValue = setting.getValue();
 
-      Class<? extends OptionsBase> optionClass = config.getOptionClass(optionName);
+      Class<? extends OptionsBase> optionClass = options.getOptionClass(optionName);
       if (optionClass == null) {
         throw new OptionsParsingException("unknown option: '" + optionName + "'");
       }
@@ -106,7 +109,7 @@ public class ConfigSetting implements RuleConfiguredTargetFactory {
       parser.parse("--" + optionName + "=" + expectedRawValue);
       Object expectedParsedValue = parser.getOptions(optionClass).asMap().get(optionName);
 
-      if (!optionMatches(config, optionName, expectedParsedValue)) {
+      if (!optionMatches(options, optionName, expectedParsedValue)) {
         foundMismatch = true;
       }
     }
@@ -116,23 +119,23 @@ public class ConfigSetting implements RuleConfiguredTargetFactory {
   /**
    * For single-value options, returns true iff the option's value matches the expected value.
    *
-   * <p>For multi-value List options, returns true iff any of the option's values matches
-   * the expected value. This means, e.g. "--tool_tag=foo --tool_tag=bar" would match the
-   * expected condition { 'tool_tag': 'bar' }.
+   * <p>For multi-value List options, returns true iff any of the option's values matches the
+   * expected value. This means, e.g. "--tool_tag=foo --tool_tag=bar" would match the expected
+   * condition { 'tool_tag': 'bar' }.
    *
    * <p>For multi-value Map options, returns true iff the last instance with the same key as the
-   * expected key has the same value. This means, e.g. "--define foo=1 --define bar=2" would
-   * match { 'define': 'foo=1' }, but "--define foo=1 --define bar=2 --define foo=3" would not
-   * match. Note that the definition of --define states that the last instance takes precedence.
+   * expected key has the same value. This means, e.g. "--define foo=1 --define bar=2" would match {
+   * 'define': 'foo=1' }, but "--define foo=1 --define bar=2 --define foo=3" would not match. Note
+   * that the definition of --define states that the last instance takes precedence.
    */
-  private static boolean optionMatches(BuildConfiguration config, String optionName,
-      Object expectedValue) {
-    Object actualValue = config.getOptionValue(optionName);
+  private static boolean optionMatches(
+      TransitiveOptionDetails options, String optionName, Object expectedValue) {
+    Object actualValue = options.getOptionValue(optionName);
     if (actualValue == null) {
       return expectedValue == null;
 
-    // Single-value case:
-    } else if (!config.allowsMultipleValues(optionName)) {
+      // Single-value case:
+    } else if (!options.allowsMultipleValues(optionName)) {
       return actualValue.equals(expectedValue);
     }
 
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/TransitiveOptionDetails.java b/src/main/java/com/google/devtools/build/lib/analysis/config/TransitiveOptionDetails.java
new file mode 100644
index 0000000000..8f1a9b66b1
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/TransitiveOptionDetails.java
@@ -0,0 +1,142 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.analysis.config;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.devtools.common.options.Option;
+import com.google.devtools.common.options.OptionsBase;
+import java.io.Serializable;
+import java.lang.reflect.Field;
+import java.util.Map;
+import javax.annotation.Nullable;
+
+/**
+ * Introspector for option details - what OptionsBase class the option is defined in, the option's
+ * current value, and whether the option allows multiple values to be specified.
+ *
+ * <p>This is "transitive" in that it includes *all* options recognizable by a given configuration,
+ * including those defined in child fragments.
+ */
+public final class TransitiveOptionDetails implements Serializable {
+
+  /**
+   * Computes and returns the transitive optionName -> "option info" map for the given set of
+   * options sets, using the given map as defaults for options which would otherwise be null.
+   */
+  public static TransitiveOptionDetails forOptionsWithDefaults(
+      Iterable<? extends OptionsBase> buildOptions, Map<String, Object> lateBoundDefaults) {
+    ImmutableMap.Builder<String, OptionDetails> map = ImmutableMap.builder();
+    try {
+      for (OptionsBase options : buildOptions) {
+        for (Field field : options.getClass().getFields()) {
+          if (field.isAnnotationPresent(Option.class)) {
+            Option option = field.getAnnotation(Option.class);
+            if (option.category().equals("internal")) {
+              // ignore internal options
+              continue;
+            }
+            Object value = field.get(options);
+            if (value == null) {
+              if (lateBoundDefaults.containsKey(option.name())) {
+                value = lateBoundDefaults.get(option.name());
+              } else if (!option.defaultValue().equals("null")) {
+                // See {@link Option#defaultValue} for an explanation of default "null" strings.
+                value = option.defaultValue();
+              }
+            }
+            map.put(option.name(),
+                new OptionDetails(options.getClass(), value, option.allowMultiple()));
+          }
+        }
+      }
+    } catch (IllegalAccessException e) {
+      throw new IllegalStateException(
+          "Unexpected illegal access trying to create this configuration's options map: ", e);
+    }
+    return new TransitiveOptionDetails(map.build());
+  }
+
+  private static final class OptionDetails implements Serializable {
+    private OptionDetails(Class<? extends OptionsBase> optionsClass, Object value,
+        boolean allowsMultiple) {
+      this.optionsClass = optionsClass;
+      this.value = value;
+      this.allowsMultiple = allowsMultiple;
+    }
+
+    /** The {@link FragmentOptions} class that defines this option. */
+    private final Class<? extends OptionsBase> optionsClass;
+
+    /** The value of the given option (either explicitly defined or default). May be null. */
+    @Nullable private final Object value;
+
+    /** Whether or not this option supports multiple values. */
+    private final boolean allowsMultiple;
+  }
+
+  /**
+   * Maps option names to the {@link OptionDetails} the option takes for this configuration.
+   *
+   * <p>This can be used to:
+   *
+   * <ol>
+   *   <li>Find an option's (parsed) value given its command-line name
+   *   <li>Parse alternative values for the option.
+   * </ol>
+   */
+  private final ImmutableMap<String, OptionDetails> transitiveOptionsMap;
+
+  private TransitiveOptionDetails(ImmutableMap<String, OptionDetails> transitiveOptionsMap) {
+    this.transitiveOptionsMap = transitiveOptionsMap;
+  }
+
+  /**
+   * Returns the {@link Option} class the defines the given option, null if the option isn't
+   * recognized.
+   *
+   * <p>optionName is the name of the option as it appears on the command line e.g. {@link
+   * Option#name}).
+   */
+  public Class<? extends OptionsBase> getOptionClass(String optionName) {
+    OptionDetails optionData = transitiveOptionsMap.get(optionName);
+    return optionData == null ? null : optionData.optionsClass;
+  }
+
+  /**
+   * Returns the value of the specified option for this configuration or null if the option isn't
+   * recognized. Since an option's legitimate value could be null, use {@link #getOptionClass} to
+   * distinguish between that and an unknown option.
+   *
+   * <p>optionName is the name of the option as it appears on the command line e.g. {@link
+   * Option#name}).
+   */
+  public Object getOptionValue(String optionName) {
+    OptionDetails optionData = transitiveOptionsMap.get(optionName);
+    return (optionData == null) ? null : optionData.value;
+  }
+
+  /**
+   * Returns whether or not the given option supports multiple values at the command line (e.g.
+   * "--myoption value1 --myOption value2 ..."). Returns false for unrecognized options. Use
+   * {@link #getOptionClass} to distinguish between those and legitimate single-value options.
+   *
+   * <p>As declared in {@link Option#allowMultiple}, multi-value options are expected to be
+   * of type {@code List<T>}.
+   */
+  boolean allowsMultipleValues(String optionName) {
+    OptionDetails optionData = transitiveOptionsMap.get(optionName);
+    return (optionData == null) ? false : optionData.allowsMultiple;
+  }
+}