diff --git a/changelog/@unreleased/pr-2389.v2.yml b/changelog/@unreleased/pr-2389.v2.yml new file mode 100644 index 000000000..36e09bbaa --- /dev/null +++ b/changelog/@unreleased/pr-2389.v2.yml @@ -0,0 +1,5 @@ +type: feature +feature: + description: '[Feature] Non-boxing ''addAll'' Methods for Primitive Types' + links: + - https://github.com/palantir/conjure-java/pull/2389 diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/ListExample.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/ListExample.java index 885402021..8abc8ee18 100644 --- a/conjure-java-core/src/integrationInput/java/com/palantir/product/ListExample.java +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/ListExample.java @@ -235,6 +235,13 @@ public Builder primitiveItems(@Nonnull Iterable primitiveItems) { return this; } + public Builder addAllPrimitiveItems(@Nonnull int... primitiveItems) { + checkNotBuilt(); + ConjureCollections.addAllToIntegerList( + this.primitiveItems, Preconditions.checkNotNull(primitiveItems, "primitiveItems cannot be null")); + return this; + } + public Builder addAllPrimitiveItems(@Nonnull Iterable primitiveItems) { checkNotBuilt(); ConjureCollections.addAllAndCheckNonNull( @@ -257,6 +264,13 @@ public Builder doubleItems(@Nonnull Iterable doubleItems) { return this; } + public Builder addAllDoubleItems(@Nonnull double... doubleItems) { + checkNotBuilt(); + ConjureCollections.addAllToDoubleList( + this.doubleItems, Preconditions.checkNotNull(doubleItems, "doubleItems cannot be null")); + return this; + } + public Builder addAllDoubleItems(@Nonnull Iterable doubleItems) { checkNotBuilt(); ConjureCollections.addAllAndCheckNonNull( diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeLongExample.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeLongExample.java index ba755c61a..dde2cd290 100644 --- a/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeLongExample.java +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/SafeLongExample.java @@ -135,6 +135,13 @@ public Builder safeLongList(@Nonnull Iterable safeLongList) { return this; } + public Builder addAllSafeLongList(@Nonnull long... safeLongList) { + checkNotBuilt(); + ConjureCollections.addAllToSafeLongList( + this.safeLongList, Preconditions.checkNotNull(safeLongList, "safeLongList cannot be null")); + return this; + } + public Builder addAllSafeLongList(@Nonnull Iterable safeLongList) { checkNotBuilt(); ConjureCollections.addAllAndCheckNonNull( diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderAuxiliarySettersUtils.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderAuxiliarySettersUtils.java index 3adef817e..fd9e29408 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderAuxiliarySettersUtils.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderAuxiliarySettersUtils.java @@ -17,6 +17,7 @@ package com.palantir.conjure.java.types; import com.palantir.conjure.java.ConjureAnnotations; +import com.palantir.conjure.java.lib.SafeLong; import com.palantir.conjure.java.types.BeanGenerator.EnrichedField; import com.palantir.conjure.java.util.Javadoc; import com.palantir.conjure.java.util.Primitives; @@ -28,6 +29,7 @@ import com.palantir.conjure.spec.PrimitiveType; import com.palantir.conjure.spec.Type; import com.palantir.conjure.visitor.TypeVisitor; +import com.palantir.javapoet.ArrayTypeName; import com.palantir.javapoet.ClassName; import com.palantir.javapoet.CodeBlock; import com.palantir.javapoet.FieldSpec; @@ -39,11 +41,43 @@ import javax.lang.model.element.Modifier; import org.apache.commons.lang3.StringUtils; -public final class BeanBuilderAuxiliarySettersUtils { +final class BeanBuilderAuxiliarySettersUtils { private BeanBuilderAuxiliarySettersUtils() {} - public static MethodSpec.Builder createCollectionSetterBuilder( + static MethodSpec.Builder createPrimitiveCollectionSetterBuilder( + EnrichedField enriched, TypeMapper typeMapper, ClassName returnClass, SafetyEvaluator safetyEvaluator) { + FieldSpec field = enriched.poetSpec(); + FieldDefinition definition = enriched.conjureDef(); + TypeName innerTypeName = extractInnerTypeFromList(definition, typeMapper, safetyEvaluator); + + return MethodSpec.methodBuilder("addAll" + StringUtils.capitalize(field.name())) + .addJavadoc(Javadoc.render(definition.getDocs(), definition.getDeprecated()) + .map(rendered -> CodeBlock.of("$L", rendered)) + .orElseGet(() -> CodeBlock.builder().build())) + .addAnnotations(ConjureAnnotations.deprecation(definition.getDeprecated())) + .addModifiers(Modifier.PUBLIC) + // Forces the array argument to instead be variadic + .varargs() + .addParameter(Parameters.nonnullParameter(ArrayTypeName.of(innerTypeName), field.name())) + .returns(returnClass); + } + + private static TypeName extractInnerTypeFromList( + FieldDefinition conjureDef, TypeMapper typeMapper, SafetyEvaluator safetyEvaluator) { + Type innerType = conjureDef.getType().accept(TypeVisitor.LIST).getItemType(); + TypeName boxedTypeName = typeMapper.getClassName(innerType); + + // SafeLong is just a special case of long + if (boxedTypeName.equals(ClassName.get(SafeLong.class))) { + return ConjureAnnotations.withSafety(TypeName.LONG, safetyEvaluator.getUsageTimeSafety(conjureDef)); + } else { + return ConjureAnnotations.withSafety( + Primitives.unbox(boxedTypeName), safetyEvaluator.getUsageTimeSafety(conjureDef)); + } + } + + static MethodSpec.Builder createCollectionSetterBuilder( String prefix, EnrichedField enriched, TypeMapper typeMapper, @@ -66,7 +100,7 @@ public static MethodSpec.Builder createCollectionSetterBuilder( field.name())); } - public static MethodSpec.Builder createOptionalSetterBuilder( + static MethodSpec.Builder createOptionalSetterBuilder( EnrichedField enriched, TypeMapper typeMapper, ClassName returnClass, SafetyEvaluator safetyEvaluator) { FieldSpec field = enriched.poetSpec(); OptionalType type = enriched.conjureDef().getType().accept(TypeVisitor.OPTIONAL); @@ -78,7 +112,7 @@ public static MethodSpec.Builder createOptionalSetterBuilder( field.name())); } - public static MethodSpec.Builder createItemSetterBuilder( + static MethodSpec.Builder createItemSetterBuilder( EnrichedField enriched, Type itemType, TypeMapper typeMapper, @@ -89,7 +123,7 @@ public static MethodSpec.Builder createItemSetterBuilder( .addParameter(ConjureAnnotations.withSafety(typeMapper.getClassName(itemType), safety), field.name()); } - public static MethodSpec.Builder createMapSetterBuilder( + static MethodSpec.Builder createMapSetterBuilder( EnrichedField enriched, TypeMapper typeMapper, ClassName returnClass) { MapType type = enriched.conjureDef().getType().accept(TypeVisitor.MAP); return publicSetter(enriched, returnClass) @@ -97,7 +131,7 @@ public static MethodSpec.Builder createMapSetterBuilder( .addParameter(typeMapper.getClassName(type.getValueType()), "value"); } - public static MethodSpec.Builder publicSetter(EnrichedField enriched, ClassName returnClass) { + static MethodSpec.Builder publicSetter(EnrichedField enriched, ClassName returnClass) { FieldDefinition definition = enriched.conjureDef(); return MethodSpec.methodBuilder(enriched.poetSpec().name()) .addJavadoc(Javadoc.render(definition.getDocs(), definition.getDeprecated()) @@ -108,7 +142,7 @@ public static MethodSpec.Builder publicSetter(EnrichedField enriched, ClassName .returns(returnClass); } - public static TypeName widenParameterIfPossible( + static TypeName widenParameterIfPossible( TypeName current, Type type, TypeMapper typeMapper, Optional safety) { if (type.accept(TypeVisitor.IS_LIST)) { Type innerType = type.accept(TypeVisitor.LIST).getItemType(); @@ -147,7 +181,7 @@ public static TypeName widenParameterIfPossible( // we want to widen containers of anything that's not a primitive, a conjure reference or an optional // since we know all of those are final. - public static boolean isWidenableContainedType(Type containedType) { + static boolean isWidenableContainedType(Type containedType) { return containedType.accept(new DefaultTypeVisitor() { @Override public Boolean visitPrimitive(PrimitiveType value) { diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderGenerator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderGenerator.java index e791a2298..ea924e594 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderGenerator.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderGenerator.java @@ -671,6 +671,26 @@ private MethodSpec createCollectionSetter(String prefix, EnrichedField enriched, .build(); } + private MethodSpec createPrimitiveCollectionSetter(EnrichedField enriched, boolean override) { + FieldSpec field = enriched.poetSpec(); + Type type = enriched.conjureDef().getType(); + + CollectionType collectionType = getCollectionType(type); + return BeanBuilderAuxiliarySettersUtils.createPrimitiveCollectionSetterBuilder( + enriched, typeMapper, builderClass, safetyEvaluator) + .addAnnotations(ConjureAnnotations.override(override)) + .addCode(verifyNotBuilt()) + .addCode(CodeBlocks.statement( + "$1T.addAllTo$2L(this.$3N, $4L)", + ConjureCollections.class, + collectionType.getConjureCollectionType().getCollectionName(), + field.name(), + Expressions.requireNonNull( + field.name(), enriched.fieldName().get() + " cannot be null"))) + .addStatement("return this") + .build(); + } + @SuppressWarnings("checkstyle:CyclomaticComplexity") private CodeBlock typeAwareAssignment(EnrichedField enriched, Type type, boolean shouldClearFirst) { FieldSpec spec = enriched.poetSpec(); @@ -767,10 +787,19 @@ private List createAuxiliarySetters(EnrichedField enriched, boolean Type type = enriched.conjureDef().getType(); Optional safety = safetyEvaluator.getUsageTimeSafety(enriched.conjureDef()); + ImmutableList.Builder builder = ImmutableList.builder(); + if (type.accept(TypeVisitor.IS_LIST)) { - return ImmutableList.of( + CollectionType collectionType = getCollectionType(type); + if (collectionType.getConjureCollectionType().isPrimitiveCollection() + && collectionType.useNonNullFactory()) { + builder.add(createPrimitiveCollectionSetter(enriched, override)); + } + + builder.add( createCollectionSetter("addAll", enriched, override), createItemSetter(enriched, type.accept(TypeVisitor.LIST).getItemType(), override, safety)); + return builder.build(); } if (type.accept(TypeVisitor.IS_SET)) { @@ -822,7 +851,9 @@ private CodeBlock optionalAssignmentStatement(EnrichedField enriched, OptionalTy private static final EnumSet 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( @@ -1041,25 +1072,31 @@ public boolean useNonNullFactory() { } private enum ConjureCollectionType { - LIST("List"), - DOUBLE_LIST("DoubleList"), - INTEGER_LIST("IntegerList"), + LIST("List", false), + DOUBLE_LIST("DoubleList", true), + INTEGER_LIST("IntegerList", true), // Eclipse has a BooleanList type, but this use case implies // bit mask and it doesn't serialize efficiently as a collection // so let's just use the "naive" boxed collection - BOOLEAN_LIST("List"), - SAFE_LONG_LIST("SafeLongList"), - SET("Set"); + BOOLEAN_LIST("List", false), + 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 { diff --git a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/SafeLong.java b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/SafeLong.java index 4ff79b24e..d70994b38 100644 --- a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/SafeLong.java +++ b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/SafeLong.java @@ -40,7 +40,7 @@ public long longValue() { return longValue; } - private static long check(long value) { + public static long check(long value) { Preconditions.checkArgument( MIN_SAFE_VALUE <= value && value <= MAX_SAFE_VALUE, "number must be safely representable in javascript i.e. " diff --git a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureCollections.java b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureCollections.java index c8a5a4df8..da756357c 100644 --- a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureCollections.java +++ b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureCollections.java @@ -19,6 +19,7 @@ import com.palantir.conjure.java.lib.SafeLong; import com.palantir.logsafe.Preconditions; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.LinkedHashSet; @@ -170,8 +171,12 @@ public static List newNonNullDoubleList(Iterable iterable) { // This method modifies a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set public static void addAllToDoubleList(Collection addTo, double[] elementsToAdd) { - for (double el : elementsToAdd) { - addTo.add(el); + if (addTo instanceof ConjureDoubleList) { + ((ConjureDoubleList) addTo).addAll(elementsToAdd); + } else { + for (double el : elementsToAdd) { + addTo.add(el); + } } } @@ -195,8 +200,12 @@ public static List newNonNullIntegerList(Iterable iterable) { // This method modifies a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set public static void addAllToIntegerList(Collection addTo, int[] elementsToAdd) { - for (int el : elementsToAdd) { - addTo.add(el); + if (addTo instanceof ConjureIntegerList) { + ((ConjureIntegerList) addTo).addAll(elementsToAdd); + } else { + for (int el : elementsToAdd) { + addTo.add(el); + } } } @@ -235,8 +244,12 @@ public static List newNonNullSafeLongList(Iterable iterable) // This method modifies a list that can't handle nulls. Do not use this unless the nonNullCollections flag is set public static void addAllToSafeLongList(Collection addTo, long[] elementsToAdd) { - for (long el : elementsToAdd) { - addTo.add(SafeLong.of(el)); + if (addTo instanceof ConjureSafeLongList) { + ((ConjureSafeLongList) addTo).addAll(elementsToAdd); + } else { + for (long el : elementsToAdd) { + addTo.add(SafeLong.of(el)); + } } } } diff --git a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureDoubleList.java b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureDoubleList.java index 58d63bbaa..b936eb604 100644 --- a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureDoubleList.java +++ b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureDoubleList.java @@ -55,6 +55,10 @@ public boolean addAll(int index, Collection collection) { return delegate.addAllAtIndex(index, target); } + public void addAll(double... source) { + this.delegate.addAll(source); + } + @Override public Double remove(int index) { return delegate.removeAtIndex(index); diff --git a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureIntegerList.java b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureIntegerList.java index 79ecd32b6..31a34062b 100644 --- a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureIntegerList.java +++ b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureIntegerList.java @@ -55,6 +55,10 @@ public boolean addAll(int index, Collection collection) { return delegate.addAllAtIndex(index, target); } + public void addAll(int... source) { + this.delegate.addAll(source); + } + @Override public Integer remove(int index) { return delegate.removeAtIndex(index); diff --git a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureSafeLongList.java b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureSafeLongList.java index 7995d8285..1d4917703 100644 --- a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureSafeLongList.java +++ b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureSafeLongList.java @@ -56,6 +56,14 @@ public boolean addAll(int index, Collection collection) { return delegate.addAllAtIndex(index, target); } + public void addAll(long... source) { + for (long value : source) { + // Doesn't use SafeLong creation because this causes unnecessary boxing + SafeLong.check(value); + } + this.delegate.addAll(source); + } + @Override public SafeLong remove(int index) { return SafeLong.of(delegate.removeAtIndex(index));