Don't emit duplicate toString, equals, and hashCode methods in …

…records These methods are only emitted if they don't already exist, per JLS 8.10.3. bazelbuild/bazel#17181 PiperOrigin-RevId: 501618070
treblereel · Sep 28, 2023 · e6d6f3c · e6d6f3c
1 parent 8b280f3
commit e6d6f3c
Show file tree

Hide file tree

Showing 3 changed files with 120 additions and 51 deletions.
diff --git a/java/com/google/turbine/binder/TypeBinder.java b/java/com/google/turbine/binder/TypeBinder.java
@@ -17,6 +17,7 @@
 package com.google.turbine.binder;
 
 import static com.google.common.collect.Iterables.getLast;
+import static com.google.common.collect.Iterables.getOnlyElement;
 import static java.util.Objects.requireNonNull;
 
 import com.google.common.base.Joiner;
@@ -248,12 +249,13 @@ private SourceTypeBoundClass bind() {
 
  ImmutableList<RecordComponentInfo> components = bindComponents(scope, base.decl().components());
 
+ ImmutableList<MethodInfo> boundSyntheticMethods =
+ syntheticMethods(syntheticMethods, components);
+ List<MethodInfo> boundMethods = bindMethods(scope, base.decl().members(), components);
  ImmutableList.Builder<MethodInfo> methods =
- ImmutableList.<MethodInfo>builder()
- .addAll(syntheticMethods(syntheticMethods, components))
- .addAll(bindMethods(scope, base.decl().members(), components));
+ ImmutableList.<MethodInfo>builder().addAll(boundSyntheticMethods).addAll(boundMethods);
  if (base.kind().equals(TurbineTyKind.RECORD)) {
- methods.addAll(syntheticRecordMethods(syntheticMethods, components));
+ methods.addAll(syntheticRecordMethods(syntheticMethods, components, boundMethods));
  }
 
  ImmutableList<FieldInfo> fields = bindFields(scope, base.decl().members());
@@ -467,52 +469,79 @@ private ImmutableList<MethodInfo> syntheticEnumMethods(SyntheticMethods syntheti
  }
 
  private ImmutableList<MethodInfo> syntheticRecordMethods(
- SyntheticMethods syntheticMethods, ImmutableList<RecordComponentInfo> components) {
+ SyntheticMethods syntheticMethods,
+ ImmutableList<RecordComponentInfo> components,
+ List<MethodInfo> boundMethods) {
+ boolean hasToString = false;
+ boolean hasEquals = false;
+ boolean hasHashCode = false;
+ for (MethodInfo m : boundMethods) {
+ switch (m.name()) {
+ case "toString":
+ hasToString = m.parameters().isEmpty();
+ break;
+ case "equals":
+ hasEquals =
+ m.parameters().size() == 1
+ && getOnlyElement(m.parameters()).type().equals(Type.ClassTy.OBJECT);
+ break;
+ case "hashCode":
+ hasHashCode = m.parameters().isEmpty();
+ break;
+ default: // fall out
+ }
+ }
  ImmutableList.Builder<MethodInfo> methods = ImmutableList.builder();
- MethodSymbol toStringMethod = syntheticMethods.create(owner, "toString");
- methods.add(
- new MethodInfo(
- toStringMethod,
- ImmutableMap.of(),
- Type.ClassTy.STRING,
- ImmutableList.of(),
- ImmutableList.of(),
- TurbineFlag.ACC_PUBLIC | TurbineFlag.ACC_FINAL,
- null,
- null,
- ImmutableList.of(),
- null));
- MethodSymbol hashCodeMethod = syntheticMethods.create(owner, "hashCode");
- methods.add(
- new MethodInfo(
- hashCodeMethod,
- ImmutableMap.of(),
- Type.PrimTy.create(TurbineConstantTypeKind.INT, ImmutableList.of()),
- ImmutableList.of(),
- ImmutableList.of(),
- TurbineFlag.ACC_PUBLIC | TurbineFlag.ACC_FINAL,
- null,
- null,
- ImmutableList.of(),
- null));
- MethodSymbol equalsMethod = syntheticMethods.create(owner, "equals");
- methods.add(
- new MethodInfo(
- equalsMethod,
- ImmutableMap.of(),
- Type.PrimTy.create(TurbineConstantTypeKind.BOOLEAN, ImmutableList.of()),
- ImmutableList.of(
- new ParamInfo(
- new ParamSymbol(equalsMethod, "other"),
- Type.ClassTy.OBJECT,
- ImmutableList.of(),
- TurbineFlag.ACC_MANDATED)),
- ImmutableList.of(),
- TurbineFlag.ACC_PUBLIC | TurbineFlag.ACC_FINAL,
- null,
- null,
- ImmutableList.of(),
- null));
+ if (!hasToString) {
+ MethodSymbol toStringMethod = syntheticMethods.create(owner, "toString");
+ methods.add(
+ new MethodInfo(
+ toStringMethod,
+ ImmutableMap.of(),
+ Type.ClassTy.STRING,
+ ImmutableList.of(),
+ ImmutableList.of(),
+ TurbineFlag.ACC_PUBLIC | TurbineFlag.ACC_FINAL,
+ null,
+ null,
+ ImmutableList.of(),
+ null));
+ }
+ if (!hasHashCode) {
+ MethodSymbol hashCodeMethod = syntheticMethods.create(owner, "hashCode");
+ methods.add(
+ new MethodInfo(
+ hashCodeMethod,
+ ImmutableMap.of(),
+ Type.PrimTy.create(TurbineConstantTypeKind.INT, ImmutableList.of()),
+ ImmutableList.of(),
+ ImmutableList.of(),
+ TurbineFlag.ACC_PUBLIC | TurbineFlag.ACC_FINAL,
+ null,
+ null,
+ ImmutableList.of(),
+ null));
+ }
+ if (!hasEquals) {
+ MethodSymbol equalsMethod = syntheticMethods.create(owner, "equals");
+ methods.add(
+ new MethodInfo(
+ equalsMethod,
+ ImmutableMap.of(),
+ Type.PrimTy.create(TurbineConstantTypeKind.BOOLEAN, ImmutableList.of()),
+ ImmutableList.of(
+ new ParamInfo(
+ new ParamSymbol(equalsMethod, "other"),
+ Type.ClassTy.OBJECT,
+ ImmutableList.of(),
+ TurbineFlag.ACC_MANDATED)),
+ ImmutableList.of(),
+ TurbineFlag.ACC_PUBLIC | TurbineFlag.ACC_FINAL,
+ null,
+ null,
+ ImmutableList.of(),
+ null));
+ }
  for (RecordComponentInfo c : components) {
  MethodSymbol componentMethod = syntheticMethods.create(owner, c.name());
  methods.add(

diff --git a/javatests/com/google/turbine/lower/LowerIntegrationTest.java b/javatests/com/google/turbine/lower/LowerIntegrationTest.java
@@ -23,6 +23,7 @@
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
 import java.io.IOError;
 import java.io.IOException;
@@ -47,6 +48,7 @@ public class LowerIntegrationTest {
  ImmutableMap.of(
  "record.test", 16, //
  "record2.test", 16,
+ "record_tostring.test", 16,
  "sealed.test", 17,
  "sealed_nested.test", 17,
  "textblock.test", 15);
@@ -269,9 +271,11 @@ public static Iterable<Object[]> parameters() {
  "receiver_param.test",
  "record.test",
  "record2.test",
+ "record_tostring.test",
  "rek.test",
  "samepkg.test",
  "sealed.test",
+ "sealed_nested.test",
  "self.test",
  "semi.test",
  // https://bugs.openjdk.java.net/browse/JDK-8054064 ?
@@ -333,8 +337,9 @@ public static Iterable<Object[]> parameters() {
  "wildcanon.test",
  // keep-sorted end
  };
- List<Object[]> tests =
- ImmutableList.copyOf(testCases).stream().map(x -> new Object[] {x}).collect(toList());
+ ImmutableSet<String> cases = ImmutableSet.copyOf(testCases);
+ assertThat(cases).containsAtLeastElementsIn(SOURCE_VERSION.keySet());
+ List<Object[]> tests = cases.stream().map(x -> new Object[] {x}).collect(toList());
  String testShardIndex = System.getenv("TEST_SHARD_INDEX");
  String testTotalShards = System.getenv("TEST_TOTAL_SHARDS");
  if (testShardIndex == null || testTotalShards == null) {

diff --git a/javatests/com/google/turbine/lower/testdata/record_tostring.test b/javatests/com/google/turbine/lower/testdata/record_tostring.test
@@ -0,0 +1,35 @@
+=== Records.java ===
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Target;
+import java.util.Objects;
+
+class Records {
+ public record A() {
+ @Override
+ public String toString() {
+ return "A";
+ }
+ }
+
+ public record B() {
+ @Override
+ public final String toString() {
+ return "B";
+ }
+ }
+
+ public record C() {
+ @Override
+ public final boolean equals(Object o) {
+ return false;
+ }
+ }
+
+ public record D() {
+ @Override
+ public final int hashCode() {
+ return -1;
+ }
+ }
+}