Change LocationFunction to not extend Function.

This is necessary for subsequent changes to the apply method for diamond splitting, which will require apply() to take more than one argument. But it seems like the only reason LocationFunction ever extended Function was for tests and so this is an improvement on its own.

RELNOTES: None
PiperOrigin-RevId: 194796136

2 files changed, 54 insertions, 49 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/LocationExpanderTest.java b/src/test/java/com/google/devtools/build/lib/analysis/LocationExpanderTest.java
index f49cf5901a..c334cce1c7 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/LocationExpanderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/LocationExpanderTest.java
@@ -17,11 +17,11 @@ package com.google.devtools.build.lib.analysis;
 import static com.google.common.truth.Truth.assertThat;
 
 import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.analysis.LocationExpander.LocationFunction;
 import com.google.devtools.build.lib.packages.AbstractRuleErrorConsumer;
 import com.google.devtools.build.lib.packages.RuleErrorConsumer;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.function.Function;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -60,11 +60,22 @@ public class LocationExpanderTest {
   }
 
   private LocationExpander makeExpander(RuleErrorConsumer ruleErrorConsumer) throws Exception {
+
+    LocationFunction f1 = new LocationFunctionBuilder("//a", false)
+        .setExecPaths(false)
+        .add("//a", "/exec/src/a")
+        .build();
+
+    LocationFunction f2 = new LocationFunctionBuilder("//b", true)
+        .setExecPaths(false)
+        .add("//b", "/exec/src/b")
+        .build();
+
     return new LocationExpander(
         ruleErrorConsumer,
-        ImmutableMap.<String, Function<String, String>>of(
-            "location", (String s) -> "one(" + s + ")",
-            "locations", (String s) -> "more(" + s + ")"));
+        ImmutableMap.<String, LocationFunction>of(
+            "location", f1,
+            "locations", f2));
   }
 
   private String expand(String input) throws Exception {
@@ -78,14 +89,14 @@ public class LocationExpanderTest {
 
   @Test
   public void oneOrMore() throws Exception {
-    assertThat(expand("$(location a)")).isEqualTo("one(a)");
-    assertThat(expand("$(locations b)")).isEqualTo("more(b)");
-    assertThat(expand("---$(location a)---")).isEqualTo("---one(a)---");
+    assertThat(expand("$(location a)")).isEqualTo("src/a");
+    assertThat(expand("$(locations b)")).isEqualTo("src/b");
+    assertThat(expand("---$(location a)---")).isEqualTo("---src/a---");
   }
 
   @Test
   public void twoInOne() throws Exception {
-    assertThat(expand("$(location a) $(locations b)")).isEqualTo("one(a) more(b)");
+    assertThat(expand("$(location a) $(locations b)")).isEqualTo("src/a src/b");
   }
 
   @Test
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/LocationFunctionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/LocationFunctionTest.java
index 491d54e1d7..c6ce084d7a 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/LocationFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/LocationFunctionTest.java
@@ -30,7 +30,6 @@ import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.stream.Collectors;
-import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -38,23 +37,6 @@ import org.junit.runners.JUnit4;
 /** Unit tests for {@link LocationExpander.LocationFunction}. */
 @RunWith(JUnit4.class)
 public class LocationFunctionTest {
-  private FileSystem fs;
-
-  @Before
-  public void createFileSystem() throws Exception {
-    fs = new InMemoryFileSystem();
-  }
-
-  private Artifact makeArtifact(String path) {
-    if (path.startsWith("/exec/out")) {
-      return new Artifact(
-          fs.getPath(path),
-          ArtifactRoot.asDerivedRoot(fs.getPath("/exec"), fs.getPath("/exec/out")));
-    } else {
-      return new Artifact(
-          fs.getPath(path), ArtifactRoot.asSourceRoot(Root.fromPath(fs.getPath("/exec"))));
-    }
-  }
 
   @Test
   public void absoluteAndRelativeLabels() throws Exception {
@@ -159,34 +141,46 @@ public class LocationFunctionTest {
         .build();
     assertThat(func.apply("//foo")).isEqualTo("./bar out/foobar");
   }
+}
 
-  private final class LocationFunctionBuilder {
-    private final Label root;
-    private final boolean multiple;
-    private boolean execPaths;
-    private final Map<Label, Collection<Artifact>> labelMap = new HashMap<>();
+final class LocationFunctionBuilder {
+  private final Label root;
+  private final boolean multiple;
+  private boolean execPaths;
+  private final Map<Label, Collection<Artifact>> labelMap = new HashMap<>();
 
-    LocationFunctionBuilder(String rootLabel, boolean multiple) {
-      this.root = Label.parseAbsoluteUnchecked(rootLabel);
-      this.multiple = multiple;
-    }
+  LocationFunctionBuilder(String rootLabel, boolean multiple) {
+    this.root = Label.parseAbsoluteUnchecked(rootLabel);
+    this.multiple = multiple;
+  }
 
-    public LocationFunction build() {
-      return new LocationFunction(root, Suppliers.ofInstance(labelMap), execPaths, multiple);
-    }
+  public LocationFunction build() {
+    return new LocationFunction(root, Suppliers.ofInstance(labelMap), execPaths, multiple);
+  }
 
-    public LocationFunctionBuilder setExecPaths(boolean execPaths) {
-      this.execPaths = execPaths;
-      return this;
-    }
+  public LocationFunctionBuilder setExecPaths(boolean execPaths) {
+    this.execPaths = execPaths;
+    return this;
+  }
+
+  public LocationFunctionBuilder add(String label, String... paths) {
+    labelMap.put(
+        Label.parseAbsoluteUnchecked(label),
+        Arrays.stream(paths)
+            .map(LocationFunctionBuilder::makeArtifact)
+            .collect(Collectors.toList()));
+    return this;
+  }
 
-    public LocationFunctionBuilder add(String label, String... paths) {
-      labelMap.put(
-          Label.parseAbsoluteUnchecked(label),
-          Arrays.stream(paths)
-              .map(LocationFunctionTest.this::makeArtifact)
-              .collect(Collectors.toList()));
-      return this;
+  private static Artifact makeArtifact(String path) {
+    FileSystem fs = new InMemoryFileSystem();
+    if (path.startsWith("/exec/out")) {
+      return new Artifact(
+          fs.getPath(path),
+          ArtifactRoot.asDerivedRoot(fs.getPath("/exec"), fs.getPath("/exec/out")));
+    } else {
+      return new Artifact(
+          fs.getPath(path), ArtifactRoot.asSourceRoot(Root.fromPath(fs.getPath("/exec"))));
     }
   }
 }