Skip to content

Commit

Permalink
For @AutoBuilder classes, generate a "copy constructor" if appropri…
Browse files Browse the repository at this point in the history
…ate.

For example, given this:

```
record Foo(String bar) {
  @autobuilder
  abstract class Builder {
    Builder setBar(String bar);
    Foo build();
  }
}
```

The built type is `Foo` (it is the return type of the `build()` method), and the record does have an implicit method `Foo.bar()`, so this qualifies. In addition to the `AutoBuilder_Foo_Builder()` constructor, we will declare `AutoBuilder_Foo_Builder(Foo source)`, which copies initial values for the builder from the given `source`.

RELNOTES=The generated AutoBuilder class now has a "copy constructor" if values for the builder properties can be obtained from the built type.
PiperOrigin-RevId: 448049949
  • Loading branch information
eamonnmcmanus authored and Google Java Core Libraries committed May 11, 2022
1 parent 26f073f commit b3b53a3
Show file tree
Hide file tree
Showing 9 changed files with 148 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down Expand Up @@ -248,7 +251,9 @@ public void buildWithoutAutoAnnotation() {
@AutoBuilder(ofClass = MyAnnotation.class)
public interface MyAnnotationSimplerBuilder {
MyAnnotationSimplerBuilder value(String x);

MyAnnotationSimplerBuilder id(int x);

MyAnnotation build();
}

Expand Down Expand Up @@ -665,4 +670,47 @@ public void genericGetters() {
assertThat(builder.getFirst()).isEqualTo(1.0);
assertThat(builder.build()).containsExactly(1.0, 2).inOrder();
}

static class NumberHolder<T extends Number> {
private final T number;

NumberHolder(T number) {
this.number = number;
}

T getNumber() {
return number;
}
}

static <T extends Number> NumberHolder<T> buildNumberHolder(T number) {
return new NumberHolder<>(number);
}

@AutoBuilder(callMethod = "buildNumberHolder")
interface NumberHolderBuilder<T extends Number> {
NumberHolderBuilder<T> setNumber(T number);

NumberHolder<T> build();
}

static <T extends Number> NumberHolderBuilder<T> numberHolderBuilder() {
return new AutoBuilder_AutoBuilderTest_NumberHolderBuilder<>();
}

static <T extends Number> NumberHolderBuilder<T> numberHolderBuilder(
NumberHolder<T> numberHolder) {
return new AutoBuilder_AutoBuilderTest_NumberHolderBuilder<>(numberHolder);
}

@Test
public void builderFromInstance() {
NumberHolder<Integer> instance1 =
AutoBuilderTest.<Integer>numberHolderBuilder().setNumber(23).build();
assertThat(instance1.getNumber()).isEqualTo(23);
NumberHolder<Integer> instance2 = numberHolderBuilder(instance1).build();
assertThat(instance2.getNumber()).isEqualTo(23);
NumberHolder<Integer> instance3 = numberHolderBuilder(instance2).setNumber(17).build();
assertThat(instance3.getNumber()).isEqualTo(17);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -203,8 +202,8 @@ private void processType(TypeElement autoBuilderType, TypeElement ofClass, Strin
return;
}
BuilderMethodClassifier<VariableElement> classifier = maybeClassifier.get();
Map<String, String> propertyToGetterName =
Maps.transformValues(classifier.builderGetters(), PropertyGetter::getName);
ImmutableMap<String, String> propertyToGetterName =
propertyToGetterName(executable, autoBuilderType);
AutoBuilderTemplateVars vars = new AutoBuilderTemplateVars();
vars.props = propertySet(executable, propertyToGetterName, propertyInitializers);
builder.defineVars(vars, classifier);
Expand All @@ -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();
Expand Down Expand Up @@ -350,6 +349,72 @@ private ImmutableMap<String, String> 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.
*
* <p>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).
*
* <p>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<Baz>} and the parameter has type
* {@code Baz[]}. We already have similar logic for the parameter types of builder setters.
*/
private ImmutableMap<String, String> propertyToGetterName(
Executable executable, TypeElement autoBuilderType) {
TypeMirror builtType = executable.builtType();
if (builtType.getKind() != TypeKind.DECLARED) {
return ImmutableMap.of();
}
TypeElement type = MoreTypes.asTypeElement(builtType);
Map<String, ExecutableElement> 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<String, String> 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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<SimpleMethod> toBuilderMethods;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ private static <E> ImmutableSet<E> immutableSetDifference(ImmutableSet<E> a, Imm
if (Collections.disjoint(a, b)) {
return a;
} else {
return ImmutableSet.copyOf(difference(a, b));
return difference(a, b).immutableCopy();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
20 changes: 11 additions & 9 deletions value/src/main/java/com/google/auto/value/processor/builder.vm
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;",
Expand Down

3 comments on commit b3b53a3

@sesamzoo
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @eamonnmcmanus, what's the ETA for a release with these changes?

@eamonnmcmanus
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the reminder! I see it's been 10 months since AutoValue 1.9, so a release was overdue, especially for this and other improvements to AutoBuilder. I've released 1.10 now.

@sesamzoo
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, that was super quick. Thanks, @eamonnmcmanus!

Please sign in to comment.