Simplify RuleContext API

Use ConfigurationMakeVariableContext instead of passing in lists of
MakeVariableSuppliers.

Progress on #2475.

PiperOrigin-RevId: 170182945

5 files changed, 56 insertions, 81 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index 41876d6b99..64be9bbb4a 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -92,7 +92,6 @@ import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
@@ -975,7 +974,8 @@ public final class RuleContext extends TargetContext
    * @return a list of strings containing the expanded and tokenized values for the attribute
    */
   public ImmutableList<String> getTokenizedStringListAttr(String attributeName) {
-    return getExpandedStringListAttr(attributeName, Tokenize.YES, Collections.EMPTY_LIST);
+    return getExpandedStringListAttr(
+        attributeName, Tokenize.YES, getConfigurationMakeVariableContext());
   }
 
   /**
@@ -983,12 +983,13 @@ public final class RuleContext extends TargetContext
    * dependency location (see {@link LocationExpander} for details) and tokenizes the result.
    *
    * @param attributeName the name of the attribute to process
-   * @param makeVariableSuppliers to be used with {@link ConfigurationMakeVariableContext}
+   * @param makeVariableContext the make variable context
    * @return a list of strings containing the expanded and tokenized values for the attribute
    */
   public ImmutableList<String> getTokenizedStringListAttr(
-      String attributeName, Iterable<? extends MakeVariableSupplier> makeVariableSuppliers) {
-    return getExpandedStringListAttr(attributeName, Tokenize.YES, makeVariableSuppliers);
+      String attributeName, ConfigurationMakeVariableContext makeVariableContext) {
+    return getExpandedStringListAttr(
+        attributeName, Tokenize.YES, makeVariableContext);
   }
 
   /**
@@ -998,9 +999,9 @@ public final class RuleContext extends TargetContext
    * @param attributeName the name of the attribute to process
    * @return a list of strings containing the processed values for the attribute
    */
-  public ImmutableList<String> getExpandedStringListAttr(String attributeName, Tokenize tokenize) {
+  public ImmutableList<String> getExpandedStringListAttr(String attributeName) {
     return getExpandedStringListAttr(
-        attributeName, tokenize, ImmutableList.<MakeVariableSupplier>of());
+        attributeName, Tokenize.NO, getConfigurationMakeVariableContext());
   }
 
   /**
@@ -1008,13 +1009,13 @@ public final class RuleContext extends TargetContext
    * optionally tokenizes the result.
    *
    * @param attributeName the name of the attribute to process
-   * @param makeVariableSuppliers to be used with {@link ConfigurationMakeVariableContext}
+   * @param makeVariableContext the make variable context
    * @return a list of strings containing the processed values for the attribute
    */
-  public ImmutableList<String> getExpandedStringListAttr(
+  private ImmutableList<String> getExpandedStringListAttr(
       String attributeName,
       Tokenize tokenize,
-      Iterable<? extends MakeVariableSupplier> makeVariableSuppliers) {
+      ConfigurationMakeVariableContext makeVariableContext) {
     if (!getRule().isAttrDefined(attributeName, Type.STRING_LIST)) {
       // TODO(bazel-team): This should be an error.
       return ImmutableList.of();
@@ -1028,7 +1029,7 @@ public final class RuleContext extends TargetContext
         new LocationExpander(this, LocationExpander.Options.ALLOW_DATA);
 
     for (String token : original) {
-      expandValue(tokens, attributeName, token, locationExpander, tokenize, makeVariableSuppliers);
+      expandValue(tokens, attributeName, token, locationExpander, tokenize, makeVariableContext);
     }
     return ImmutableList.copyOf(tokens);
   }
@@ -1036,53 +1037,35 @@ public final class RuleContext extends TargetContext
   /**
    * Expands make variables in value and tokenizes the result into tokens.
    *
-   * @param makeVariableSuppliers to be used with {@link ConfigurationMakeVariableContext}
+   * @param makeVariableContext the make variable context
    *     <p>This methods should be called only during initialization.
    */
   public void tokenizeAndExpandMakeVars(
-      List<String> tokens,
+      List<String> result,
       String attributeName,
       String value,
-      Iterable<? extends MakeVariableSupplier> makeVariableSuppliers) {
+      ConfigurationMakeVariableContext makeVariableContext) {
     LocationExpander locationExpander =
         new LocationExpander(this, Options.ALLOW_DATA, Options.EXEC_PATHS);
-    tokenizeAndExpandMakeVars(
-        tokens, attributeName, value, locationExpander, makeVariableSuppliers);
-  }
-
-  /**
-   * Expands make variables and $(location) tags in value and tokenizes the result into tokens.
-   *
-   * @param makeVariableSuppliers to be used with {@link ConfigurationMakeVariableContext}
-   *     <p>This methods should be called only during initialization.
-   */
-  public void tokenizeAndExpandMakeVars(
-      List<String> tokens,
-      String attributeName,
-      String value,
-      @Nullable LocationExpander locationExpander,
-      Iterable<? extends MakeVariableSupplier> makeVariableSuppliers) {
     expandValue(
-        tokens, attributeName, value, locationExpander, Tokenize.YES, makeVariableSuppliers);
+        result, attributeName, value, locationExpander, Tokenize.YES,
+        makeVariableContext);
   }
 
   /**
    * Expands make variables and $(location) tags in value, and optionally tokenizes the result.
-   *
-   * @param makeVariableSuppliers to be used with {@link ConfigurationMakeVariableContext}
-   *     <p>This methods should be called only during initialization.
    */
-  public void expandValue(
+  private void expandValue(
       List<String> tokens,
       String attributeName,
       String value,
       @Nullable LocationExpander locationExpander,
       Tokenize tokenize,
-      Iterable<? extends MakeVariableSupplier> makeVariableSuppliers) {
+      ConfigurationMakeVariableContext makeVariableContext) {
     if (locationExpander != null) {
       value = locationExpander.expandAttribute(attributeName, value);
     }
-    value = expandMakeVariables(attributeName, value, makeVariableSuppliers);
+    value = expandMakeVariables(attributeName, value, makeVariableContext);
     if (tokenize == Tokenize.YES) {
       try {
         ShellUtils.tokenize(tokens, value);
@@ -1155,25 +1138,7 @@ public final class RuleContext extends TargetContext
    * @return the expanded string.
    */
   public String expandMakeVariables(String attributeName, String expression) {
-    return expandMakeVariables(attributeName, expression, ImmutableList.<MakeVariableSupplier>of());
-  }
-
-  /**
-   * Returns the string "expression" after expanding all embedded references to "Make" variables. If
-   * any errors are encountered, they are reported, and "expression" is returned unchanged.
-   *
-   * @param attributeName the name of the attribute from which "expression" comes; used for error
-   *     reporting.
-   * @param expression the string to expand.
-   * @param makeVariableSuppliers to be used with {@link ConfigurationMakeVariableContext}
-   * @return the expansion of "expression".
-   */
-  public String expandMakeVariables(
-      String attributeName,
-      String expression,
-      Iterable<? extends MakeVariableSupplier> makeVariableSuppliers) {
-    return expandMakeVariables(
-        attributeName, expression, getConfigurationMakeVariableContext(makeVariableSuppliers));
+    return expandMakeVariables(attributeName, expression, getConfigurationMakeVariableContext());
   }
 
   /**
@@ -1188,8 +1153,8 @@ public final class RuleContext extends TargetContext
    *     lookupMakeVariable(String) method.
    * @return the expansion of "expression".
    */
-  public String expandMakeVariables(String attributeName, String expression,
-      ConfigurationMakeVariableContext context) {
+  public String expandMakeVariables(
+      String attributeName, String expression, ConfigurationMakeVariableContext context) {
     try {
       return MakeVariableExpander.expand(expression, context);
     } catch (MakeVariableExpander.ExpansionException e) {
@@ -1205,7 +1170,7 @@ public final class RuleContext extends TargetContext
     List<String> variables = new ArrayList<>();
     for (String variable : attributes().get(attrName, Type.STRING_LIST)) {
       variables.add(
-          expandMakeVariables(attrName, variable, ImmutableList.<MakeVariableSupplier>of()));
+          expandMakeVariables(attrName, variable, getConfigurationMakeVariableContext()));
     }
     return variables;
   }
@@ -1217,16 +1182,16 @@ public final class RuleContext extends TargetContext
    * @param attrName the name of the attribute from which "expression" comes; used for error
    *     reporting.
    * @param expression the string to expand.
-   * @param makeVariableSuppliers to be used with {@link ConfigurationMakeVariableContext}
+   * @param makeVariableContext the make variable context
    * @return the expansion of "expression", or null.
    */
+  @Nullable
   public String expandSingleMakeVariable(
       String attrName,
       String expression,
-      ImmutableList<? extends MakeVariableSupplier> makeVariableSuppliers) {
+      ConfigurationMakeVariableContext makeVariableContext) {
     try {
-      return MakeVariableExpander.expandSingleVariable(
-          expression, getConfigurationMakeVariableContext(makeVariableSuppliers));
+      return MakeVariableExpander.expandSingleVariable(expression, makeVariableContext);
     } catch (MakeVariableExpander.ExpansionException e) {
       attributeError(attrName, e.getMessage());
       return expression;
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java
index f567fcda95..d8cc52f7c2 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java
@@ -395,7 +395,10 @@ public final class RunfilesSupport {
         ruleContext,
         executable,
         runfiles,
-        computeArgs(ruleContext, CommandLine.EMPTY, ImmutableList.<MakeVariableSupplier>of()));
+        computeArgs(
+            ruleContext,
+            CommandLine.EMPTY,
+            ruleContext.getConfigurationMakeVariableContext()));
   }
 
   /**
@@ -409,7 +412,9 @@ public final class RunfilesSupport {
         executable,
         runfiles,
         computeArgs(
-            ruleContext, CommandLine.of(appendingArgs), ImmutableList.<MakeVariableSupplier>of()));
+            ruleContext,
+            CommandLine.of(appendingArgs),
+            ruleContext.getConfigurationMakeVariableContext()));
   }
 
   /**
@@ -422,26 +427,26 @@ public final class RunfilesSupport {
         ruleContext,
         executable,
         runfiles,
-        computeArgs(ruleContext, appendingArgs, ImmutableList.<MakeVariableSupplier>of()));
+        computeArgs(ruleContext, appendingArgs, ruleContext.getConfigurationMakeVariableContext()));
   }
 
   public static RunfilesSupport withExecutable(
       RuleContext ruleContext,
       Runfiles runfiles,
       Artifact executable,
-      ImmutableList<? extends MakeVariableSupplier> makeVariableSuppliers) {
+      ConfigurationMakeVariableContext makeVariableContext) {
     return new RunfilesSupport(
         ruleContext,
         executable,
         runfiles,
-        computeArgs(ruleContext, CommandLine.EMPTY, makeVariableSuppliers));
+        computeArgs(ruleContext, CommandLine.EMPTY, makeVariableContext));
   }
 
   private static CommandLine computeArgs(
       RuleContext ruleContext,
       CommandLine additionalArgs,
-      ImmutableList<? extends MakeVariableSupplier> makeVariableSuppliers) {
+      ConfigurationMakeVariableContext makeVariableContext) {
     return CommandLine.concat(
-        ruleContext.getTokenizedStringListAttr("args", makeVariableSuppliers), additionalArgs);
+        ruleContext.getTokenizedStringListAttr("args", makeVariableContext), additionalArgs);
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
index aff9c39e18..21714bbf33 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
@@ -25,6 +25,7 @@ import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.MiddlemanFactory;
 import com.google.devtools.build.lib.analysis.AnalysisUtils;
+import com.google.devtools.build.lib.analysis.ConfigurationMakeVariableContext;
 import com.google.devtools.build.lib.analysis.FileProvider;
 import com.google.devtools.build.lib.analysis.MakeVariableSupplier;
 import com.google.devtools.build.lib.analysis.PlatformConfiguration;
@@ -158,20 +159,22 @@ public class CppHelper {
     List<String> tokens = new ArrayList<>();
     ImmutableList<? extends MakeVariableSupplier> makeVariableSuppliers =
         ImmutableList.of(new CcFlagsSupplier(ruleContext));
+    ConfigurationMakeVariableContext makeVariableContext =
+        ruleContext.getConfigurationMakeVariableContext(makeVariableSuppliers);
     for (String token : input) {
       try {
         // Legacy behavior: tokenize all items.
         if (tokenization) {
           ruleContext.tokenizeAndExpandMakeVars(
-              tokens, attributeName, token, makeVariableSuppliers);
+              tokens, attributeName, token, makeVariableContext);
         } else {
           String exp =
-              ruleContext.expandSingleMakeVariable(attributeName, token, makeVariableSuppliers);
+              ruleContext.expandSingleMakeVariable(attributeName, token, makeVariableContext);
           if (exp != null) {
             ShellUtils.tokenize(tokens, exp);
           } else {
             tokens.add(
-                ruleContext.expandMakeVariables(attributeName, token, makeVariableSuppliers));
+                ruleContext.expandMakeVariables(attributeName, token, makeVariableContext));
           }
         }
       } catch (ShellUtils.TokenizationException e) {
@@ -197,17 +200,18 @@ public class CppHelper {
    * variable or flag) or tokenizes and expands make variables.
    */
   public static void expandAttribute(RuleContext ruleContext,
-      List<String> values, String attrName, String attrValue, boolean attemptLabelExpansion) {
+      List<String> result, String attrName, String attrValue, boolean attemptLabelExpansion) {
     if (attemptLabelExpansion && CppHelper.isLinkoptLabel(attrValue)) {
-      if (!CppHelper.expandLabel(ruleContext, values, attrValue)) {
+      if (!CppHelper.expandLabel(ruleContext, result, attrValue)) {
         ruleContext.attributeError(attrName, "could not resolve label '" + attrValue + "'");
       }
     } else {
       ruleContext.tokenizeAndExpandMakeVars(
-          values,
+          result,
           attrName,
           attrValue,
-          ImmutableList.of(new CcFlagsSupplier(ruleContext)));
+          ruleContext.getConfigurationMakeVariableContext(
+              ImmutableList.of(new CcFlagsSupplier(ruleContext))));
     }
   }
 
@@ -232,8 +236,8 @@ public class CppHelper {
    * @param labelName the name of the label to expand
    * @return true if the label was expanded successfully, false otherwise
    */
-  private static boolean expandLabel(RuleContext ruleContext, List<String> linkopts,
-      String labelName) {
+  private static boolean expandLabel(
+      RuleContext ruleContext, List<String> linkopts, String labelName) {
     try {
       Label label = ruleContext.getLabel().getRelative(labelName);
       for (String prereqKind : LINKOPTS_PREREQUISITE_LABEL_KINDS) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java
index 73e4f91b3c..029c0f507c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java
@@ -547,7 +547,7 @@ public class JavaCommon {
   public static List<String> getJvmFlags(RuleContext ruleContext) {
     List<String> jvmFlags = new ArrayList<>();
     jvmFlags.addAll(ruleContext.getFragment(JavaConfiguration.class).getDefaultJvmFlags());
-    jvmFlags.addAll(ruleContext.getExpandedStringListAttr("jvm_flags", RuleContext.Tokenize.NO));
+    jvmFlags.addAll(ruleContext.getExpandedStringListAttr("jvm_flags"));
     return jvmFlags;
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyBinary.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyBinary.java
index b0dd535c95..e1e779a13b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PyBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyBinary.java
@@ -92,7 +92,8 @@ public abstract class PyBinary implements RuleConfiguredTargetFactory {
             ruleContext,
             defaultRunfiles,
             common.getExecutable(),
-            ImmutableList.of(new CcFlagsSupplier(ruleContext)));
+            ruleContext.getConfigurationMakeVariableContext(
+                ImmutableList.of(new CcFlagsSupplier(ruleContext))));
 
     if (ruleContext.hasErrors()) {
       return null;