Move expansion functionality to a new class

Progress on #2475.

PiperOrigin-RevId: 170473111

5 files changed, 225 insertions, 173 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/Expander.java b/src/main/java/com/google/devtools/build/lib/analysis/Expander.java
new file mode 100644
index 0000000000..69f45f9cc4
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/analysis/Expander.java
@@ -0,0 +1,215 @@
+// 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;
+
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.analysis.LocationExpander.Options;
+import com.google.devtools.build.lib.shell.ShellUtils;
+import com.google.devtools.build.lib.syntax.Type;
+import java.util.ArrayList;
+import java.util.List;
+import javax.annotation.Nullable;
+
+/**
+ * Expansion of strings and string lists by replacing make variables and $(location) functions.
+ */
+public final class Expander {
+  /** Indicates whether a string list attribute should be tokenized. */
+  private enum Tokenize {
+    YES,
+    NO
+  }
+
+  private final RuleContext ruleContext;
+  private final ConfigurationMakeVariableContext makeVariableContext;
+  @Nullable private final LocationExpander locationExpander;
+
+  private Expander(
+      RuleContext ruleContext,
+      ConfigurationMakeVariableContext makeVariableContext,
+      @Nullable LocationExpander locationExpander) {
+    this.ruleContext = ruleContext;
+    this.makeVariableContext = makeVariableContext;
+    this.locationExpander = locationExpander;
+  }
+
+  Expander(
+      RuleContext ruleContext,
+      ConfigurationMakeVariableContext makeVariableContext) {
+    this(ruleContext, makeVariableContext, null);
+  }
+
+  /**
+   * Returns a new instance that also expands locations using the default configuration of
+   * {@link LocationExpander}.
+   */
+  public Expander withLocations(Options... options) {
+    LocationExpander newLocationExpander =
+        new LocationExpander(ruleContext, options);
+    return new Expander(ruleContext, makeVariableContext, newLocationExpander);
+  }
+
+  /**
+   * Returns a new instance that also expands locations, passing {@link Options#ALLOW_DATA} to the
+   * underlying {@link LocationExpander}.
+   */
+  public Expander withDataLocations() {
+    return withLocations(Options.ALLOW_DATA);
+  }
+
+  /**
+   * Returns a new instance that also expands locations, passing {@link Options#ALLOW_DATA} and
+   * {@link Options#EXEC_PATHS} to the underlying {@link LocationExpander}.
+   */
+  public Expander withDataExecLocations() {
+    return withLocations(Options.ALLOW_DATA, Options.EXEC_PATHS);
+  }
+
+  /**
+   * Expands the given value string, tokenizes it, and then adds it to the given list. The attribute
+   * name is only used for error reporting.
+   */
+  public void tokenizeAndExpandMakeVars(
+      List<String> result,
+      String attributeName,
+      String value) {
+    expandValue(result, attributeName, value, Tokenize.YES);
+  }
+
+  /**
+   * Expands make variables and $(location) tags in value, and optionally tokenizes the result.
+   */
+  private void expandValue(
+      List<String> tokens,
+      String attributeName,
+      String value,
+      Tokenize tokenize) {
+    value = expand(attributeName, value);
+    if (tokenize == Tokenize.YES) {
+      try {
+        ShellUtils.tokenize(tokens, value);
+      } catch (ShellUtils.TokenizationException e) {
+        ruleContext.attributeError(attributeName, e.getMessage());
+      }
+    } else {
+      tokens.add(value);
+    }
+  }
+
+  /**
+   * 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
+   * @return the expansion of "expression".
+   */
+  public String expand(String attributeName) {
+    return expand(attributeName, ruleContext.attributes().get(attributeName, Type.STRING));
+  }
+
+  /**
+   * 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.
+   * @return the expansion of "expression".
+   */
+  public String expand(String attributeName, String expression) {
+    if (locationExpander != null) {
+      expression = locationExpander.expandAttribute(attributeName, expression);
+    }
+    try {
+      return MakeVariableExpander.expand(expression, makeVariableContext);
+    } catch (MakeVariableExpander.ExpansionException e) {
+      ruleContext.attributeError(attributeName, e.getMessage());
+      return expression;
+    }
+  }
+
+  /**
+   * Expands all the strings in the given list, optionally tokenizing them after expansion. The
+   * attribute name is only used for error reporting.
+   */
+  private ImmutableList<String> expandAndTokenizeList(
+      String attrName, List<String> values, Tokenize tokenize) {
+    List<String> variables = new ArrayList<>();
+    for (String variable : values) {
+      expandValue(variables, attrName, variable, tokenize);
+    }
+    return ImmutableList.copyOf(variables);
+  }
+
+  /**
+   * Obtains the value of the attribute, expands all values, and returns the resulting list. If the
+   * attribute does not exist or is not of type {@link Type#STRING_LIST}, then this method returns
+   * an empty list.
+   */
+  public ImmutableList<String> list(String attrName) {
+    if (!ruleContext.getRule().isAttrDefined(attrName, Type.STRING_LIST)) {
+      // TODO(bazel-team): This should be an error.
+      return ImmutableList.of();
+    }
+    return list(attrName, ruleContext.attributes().get(attrName, Type.STRING_LIST));
+  }
+
+  /**
+   * Expands all the strings in the given list. The attribute name is only used for error reporting.
+   */
+  public ImmutableList<String> list(String attrName, List<String> values) {
+    return expandAndTokenizeList(attrName, values, Tokenize.NO);
+  }
+
+  /**
+   * Obtains the value of the attribute, expands, and tokenizes all values. If the attribute does
+   * not exist or is not of type {@link Type#STRING_LIST}, then this method returns an empty list.
+   */
+  public ImmutableList<String> tokenized(String attrName) {
+    if (!ruleContext.getRule().isAttrDefined(attrName, Type.STRING_LIST)) {
+      // TODO(bazel-team): This should be an error.
+      return ImmutableList.of();
+    }
+    return tokenized(attrName, ruleContext.attributes().get(attrName, Type.STRING_LIST));
+  }
+
+  /**
+   * Expands all the strings in the given list, and tokenizes them after expansion. The attribute
+   * name is only used for error reporting.
+   */
+  public ImmutableList<String> tokenized(String attrName, List<String> values) {
+    return expandAndTokenizeList(attrName, values, Tokenize.YES);
+  }
+
+  /**
+   * If the string consists of a single variable, returns the expansion of that variable. Otherwise,
+   * returns null. Syntax errors are reported.
+   *
+   * @param attrName the name of the attribute from which "expression" comes; used for error
+   *     reporting.
+   * @param expression the string to expand.
+   * @return the expansion of "expression", or null.
+   */
+  @Nullable
+  public String expandSingleMakeVariable(String attrName, String expression) {
+    try {
+      return MakeVariableExpander.expandSingleVariable(expression, makeVariableContext);
+    } catch (MakeVariableExpander.ExpansionException e) {
+      ruleContext.attributeError(attrName, e.getMessage());
+      return expression;
+    }
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java b/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java
index 5d685160b7..9d3fa1d127 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java
@@ -131,7 +131,7 @@ public class LocationExpander {
    * Expands attribute's location and locations tags based on the target and
    * location map.
    *
-   * @param attrName  name of the attribute
+   * @param attrName  name of the attribute; only used for error reporting
    * @param attrValue initial value of the attribute
    * @return attribute value with expanded location tags or original value in
    *         case of errors
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 9a114f544f..e17f74a21a 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
@@ -38,7 +38,6 @@ import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ArtifactOwner;
 import com.google.devtools.build.lib.actions.Root;
 import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider.PrerequisiteValidator;
-import com.google.devtools.build.lib.analysis.LocationExpander.Options;
 import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext;
 import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory.BuildInfoKey;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
@@ -80,7 +79,6 @@ import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.
 import com.google.devtools.build.lib.packages.RuleErrorConsumer;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.packages.TargetUtils;
-import com.google.devtools.build.lib.shell.ShellUtils;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.syntax.Type.LabelClass;
@@ -972,96 +970,12 @@ public final class RuleContext extends TargetContext
     initConfigurationMakeVariableContext(ImmutableList.copyOf(makeVariableSuppliers));
   }
 
-  /** Indicates whether a string list attribute should be tokenized. */
-  public enum Tokenize {
-    YES,
-    NO
+  public Expander getExpander(ConfigurationMakeVariableContext makeVariableContext) {
+    return new Expander(this, makeVariableContext);
   }
 
-  /**
-   * Gets an attribute of type STRING_LIST expanding Make variables, $(location) tags into the
-   * dependency location (see {@link LocationExpander} for details) and tokenizes the result.
-   *
-   * @param attributeName the name of the attribute to process
-   * @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);
-  }
-
-  /**
-   * Gets an attribute of type STRING_LIST expanding Make variables and $(location) tags, and
-   * optionally tokenizes the result. Doesn't register any {@link MakeVariableSupplier}.
-   *
-   * @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) {
-    return getExpandedStringListAttr(attributeName, Tokenize.NO);
-  }
-
-  /**
-   * Gets an attribute of type STRING_LIST expanding Make variables and $(location) tags, and
-   * optionally tokenizes the result.
-   *
-   * @param attributeName the name of the attribute to process
-   * @return a list of strings containing the processed values for the attribute
-   */
-  private ImmutableList<String> getExpandedStringListAttr(
-      String attributeName,
-      Tokenize tokenize) {
-    if (!getRule().isAttrDefined(attributeName, Type.STRING_LIST)) {
-      // TODO(bazel-team): This should be an error.
-      return ImmutableList.of();
-    }
-    List<String> original = attributes().get(attributeName, Type.STRING_LIST);
-    if (original.isEmpty()) {
-      return ImmutableList.of();
-    }
-    List<String> tokens = new ArrayList<>();
-    LocationExpander locationExpander =
-        new LocationExpander(this, LocationExpander.Options.ALLOW_DATA);
-
-    for (String token : original) {
-      expandValue(tokens, attributeName, token, locationExpander, tokenize);
-    }
-    return ImmutableList.copyOf(tokens);
-  }
-
-  /**
-   * Expands make variables in value and tokenizes the result into tokens.
-   */
-  public void tokenizeAndExpandMakeVars(
-      List<String> result,
-      String attributeName,
-      String value) {
-    LocationExpander locationExpander =
-        new LocationExpander(this, Options.ALLOW_DATA, Options.EXEC_PATHS);
-    expandValue(result, attributeName, value, locationExpander, Tokenize.YES);
-  }
-
-  /**
-   * Expands make variables and $(location) tags in value, and optionally tokenizes the result.
-   */
-  private void expandValue(
-      List<String> tokens,
-      String attributeName,
-      String value,
-      @Nullable LocationExpander locationExpander,
-      Tokenize tokenize) {
-    if (locationExpander != null) {
-      value = locationExpander.expandAttribute(attributeName, value);
-    }
-    value = expandMakeVariables(attributeName, value);
-    if (tokenize == Tokenize.YES) {
-      try {
-        ShellUtils.tokenize(tokens, value);
-      } catch (ShellUtils.TokenizationException e) {
-        attributeError(attributeName, e.getMessage());
-      }
-    } else {
-      tokens.add(value);
-    }
+  public Expander getExpander() {
+    return new Expander(this, getConfigurationMakeVariableContext());
   }
 
   public ImmutableMap<String, String> getMakeVariables(Iterable<String> attributeNames) {
@@ -1095,83 +1009,6 @@ public final class RuleContext extends TargetContext
     return configurationMakeVariableContext;
   }
 
-  /**
-   * Expands the make variables in {@code expression}.
-   *
-   * @param attributeName the name of the attribute from which "expression" comes; used for error
-   *     reporting.
-   * @return the expanded string.
-   */
-  public String expandedMakeVariables(String attributeName) {
-    String expression = attributes().get(attributeName, Type.STRING);
-    return expandMakeVariables(attributeName, expression);
-  }
-
-  /**
-   * Expands the make variables in {@code expression}.
-   *
-   * @param attributeName the name of the attribute from which "expression" comes; used for error
-   *     reporting.
-   * @param expression the string to expand.
-   * @return the expanded string.
-   */
-  public String expandMakeVariables(String attributeName, String expression) {
-    return expandMakeVariables(attributeName, expression, getConfigurationMakeVariableContext());
-  }
-
-  /**
-   * 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 context the ConfigurationMakeVariableContext which can have a customized
-   *     lookupMakeVariable(String) method.
-   * @return the expansion of "expression".
-   */
-  public String expandMakeVariables(
-      String attributeName, String expression, ConfigurationMakeVariableContext context) {
-    try {
-      return MakeVariableExpander.expand(expression, context);
-    } catch (MakeVariableExpander.ExpansionException e) {
-      attributeError(attributeName, e.getMessage());
-      return expression;
-    }
-  }
-
-  /**
-   * Gets the value of the STRING_LIST attribute expanding all make variables.
-   */
-  public List<String> expandedMakeVariablesList(String attrName) {
-    List<String> variables = new ArrayList<>();
-    for (String variable : attributes().get(attrName, Type.STRING_LIST)) {
-      variables.add(expandMakeVariables(attrName, variable));
-    }
-    return variables;
-  }
-
-  /**
-   * If the string consists of a single variable, returns the expansion of that variable. Otherwise,
-   * returns null. Syntax errors are reported.
-   *
-   * @param attrName the name of the attribute from which "expression" comes; used for error
-   *     reporting.
-   * @param expression the string to expand.
-   * @return the expansion of "expression", or null.
-   */
-  @Nullable
-  public String expandSingleMakeVariable(String attrName, String expression) {
-    try {
-      return MakeVariableExpander
-          .expandSingleVariable(expression, getConfigurationMakeVariableContext());
-    } catch (MakeVariableExpander.ExpansionException e) {
-      attributeError(attrName, e.getMessage());
-      return expression;
-    }
-  }
-
   @Nullable
   public ToolchainContext getToolchainContext() {
     return toolchainContext;
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 7f4c12d14a..230b1fd61e 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
@@ -428,6 +428,7 @@ public final class RunfilesSupport {
       RuleContext ruleContext,
       CommandLine additionalArgs) {
     return CommandLine.concat(
-        ruleContext.getTokenizedStringListAttr("args"), additionalArgs);
+        ruleContext.getExpander().withDataLocations().tokenized("args"),
+        additionalArgs);
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java
index deb4595015..fc82668063 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java
@@ -1019,9 +1019,7 @@ public final class SkylarkRuleContext implements SkylarkValue {
   public String expandMakeVariables(String attributeName, String command,
       final Map<String, String> additionalSubstitutions) throws EvalException {
     checkMutable("expand_make_variables");
-    return ruleContext.expandMakeVariables(
-        attributeName,
-        command,
+    ConfigurationMakeVariableContext makeVariableContext =
         new ConfigurationMakeVariableContext(
             // TODO(lberki): This should be removed. But only after either verifying that no one
             // uses it or providing an alternative.
@@ -1036,7 +1034,8 @@ public final class SkylarkRuleContext implements SkylarkValue {
               return super.lookupMakeVariable(variableName);
             }
           }
-        });
+        };
+    return ruleContext.getExpander(makeVariableContext).expand(attributeName, command);
   }