Skip to content

Commit

Permalink
address some self review criticism
Browse files Browse the repository at this point in the history
  • Loading branch information
@bmarcaur
bmarcaur committed Oct 23, 2024
1 parent ae51640 commit df23c89
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 35 deletions.
55 changes: 27 additions & 28 deletions ...-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderAuxiliarySettersUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,44 +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 createPrimitiveCollectionSetterBuilder(
String prefix,
EnrichedField enriched,
TypeMapper typeMapper,
ClassName returnClass,
SafetyEvaluator safetyEvaluator) {
static MethodSpec.Builder createPrimitiveCollectionSetterBuilder(
EnrichedField enriched, TypeMapper typeMapper, ClassName returnClass, SafetyEvaluator safetyEvaluator) {
FieldSpec field = enriched.poetSpec();
FieldDefinition definition = enriched.conjureDef();
Type type = definition.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()));
}
TypeName innerTypeName = extractInnerTypeFromList(definition, typeMapper, safetyEvaluator);

return MethodSpec.methodBuilder(prefix + StringUtils.capitalize(field.name))
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)
// Var arg of the primitive type
// Forces the array argument to instead be variadic
.varargs()
.addParameter(Parameters.nonnullParameter(ArrayTypeName.of(innerTypeName), field.name))
.returns(returnClass);
}

public static MethodSpec.Builder createCollectionSetterBuilder(
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,
Expand All @@ -101,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);
Expand All @@ -113,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,
Expand All @@ -124,15 +123,15 @@ 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)
.addParameter(typeMapper.getClassName(type.getKeyType()), "key")
.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())
Expand All @@ -143,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<LogSafety> safety) {
if (type.accept(TypeVisitor.IS_LIST)) {
Type innerType = type.accept(TypeVisitor.LIST).getItemType();
Expand Down Expand Up @@ -182,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<Boolean>() {
@Override
public Boolean visitPrimitive(PrimitiveType value) {
Expand Down
8 changes: 1 addition & 7 deletions conjure-java-core/src/main/java/com/palantir/conjure/java/types/BeanBuilderGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,7 @@ private MethodSpec createPrimitiveCollectionSetter(EnrichedField enriched, boole

CollectionType collectionType = getCollectionType(type);
return BeanBuilderAuxiliarySettersUtils.createPrimitiveCollectionSetterBuilder(
"addAll", enriched, typeMapper, builderClass, safetyEvaluator)
enriched, typeMapper, builderClass, safetyEvaluator)
.addAnnotations(ConjureAnnotations.override(override))
.addCode(verifyNotBuilt())
.addCode(CodeBlocks.statement(
Expand All @@ -689,12 +689,6 @@ private MethodSpec createPrimitiveCollectionSetter(EnrichedField enriched, boole
field.name, enriched.fieldName().get() + " cannot be null")))
.addStatement("return this")
.build();
/*
ConjureCollections.class,
spec.name,
Expressions.requireNonNull(
spec.name, enriched.fieldName().get() + " cannot be null"));
*/
}

@SuppressWarnings("checkstyle:CyclomaticComplexity")
Expand Down

0 comments on commit df23c89

Please sign in to comment.