Allow an AutoValue property builder to have a parameter.

Previously, a property builder for `abstract ImmutableSet<String> foos()` had to look like `abstract ImmutableSet.Builder<String> foosBuilder()`. For `ImmutableSortedSet`, this meant that you could only construct a builder using `ImmutableSortedSet.naturalOrder()`. Now, if you have `abstract ImmutableSortedSet<String> foos()` you can optionally write `abstract ImmutableSortedSet.Builder<String> foosBuilder(Comparator<String> comparator)`. This works because `ImmutableSortedSet.Builder<T>` has a constructor `ImmutableSortedSet.Builder(Comparator<T>)`.

This change is closely based on https://github.com/google/auto/pull/983 by Paweł Łabaj. I adjusted the formatting slightly, renamed a couple of methods, and reworded the new text in the user guide. The only substantive change is that I removed the special knowledge of the static `orderedBy` method (from `ImmutableSortedSet` and `ImmutableSortedMap`) since the builder constructor works just as well in those cases.

Closes https://github.com/google/auto/pull/983.
Fixes https://github.com/google/auto/issues/984.

RELNOTES=AutoValue property builders can now have a single parameter which is passed to the constructor of the new builder.
PiperOrigin-RevId: 358583454

3 files changed, 144 insertions, 37 deletions

diff --git a/value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifier.java b/value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifier.java
index c1db0f73..08eba48d 100644
--- a/value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifier.java
+++ b/value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifier.java
@@ -350,11 +350,21 @@ class BuilderMethodClassifier {
   }
 
   /**
-   * Classifies a method given that it has one argument. Currently, a method with one argument can
-   * only be a setter, meaning that it must look like {@code foo(T)} or {@code setFoo(T)}, where the
-   * {@code AutoValue} class has a property called {@code foo} of type {@code T}.
+   * Classifies a method given that it has one argument. A method with one argument can be:
+   *
+   * <ul>
+   *   <li>a setter, meaning that it looks like {@code foo(T)} or {@code setFoo(T)}, where the
+   *       {@code AutoValue} class has a property called {@code foo} of type {@code T};
+   *   <li>a property builder with one argument, meaning it looks like {@code
+   *       ImmutableSortedSet.Builder<V> foosBuilder(Comparator<V>)}, where the {@code AutoValue}
+   *       class has a property called {@code foos} with a type whose builder can be made with an
+   *       argument of the given type.
+   * </ul>
    */
   private void classifyMethodOneArg(ExecutableElement method) {
+    if (classifyPropertyBuilderOneArg(method)) {
+      return;
+    }
     String methodName = method.getSimpleName().toString();
     Map<String, ExecutableElement> propertyNameToGetter = getterToPropertyName.inverse();
     String propertyName = null;
@@ -417,6 +427,38 @@ class BuilderMethodClassifier {
     }
   }
 
+  /**
+   * Classifies a method given that it has one argument and is a property builder with a parameter,
+   * like {@code ImmutableSortedSet.Builder<String> foosBuilder(Comparator<String>)}.
+   *
+   * @param method A method to classify
+   * @return true if method has been classified successfully
+   */
+  private boolean classifyPropertyBuilderOneArg(ExecutableElement method) {
+    String methodName = method.getSimpleName().toString();
+    if (!methodName.endsWith("Builder")) {
+      return false;
+    }
+    String property = methodName.substring(0, methodName.length() - "Builder".length());
+    if (!getterToPropertyName.containsValue(property)) {
+      return false;
+    }
+    PropertyBuilderClassifier propertyBuilderClassifier =
+        new PropertyBuilderClassifier(
+            errorReporter,
+            typeUtils,
+            elementUtils,
+            this,
+            getterToPropertyName,
+            getterToPropertyType,
+            eclipseHack);
+    Optional<PropertyBuilder> maybePropertyBuilder =
+        propertyBuilderClassifier.makePropertyBuilder(method, property);
+    maybePropertyBuilder.ifPresent(
+        propertyBuilder -> propertyNameToPropertyBuilder.put(property, propertyBuilder));
+    return maybePropertyBuilder.isPresent();
+  }
+
   // A frequent source of problems is where the JavaBeans conventions have been followed for
   // most but not all getters. Then AutoValue considers that they haven't been followed at all,
   // so you might have a property called getFoo where you thought it was called just foo, and
diff --git a/value/src/main/java/com/google/auto/value/processor/PropertyBuilderClassifier.java b/value/src/main/java/com/google/auto/value/processor/PropertyBuilderClassifier.java
index 2565cddc..384d730a 100644
--- a/value/src/main/java/com/google/auto/value/processor/PropertyBuilderClassifier.java
+++ b/value/src/main/java/com/google/auto/value/processor/PropertyBuilderClassifier.java
@@ -15,16 +15,21 @@
  */
 package com.google.auto.value.processor;
 
+import static java.util.stream.Collectors.collectingAndThen;
+import static java.util.stream.Collectors.joining;
+import static java.util.stream.Collectors.toMap;
+
 import com.google.auto.common.MoreElements;
 import com.google.auto.common.MoreTypes;
 import com.google.common.collect.ImmutableBiMap;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
-import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
+import java.util.function.Function;
 import javax.lang.model.element.AnnotationMirror;
 import javax.lang.model.element.ElementKind;
 import javax.lang.model.element.ExecutableElement;
@@ -115,6 +120,14 @@ class PropertyBuilderClassifier {
       return propertyBuilderMethod;
     }
 
+    /** The property builder method parameters, for example {@code Comparator<T> comparator} */
+    public String getPropertyBuilderMethodParameters() {
+      return propertyBuilderMethod.getParameters().stream()
+          .map(
+              parameter -> TypeEncoder.encode(parameter.asType()) + " " + parameter.getSimpleName())
+          .collect(joining(", "));
+    }
+
     public String getAccess() {
       return SimpleMethod.access(propertyBuilderMethod);
     }
@@ -251,8 +264,13 @@ class PropertyBuilderClassifier {
       return Optional.empty();
     }
 
-    Optional<ExecutableElement> maybeBuilderMaker =
-        builderMaker(barNoArgMethods, barBuilderTypeElement);
+    Optional<ExecutableElement> maybeBuilderMaker;
+    if (method.getParameters().isEmpty()) {
+      maybeBuilderMaker = noArgBuilderMaker(barNoArgMethods, barBuilderTypeElement);
+    } else {
+      Map<String, ExecutableElement> barOneArgMethods = oneArgumentMethodsOf(barTypeElement);
+      maybeBuilderMaker = oneArgBuilderMaker(barOneArgMethods, barBuilderTypeElement);
+    }
     if (!maybeBuilderMaker.isPresent()) {
       errorReporter.reportError(
           method,
@@ -268,10 +286,14 @@ class PropertyBuilderClassifier {
 
     String barBuilderType = TypeEncoder.encodeWithAnnotations(barBuilderTypeMirror);
     String rawBarType = TypeEncoder.encodeRaw(barTypeMirror);
+    String arguments =
+        method.getParameters().isEmpty()
+            ? "()"
+            : "(" + method.getParameters().get(0).getSimpleName() + ")";
     String initializer =
         (builderMaker.getKind() == ElementKind.CONSTRUCTOR)
-            ? "new " + barBuilderType + "()"
-            : rawBarType + "." + builderMaker.getSimpleName() + "()";
+            ? "new " + barBuilderType + arguments
+            : rawBarType + "." + builderMaker.getSimpleName() + arguments;
     String builtToBuilder = null;
     String copyAll = null;
     ExecutableElement toBuilder = barNoArgMethods.get("toBuilder");
@@ -324,41 +346,75 @@ class PropertyBuilderClassifier {
   private static final ImmutableSet<String> BUILDER_METHOD_NAMES =
       ImmutableSet.of("naturalOrder", "builder", "newBuilder");
 
-  // (2) `BarBuilder must have a public no-arg constructor, or `Bar` must have a visible static
-  //      method `naturalOrder(), `builder()`, or `newBuilder()` that returns `BarBuilder`.
-  private Optional<ExecutableElement> builderMaker(
+  // (2) `BarBuilder` must have a public no-arg constructor, or `Bar` must have a visible static
+  //      method `naturalOrder(), `builder()`, or `newBuilder()` that returns `BarBuilder`; or,
+  //      if we have a foosBuilder(T) method, then `BarBuilder` must have a public constructor with
+  //      a single parameter assignable from T, or a visible static `builder(T)` method.
+  private Optional<ExecutableElement> noArgBuilderMaker(
       Map<String, ExecutableElement> barNoArgMethods, TypeElement barBuilderTypeElement) {
-    for (String builderMethodName : BUILDER_METHOD_NAMES) {
-      ExecutableElement method = barNoArgMethods.get(builderMethodName);
-      if (method != null
-          && method.getModifiers().contains(Modifier.STATIC)
-          && typeUtils.isSameType(
-              typeUtils.erasure(method.getReturnType()),
-              typeUtils.erasure(barBuilderTypeElement.asType()))) {
-        // TODO(emcmanus): check visibility. We don't want to require public for @AutoValue
-        // builders. By not checking visibility we risk accepting something as a builder maker
-        // and then failing when the generated code tries to call Bar.builder(). But the risk
-        // seems small.
-        return Optional.of(method);
-      }
+    return builderMaker(BUILDER_METHOD_NAMES, barNoArgMethods, barBuilderTypeElement, 0);
+  }
+
+  private static final ImmutableSet<String> ONE_ARGUMENT_BUILDER_METHOD_NAMES =
+      ImmutableSet.of("builder");
+
+  private Optional<ExecutableElement> oneArgBuilderMaker(
+      Map<String, ExecutableElement> barOneArgMethods, TypeElement barBuilderTypeElement) {
+
+    return builderMaker(
+        ONE_ARGUMENT_BUILDER_METHOD_NAMES, barOneArgMethods, barBuilderTypeElement, 1);
+  }
+
+  private Optional<ExecutableElement> builderMaker(
+      ImmutableSet<String> methodNamesToCheck,
+      Map<String, ExecutableElement> methods,
+      TypeElement barBuilderTypeElement,
+      int argumentCount) {
+    Optional<ExecutableElement> maybeMethod =
+        methodNamesToCheck.stream()
+            .map(methods::get)
+            .filter(Objects::nonNull)
+            .filter(method -> method.getModifiers().contains(Modifier.STATIC))
+            .filter(
+                method ->
+                    typeUtils.isSameType(
+                        typeUtils.erasure(method.getReturnType()),
+                        typeUtils.erasure(barBuilderTypeElement.asType())))
+            .findFirst();
+
+    if (maybeMethod.isPresent()) {
+      // TODO(emcmanus): check visibility. We don't want to require public for @AutoValue
+      // builders. By not checking visibility we risk accepting something as a builder maker
+      // and then failing when the generated code tries to call Bar.builder(). But the risk
+      // seems small.
+      return maybeMethod;
     }
-    return ElementFilter.constructorsIn(barBuilderTypeElement.getEnclosedElements())
-        .stream()
-        .filter(c -> c.getParameters().isEmpty())
+
+    return ElementFilter.constructorsIn(barBuilderTypeElement.getEnclosedElements()).stream()
+        .filter(c -> c.getParameters().size() == argumentCount)
         .filter(c -> c.getModifiers().contains(Modifier.PUBLIC))
         .findFirst();
   }
 
   private Map<String, ExecutableElement> noArgMethodsOf(TypeElement type) {
-    // Can't easily use ImmutableMap here because getAllMembers could return more than one method
-    // with the same name.
-    Map<String, ExecutableElement> methods = new LinkedHashMap<>();
-    for (ExecutableElement method : ElementFilter.methodsIn(elementUtils.getAllMembers(type))) {
-      if (method.getParameters().isEmpty() && !isStaticInterfaceMethodNotIn(method, type)) {
-        methods.put(method.getSimpleName().toString(), method);
-      }
-    }
-    return methods;
+    return methodsOf(type, 0);
+  }
+
+  private ImmutableMap<String, ExecutableElement> oneArgumentMethodsOf(TypeElement type) {
+    return methodsOf(type, 1);
+  }
+
+  private ImmutableMap<String, ExecutableElement> methodsOf(TypeElement type, int argumentCount) {
+    return ElementFilter.methodsIn(elementUtils.getAllMembers(type)).stream()
+        .filter(method -> method.getParameters().size() == argumentCount)
+        .filter(method -> !isStaticInterfaceMethodNotIn(method, type))
+        .collect(
+            collectingAndThen(
+                toMap(
+                    method -> method.getSimpleName().toString(),
+                    Function.identity(),
+                    (method1, method2) -> method1),
+                ImmutableMap::copyOf));
   }
 
   // Work around an Eclipse compiler bug: https://bugs.eclipse.org/bugs/show_bug.cgi?id=547185
diff --git a/value/src/main/java/com/google/auto/value/processor/autovalue.vm b/value/src/main/java/com/google/auto/value/processor/autovalue.vm
index 6f50e934..8287c5b2 100644
--- a/value/src/main/java/com/google/auto/value/processor/autovalue.vm
+++ b/value/src/main/java/com/google/auto/value/processor/autovalue.vm
@@ -291,7 +291,7 @@ ${modifiers}class $subclass$formalTypes extends $origClass$actualTypes {
     #if ($propertyBuilder)
 
     @`java.lang.Override`
-    ${propertyBuilder.access}$propertyBuilder.builderType ${p.name}Builder() {
+    ${propertyBuilder.access}$propertyBuilder.builderType ${p.name}Builder($propertyBuilder.propertyBuilderMethodParameters) {
       if (${propertyBuilder.name} == null) {
 
         ## This is the first time someone has asked for the builder. If the property it sets already
@@ -327,7 +327,16 @@ ${modifiers}class $subclass$formalTypes extends $origClass$actualTypes {
 
         #end
 
+      } #if (!$propertyBuilder.propertyBuilderMethodParameters.empty) else {
+          ## This check only happens if the property-builder method has a parameter.
+          ## We don't know if the existing builder was created with the same parameter,
+          ## so we throw to avoid possibly giving you a builder that is different from
+          ## the one you asked for.
+
+        throw new IllegalStateException("Property builder for $p.name is already defined");
       }
+        #end
+
       return $propertyBuilder.name;
     }