diff --git a/value/src/it/functional/src/test/java/com/google/auto/value/AutoBuilderKotlinTest.java b/value/src/it/functional/src/test/java/com/google/auto/value/AutoBuilderKotlinTest.java index 07934c848d..d65607f883 100644 --- a/value/src/it/functional/src/test/java/com/google/auto/value/AutoBuilderKotlinTest.java +++ b/value/src/it/functional/src/test/java/com/google/auto/value/AutoBuilderKotlinTest.java @@ -117,6 +117,13 @@ public void kotlinWithDefaults_defaulted() { assertThat(x.getAnImmutableList()).containsExactly("foo"); assertThat(x.getAString()).isEqualTo("skidoo"); assertThat(x.getNotDefaulted()).isEqualTo(100L); + KotlinDataWithDefaults copy = + new AutoBuilder_AutoBuilderKotlinTest_KotlinDataWithDefaultsBuilder(x).build(); + assertThat(copy).isEqualTo(x); + assertThat(copy).isNotSameInstanceAs(x); + KotlinDataWithDefaults modified = + new AutoBuilder_AutoBuilderKotlinTest_KotlinDataWithDefaultsBuilder(x).setAnInt(17).build(); + assertThat(modified.getAnInt()).isEqualTo(17); } @Test diff --git a/value/src/it/functional/src/test/java/com/google/auto/value/AutoBuilderTest.java b/value/src/it/functional/src/test/java/com/google/auto/value/AutoBuilderTest.java index a9baa51646..0d8735958c 100644 --- a/value/src/it/functional/src/test/java/com/google/auto/value/AutoBuilderTest.java +++ b/value/src/it/functional/src/test/java/com/google/auto/value/AutoBuilderTest.java @@ -210,8 +210,11 @@ public void simpleAutoAnnotation() { @AutoBuilder(ofClass = MyAnnotation.class) public interface MyAnnotationSimpleBuilder { MyAnnotationSimpleBuilder value(String x); + MyAnnotationSimpleBuilder id(int x); + MyAnnotationSimpleBuilder truthiness(Truthiness x); + MyAnnotation build(); } @@ -248,7 +251,9 @@ public void buildWithoutAutoAnnotation() { @AutoBuilder(ofClass = MyAnnotation.class) public interface MyAnnotationSimplerBuilder { MyAnnotationSimplerBuilder value(String x); + MyAnnotationSimplerBuilder id(int x); + MyAnnotation build(); } @@ -665,4 +670,47 @@ public void genericGetters() { assertThat(builder.getFirst()).isEqualTo(1.0); assertThat(builder.build()).containsExactly(1.0, 2).inOrder(); } + + static class NumberHolder { + private final T number; + + NumberHolder(T number) { + this.number = number; + } + + T getNumber() { + return number; + } + } + + static NumberHolder buildNumberHolder(T number) { + return new NumberHolder<>(number); + } + + @AutoBuilder(callMethod = "buildNumberHolder") + interface NumberHolderBuilder { + NumberHolderBuilder setNumber(T number); + + NumberHolder build(); + } + + static NumberHolderBuilder numberHolderBuilder() { + return new AutoBuilder_AutoBuilderTest_NumberHolderBuilder<>(); + } + + static NumberHolderBuilder numberHolderBuilder( + NumberHolder numberHolder) { + return new AutoBuilder_AutoBuilderTest_NumberHolderBuilder<>(numberHolder); + } + + @Test + public void builderFromInstance() { + NumberHolder instance1 = + AutoBuilderTest.numberHolderBuilder().setNumber(23).build(); + assertThat(instance1.getNumber()).isEqualTo(23); + NumberHolder instance2 = numberHolderBuilder(instance1).build(); + assertThat(instance2.getNumber()).isEqualTo(23); + NumberHolder instance3 = numberHolderBuilder(instance2).setNumber(17).build(); + assertThat(instance3.getNumber()).isEqualTo(17); + } } diff --git a/value/src/main/java/com/google/auto/value/processor/AutoBuilderProcessor.java b/value/src/main/java/com/google/auto/value/processor/AutoBuilderProcessor.java index 8fdede6294..620d80d4a8 100644 --- a/value/src/main/java/com/google/auto/value/processor/AutoBuilderProcessor.java +++ b/value/src/main/java/com/google/auto/value/processor/AutoBuilderProcessor.java @@ -37,13 +37,11 @@ import com.google.auto.common.MoreTypes; import com.google.auto.common.Visibility; import com.google.auto.service.AutoService; -import com.google.auto.value.processor.BuilderSpec.PropertyGetter; import com.google.auto.value.processor.MissingTypes.MissingTypeException; import com.google.common.base.Ascii; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Maps; import java.io.IOException; import java.io.OutputStream; import java.io.UncheckedIOException; @@ -56,6 +54,7 @@ import java.util.NavigableSet; import java.util.Optional; import java.util.Set; +import java.util.TreeMap; import java.util.TreeSet; import java.util.stream.Stream; import javax.annotation.processing.ProcessingEnvironment; @@ -203,8 +202,8 @@ private void processType(TypeElement autoBuilderType, TypeElement ofClass, Strin return; } BuilderMethodClassifier classifier = maybeClassifier.get(); - Map propertyToGetterName = - Maps.transformValues(classifier.builderGetters(), PropertyGetter::getName); + ImmutableMap propertyToGetterName = + propertyToGetterName(executable, autoBuilderType); AutoBuilderTemplateVars vars = new AutoBuilderTemplateVars(); vars.props = propertySet(executable, propertyToGetterName, propertyInitializers); builder.defineVars(vars, classifier); @@ -217,7 +216,7 @@ private void processType(TypeElement autoBuilderType, TypeElement ofClass, Strin forwardingClassName .map(n -> TypeSimplifier.simpleNameOf(n) + ".of") .orElseGet(executable::invoke); - vars.toBuilderConstructor = false; + vars.toBuilderConstructor = !propertyToGetterName.isEmpty(); vars.toBuilderMethods = ImmutableList.of(); defineSharedVarsForType(autoBuilderType, ImmutableSet.of(), vars); String text = vars.toText(); @@ -350,6 +349,72 @@ private ImmutableMap propertyInitializers( return builder.build(); } + /** + * Returns a map from property names to the corresponding getters in the built type. The built + * type is the return type of the given {@code executable}, and the property names are the names + * of its parameters. If the return type is a {@link DeclaredType} {@code Foo} and if every + * property name {@code bar} matches a method {@code bar()} or {@code getBar()} in {@code Foo}, + * then the method returns a map where {@code bar} maps to {@code bar} or {@code getBar}. If these + * conditions are not met then the method returns an empty map. + * + *

The method name match is case-insensitive, so we will also accept {@code baR()} or {@code + * getbar()}. For a property of type {@code boolean}, we also accept {@code isBar()} (or {@code + * isbar()} etc). + * + *

The return type of each getter method must match the type of the corresponding parameter + * exactly. This will always be true for our principal use cases, Java records and Kotlin data + * classes. For other use cases, we may in the future accept getters where we know how to convert, + * for example if the getter has type {@code ImmutableList} and the parameter has type + * {@code Baz[]}. We already have similar logic for the parameter types of builder setters. + */ + private ImmutableMap propertyToGetterName( + Executable executable, TypeElement autoBuilderType) { + TypeMirror builtType = executable.builtType(); + if (builtType.getKind() != TypeKind.DECLARED) { + return ImmutableMap.of(); + } + TypeElement type = MoreTypes.asTypeElement(builtType); + Map noArgInstanceMethods = + MoreElements.getLocalAndInheritedMethods(type, typeUtils(), elementUtils()).stream() + .filter(m -> m.getParameters().isEmpty()) + .filter(m -> !m.getModifiers().contains(Modifier.STATIC)) + .filter(m -> visibleFrom(autoBuilderType, getPackage(autoBuilderType))) + .collect( + toMap( + m -> m.getSimpleName().toString(), + m -> m, + (a, b) -> a, + () -> new TreeMap<>(String.CASE_INSENSITIVE_ORDER))); + ImmutableMap propertyToGetterName = + executable.parameters().stream() + .map( + param -> { + String name = param.getSimpleName().toString(); + // Parameter name is `bar`; we look for `bar()` and `getBar()` (or `getbar()` etc) + // in that order. If `bar` is boolean we also look for `isBar()`. + ExecutableElement getter = noArgInstanceMethods.get(name); + if (getter == null) { + getter = noArgInstanceMethods.get("get" + name); + if (getter == null && param.asType().getKind() == TypeKind.BOOLEAN) { + getter = noArgInstanceMethods.get("is" + name); + } + } + if (getter != null + && !MoreTypes.equivalence() + .equivalent(getter.getReturnType(), param.asType())) { + getter = null; + } + return new SimpleEntry<>(name, getter); + }) + .filter(entry -> entry.getValue() != null) + .collect( + toImmutableMap( + Map.Entry::getKey, entry -> entry.getValue().getSimpleName().toString())); + return (propertyToGetterName.size() == executable.parameters().size()) + ? propertyToGetterName + : ImmutableMap.of(); + } + private Executable findExecutable( TypeElement ofClass, String callMethod, diff --git a/value/src/main/java/com/google/auto/value/processor/AutoValueOrBuilderTemplateVars.java b/value/src/main/java/com/google/auto/value/processor/AutoValueOrBuilderTemplateVars.java index eddb9ae25e..db711857bd 100644 --- a/value/src/main/java/com/google/auto/value/processor/AutoValueOrBuilderTemplateVars.java +++ b/value/src/main/java/com/google/auto/value/processor/AutoValueOrBuilderTemplateVars.java @@ -124,15 +124,14 @@ abstract class AutoValueOrBuilderTemplateVars extends AutoValueishTemplateVars { /** * True if the generated builder should have a second constructor with a parameter of the built - * class. The constructor produces a new builder that starts off with the values from the + * type. The constructor produces a new builder that starts off with the values from the * parameter. */ Boolean toBuilderConstructor; /** * Any {@code toBuilder()} methods, that is methods that return the builder type. AutoBuilder does - * not currently support this, but it's included in these shared variables to simplify the - * template. + * not support this, but it's included in these shared variables to simplify the template. */ ImmutableList toBuilderMethods; diff --git a/value/src/main/java/com/google/auto/value/processor/AutoValueProcessor.java b/value/src/main/java/com/google/auto/value/processor/AutoValueProcessor.java index 78cb899377..e69587913e 100644 --- a/value/src/main/java/com/google/auto/value/processor/AutoValueProcessor.java +++ b/value/src/main/java/com/google/auto/value/processor/AutoValueProcessor.java @@ -492,7 +492,7 @@ private static ImmutableSet immutableSetDifference(ImmutableSet a, Imm if (Collections.disjoint(a, b)) { return a; } else { - return ImmutableSet.copyOf(difference(a, b)); + return difference(a, b).immutableCopy(); } } } diff --git a/value/src/main/java/com/google/auto/value/processor/AutoValueishProcessor.java b/value/src/main/java/com/google/auto/value/processor/AutoValueishProcessor.java index 054810dfd8..23456477e2 100644 --- a/value/src/main/java/com/google/auto/value/processor/AutoValueishProcessor.java +++ b/value/src/main/java/com/google/auto/value/processor/AutoValueishProcessor.java @@ -280,8 +280,8 @@ public boolean isNullable() { /** * Returns the name of the getter method for this property as defined by the {@code @AutoValue} * or {@code @AutoBuilder} class. For property {@code foo}, this will be {@code foo} or {@code - * getFoo} or {@code isFoo}. For AutoValue, this will also be the name of a getter method in a - * builder; in the case of AutoBuilder it will only be that and may be null. + * getFoo} or {@code isFoo}. For AutoBuilder, the getter in question is the one that will be + * called on the built type to derive the value of the property, in the copy constructor. */ public String getGetter() { return getter; diff --git a/value/src/main/java/com/google/auto/value/processor/BuilderSpec.java b/value/src/main/java/com/google/auto/value/processor/BuilderSpec.java index c67359cae3..7b734ca9b6 100644 --- a/value/src/main/java/com/google/auto/value/processor/BuilderSpec.java +++ b/value/src/main/java/com/google/auto/value/processor/BuilderSpec.java @@ -390,8 +390,7 @@ public static class PropertyGetter { this.optional = optional; } - // Not accessed from templates so doesn't have to be public. - String getName() { + public String getName() { return name; } diff --git a/value/src/main/java/com/google/auto/value/processor/builder.vm b/value/src/main/java/com/google/auto/value/processor/builder.vm index 6a0032ceac..5c4f4d5dbd 100644 --- a/value/src/main/java/com/google/auto/value/processor/builder.vm +++ b/value/src/main/java/com/google/auto/value/processor/builder.vm @@ -60,7 +60,8 @@ class ${builderName}${builderFormalTypes} ## #if ($toBuilderConstructor) - private ${builderName}(${origClass}${actualTypes} source) { + #if (!$autoBuilder) private #end## + ${builderName}($builtType source) { #foreach ($p in $props) @@ -128,13 +129,13 @@ class ${builderName}${builderFormalTypes} ## if (${propertyBuilder.name} == null) { ## This is the first time someone has asked for the builder. If the property it sets already - ## 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. + ## has a value (because it came from the copy constructor, 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) + #if ($builderSetters[$p.name].empty && !$toBuilderConstructor) ${propertyBuilder.name} = ${propertyBuilder.initializer}; $builderRequiredProperties.markAsSet($p) @@ -179,23 +180,24 @@ class ${builderName}${builderFormalTypes} ## ## Getter - #if ($builderGetters[$p.name]) + #set ($builderGetter = $builderGetters[$p.name]) + #if ($builderGetter) @`java.lang.Override` - ${p.nullableAnnotation}${builderGetters[$p.name].access}$builderGetters[$p.name].type ${p.getter}() { + ${p.nullableAnnotation}${builderGetter.access}$builderGetter.type ${builderGetter.name}() { #set ($noValueToGetCondition = $builderRequiredProperties.noValueToGet($p)) #if ($builderGetters[$p.name].optional) #if ($noValueToGetCondition) if ($noValueToGetCondition) { - return $builderGetters[$p.name].optional.empty; + return $builderGetter.optional.empty; } #elseif ($p.nullable) if ($p == null) { - return $builderGetters[$p.name].optional.empty; + return $builderGetter.optional.empty; } #end - return ${builderGetters[$p.name].optional.rawType}.of($p); + return ${builderGetter.optional.rawType}.of($p); #else #if ($noValueToGetCondition) diff --git a/value/src/test/java/com/google/auto/value/processor/AutoBuilderCompilationTest.java b/value/src/test/java/com/google/auto/value/processor/AutoBuilderCompilationTest.java index 06ebc14bbf..41ee4f183f 100644 --- a/value/src/test/java/com/google/auto/value/processor/AutoBuilderCompilationTest.java +++ b/value/src/test/java/com/google/auto/value/processor/AutoBuilderCompilationTest.java @@ -44,6 +44,12 @@ public final class AutoBuilderCompilationTest { "", " AutoBuilder_Baz_Builder() {}", "", + " AutoBuilder_Baz_Builder(Baz source) {", + " this.anInt = source.anInt();", + " this.aString = source.aString();", + " set$0 = (byte) 1;", + " }", + "", " @Override public Baz.Builder setAnInt(int anInt) {", " this.anInt = anInt;", " set$0 |= 0x1;",