When property Bar bar() has a builder BarBuilder barBuilder(), do…

…n't require there to be a `Bar.toBuilder()` unless it is actually needed.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=141600782

value/src/it/functional/src/test/java/com/google/auto/value/AutoValueTest.java

@@ -2155,10 +2155,6 @@ public MyMap() {}
     public MyMap(Map<K, V> map) {
       super(map);
     }
-
-    public MyMapBuilder<K, V> toBuilder() {
-      return new MyMapBuilder<K, V>(this);
-    }
   }
 
   public static class MyMapBuilder<K, V> extends LinkedHashMap<K, V> {
@@ -2203,7 +2199,7 @@ public MyStringMap(Map<String, V> map) {
       super(map);
     }
 
-    @Override public MyStringMapBuilder<V> toBuilder() {
+    public MyStringMapBuilder<V> toBuilder() {
       return new MyStringMapBuilder<V>(this);
     }
   }

value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifier.java

@@ -102,9 +102,13 @@ private BuilderMethodClassifier(
    *
    * @param methods the methods in {@code builderType} and its ancestors.
    * @param errorReporter where to report errors.
+   * @param processingEnv the ProcessingEnvironment for annotation processing.
    * @param autoValueClass the {@code AutoValue} class containing the builder.
    * @param builderType the builder class or interface within {@code autoValueClass}.
    * @param getterToPropertyName a map from getter methods to the properties they get.
+   * @param typeSimplifier the TypeSimplifier that will be used to control imports.
+   * @param autoValueHasToBuilder true if the containing {@code @AutoValue} class has a
+   *     {@code toBuilder()} method.
    *
    * @return an {@code Optional} that contains the results of the classification if it was
    *     successful or nothing if it was not.
@@ -116,15 +120,16 @@ static Optional<BuilderMethodClassifier> classify(
       TypeElement autoValueClass,
       TypeElement builderType,
       ImmutableBiMap<ExecutableElement, String> getterToPropertyName,
-      TypeSimplifier typeSimplifier) {
+      TypeSimplifier typeSimplifier,
+      boolean autoValueHasToBuilder) {
     BuilderMethodClassifier classifier = new BuilderMethodClassifier(
         errorReporter,
         processingEnv,
         autoValueClass,
         builderType,
         getterToPropertyName,
         typeSimplifier);
-    if (classifier.classifyMethods(methods)) {
+    if (classifier.classifyMethods(methods, autoValueHasToBuilder)) {
       return Optional.of(classifier);
     } else {
       return Optional.absent();
@@ -169,7 +174,8 @@ Set<ExecutableElement> buildMethods() {
   /**
    * Classifies the given methods and sets the state of this object based on what is found.
    */
-  private boolean classifyMethods(Iterable<ExecutableElement> methods) {
+  private boolean classifyMethods(
+      Iterable<ExecutableElement> methods, boolean autoValueHasToBuilder) {
     boolean ok = true;
     for (ExecutableElement method : methods) {
       ok &= classifyMethod(method);
@@ -191,16 +197,37 @@ private boolean classifyMethods(Iterable<ExecutableElement> methods) {
     }
     for (Map.Entry<ExecutableElement, String> getterEntry : getterToPropertyName.entrySet()) {
       String property = getterEntry.getValue();
+      String propertyType = typeSimplifier.simplify(getterEntry.getKey().getReturnType());
       boolean hasSetter = propertyNameToSetter.containsKey(property);
-      boolean hasBuilder = propertyNameToPropertyBuilder.containsKey(property);
-      if (!hasSetter && !hasBuilder) {
-        // TODO(emcmanus): also mention the possible builder method if the property type allows one
+      PropertyBuilder propertyBuilder = propertyNameToPropertyBuilder.get(property);
+      boolean hasBuilder = propertyBuilder != null;
+      if (hasBuilder) {
+        // If property bar of type Bar has a barBuilder() that returns BarBuilder, then it must be
+        // possible to make a BarBuilder from a Bar if either (1) the @AutoValue class has a
+        // toBuilder() or (2) there is also a setBar(Bar). Making BarBuilder from Bar is possible
+        // if Bar either has a toBuilder() method or is a Guava immutable collection (in which case
+        // we can use addAll or putAll).
+        boolean canMakeBarBuilder =
+            (propertyBuilder.getBuiltToBuilder() != null || propertyBuilder.getCopyAll() != null);
+        boolean needToMakeBarBuilder = (autoValueHasToBuilder || hasSetter);
+        if (needToMakeBarBuilder && !canMakeBarBuilder) {
+          String error = String.format(
+              "Property builder method returns %1$s but there is no way to make that type from "
+                  + "%2$s: %2$s does not have a non-static toBuilder() method that returns %1$s",
+              propertyBuilder.getBuilderType(),
+              propertyType);
+          errorReporter.reportError(error, propertyBuilder.getPropertyBuilderMethod());
+        }
+      } else if (!hasSetter) {
+        // We have neither barBuilder() nor setBar(Bar), so we should complain.
         String setterName = settersPrefixed ? prefixWithSet(property) : property;
-        String error = String.format("Expected a method with this signature: %s%s %s(%s)",
+        String error = String.format(
+            "Expected a method with this signature: %s%s %s(%s), or a %sBuilder() method",
             builderType,
             typeParamsString(),
             setterName,
-            getterEntry.getKey().getReturnType());
+            propertyType,
+            property);
         errorReporter.reportError(error, builderType);
         ok = false;
       }

value/src/main/java/com/google/auto/value/processor/BuilderSpec.java

@@ -102,6 +102,7 @@ Optional<Builder> getBuilder() {
    */
   class Builder {
     private final TypeElement builderTypeElement;
+    private ImmutableSet<ExecutableElement> toBuilderMethods;
 
     Builder(TypeElement builderTypeElement) {
       this.builderTypeElement = builderTypeElement;
@@ -157,6 +158,7 @@ ImmutableSet<ExecutableElement> toBuilderMethods(
         errorReporter.reportError(
             "There can be at most one builder converter method", builderMethods.iterator().next());
       }
+      this.toBuilderMethods = builderMethods;
       return builderMethods;
     }
 
@@ -183,14 +185,16 @@ void defineVars(
         TypeSimplifier typeSimplifier,
         ImmutableBiMap<ExecutableElement, String> getterToPropertyName) {
       Iterable<ExecutableElement> builderMethods = abstractMethods(builderTypeElement);
+      boolean autoValueHasToBuilder = !toBuilderMethods.isEmpty();
       Optional<BuilderMethodClassifier> optionalClassifier = BuilderMethodClassifier.classify(
           builderMethods,
           errorReporter,
           processingEnv,
           autoValueClass,
           builderTypeElement,
           getterToPropertyName,
-          typeSimplifier);
+          typeSimplifier,
+          autoValueHasToBuilder);
       if (!optionalClassifier.isPresent()) {
         return;
       }

value/src/main/java/com/google/auto/value/processor/PropertyBuilderClassifier.java

@@ -82,6 +82,7 @@ class PropertyBuilderClassifier {
    * {@code barBuilder()} might return {@code ImmutableSet.Builder<String>}.
    */
   public static class PropertyBuilder {
+    private final ExecutableElement propertyBuilderMethod;
     private final String name;
     private final String builderType;
     private final String initializer;
@@ -96,6 +97,7 @@ public static class PropertyBuilder {
         String initEmpty,
         String builtToBuilder,
         String copyAll) {
+      this.propertyBuilderMethod = propertyBuilderMethod;
       this.name = propertyBuilderMethod.getSimpleName() + "$";
       this.builderType = builderType;
       this.initializer = initializer;
@@ -104,6 +106,11 @@ public static class PropertyBuilder {
       this.copyAll = copyAll;
     }
 
+    /** The property builder method, for example {@code barBuilder()}. */
+    public ExecutableElement getPropertyBuilderMethod() {
+      return propertyBuilderMethod;
+    }
+
     /** The name of the field to hold this builder. */
     public String getName() {
       return name;
@@ -238,19 +245,6 @@ Optional<PropertyBuilder> makePropertyBuilder(ExecutableElement method, String p
       Optional<ExecutableElement> maybeCopyAll = addAllPutAll(barBuilderTypeElement);
       if (maybeCopyAll.isPresent() && isGuavaBuilder) {
         copyAll = maybeCopyAll.get().getSimpleName().toString();
-      } else {
-        // TODO(emcmanus): relax the condition here by not requiring that there be a way to make
-        // BarBuilder from Bar. That is needed if the containing @AutoValue class Foo has a
-        // toBuilder() method, or if there is also a setter for bar. Unfortunately we emit a
-        // second package-private constructor AutoValue_Foo.Builder(Foo) which can be used instead
-        // of toBuilder(). That's not documented, and we should stop doing it, because we can't
-        // know if anyone calls the constructor and therefore we have to assume that we'll need
-        // to make BarBuilder from Bar.
-        errorReporter.reportError("Property builder method returns " + barBuilderTypeMirror
-            + " but there is no way to make that type from " + barTypeMirror + ": "
-            + barTypeMirror + " does not have a non-static toBuilder() method that returns "
-            + barBuilderTypeMirror, method);
-        return Optional.absent();
       }
     }
     ExecutableElement barOf = barNoArgMethods.get("of");

value/src/main/java/com/google/auto/value/processor/autovalue.vm

@@ -281,6 +281,14 @@ $a
         ## has a value (because it came from a toBuilder() call on the AutoValue class, or because
         ## there is also a setter for this property) then we copy that value into the builder.
         ## Otherwise the builder starts out empty.
+        ## If we have neither a setter nor a toBuilder() method, then the builder always starts
+        ## off empty.
+
+        #if ($builderSetters[$p.name].empty && $toBuilderMethods.empty)
+
+        ${propertyBuilder.name} = ${propertyBuilder.initializer};
+
+        #else
 
         if ($p == null) {
           ${propertyBuilder.name} = ${propertyBuilder.initializer};
@@ -299,6 +307,9 @@ $a
 
           $p = null;
         }
+
+        #end
+
       }
       return $propertyBuilder.name;
     }

value/src/test/java/com/google/auto/value/processor/CompilationTest.java

@@ -1558,6 +1558,7 @@ public void autoValueBuilderPropertyBuilderCantReconstruct() {
         "@AutoValue",
         "public abstract class Baz<E> {",
         "  abstract String blim();",
+        "  abstract Builder toBuilder();",
         "",
         "  public static class StringFactory {",
         "    public String build() {",
@@ -1576,10 +1577,47 @@ public void autoValueBuilderPropertyBuilderCantReconstruct() {
         .processedWith(new AutoValueProcessor(), new AutoValueBuilderProcessor())
         .failsToCompile()
         .withErrorContaining(
-            "Property builder method returns foo.bar.Baz.StringFactory but there is no way to make "
-                + "that type from java.lang.String: java.lang.String does not have a non-static "
-                + "toBuilder() method that returns foo.bar.Baz.StringFactory")
-        .in(javaFileObject).onLine(18);
+            "Property builder method returns Baz.StringFactory but there is no way to make "
+                + "that type from String: String does not have a non-static "
+                + "toBuilder() method that returns Baz.StringFactory")
+        .in(javaFileObject).onLine(19);
+  }
+
+  @Test
+  public void autoValueBuilderPropertyBuilderCantSet() {
+    JavaFileObject javaFileObject = JavaFileObjects.forSourceLines(
+        "foo.bar.Baz",
+        "package foo.bar;",
+        "",
+        "import com.google.auto.value.AutoValue;",
+        "import com.google.common.collect.ImmutableSet;",
+        "",
+        "@AutoValue",
+        "public abstract class Baz<E> {",
+        "  abstract String blim();",
+        "",
+        "  public static class StringFactory {",
+        "    public String build() {",
+        "      return null;",
+        "    }",
+        "  }",
+        "",
+        "  @AutoValue.Builder",
+        "  public interface Builder<E> {",
+        "    Builder<E> setBlim(String s);",
+        "    StringFactory blimBuilder();",
+        "    Baz<E> build();",
+        "  }",
+        "}");
+    assertAbout(javaSource())
+        .that(javaFileObject)
+        .processedWith(new AutoValueProcessor(), new AutoValueBuilderProcessor())
+        .failsToCompile()
+        .withErrorContaining(
+            "Property builder method returns Baz.StringFactory but there is no way to make "
+                + "that type from String: String does not have a non-static "
+                + "toBuilder() method that returns Baz.StringFactory")
+        .in(javaFileObject).onLine(19);
   }
 
   @Test
@@ -1594,6 +1632,7 @@ public void autoValueBuilderPropertyBuilderWrongTypeToBuilder() {
         "@AutoValue",
         "public abstract class Baz<E> {",
         "  abstract Buh blim();",
+        "  abstract Builder<E> toBuilder();",
         "",
         "  public static class Buh {",
         "    StringBuilder toBuilder() {",
@@ -1618,10 +1657,10 @@ public void autoValueBuilderPropertyBuilderWrongTypeToBuilder() {
         .processedWith(new AutoValueProcessor(), new AutoValueBuilderProcessor())
         .failsToCompile()
         .withErrorContaining(
-            "Property builder method returns foo.bar.Baz.BuhBuilder but there is no way to make "
-                + "that type from foo.bar.Baz.Buh: foo.bar.Baz.Buh does not have a non-static "
-                + "toBuilder() method that returns foo.bar.Baz.BuhBuilder")
-        .in(javaFileObject).onLine(24);
+            "Property builder method returns Baz.BuhBuilder but there is no way to make "
+                + "that type from Baz.Buh: Baz.Buh does not have a non-static "
+                + "toBuilder() method that returns Baz.BuhBuilder")
+        .in(javaFileObject).onLine(25);
   }
 
   @Test