Add primitive array setter optimization [Primitive Arrays Pt.4]

conjure-java-core/src/main/java/com/palantir/conjure/java/Options.java

@@ -169,6 +169,15 @@ default boolean externalFallbackTypes() {
         return false;
     }
 
+    /**
+     * When set, enables codegen for array setters for primitive optimized collections.
+     * This feature is experimental and subject to change.
+     */
+    @Value.Default
+    default boolean primitiveCollectionArraySetters() {
+        return false;
+    }
+
     Optional<String> packagePrefix();
 
     Optional<String> apiVersion();

conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderGenerator.java

@@ -28,6 +28,7 @@
 import com.google.errorprone.annotations.CheckReturnValue;
 import com.palantir.conjure.java.ConjureAnnotations;
 import com.palantir.conjure.java.Options;
+import com.palantir.conjure.java.lib.SafeLong;
 import com.palantir.conjure.java.lib.internal.ConjureCollections;
 import com.palantir.conjure.java.types.BeanGenerator.EnrichedField;
 import com.palantir.conjure.java.util.JavaNameSanitizer;
@@ -57,6 +58,7 @@
 import com.palantir.logsafe.exceptions.SafeIllegalArgumentException;
 import com.palantir.logsafe.exceptions.SafeIllegalStateException;
 import com.squareup.javapoet.AnnotationSpec;
+import com.squareup.javapoet.ArrayTypeName;
 import com.squareup.javapoet.ClassName;
 import com.squareup.javapoet.CodeBlock;
 import com.squareup.javapoet.FieldSpec;
@@ -671,6 +673,36 @@ private MethodSpec createCollectionSetter(String prefix, EnrichedField enriched,
                 .build();
     }
 
+    private MethodSpec createPrimitiveCollectionSetter(EnrichedField enriched) {
+        FieldSpec field = enriched.poetSpec();
+        Type type = enriched.conjureDef().getType();
+
+        Type innerType = type.accept(TypeVisitor.LIST).getItemType();
+        TypeName boxedTypeName = typeMapper.getClassName(innerType);
+        TypeName innerTypeName;
+        // SafeLong is just a special case of long
+        if (boxedTypeName.equals(ClassName.get(SafeLong.class))) {
+            innerTypeName = ConjureAnnotations.withSafety(
+                    TypeName.LONG, safetyEvaluator.getUsageTimeSafety(enriched.conjureDef()));
+        } else {
+            innerTypeName = ConjureAnnotations.withSafety(
+                    Primitives.unbox(boxedTypeName), safetyEvaluator.getUsageTimeSafety(enriched.conjureDef()));
+        }
+        CollectionType collectionType = getCollectionType(type);
+        MethodSpec.Builder setterBuilder = BeanBuilderAuxiliarySettersUtils.publicSetter(enriched, builderClass)
+                .addParameter(Parameters.nonnullParameter(ArrayTypeName.of(innerTypeName), field.name))
+                .addCode(verifyNotBuilt())
+                .addCode(CodeBlocks.statement(
+                        "this.$1N = $2T.newNonNull$3L($4L)",
+                        enriched.poetSpec().name,
+                        ConjureCollections.class,
+                        collectionType.getConjureCollectionType().getCollectionName(),
+                        Expressions.requireNonNull(
+                                enriched.poetSpec().name, enriched.fieldName().get() + " cannot be null")));
+
+        return setterBuilder.addStatement("return this").build();
+    }
+
     @SuppressWarnings("checkstyle:CyclomaticComplexity")
     private CodeBlock typeAwareAssignment(EnrichedField enriched, Type type, boolean shouldClearFirst) {
         FieldSpec spec = enriched.poetSpec();
@@ -766,9 +798,19 @@ private List<MethodSpec> createAuxiliarySetters(EnrichedField enriched, boolean
         Optional<LogSafety> safety = safetyEvaluator.getUsageTimeSafety(enriched.conjureDef());
 
         if (type.accept(TypeVisitor.IS_LIST)) {
-            return ImmutableList.of(
-                    createCollectionSetter("addAll", enriched, override),
-                    createItemSetter(enriched, type.accept(TypeVisitor.LIST).getItemType(), override, safety));
+            CollectionType collectionType = getCollectionType(type);
+            if (collectionType.getConjureCollectionType().isPrimitiveCollection()
+                    && collectionType.useNonNullFactory()
+                    && options.primitiveCollectionArraySetters()) {
+                return ImmutableList.of(
+                        createPrimitiveCollectionSetter(enriched),
+                        createCollectionSetter("addAll", enriched, override),
+                        createItemSetter(enriched, type.accept(TypeVisitor.LIST).getItemType(), override, safety));
+            } else {
+                return ImmutableList.of(
+                        createCollectionSetter("addAll", enriched, override),
+                        createItemSetter(enriched, type.accept(TypeVisitor.LIST).getItemType(), override, safety));
+            }
         }
 
         if (type.accept(TypeVisitor.IS_SET)) {
@@ -820,7 +862,9 @@ private CodeBlock optionalAssignmentStatement(EnrichedField enriched, OptionalTy
     private static final EnumSet<PrimitiveType.Value> OPTIONAL_PRIMITIVES =
             EnumSet.of(PrimitiveType.Value.INTEGER, PrimitiveType.Value.DOUBLE, PrimitiveType.Value.BOOLEAN);
 
-    /** Check if the optionalType contains a primitive boolean, double or integer. */
+    /**
+     * Check if the optionalType contains a primitive boolean, double or integer.
+     */
     private boolean isPrimitiveOptional(OptionalType optionalType) {
         return optionalType.getItemType().accept(TypeVisitor.IS_PRIMITIVE)
                 && OPTIONAL_PRIMITIVES.contains(
@@ -1039,22 +1083,28 @@ public boolean useNonNullFactory() {
     }
 
     private enum ConjureCollectionType {
-        LIST("List"),
-        DOUBLE_LIST("DoubleList"),
-        INTEGER_LIST("IntegerList"),
-        BOOLEAN_LIST("BooleanList"),
-        SAFE_LONG_LIST("SafeLongList"),
-        SET("Set");
+        LIST("List", false),
+        DOUBLE_LIST("DoubleList", true),
+        INTEGER_LIST("IntegerList", true),
+        BOOLEAN_LIST("BooleanList", true),
+        SAFE_LONG_LIST("SafeLongList", true),
+        SET("Set", false);
 
         private final String collectionName;
+        private final Boolean primitiveCollection;
 
-        ConjureCollectionType(String collectionName) {
+        ConjureCollectionType(String collectionName, Boolean primitiveCollection) {
             this.collectionName = collectionName;
+            this.primitiveCollection = primitiveCollection;
         }
 
         public String getCollectionName() {
             return collectionName;
         }
+
+        public Boolean isPrimitiveCollection() {
+            return primitiveCollection;
+        }
     }
 
     private enum ConjureCollectionNullHandlingMode {

conjure-java-core/src/test/java/com/palantir/conjure/java/types/ObjectGeneratorTests.java

@@ -66,6 +66,7 @@ public void testObjectGenerator_allExamples() throws IOException {
                                 .excludeEmptyOptionals(true)
                                 .unionsWithUnknownValues(true)
                                 .jetbrainsContractAnnotations(true)
+                                .primitiveCollectionArraySetters(true)
                                 .build())))
                 .emit(def, tempDir);
 

conjure-java/src/main/java/com/palantir/conjure/java/cli/ConjureJavaCli.java

@@ -229,6 +229,12 @@ public static final class GenerateCommand implements Runnable {
                 description = "Java external type imports are generated using their fallback type.")
         private boolean externalFallbackTypes;
 
+        @CommandLine.Option(
+                names = "--primitiveCollectionArraySetters",
+                defaultValue = "false",
+                description = "Ecodegen for array setters for primitive optimized collections.")
+        private boolean primitiveCollectionArraySetters;
+
         @SuppressWarnings("unused")
         @CommandLine.Unmatched
         private List<String> unmatchedOptions;
@@ -297,6 +303,7 @@ CliConfiguration getConfiguration() {
                             .excludeEmptyCollections(excludeEmptyCollections)
                             .unionsWithUnknownValues(unionsWithUnknownValues)
                             .externalFallbackTypes(externalFallbackTypes)
+                            .primitiveCollectionArraySetters(primitiveCollectionArraySetters)
                             .build())
                     .build();
         }

conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureCollections.java

@@ -18,6 +18,7 @@
 
 import com.palantir.conjure.java.lib.SafeLong;
 import com.palantir.logsafe.Preconditions;
+import com.palantir.logsafe.exceptions.SafeIllegalArgumentException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.LinkedHashSet;
@@ -140,11 +141,22 @@ public static <T> Set<T> newNonNullSet(Iterable<? extends T> iterable) {
         return set;
     }
 
+    /**
+     * The following Conjure boxed list wrappers for the eclipse-collections [type]ArrayList are temporary (except
+     * ConjureSafeLongList). In eclipse-collections 12, a BoxedMutable[type]List will be released. Once available,
+     * Conjure[type]List should be replaced with that.
+     */
+
     // This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set
     public static List<Double> newNonNullDoubleList() {
         return new ConjureDoubleList(new DoubleArrayList());
     }
 
+    // This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set
+    public static List<Double> newNonNullDoubleList(double[] doubles) {
+        return new ConjureDoubleList(DoubleArrayList.newListWith(doubles.clone()));
+    }
+
     // This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set
     public static List<Double> newNonNullDoubleList(Iterable<Double> iterable) {
         List<Double> doubleList;
@@ -158,17 +170,16 @@ public static List<Double> newNonNullDoubleList(Iterable<Double> iterable) {
         return doubleList;
     }
 
-    /**
-     * The following Conjure boxed list wrappers for the eclipse-collections [type]ArrayList are temporary (except
-     * ConjureSafeLongList). In eclipse-collections 12, a BoxedMutable[type]List will be released. Once available,
-     * Conjure[type]List should be replaced with that.
-     */
-
     // This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set
     public static List<Integer> newNonNullIntegerList() {
         return new ConjureIntegerList(new IntArrayList());
     }
 
+    // This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set
+    public static List<Integer> newNonNullIntegerList(int[] integers) {
+        return new ConjureIntegerList(IntArrayList.newListWith(integers.clone()));
+    }
+
     // This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set
     public static List<Integer> newNonNullIntegerList(Iterable<Integer> iterable) {
         List<Integer> integerList;
@@ -187,6 +198,11 @@ public static List<Boolean> newNonNullBooleanList() {
         return new ConjureBooleanList(new BooleanArrayList());
     }
 
+    // This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set
+    public static List<Boolean> newNonNullBooleanList(boolean[] booleans) {
+        return new ConjureBooleanList(BooleanArrayList.newListWith(booleans.clone()));
+    }
+
     // This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set
     public static List<Boolean> newNonNullBooleanList(Iterable<Boolean> iterable) {
         List<Boolean> booleanList;
@@ -205,6 +221,19 @@ public static List<SafeLong> newNonNullSafeLongList() {
         return new ConjureSafeLongList(new LongArrayList());
     }
 
+    // This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set
+    public static List<SafeLong> newNonNullSafeLongList(long[] longs) {
+        long[] conjureCopy = longs.clone();
+        for (long value : conjureCopy) {
+            if (!safeLongCheck(value)) {
+                throw new SafeIllegalArgumentException(
+                        "number must be safely representable in javascript i.e. lie between -9007199254740991 and "
+                                + "9007199254740991");
+            }
+        }
+        return new ConjureSafeLongList(LongArrayList.newListWith(conjureCopy));
+    }
+
     // This method returns a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set
     public static List<SafeLong> newNonNullSafeLongList(Iterable<SafeLong> iterable) {
         List<SafeLong> safeLongList;
@@ -217,4 +246,8 @@ public static List<SafeLong> newNonNullSafeLongList(Iterable<SafeLong> iterable)
 
         return safeLongList;
     }
+
+    private static boolean safeLongCheck(long value) {
+        return SafeLong.MIN_VALUE.longValue() <= value && value <= SafeLong.MAX_VALUE.longValue();
+    }
 }

conjure-lib/src/test/java/com/palantir/conjure/java/lib/internal/ConjureCollectionsTest.java

@@ -17,8 +17,10 @@
 package com.palantir.conjure.java.lib.internal;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 
 import com.palantir.conjure.java.lib.SafeLong;
+import com.palantir.logsafe.exceptions.SafeIllegalArgumentException;
 import java.util.List;
 import org.junit.jupiter.api.Test;
 
@@ -51,6 +53,17 @@ public void newNonNullDoubleList() {
 
         doubleList.clear();
         assertThat(doubleList).hasSize(0);
+
+        double[] rawList = new double[] {0.1, 0.2, 0.3};
+        doubleList = ConjureCollections.newNonNullDoubleList(rawList);
+        assertThat(doubleList).hasSize(3);
+        assertThat(doubleList.get(0)).isEqualTo(0.1);
+        assertThat(doubleList.get(1)).isEqualTo(0.2);
+        assertThat(doubleList.get(2)).isEqualTo(0.3);
+
+        // Check we made a copy of the array
+        rawList[0] = 0.4;
+        assertThat(doubleList.get(2)).isEqualTo(0.3);
     }
 
     @Test
@@ -81,5 +94,19 @@ public void newNonNullSafeLongList() {
 
         safeLongList.clear();
         assertThat(safeLongList).hasSize(0);
+
+        long[] rawList = new long[] {1L, 2L, 3L};
+        safeLongList = ConjureCollections.newNonNullSafeLongList(rawList);
+        assertThat(safeLongList).hasSize(3);
+        assertThat(safeLongList.get(0)).isEqualTo(SafeLong.of(1L));
+        assertThat(safeLongList.get(1)).isEqualTo(SafeLong.of(2L));
+        assertThat(safeLongList.get(2)).isEqualTo(SafeLong.of(3L));
+
+        rawList[0] = 42L;
+        assertThat(safeLongList.get(0)).isEqualTo(SafeLong.of(1L));
+
+        assertThatExceptionOfType(SafeIllegalArgumentException.class)
+                .isThrownBy(() -> ConjureCollections.newNonNullSafeLongList(
+                        new long[] {1L, 2L, SafeLong.MAX_VALUE.longValue() + 1}));
     }
 }