Merge branch 'release/0.7.0'

README.md

@@ -20,12 +20,13 @@ Add the following plugin to the `<build>` part of your Maven pom.xml file
 <plugin>
     <groupId>com.github.timvlaer</groupId>
     <artifactId>thrifty-maven-plugin</artifactId>
-    <version>0.6.0</version>
+    <version>0.7.0</version>
     <configuration>
         <thriftFiles>
             <file>thrift-schema/internal.thrift</file>
         </thriftFiles>
-        <enableConvenienceMethods>true</enableConvenienceMethods>
+        <enableConvenienceMethods>true</enableConvenienceMethods><!--default is false-->
+        <generateGettersInBuilders>false</generateGettersInBuilders><!--default is false-->
     </configuration>
     <executions>
         <execution>
@@ -42,7 +43,7 @@ The generated code depends on `thrifty-runtime`, so add the following to your de
 <dependency>
     <groupId>com.microsoft.thrifty</groupId>
     <artifactId>thrifty-runtime</artifactId>
-    <version>2.1.0</version>
+    <version>2.1.1</version>
 </dependency>
 ```
 
@@ -64,3 +65,9 @@ On all classes:
 On classes that are based on Thrift `union` types:
 * `public String tag()` which returns the name of the filled field
 * `public Object value()` which returns the value of the filled field (untyped)
+* For each union field, a static factory method, e.g. `public static UnionName unionValue(String unionValue)`
+
+If you set `<generateGettersInBuilders>` to `true`, this plugin will generate getter methods on builders. 
+This might ease migration from the default (mutable) thrift generated code. 
+[Effective Java](https://www.goodreads.com/book/show/34927404-effective-java) doesn't recommend Getters on builders, 
+so the default for this setting is `false`.

pom.xml

@@ -5,7 +5,7 @@
 
   <groupId>com.github.timvlaer</groupId>
   <artifactId>thrifty-maven-plugin</artifactId>
-  <version>0.6.0</version>
+  <version>0.7.0</version>
   <packaging>maven-plugin</packaging>
 
   <name>Thrifty Maven Plugin</name>
@@ -38,7 +38,7 @@
 
     <maven.version>3.6.3</maven.version>
 
-    <thrifty.version>2.1.0</thrifty.version>
+    <thrifty.version>2.1.1</thrifty.version>
 
     <sourceDirectory>${project.basedir}/src/main/java</sourceDirectory>
     <testSourceDirectory>${project.basedir}/src/test/java</testSourceDirectory>

src/main/java/com/github/timvlaer/thrifty/ThriftyCompilerMojo.java

@@ -30,7 +30,7 @@ public class ThriftyCompilerMojo extends AbstractMojo {
   @Parameter(defaultValue = "false")
   private boolean enableConvenienceMethods;
 
-  @Parameter(defaultValue = "true")
+  @Parameter(defaultValue = "false")
   private boolean generateGettersInBuilders;
 
   public void execute() {

src/main/java/com/github/timvlaer/thrifty/plugin/BuilderMethodsTypeProcessor.java

@@ -10,17 +10,15 @@ public class BuilderMethodsTypeProcessor implements TypeProcessor {
   public static final String NEW_BUILDER_METHOD_NAME = "builder";
   public static final String TO_BUILDER_METHOD_NAME = "toBuilder";
 
-  private static final String BUILDER_CLASS_NAME = "Builder";
-
   @Override
   public TypeSpec process(TypeSpec type) {
     if (TypeSpecUtil.getBuilderClass(type).isPresent()) {
-        return type.toBuilder()
-            .addMethod(createNewBuilderMethod())
-            .addMethod(createNewBuilderMethodWithParameter(type))
-            .addMethod(createToBuilderMethod())
-            .build();
-      }
+      return type.toBuilder()
+          .addMethod(createNewBuilderMethod())
+          .addMethod(createNewBuilderMethodWithParameter(type))
+          .addMethod(createToBuilderMethod())
+          .build();
+    }
     return type;
   }
 
@@ -40,7 +38,8 @@ private MethodSpec createNewBuilderMethod() {
   private MethodSpec createNewBuilderMethodWithParameter(TypeSpec type) {
     ClassName builder = ClassName.bestGuess("Builder");
 
-    ParameterSpec param = ParameterSpec.builder(ClassName.bestGuess(type.name), "prototype").build();
+    ParameterSpec param =
+        ParameterSpec.builder(ClassName.bestGuess(type.name), "prototype").build();
 
     CodeBlock.Builder codeBlockBuilder = CodeBlock.builder();
     codeBlockBuilder.addStatement("return new $T($N)", builder, param.name);
@@ -65,5 +64,4 @@ private MethodSpec createToBuilderMethod() {
         .addCode(codeBlockBuilder.build())
         .build();
   }
-
 }

src/main/java/com/github/timvlaer/thrifty/plugin/TaggedUnionMethodTypeProcessor.java

@@ -21,6 +21,7 @@ public TypeSpec process(TypeSpec type) {
       TypeSpec.Builder builder = type.toBuilder();
       builder.addMethod(createTagMethod(type));
       builder.addMethod(createValueMethod(type));
+      builder.addMethods(createStaticFactoryMethods(type));
       return builder.build();
     }
 
@@ -29,18 +30,26 @@ public TypeSpec process(TypeSpec type) {
 
   private MethodSpec createTagMethod(TypeSpec type) {
     CodeBlock.Builder codeBlockBuilder = CodeBlock.builder();
-    getUnionFieldSpecs(type).forEach(fieldSpec ->
-        codeBlockBuilder.addStatement("if ($N != null) return $S", fieldSpec.name, fieldSpec.name));
-    codeBlockBuilder.addStatement("throw new $T($S)", IllegalStateException.class, "Union type should have one value!");
+    getUnionFieldSpecs(type)
+        .forEach(
+            fieldSpec ->
+                codeBlockBuilder.addStatement(
+                    "if ($N != null) return $S", fieldSpec.name, fieldSpec.name));
+    codeBlockBuilder.addStatement(
+        "throw new $T($S)", IllegalStateException.class, "Union type should have one value!");
 
     return createPublicMethod(TAG_METHOD_NAME, String.class, codeBlockBuilder.build());
   }
 
   private MethodSpec createValueMethod(TypeSpec type) {
     CodeBlock.Builder codeBlockBuilder = CodeBlock.builder();
     getUnionFieldSpecs(type)
-        .forEach(fieldSpec -> codeBlockBuilder.addStatement("if ($N != null) return $N", fieldSpec.name, fieldSpec.name));
-    codeBlockBuilder.addStatement("throw new $T($S)", IllegalStateException.class, "Union type should have one value!");
+        .forEach(
+            fieldSpec ->
+                codeBlockBuilder.addStatement(
+                    "if ($N != null) return $N", fieldSpec.name, fieldSpec.name));
+    codeBlockBuilder.addStatement(
+        "throw new $T($S)", IllegalStateException.class, "Union type should have one value!");
 
     return createPublicMethod(VALUE_METHOD_NAME, Object.class, codeBlockBuilder.build());
   }
@@ -59,14 +68,33 @@ private MethodSpec createPublicMethod(String methodName, Class returnType, CodeB
         .build();
   }
 
+  private List<MethodSpec> createStaticFactoryMethods(TypeSpec type) {
+    return getUnionFieldSpecs(type).stream()
+        .map(
+            f ->
+                MethodSpec.methodBuilder(f.name)
+                    .returns(ClassName.bestGuess(type.name))
+                    .addParameter(f.type, f.name)
+                    .addModifiers(Modifier.PUBLIC, Modifier.STATIC)
+                    .addCode(
+                        CodeBlock.builder()
+                            .addStatement("return new Builder().$L($L).build()", f.name, f.name)
+                            .build())
+                    .build())
+        .collect(Collectors.toList());
+  }
+
   private Boolean wasThriftUnion(TypeSpec type) {
-    return type.superinterfaces.contains(ClassName.get(com.microsoft.thrifty.Struct.class)) &&
-        type.typeSpecs.stream()
+    return type.superinterfaces.contains(ClassName.get(com.microsoft.thrifty.Struct.class))
+        && type.typeSpecs.stream()
             .filter(e -> BUILDER_CLASS_NAME.equals(e.name))
             .findFirst()
-            .flatMap(t -> t.methodSpecs.stream().filter(m -> BUILD_METHOD_NAME.equals(m.name)).findFirst())
+            .flatMap(
+                t ->
+                    t.methodSpecs.stream()
+                        .filter(m -> BUILD_METHOD_NAME.equals(m.name))
+                        .findFirst())
             .map(m -> m.code.toString().contains("Invalid union;"))
             .orElse(false);
   }
-
 }

src/test/java/com/github/timvlaer/thrifty/ThriftyCompilerMojoTest.java

@@ -28,28 +28,25 @@ public class ThriftyCompilerMojoTest {
 
   private final ThriftyCompilerMojo mojo = new ThriftyCompilerMojo();
 
-  @Rule
-  public TemporaryFolder outputFolder = new TemporaryFolder();
-  @Rule
-  public ExpectedException expectedException = ExpectedException.none();
+  @Rule public TemporaryFolder outputFolder = new TemporaryFolder();
+  @Rule public ExpectedException expectedException = ExpectedException.none();
 
-  @Mock
-  private MavenProject project;
-  @Mock
-  private Build build;
+  @Mock private MavenProject project;
+  @Mock private Build build;
 
   @Before
   public void setUp() {
     when(project.getBuild()).thenReturn(build);
     when(project.getProperties()).thenReturn(new Properties());
-    when(build.getOutputDirectory()).thenReturn(outputFolder.getRoot().getAbsolutePath() + "/classes");
+    when(build.getOutputDirectory())
+        .thenReturn(outputFolder.getRoot().getAbsolutePath() + "/classes");
     mojo.setProject(project);
     mojo.setOutputDirectory(outputFolder.getRoot().getAbsolutePath());
   }
 
   @Test
   public void execute() throws Exception {
-    mojo.setThriftFiles(new File[]{new File("src/test/resources/testcase.thrift")});
+    mojo.setThriftFiles(new File[] {new File("src/test/resources/testcase.thrift")});
 
     mojo.execute();
 
@@ -69,7 +66,7 @@ public void executeWithJava11() throws Exception {
     Properties properties = new Properties();
     properties.setProperty("maven.compiler.release", "11");
     when(project.getProperties()).thenReturn(properties);
-    mojo.setThriftFiles(new File[]{new File("src/test/resources/testcase.thrift")});
+    mojo.setThriftFiles(new File[] {new File("src/test/resources/testcase.thrift")});
 
     mojo.execute();
 
@@ -82,7 +79,7 @@ public void executeWithJava11() throws Exception {
   public void executeWithProcessor() throws Exception {
     mojo.setEnableConvenienceMethods(true);
     mojo.setGenerateGettersInBuilders(true);
-    mojo.setThriftFiles(new File[]{new File("src/test/resources/union.thrift")});
+    mojo.setThriftFiles(new File[] {new File("src/test/resources/union.thrift")});
 
     mojo.execute();
 
@@ -92,27 +89,40 @@ public void executeWithProcessor() throws Exception {
     assertThat(codeForUnion).exists();
 
     String result = new String(Files.readAllBytes(codeForUnion.toPath()), UTF_8);
-    assertThat(result).contains("public String " + TAG_METHOD_NAME + "() {\n" +
-        "    if (value1 != null) return \"value1\";\n" +
-        "    if (value2 != null) return \"value2\";\n" +
-        "    if (value3 != null) return \"value3\";\n" +
-        "    throw new IllegalStateException(\"Union type should have one value!\");\n" +
-        "  }");
-    assertThat(result).contains("public Object " + VALUE_METHOD_NAME + "() {\n" +
-        "    if (value1 != null) return value1;\n" +
-        "    if (value2 != null) return value2;\n" +
-        "    if (value3 != null) return value3;\n" +
-        "    throw new IllegalStateException(\"Union type should have one value!\");\n" +
-        "  }");
-    assertThat(result).contains("public static Builder builder() {\n" +
-        "    return new Builder();\n" +
-        "  }");
-    assertThat(result).contains("public static Builder builder(TestUnion prototype) {\n" +
-        "    return new Builder(prototype);\n" +
-        "  }");
-    assertThat(result).contains("public Builder toBuilder() {\n" +
-        "    return new Builder(this);\n" +
-        "  }");
+    assertThat(result)
+        .contains(
+            "public String "
+                + TAG_METHOD_NAME
+                + "() {\n"
+                + "    if (value1 != null) return \"value1\";\n"
+                + "    if (value2 != null) return \"value2\";\n"
+                + "    if (value3 != null) return \"value3\";\n"
+                + "    throw new IllegalStateException(\"Union type should have one value!\");\n"
+                + "  }");
+    assertThat(result)
+        .contains(
+            "public Object "
+                + VALUE_METHOD_NAME
+                + "() {\n"
+                + "    if (value1 != null) return value1;\n"
+                + "    if (value2 != null) return value2;\n"
+                + "    if (value3 != null) return value3;\n"
+                + "    throw new IllegalStateException(\"Union type should have one value!\");\n"
+                + "  }");
+    assertThat(result)
+        .contains("public static Builder builder() {\n" + "    return new Builder();\n" + "  }");
+    assertThat(result)
+        .contains(
+            "public static Builder builder(TestUnion prototype) {\n"
+                + "    return new Builder(prototype);\n"
+                + "  }");
+    assertThat(result)
+        .contains("public Builder toBuilder() {\n" + "    return new Builder(this);\n" + "  }");
+    assertThat(result)
+        .contains(
+            "public static TestUnion value1(String value1) {\n"
+                + "    return new Builder().value1(value1).build();\n"
+                + "  }\n");
 
     File codeForStruct = new File(outputFolder.getRoot(), "com/sentiance/thrift/TestStruct.java");
     assertThat(codeForStruct).exists();
@@ -123,7 +133,7 @@ public void executeWithProcessor() throws Exception {
 
   @Test
   public void executeWithInclude() throws Exception {
-    mojo.setThriftFiles(new File[]{new File("src/test/resources/include.thrift")});
+    mojo.setThriftFiles(new File[] {new File("src/test/resources/include.thrift")});
 
     mojo.execute();
 
@@ -134,9 +144,6 @@ public void executeWithInclude() throws Exception {
         .containsExactlyInAnyOrder(
             Paths.get(outputFolder.getRoot().getAbsolutePath(), "classes/include.thrift"),
             Paths.get(outputFolder.getRoot().getAbsolutePath(), "classes/testcase.thrift"),
-            Paths.get(outputFolder.getRoot().getAbsolutePath(), "classes/union.thrift")
-        );
-
+            Paths.get(outputFolder.getRoot().getAbsolutePath(), "classes/union.thrift"));
   }
-
-}
+}