@AutoCodec stray usage of AndroidDataConverter. It should always be a static constant.

This allows us to continue using lambdas in its definition.

This is a partial rollback of https://github.com/bazelbuild/bazel/commit/ed1e7594b23100f89755491f36e46886b4a51c8d, since the work done to class-ify things there is unnecessary once every instance is @AutoCodec-ed.

PiperOrigin-RevId: 200504678

3 files changed, 22 insertions, 54 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidDataConverter.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidDataConverter.java
index 0db9971af0..f0eaf20299 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidDataConverter.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidDataConverter.java
@@ -22,7 +22,6 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.errorprone.annotations.CompileTimeConstant;
-import java.util.Objects;
 import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.stream.Collectors;
@@ -31,6 +30,8 @@ import javax.annotation.Nullable;
 /**
  * Factory for functions to convert a {@code T} to a commandline argument. Uses a certain convention
  * for commandline arguments (e.g., separators, and ordering of container elements).
+ *
+ * <p>Should only need to be created statically, and in limited quantity.
  */
 public class AndroidDataConverter<T> extends ParametrizedMapFn<T> {
 
@@ -76,19 +77,17 @@ public class AndroidDataConverter<T> extends ParametrizedMapFn<T> {
     this.joinerType = joinerType;
   }
 
+  // We must override equals and hashCode as per the contract of ParametrizedMapFn, but we
+  // statically create a very small number of these objects, so we know that reference equality is
+  // enough.
   @Override
   public boolean equals(Object obj) {
-    if (!(obj instanceof AndroidDataConverter)) {
-      return false;
-    }
-
-    AndroidDataConverter<?> other = (AndroidDataConverter) obj;
-    return suppliers.equals(other.suppliers) && joinerType.equals(other.joinerType);
+    return this == obj;
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(suppliers, joinerType);
+    return System.identityHashCode(this);
   }
 
   @Override
@@ -139,54 +138,25 @@ public class AndroidDataConverter<T> extends ParametrizedMapFn<T> {
     }
 
     Builder<T> withRoots(Function<T, ImmutableList<PathFragment>> rootsFunction) {
-      // Anonymous inner class for serialization.
-      return with(
-          new Function<T, String>() {
-            @Override
-            public String apply(T t) {
-              // Copied from rootsToString to get rid of internal Lambda.
-              return rootsFunction
-                  .apply(t)
-                  .stream()
-                  .map(PathFragment::toString)
-                  .collect(Collectors.joining("#"));
-            }
-          });
+      return with(t -> rootsToString(rootsFunction.apply(t)));
     }
 
     Builder<T> withArtifact(Function<T, Artifact> artifactFunction) {
-      // Anonymous inner class for serialization.
-      return with(
-          new Function<T, String>() {
-            @Override
-            public String apply(T t) {
-              return artifactFunction.apply(t).getExecPathString();
-            }
-          });
+      return with(t -> artifactFunction.apply(t).getExecPathString());
     }
 
     Builder<T> maybeWithArtifact(Function<T, Artifact> nullableArtifactFunction) {
-      // Anonymous inner class for serialization.
       return with(
-          new Function<T, String>() {
-            @Override
-            public String apply(T t) {
-              @Nullable Artifact artifact = nullableArtifactFunction.apply(t);
-              return artifact == null ? "" : artifact.getExecPathString();
-            }
+          t -> {
+            @Nullable Artifact artifact = nullableArtifactFunction.apply(t);
+            return artifact == null ? "" : artifact.getExecPathString();
           });
     }
 
     Builder<T> withLabel(Function<T, Label> labelFunction) {
       // Escape labels, since they are known to contain separating characters (specifically, ':').
       // Anonymous inner class for serialization.
-      return with(
-          new Function<T, String>() {
-            @Override
-            public String apply(T t) {
-              return joinerType.escape(labelFunction.apply(t).toString());
-            }
-          });
+      return with(t -> joinerType.escape(labelFunction.apply(t).toString()));
     }
 
     Builder<T> with(Function<T, String> stringFunction) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceMergingActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceMergingActionBuilder.java
index 9d945e2104..23be43f152 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceMergingActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceMergingActionBuilder.java
@@ -18,6 +18,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.rules.android.AndroidDataConverter.JoinerType;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import javax.annotation.Nullable;
 
 /**
@@ -32,7 +33,8 @@ public class AndroidResourceMergingActionBuilder {
   private static final AndroidDataConverter<MergableAndroidData> RESOURCE_CONTAINER_TO_ARG =
       AndroidDataConverter.MERGABLE_DATA_CONVERTER;
 
-  private static final AndroidDataConverter<CompiledMergableAndroidData>
+  @AutoCodec @AutoCodec.VisibleForSerialization
+  static final AndroidDataConverter<CompiledMergableAndroidData>
       RESOURCE_CONTAINER_TO_ARG_FOR_COMPILED =
           AndroidDataConverter.<CompiledMergableAndroidData>builder(JoinerType.SEMICOLON_AMPERSAND)
               .withRoots(CompiledMergableAndroidData::getResourceRoots)
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java
index 7798f019b2..8cb592419d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java
@@ -97,16 +97,12 @@ public class RClassGeneratorActionBuilder {
 
   private static Function<ValidatedAndroidData, String> chooseDepsToArg(
       final AndroidAaptVersion version) {
-    // Use an anonymous inner class for serialization.
-    return new Function<ValidatedAndroidData, String>() {
-      @Override
-      public String apply(ValidatedAndroidData container) {
-        Artifact rTxt =
-            version == AndroidAaptVersion.AAPT2 ? container.getAapt2RTxt() : container.getRTxt();
-        return (rTxt != null ? rTxt.getExecPath() : "")
-            + ","
-            + (container.getManifest() != null ? container.getManifest().getExecPath() : "");
-      }
+    return container -> {
+      Artifact rTxt =
+          version == AndroidAaptVersion.AAPT2 ? container.getAapt2RTxt() : container.getRTxt();
+      return (rTxt != null ? rTxt.getExecPath() : "")
+          + ","
+          + (container.getManifest() != null ? container.getManifest().getExecPath() : "");
     };
   }
 }