Alias types use memoized hashCode for collections

palantir · Apr 23, 2024 · ecc081c · ecc081c
1 parent f84d8ca
commit ecc081c
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 56 deletions.
diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/AliasGenerator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/AliasGenerator.java
@@ -52,9 +52,9 @@
 import java.lang.reflect.Method;
 import java.math.BigDecimal;
 import java.util.Collections;
+import java.util.List;
 import java.util.Optional;
 import java.util.function.BiFunction;
-import javax.annotation.Nullable;
 import javax.lang.model.element.Modifier;
 
 public final class AliasGenerator {
@@ -74,11 +74,13 @@ public static JavaFile generateAliasType(
         Optional<LogSafety> computedSafety = safetyEvaluator.evaluate(typeDef.getAlias(), safety);
         ImmutableList<AnnotationSpec> computedSafetyAnnotations = ConjureAnnotations.safety(computedSafety);
 
+        List<FieldSpec> fields = List.of(FieldSpec.builder(aliasTypeName, "value", Modifier.PRIVATE, Modifier.FINAL)
+                .build());
         TypeSpec.Builder spec = TypeSpec.classBuilder(prefixedTypeName.getName())
                 .addAnnotations(computedSafetyAnnotations)
                 .addAnnotation(ConjureAnnotations.getConjureGeneratedAnnotation(AliasGenerator.class))
                 .addModifiers(Modifier.PUBLIC, Modifier.FINAL)
-                .addField(aliasTypeName, "value", Modifier.PRIVATE, Modifier.FINAL)
+                .addFields(fields)
                 .addMethod(createConstructor(aliasTypeName))
                 .addMethod(MethodSpec.methodBuilder("get")
                         .addModifiers(Modifier.PUBLIC)
@@ -92,23 +94,19 @@ public static JavaFile generateAliasType(
                         .addAnnotations(safetyAnnotations)
                         .returns(String.class)
                         .addCode(primitiveSafeToString(aliasTypeName))
-                        .build())
-                .addMethod(MethodSpec.methodBuilder("equals")
-                        .addModifiers(Modifier.PUBLIC)
-                        .addAnnotation(Override.class)
-                        .addParameter(ParameterSpec.builder(ClassName.OBJECT, "other")
-                                .addAnnotation(Nullable.class)
-                                .build())
-                        .returns(TypeName.BOOLEAN)
-                        .addCode(primitiveSafeEquality(thisClass, aliasTypeName, typeDef))
-                        .build())
-                .addMethod(MethodSpec.methodBuilder("hashCode")
-                        .addModifiers(Modifier.PUBLIC)
-                        .addAnnotation(Override.class)
-                        .returns(TypeName.INT)
-                        .addCode(primitiveSafeHashCode(aliasTypeName))
                         .build());
 
+        if (typeDef.getAlias().accept(BeanGenerator.FieldRequiresMemoizedHashCode.INSTANCE)) {
+            // only use memoized hashCode for complex alias types
+            spec.addMethod(MethodSpecs.createEquals(thisClass))
+                    .addMethod(MethodSpecs.createEqualTo(thisClass, fields, true));
+            MethodSpecs.addCachedHashCode(spec, fields);
+        } else {
+            spec.addMethod(MethodSpecs.createEquals(thisClass))
+                    .addMethod(MethodSpecs.createEqualTo(thisClass, fields))
+                    .addMethod(MethodSpecs.createHashCode(fields));
+        }
+
         typeDef.getAlias().accept(new ComparableVisitor(thisClass)).ifPresent(compareTo -> spec.addSuperinterface(
                         ParameterizedTypeName.get(ClassName.get(Comparable.class), thisClass))
                 .addMethod(compareTo));
@@ -426,45 +424,13 @@ private static MethodSpec createConstructor(TypeName aliasTypeName) {
         return builder.build();
     }
 
-    private static CodeBlock primitiveSafeEquality(
-            ClassName thisClass, TypeName aliasTypeName, AliasDefinition typeDef) {
-        if (isAliasOfDouble(typeDef)) {
-            return CodeBlocks.statement(
-                    "return this == other || "
-                            + "("
-                            + "other instanceof $1T "
-                            + "&& "
-                            + "$2T.doubleToLongBits(this.value) == "
-                            + "$2T.doubleToLongBits((($1T) other).value)"
-                            + ")",
-                    thisClass,
-                    Double.class);
-        }
-
-        if (Primitives.isPrimitive(aliasTypeName)) {
-            return CodeBlocks.statement(
-                    "return this == other || (other instanceof $1T && this.value == (($1T) other).value)", thisClass);
-        }
-
-        return CodeBlocks.statement(
-                "return this == other || (other instanceof $1T && this.value.equals((($1T) other).value))", thisClass);
-    }
-
     private static CodeBlock primitiveSafeToString(TypeName aliasTypeName) {
         if (Primitives.isPrimitive(aliasTypeName)) {
             return CodeBlocks.statement("return $T.valueOf(value)", String.class);
         }
         return CodeBlocks.statement("return value.toString()");
     }
 
-    private static CodeBlock primitiveSafeHashCode(TypeName aliasTypeName) {
-        if (Primitives.isPrimitive(aliasTypeName)) {
-            return CodeBlocks.statement(
-                    "return $T.hashCode(value)", Primitives.box(aliasTypeName).withoutAnnotations());
-        }
-        return CodeBlocks.statement("return value.hashCode()");
-    }
-
     private static final class ComparableVisitor implements Type.Visitor<Optional<MethodSpec>> {
         private final TypeName aliasName;
 

diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanGenerator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanGenerator.java
@@ -492,8 +492,8 @@ public static String asGetterName(String fieldName) {
      * Any types we generate directly will produce an efficient hashCode, but collections may be large, and we must
      * traverse through optionals.
      */
-    private static final class FieldRequiresMemoizedHashCode extends DefaultTypeVisitor<Boolean> {
-        private static final DefaultTypeVisitor<Boolean> INSTANCE = new FieldRequiresMemoizedHashCode();
+    static final class FieldRequiresMemoizedHashCode extends DefaultTypeVisitor<Boolean> {
+        static final DefaultTypeVisitor<Boolean> INSTANCE = new FieldRequiresMemoizedHashCode();
 
         @Override
         public Boolean visitOptional(OptionalType value) {

diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/MethodSpecs.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/MethodSpecs.java
@@ -170,7 +170,10 @@ private static CodeBlock computeHashCode(FieldSpec fieldSpec) {
                 return createHashInput(fieldSpec);
             } else {
                 return CodeBlock.of(
-                        "$1T.$2N($3L)", Primitives.box(fieldSpec.type), "hashCode", createHashInput(fieldSpec));
+                        "$1T.$2N($3L)",
+                        Primitives.box(fieldSpec.type).withoutAnnotations(),
+                        "hashCode",
+                        createHashInput(fieldSpec));
             }
         }
         return CodeBlock.of("$L.hashCode()", createHashInput(fieldSpec));

diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/util/Primitives.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/util/Primitives.java
@@ -40,7 +40,7 @@ private Primitives() {}
     public static TypeName box(TypeName type) {
         if (isPrimitive(type)) {
             List<AnnotationSpec> annotations = type.annotations;
-            return PRIMITIVES.get(getPrimitiveType(type).get()).annotated(annotations);
+            return PRIMITIVES.get(getPrimitiveType(type).orElseThrow()).annotated(annotations);
         } else {
             return type;
         }
@@ -61,8 +61,6 @@ public static TypeName unbox(TypeName type) {
 
     private static Optional<TypeName> getPrimitiveType(TypeName type) {
         TypeName rawType = type.withoutAnnotations();
-        return PRIMITIVES.keySet().stream()
-                .filter(typeName -> typeName.equals(rawType))
-                .findAny();
+        return PRIMITIVES.containsKey(rawType) ? Optional.of(rawType) : Optional.empty();
     }
 }