From 4343fc6ccab7bcdd9e8cd9035903da656b5c18df Mon Sep 17 00:00:00 2001 From: janakr Date: Wed, 13 Jun 2018 22:02:17 -0700 Subject: @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 --- .../lib/rules/android/AndroidDataConverter.java | 56 +++++----------------- .../AndroidResourceMergingActionBuilder.java | 4 +- .../android/RClassGeneratorActionBuilder.java | 16 +++---- 3 files changed, 22 insertions(+), 54 deletions(-) (limited to 'src/main/java/com/google/devtools/build/lib/rules') 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). + * + *

Should only need to be created statically, and in limited quantity. */ public class AndroidDataConverter extends ParametrizedMapFn { @@ -76,19 +77,17 @@ public class AndroidDataConverter extends ParametrizedMapFn { 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 extends ParametrizedMapFn { } Builder withRoots(Function> rootsFunction) { - // Anonymous inner class for serialization. - return with( - new Function() { - @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 withArtifact(Function artifactFunction) { - // Anonymous inner class for serialization. - return with( - new Function() { - @Override - public String apply(T t) { - return artifactFunction.apply(t).getExecPathString(); - } - }); + return with(t -> artifactFunction.apply(t).getExecPathString()); } Builder maybeWithArtifact(Function nullableArtifactFunction) { - // Anonymous inner class for serialization. return with( - new Function() { - @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 withLabel(Function labelFunction) { // Escape labels, since they are known to contain separating characters (specifically, ':'). // Anonymous inner class for serialization. - return with( - new Function() { - @Override - public String apply(T t) { - return joinerType.escape(labelFunction.apply(t).toString()); - } - }); + return with(t -> joinerType.escape(labelFunction.apply(t).toString())); } Builder with(Function 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 RESOURCE_CONTAINER_TO_ARG = AndroidDataConverter.MERGABLE_DATA_CONVERTER; - private static final AndroidDataConverter + @AutoCodec @AutoCodec.VisibleForSerialization + static final AndroidDataConverter RESOURCE_CONTAINER_TO_ARG_FOR_COMPILED = AndroidDataConverter.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 chooseDepsToArg( final AndroidAaptVersion version) { - // Use an anonymous inner class for serialization. - return new Function() { - @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() : ""); }; } } -- cgit v1.2.3