From 1f0d451fee8e8fbd03b643be521a5ee863eb7a52 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Tue, 25 Dec 2018 12:33:47 +0000 Subject: [PATCH 1/4] Generate visitor interfaces for conjure enums Most uses of enums involve a switch or if-else block across values along the lines of "boolean isTerminalState(enum)" or "String getDescription(enum)". It's easy to add new enum values without updating all usages, by using a visitor we catch these bugs at compile time rather than runtime. --- .../com/palantir/product/EnumExample.java | 20 +++++ .../conjure/java/types/EnumGenerator.java | 74 +++++++++++++++++-- 2 files changed, 89 insertions(+), 5 deletions(-) diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/EnumExample.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/EnumExample.java index a4d11554f..20bf8dcf5 100644 --- a/conjure-java-core/src/integrationInput/java/com/palantir/product/EnumExample.java +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/EnumExample.java @@ -70,6 +70,17 @@ public static EnumExample valueOf(String value) { } } + public T accept(Visitor visitor) { + switch (value) { + case ONE: + return visitor.visitOne(); + case TWO: + return visitor.visitTwo(); + default: + return visitor.visitUnknown(string); + } + } + @Generated("com.palantir.conjure.java.types.EnumGenerator") public enum Value { ONE, @@ -78,4 +89,13 @@ public enum Value { UNKNOWN } + + @Generated("com.palantir.conjure.java.types.EnumGenerator") + public interface Visitor { + T visitOne(); + + T visitTwo(); + + T visitUnknown(String unknownValue); + } } diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/EnumGenerator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/EnumGenerator.java index 0ac1b64d7..a7efa56ac 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/EnumGenerator.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/EnumGenerator.java @@ -18,6 +18,8 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonValue; +import com.google.common.base.CaseFormat; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.palantir.conjure.java.ConjureAnnotations; import com.palantir.conjure.spec.EnumDefinition; @@ -28,34 +30,46 @@ import com.squareup.javapoet.JavaFile; import com.squareup.javapoet.MethodSpec; import com.squareup.javapoet.ParameterSpec; +import com.squareup.javapoet.ParameterizedTypeName; import com.squareup.javapoet.TypeName; import com.squareup.javapoet.TypeSpec; +import com.squareup.javapoet.TypeVariableName; +import java.util.List; import java.util.Locale; import javax.lang.model.element.Modifier; import org.apache.commons.lang3.StringUtils; public final class EnumGenerator { + private static final String VALUE_PARAMETER = "value"; + private static final String STRING_PARAMETER = "string"; + private static final String VISIT_METHOD_NAME = "visit"; + private static final String VISIT_UNKNOWN_METHOD_NAME = "visitUnknown"; + private static final TypeVariableName TYPE_VARIABLE = TypeVariableName.get("T"); + private EnumGenerator() {} public static JavaFile generateEnumType(EnumDefinition typeDef) { String typePackage = typeDef.getTypeName().getPackage(); ClassName thisClass = ClassName.get(typePackage, typeDef.getTypeName().getName()); ClassName enumClass = ClassName.get(typePackage, typeDef.getTypeName().getName(), "Value"); + ClassName visitorClass = ClassName.get(typePackage, typeDef.getTypeName().getName(), "Visitor"); - return JavaFile.builder(typePackage, createSafeEnum(typeDef, thisClass, enumClass)) + return JavaFile.builder(typePackage, createSafeEnum(typeDef, thisClass, enumClass, visitorClass)) .skipJavaLangImports(true) .indent(" ") .build(); } - private static TypeSpec createSafeEnum(EnumDefinition typeDef, ClassName thisClass, ClassName enumClass) { + private static TypeSpec createSafeEnum( + EnumDefinition typeDef, ClassName thisClass, ClassName enumClass, ClassName visitorClass) { TypeSpec.Builder wrapper = TypeSpec.classBuilder(typeDef.getTypeName().getName()) .addAnnotation(ConjureAnnotations.getConjureGeneratedAnnotation(EnumGenerator.class)) .addModifiers(Modifier.PUBLIC, Modifier.FINAL) .addType(createEnum(enumClass, typeDef.getValues(), true)) - .addField(enumClass, "value", Modifier.PRIVATE, Modifier.FINAL) - .addField(ClassName.get(String.class), "string", Modifier.PRIVATE, Modifier.FINAL) + .addType(createVisitor(visitorClass, typeDef.getValues())) + .addField(enumClass, VALUE_PARAMETER, Modifier.PRIVATE, Modifier.FINAL) + .addField(ClassName.get(String.class), STRING_PARAMETER, Modifier.PRIVATE, Modifier.FINAL) .addFields(createConstants(typeDef.getValues(), thisClass, enumClass)) .addMethod(createConstructor(enumClass)) .addMethod(MethodSpec.methodBuilder("get") @@ -72,7 +86,8 @@ private static TypeSpec createSafeEnum(EnumDefinition typeDef, ClassName thisCla .build()) .addMethod(createEquals(thisClass)) .addMethod(createHashCode()) - .addMethod(createValueOf(thisClass, typeDef.getValues())); + .addMethod(createValueOf(thisClass, typeDef.getValues())) + .addMethod(generateAcceptVisitMethod(visitorClass, typeDef.getValues())); typeDef.getDocs().ifPresent( docs -> wrapper.addJavadoc("$L

\n", StringUtils.appendIfMissing(docs.get(), "\n"))); @@ -133,6 +148,55 @@ private static TypeSpec createEnum(ClassName enumClass, Iterable values) { + return TypeSpec.interfaceBuilder(visitorClass.simpleName()) + .addAnnotation(ConjureAnnotations.getConjureGeneratedAnnotation(EnumGenerator.class)) + .addModifiers(Modifier.PUBLIC) + .addTypeVariable(TYPE_VARIABLE) + .addMethods(generateMemberVisitMethods(values)) + .addMethod(MethodSpec.methodBuilder(VISIT_UNKNOWN_METHOD_NAME) + .addModifiers(Modifier.PUBLIC, Modifier.ABSTRACT) + .addParameter(String.class, "unknownValue") + .returns(TYPE_VARIABLE) + .build()) + .build(); + } + + private static List generateMemberVisitMethods(Iterable values) { + ImmutableList.Builder methods = ImmutableList.builder(); + for (EnumValueDefinition value : values) { + methods.add(MethodSpec.methodBuilder(getVisitorMethodName(value)) + .addModifiers(Modifier.PUBLIC, Modifier.ABSTRACT) + .returns(TYPE_VARIABLE) + .build()); + } + return methods.build(); + } + + private static MethodSpec generateAcceptVisitMethod(ClassName visitorClass, Iterable values) { + CodeBlock.Builder switchBlock = CodeBlock.builder(); + switchBlock.beginControlFlow("switch (value)"); + for (EnumValueDefinition value : values) { + switchBlock.add("case $L:\n", value.getValue()) + .addStatement("return visitor.$L()", getVisitorMethodName(value)); + } + switchBlock.add("default:\n") + .addStatement("return visitor.$1L(string)", VISIT_UNKNOWN_METHOD_NAME) + .endControlFlow(); + ParameterizedTypeName parameterizedVisitorClass = ParameterizedTypeName.get(visitorClass, TYPE_VARIABLE); + return MethodSpec.methodBuilder("accept") + .addModifiers(Modifier.PUBLIC) + .addParameter(ParameterSpec.builder(parameterizedVisitorClass, "visitor").build()) + .addTypeVariable(TYPE_VARIABLE) + .returns(TYPE_VARIABLE) + .addCode(switchBlock.build() + ).build(); + } + + private static String getVisitorMethodName(EnumValueDefinition definition) { + return VISIT_METHOD_NAME + CaseFormat.UPPER_UNDERSCORE.to(CaseFormat.UPPER_CAMEL, definition.getValue()); + } + private static MethodSpec createConstructor(ClassName enumClass) { // Note: We generate a two arg constructor that handles both known // and unknown variants instead of using separate contructors to avoid From c9cc11bff195945e59c5db02f0b43f9d4bbe53f5 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Tue, 25 Dec 2018 13:01:36 +0000 Subject: [PATCH 2/4] Add javadoc to enum visitor methods Tests for generated enums --- .../com/palantir/product/EnumExample.java | 15 +++- .../conjure/java/types/EnumGenerator.java | 8 +- .../conjure/java/types/EnumTests.java | 73 +++++++++++++++++++ .../src/test/resources/example-types.yml | 4 +- 4 files changed, 95 insertions(+), 5 deletions(-) create mode 100644 conjure-java-core/src/test/java/com/palantir/conjure/java/types/EnumTests.java diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/EnumExample.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/EnumExample.java index 20bf8dcf5..7a5ca8305 100644 --- a/conjure-java-core/src/integrationInput/java/com/palantir/product/EnumExample.java +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/EnumExample.java @@ -7,7 +7,7 @@ import javax.annotation.Generated; /** - * This enumerates the numbers 1:2. + * This enumerates the numbers 1:2 also 100. * *

This class is used instead of a native enum to support unknown values. Rather than throw an * exception, the {@link EnumExample#valueOf} method defaults to a new instantiation of {@link @@ -25,6 +25,9 @@ public final class EnumExample { public static final EnumExample TWO = new EnumExample(Value.TWO, "TWO"); + /** Value of 100. */ + public static final EnumExample ONE_HUNDRED = new EnumExample(Value.ONE_HUNDRED, "ONE_HUNDRED"); + private final Value value; private final String string; @@ -65,6 +68,8 @@ public static EnumExample valueOf(String value) { return ONE; case "TWO": return TWO; + case "ONE_HUNDRED": + return ONE_HUNDRED; default: return new EnumExample(Value.UNKNOWN, upperCasedValue); } @@ -76,6 +81,8 @@ public T accept(Visitor visitor) { return visitor.visitOne(); case TWO: return visitor.visitTwo(); + case ONE_HUNDRED: + return visitor.visitOneHundred(); default: return visitor.visitUnknown(string); } @@ -87,6 +94,9 @@ public enum Value { TWO, + /** Value of 100. */ + ONE_HUNDRED, + UNKNOWN } @@ -96,6 +106,9 @@ public interface Visitor { T visitTwo(); + /** Value of 100. */ + T visitOneHundred(); + T visitUnknown(String unknownValue); } } diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/EnumGenerator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/EnumGenerator.java index a7efa56ac..20728065a 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/EnumGenerator.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/EnumGenerator.java @@ -165,10 +165,12 @@ private static TypeSpec createVisitor(ClassName visitorClass, Iterable generateMemberVisitMethods(Iterable values) { ImmutableList.Builder methods = ImmutableList.builder(); for (EnumValueDefinition value : values) { - methods.add(MethodSpec.methodBuilder(getVisitorMethodName(value)) + MethodSpec.Builder methodSpecBuilder = MethodSpec.methodBuilder(getVisitorMethodName(value)) .addModifiers(Modifier.PUBLIC, Modifier.ABSTRACT) - .returns(TYPE_VARIABLE) - .build()); + .returns(TYPE_VARIABLE); + value.getDocs().ifPresent(docs -> + methodSpecBuilder.addJavadoc("$L", StringUtils.appendIfMissing(docs.get(), "\n"))); + methods.add(methodSpecBuilder.build()); } return methods.build(); } diff --git a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/EnumTests.java b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/EnumTests.java new file mode 100644 index 000000000..8e2bb0755 --- /dev/null +++ b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/EnumTests.java @@ -0,0 +1,73 @@ +/* + * (c) Copyright 2018 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.conjure.java.types; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.palantir.product.EnumExample; +import org.junit.Test; + +public class EnumTests { + + @Test + public void testVisitOne() { + EnumExample enumExample = EnumExample.ONE; + assertThat(enumExample.accept(Visitor.INSTANCE)).isEqualTo("one"); + } + + @Test + public void testValueOfProducesSameInstance() { + assertThat(EnumExample.valueOf("ONE")).isSameAs(EnumExample.ONE); + } + + @Test + public void testUnknown() { + EnumExample enumExample = EnumExample.valueOf("SOME_VALUE"); + assertThat(enumExample.get()).isEqualTo(EnumExample.Value.UNKNOWN); + assertThat(enumExample.toString()).isEqualTo("SOME_VALUE"); + } + + @Test + public void testVisitUnknown() { + EnumExample enumExample = EnumExample.valueOf("SOME_VALUE"); + assertThat(enumExample.accept(Visitor.INSTANCE)).isEqualTo("SOME_VALUE"); + } + + private enum Visitor implements EnumExample.Visitor { + INSTANCE; + + @Override + public String visitOne() { + return "one"; + } + + @Override + public String visitTwo() { + return "two"; + } + + @Override + public String visitOneHundred() { + return "one hundred"; + } + + @Override + public String visitUnknown(String unknownValue) { + return unknownValue; + } + } +} diff --git a/conjure-java-core/src/test/resources/example-types.yml b/conjure-java-core/src/test/resources/example-types.yml index b17cc63e6..abc571b4c 100644 --- a/conjure-java-core/src/test/resources/example-types.yml +++ b/conjure-java-core/src/test/resources/example-types.yml @@ -56,10 +56,12 @@ types: items: map EnumExample: docs: | - This enumerates the numbers 1:2. + This enumerates the numbers 1:2 also 100. values: - ONE - TWO + - value: ONE_HUNDRED + docs: Value of 100. EnumFieldExample: fields: enum: EnumExample From 933f4c5f2b503d6509b900cc40972d4cd0da2cca Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Tue, 25 Dec 2018 13:35:17 +0000 Subject: [PATCH 3/4] Use javapoet self-reference rather than literal --- .../java/com/palantir/conjure/java/types/EnumGenerator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/EnumGenerator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/EnumGenerator.java index 20728065a..60ddb026e 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/EnumGenerator.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/EnumGenerator.java @@ -179,7 +179,7 @@ private static MethodSpec generateAcceptVisitMethod(ClassName visitorClass, Iter CodeBlock.Builder switchBlock = CodeBlock.builder(); switchBlock.beginControlFlow("switch (value)"); for (EnumValueDefinition value : values) { - switchBlock.add("case $L:\n", value.getValue()) + switchBlock.add("case $N:\n", value.getValue()) .addStatement("return visitor.$L()", getVisitorMethodName(value)); } switchBlock.add("default:\n") From 0759af217a5ecfccce494ab84dd028f180915dc6 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Tue, 8 Jan 2019 10:57:46 -0500 Subject: [PATCH 4/4] fix weird formatting --- .../java/com/palantir/conjure/java/types/EnumGenerator.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/EnumGenerator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/EnumGenerator.java index 60ddb026e..ede773391 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/EnumGenerator.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/EnumGenerator.java @@ -191,8 +191,8 @@ private static MethodSpec generateAcceptVisitMethod(ClassName visitorClass, Iter .addParameter(ParameterSpec.builder(parameterizedVisitorClass, "visitor").build()) .addTypeVariable(TYPE_VARIABLE) .returns(TYPE_VARIABLE) - .addCode(switchBlock.build() - ).build(); + .addCode(switchBlock.build()) + .build(); } private static String getVisitorMethodName(EnumValueDefinition definition) {