Merge pull request #225 from google/to.builder

Implement abstract toBuilder method

Author: alicederyn

README.md

@@ -99,15 +99,18 @@ import org.inferred.freebuilder.FreeBuilder;
 
 @FreeBuilder
 public interface Person {
-  /** Returns the person's full (English) name. */
+  /** Returns this person's full (English) name. */
   String name();
-  /** Returns the person's age in years, rounded down. */
+  /** Returns this person's age in years, rounded down. */
   int age();
+  /** Returns a new {@link Builder} with the same property values as this person. */
+  Builder toBuilder();
   /** Builder of {@link Person} instances. */
   class Builder extends Person_Builder { }
 }
 ```
 
+The `toBuilder()` method here is optional but highly recommended.
 If you are writing an abstract class, or using Java 8, you may wish to hide the
 builder's constructor and manually provide instead a static `builder()` method
 on the value type (though <em>Effective Java</em> does not do this).
@@ -133,6 +136,8 @@ If you write the Person interface shown above, you get:
      * `UnsupportedOperationException`-throwing getters for unset fields
      * `toString`
      * `equals` and `hashCode`
+     * a special `toBuilder()` method that preserves the 'partial' aspect on newly-minted
+       Person instances
 
 
 ```java
@@ -180,9 +185,7 @@ For each property `foo`, the builder gets:
 The mapper methods are very useful when modifying existing values, e.g.
 
 ```java
-Person olderPerson = Person.Builder.from(person)
-    .mapAge(age -> age + 1)
-    .build();
+Person olderPerson = person.toBuilder().mapAge(age -> age + 1).build();
 ```
 
 [UnaryOperator]: https://docs.oracle.com/javase/8/docs/api/java/util/function/UnaryOperator.html
@@ -489,7 +492,7 @@ it may violate a cross-field constraint.
 Person person = new Person.Builder()
     .name("Phil")
     .buildPartial();  // build() would throw an IllegalStateException here
-System.out.println(person);  // prints: partial Person{name="Phil"}
+System.out.println(person);  // prints: partial Person{name=Phil}
 person.age();  // throws UnsupportedOperationException
 ```
 
@@ -501,6 +504,18 @@ restrictions of the value type, partials can reduce the fragility of your test
 suite, allowing you to add new required fields or other constraints to an
 existing value type without breaking swathes of test code.
 
+To allow robust tests of modify-rebuild code, the `toBuilder()` method on
+partials returns a subclass of Builder that returns partials from the `build()`
+method.
+
+```java
+Person anotherPerson = person.toBuilder().name("Bob").build();
+System.out.println(anotherPerson);  // prints: partial Person{name=Bob}
+```
+
+(Note the `from` and `mergeFrom` methods do not behave this way; instead, they
+will throw an UnsupportedOperationException if given a partial.)
+
 
 ### Jackson
 

src/main/java/org/inferred/freebuilder/processor/Analyser.java

@@ -48,6 +48,7 @@
 import org.inferred.freebuilder.processor.Metadata.UnderrideLevel;
 import org.inferred.freebuilder.processor.PropertyCodeGenerator.Config;
 import org.inferred.freebuilder.processor.naming.NamingConvention;
+import org.inferred.freebuilder.processor.util.ModelUtils;
 import org.inferred.freebuilder.processor.util.ParameterizedType;
 import org.inferred.freebuilder.processor.util.QualifiedName;
 
@@ -138,17 +139,18 @@ Metadata analyse(TypeElement type) throws CannotGenerateCodeException {
     QualifiedName generatedBuilder = QualifiedName.of(
         pkg.getQualifiedName().toString(), generatedBuilderSimpleName(type));
     Optional<TypeElement> builder = tryFindBuilder(generatedBuilder, type);
+    Optional<BuilderFactory> builderFactory = builderFactory(builder);
     QualifiedName valueType = generatedBuilder.nestedType("Value");
     QualifiedName partialType = generatedBuilder.nestedType("Partial");
     QualifiedName propertyType = generatedBuilder.nestedType("Property");
     List<? extends TypeParameterElement> typeParameters = type.getTypeParameters();
     Map<ExecutableElement, Property> properties =
-        findProperties(type, removeUnderriddenAndConcreteMethods(methods));
+        findProperties(type, removeNonGetterMethods(builder, methods));
     Metadata.Builder metadataBuilder = new Metadata.Builder()
         .setType(QualifiedName.of(type).withParameters(typeParameters))
         .setInterfaceType(type.getKind().isInterface())
         .setBuilder(parameterized(builder, typeParameters))
-        .setBuilderFactory(builderFactory(builder))
+        .setBuilderFactory(builderFactory)
         .setGeneratedBuilder(generatedBuilder.withParameters(typeParameters))
         .setValueType(valueType.withParameters(typeParameters))
         .setPartialType(partialType.withParameters(typeParameters))
@@ -158,6 +160,7 @@ Metadata analyse(TypeElement type) throws CannotGenerateCodeException {
         .addVisibleNestedTypes(propertyType)
         .addAllVisibleNestedTypes(visibleTypesIn(type))  // Because we inherit from type
         .putAllStandardMethodUnderrides(findUnderriddenMethods(methods))
+        .setHasToBuilderMethod(hasToBuilderMethod(builder, builderFactory, methods))
         .setBuilderSerializable(shouldBuilderBeSerializable(builder))
         .addAllProperties(properties.values());
     Metadata baseMetadata = metadataBuilder.build();
@@ -306,13 +309,48 @@ private Map<StandardMethod, UnderrideLevel> findUnderriddenMethods(
     return result.build();
   }
 
-  private static Set<ExecutableElement> removeUnderriddenAndConcreteMethods(
+  /** Find a toBuilder method, if the user has provided one.
+   * @param builderFactory */
+  private boolean hasToBuilderMethod(
+      Optional<TypeElement> builder,
+      Optional<BuilderFactory> builderFactory,
       Iterable<ExecutableElement> methods) {
+    if (!builder.isPresent()) {
+      return false;
+    }
+    for (ExecutableElement method : methods) {
+      if (isToBuilderMethod(builder.get(), method)) {
+        if (!builderFactory.isPresent()) {
+          messager.printMessage(ERROR,
+              "No accessible no-args Builder constructor available to implement toBuilder",
+              method);
+        }
+        return true;
+      }
+    }
+    return false;
+  }
+
+  private static boolean isToBuilderMethod(TypeElement builder, ExecutableElement method) {
+      if (method.getSimpleName().contentEquals("toBuilder")
+          && method.getModifiers().contains(Modifier.ABSTRACT)
+          && method.getParameters().isEmpty()) {
+        Optional<TypeElement> returnType = ModelUtils.maybeAsTypeElement(method.getReturnType());
+        if (returnType.isPresent() && returnType.get().equals(builder)) {
+          return true;
+        }
+      }
+      return false;
+  }
+
+  private static Set<ExecutableElement> removeNonGetterMethods(
+      Optional<TypeElement> builder, Iterable<ExecutableElement> methods) {
     ImmutableSet.Builder<ExecutableElement> nonUnderriddenMethods = ImmutableSet.builder();
     for (ExecutableElement method : methods) {
       boolean isAbstract = method.getModifiers().contains(Modifier.ABSTRACT);
       boolean isStandardMethod = maybeStandardMethod(method).isPresent();
-      if (isAbstract && !isStandardMethod) {
+      boolean isToBuilderMethod = builder.isPresent() && isToBuilderMethod(builder.get(), method);
+      if (isAbstract && !isStandardMethod && !isToBuilderMethod) {
         nonUnderriddenMethods.add(method);
       }
     }

src/main/java/org/inferred/freebuilder/processor/BuildablePropertyFactory.java

@@ -66,6 +66,11 @@ private enum MergeBuilderMethod {
     MERGE_DIRECTLY, BUILD_PARTIAL_AND_MERGE
   }
 
+  /** How to convert a partial value into a Builder. */
+  private enum PartialToBuilderMethod {
+    MERGE_DIRECTLY, TO_BUILDER_AND_MERGE
+  }
+
   @Override
   public Optional<? extends PropertyCodeGenerator> create(Config config) {
     DeclaredType type = maybeDeclared(config.getProperty().getType()).orNull();
@@ -136,30 +141,48 @@ public Optional<? extends PropertyCodeGenerator> create(Config config) {
       }
     }
 
+    List<ExecutableElement> valueMethods = FluentIterable
+        .from(config.getElements().getAllMembers(element))
+        .filter(ExecutableElement.class)
+        .filter(new IsCallableMethod())
+        .toList();
+
+    // Check whether there is a toBuilder() method
+    PartialToBuilderMethod partialToBuilderMethod;
+    if (any(valueMethods, new IsToBuilderMethod(builder.get().asType(), config.getTypes()))) {
+      partialToBuilderMethod = PartialToBuilderMethod.TO_BUILDER_AND_MERGE;
+    } else {
+      partialToBuilderMethod = PartialToBuilderMethod.MERGE_DIRECTLY;
+    }
+
     return Optional.of(new CodeGenerator(
         config.getMetadata(),
         config.getProperty(),
         ParameterizedType.from(builder.get()),
         builderFactory.get(),
-        mergeFromBuilderMethod));
+        mergeFromBuilderMethod,
+        partialToBuilderMethod));
   }
 
   @VisibleForTesting static class CodeGenerator extends PropertyCodeGenerator {
 
-    final ParameterizedType builderType;
-    final BuilderFactory builderFactory;
-    final MergeBuilderMethod mergeFromBuilderMethod;
+    private final ParameterizedType builderType;
+    private final BuilderFactory builderFactory;
+    private final MergeBuilderMethod mergeFromBuilderMethod;
+    private final PartialToBuilderMethod partialToBuilderMethod;
 
-    CodeGenerator(
+    private CodeGenerator(
         Metadata metadata,
         Property property,
         ParameterizedType builderType,
         BuilderFactory builderFactory,
-        MergeBuilderMethod mergeFromBuilderMethod) {
+        MergeBuilderMethod mergeFromBuilderMethod,
+        PartialToBuilderMethod partialToBuilderMethod) {
       super(metadata, property);
       this.builderType = builderType;
       this.builderFactory = builderFactory;
       this.mergeFromBuilderMethod = mergeFromBuilderMethod;
+      this.partialToBuilderMethod = partialToBuilderMethod;
     }
 
     @Override
@@ -285,6 +308,17 @@ public void addMergeFromBuilder(Block code, String builder) {
       code.add(");\n");
     }
 
+    @Override
+    public void addSetBuilderFromPartial(Block code, String builder) {
+      if (partialToBuilderMethod == PartialToBuilderMethod.TO_BUILDER_AND_MERGE) {
+        code.add("%s.%s().mergeFrom(%s.toBuilder());",
+            builder, getBuilderMethod(property), setter(property), property.getName());
+      } else {
+        code.add("%s.%s().mergeFrom(%s);",
+            builder, getBuilderMethod(property), setter(property), property.getName());
+      }
+    }
+
     @Override
     public void addSetFromResult(SourceBuilder code, String builder, String variable) {
       code.addLine("%s.%s(%s);", builder, setter(property), variable);
@@ -367,6 +401,29 @@ private static final class IsMergeFromMethod implements Predicate<ExecutableElem
     }
   }
 
+  private static final class IsToBuilderMethod implements Predicate<ExecutableElement> {
+    final TypeMirror builderType;
+    final Types types;
+
+    IsToBuilderMethod(TypeMirror sourceType, Types types) {
+      this.builderType = sourceType;
+      this.types = types;
+    }
+
+    @Override public boolean apply(ExecutableElement element) {
+      if (element.getParameters().size() != 0) {
+        return false;
+      }
+      if (!element.getSimpleName().contentEquals("toBuilder")) {
+        return false;
+      }
+      if (!types.isSubtype(element.getReturnType(), builderType)) {
+        return false;
+      }
+      return true;
+    }
+  }
+
   private static final Predicate<Element> IS_BUILDER_TYPE = new Predicate<Element>() {
     @Override public boolean apply(Element element) {
       return element.getSimpleName().contentEquals("Builder")

src/main/java/org/inferred/freebuilder/processor/CodeGenerator.java

@@ -231,16 +231,26 @@ private static void addBuildPartialMethod(SourceBuilder code, Metadata metadata)
     code.addLine("")
         .addLine("/**")
         .addLine(" * Returns a newly-created partial %s", metadata.getType().javadocLink())
-        .addLine(" * based on the contents of the {@code %s}.",
-            metadata.getBuilder().getSimpleName())
-        .addLine(" * State checking will not be performed.");
+        .addLine(" * for use in unit tests. State checking will not be performed.");
     if (any(metadata.getProperties(), IS_REQUIRED)) {
       code.addLine(" * Unset properties will throw an {@link %s}",
               UnsupportedOperationException.class)
           .addLine(" * when accessed via the partial object.");
     }
+    if (metadata.getHasToBuilderMethod()
+        && metadata.getBuilderFactory() == Optional.of(BuilderFactory.NO_ARGS_CONSTRUCTOR)) {
+      code.addLine(" *")
+          .addLine(" * <p>The builder returned by a partial's {@link %s#toBuilder() toBuilder}",
+              metadata.getType())
+          .addLine(" * method overrides {@link %s#build() build()} to return another partial.",
+              metadata.getBuilder())
+          .addLine(" * This allows for robust tests of modify-rebuild code.");
+    }
     code.addLine(" *")
-        .addLine(" * <p>Partials should only ever be used in tests.")
+        .addLine(" * <p>Partials should only ever be used in tests. They permit writing robust")
+        .addLine(" * test cases that won't fail if this type gains more application-level")
+        .addLine(" * constraints (e.g. new required fields) in future. If you require partially")
+        .addLine(" * complete values in production code, consider using a Builder.")
         .addLine(" */");
     if (code.feature(GUAVA).isAvailable()) {
       code.addLine("@%s()", VisibleForTesting.class);
@@ -308,6 +318,20 @@ private static void addValueType(SourceBuilder code, Metadata metadata) {
       code.add(";\n");
       code.addLine("  }");
     }
+    // toBuilder
+    if (metadata.getHasToBuilderMethod()) {
+      code.addLine("")
+          .addLine("  @%s", Override.class)
+          .addLine("  public %s toBuilder() {", metadata.getBuilder());
+      BuilderFactory builderFactory = metadata.getBuilderFactory().orNull();
+      if (builderFactory != null) {
+        code.addLine("    return %s.mergeFrom(this);",
+                builderFactory.newBuilder(metadata.getBuilder(), EXPLICIT_TYPES));
+      } else {
+        code.addLine("    throw new %s();", UnsupportedOperationException.class);
+      }
+      code.addLine("  }");
+    }
     // Equals
     switch (metadata.standardMethodUnderride(StandardMethod.EQUALS)) {
       case ABSENT:
@@ -510,6 +534,7 @@ private static void addPartialType(SourceBuilder code, Metadata metadata) {
       code.add(";\n");
       code.addLine("  }");
     }
+    addPartialToBuilderMethod(code, metadata);
     // Equals
     if (metadata.standardMethodUnderride(StandardMethod.EQUALS) != FINAL) {
       code.addLine("")
@@ -603,6 +628,38 @@ private static void addPartialType(SourceBuilder code, Metadata metadata) {
     code.addLine("}");
   }
 
+  private static void addPartialToBuilderMethod(SourceBuilder code, Metadata metadata) {
+    if (!metadata.getHasToBuilderMethod()) {
+      return;
+    }
+    BuilderFactory builderFactory = metadata.getBuilderFactory().orNull();
+    if (builderFactory == BuilderFactory.NO_ARGS_CONSTRUCTOR) {
+      code.addLine("")
+          .addLine("  private static class PartialBuilder%s extends %s {",
+              metadata.getBuilder().declarationParameters(), metadata.getBuilder())
+          .addLine("    @Override public %s build() {", metadata.getType())
+          .addLine("      return buildPartial();")
+          .addLine("    }")
+          .addLine("  }");
+    }
+    code.addLine("")
+        .addLine("  @%s", Override.class)
+        .addLine("  public %s toBuilder() {", metadata.getBuilder());
+    if (builderFactory == BuilderFactory.NO_ARGS_CONSTRUCTOR) {
+      code.addLine("    %s builder = new PartialBuilder%s();",
+              metadata.getBuilder(), metadata.getBuilder().typeParametersOrDiamondOperator());
+      Block block = new Block(code);
+      for (Property property : metadata.getProperties()) {
+        property.getCodeGenerator().addSetBuilderFromPartial(block, "builder");
+      }
+      code.add(block)
+          .addLine("    return builder;");
+    } else {
+      code.addLine("    throw new %s();", UnsupportedOperationException.class);
+    }
+    code.addLine("  }");
+  }
+
   private static void writeToStringWithBuilder(
       SourceBuilder code, Metadata metadata, boolean isPartial) {
     code.addLine("%1$s result = new %1$s(\"%2$s%3$s{\");",

src/main/java/org/inferred/freebuilder/processor/DefaultPropertyFactory.java

@@ -223,6 +223,18 @@ public void addMergeFromBuilder(Block code, String builder) {
       }
     }
 
+    @Override
+    public void addSetBuilderFromPartial(Block code, String builder) {
+      if (!hasDefault) {
+        code.add("if (!_unsetProperties.contains(%s.%s)) {",
+            metadata.getPropertyEnum(), property.getAllCapsName());
+      }
+      code.addLine("  %s.%s(%s);", builder, setter(property), property.getName());
+      if (!hasDefault) {
+        code.addLine("}");
+      }
+    }
+
     @Override
     public void addSetFromResult(SourceBuilder code, String builder, String variable) {
       code.addLine("%s.%s(%s);", builder, setter(property), variable);