Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Optional-wrapping property builders #444

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1851,6 +1851,91 @@ public void testBuilderWithPropertyBuilders() {
}
}

@AutoValue
public abstract static class BuilderWithOptionalPropertyBuilders {
public abstract com.google.common.base.Optional<BuilderWithSetAndGet> getFoo();

public abstract BuilderWithOptionalPropertyBuilders.Builder toBuilder();

public static Builder builder() {
return new AutoValue_AutoValueTest_BuilderWithOptionalPropertyBuilders.Builder();
}

@AutoValue.Builder
public abstract static class Builder {
public abstract com.google.common.base.Optional<BuilderWithSetAndGet> getFoo();

public abstract Builder foo(com.google.common.base.Optional<BuilderWithSetAndGet> foo);

public abstract BuilderWithSetAndGet.Builder fooBuilder();

public abstract BuilderWithOptionalPropertyBuilders build();
}
}

@Test
public void testBuilderWithOptionalPropertyBuilders() {
BuilderWithOptionalPropertyBuilders a =
BuilderWithOptionalPropertyBuilders.builder()
.build();

assertThat(a.getFoo()).isAbsent();

BuilderWithSetAndGet foo = BuilderWithSetAndGet.builder()
.setAnInt(0)
.setAList(ImmutableList.of(1))
.build();
BuilderWithOptionalPropertyBuilders b =
BuilderWithOptionalPropertyBuilders.builder()
.foo(com.google.common.base.Optional.of(foo))
.build();

assertThat(b.getFoo()).hasValue(foo);

BuilderWithOptionalPropertyBuilders c =
BuilderWithOptionalPropertyBuilders.builder()
.foo(com.google.common.base.Optional.<BuilderWithSetAndGet>absent())
.build();

assertThat(c.getFoo()).isAbsent();

BuilderWithOptionalPropertyBuilders.Builder buildD =
BuilderWithOptionalPropertyBuilders.builder()
.foo(com.google.common.base.Optional.of(foo));

// get fooBuilder to trigger toBuilder logic
buildD.fooBuilder();
BuilderWithOptionalPropertyBuilders d = buildD.build();

assertThat(d.getFoo()).hasValue(foo);

BuilderWithOptionalPropertyBuilders.Builder buildE =
BuilderWithOptionalPropertyBuilders.builder()
.foo(com.google.common.base.Optional.of(foo));

buildE.fooBuilder().setAnInt(1);
BuilderWithOptionalPropertyBuilders e = buildE.build();

assertThat(e.getFoo()).hasValue(foo.toBuilder().setAnInt(1).build());

BuilderWithOptionalPropertyBuilders.Builder buildF =
BuilderWithOptionalPropertyBuilders.builder();

buildF.fooBuilder().setAList(ImmutableList.of(1)).setAnInt(0);
BuilderWithOptionalPropertyBuilders f = buildF.build();

assertThat(f.getFoo()).hasValue(foo);

try {
BuilderWithOptionalPropertyBuilders.builder().foo(null).build();
fail("Did not get expected exception");
} catch (RuntimeException expected) {
// We don't specify whether you get the exception on foo(null) or on
// build(), nor
// which exception it is exactly.
}
}

@AutoValue
public abstract static class
BuilderWithExoticPropertyBuilders<K extends Number, V extends Comparable<K>> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,29 @@ public boolean isNullable() {
return !getNullableAnnotation().isEmpty();
}

/**
* Returns an expression to check if the property is able to be retrieved as a non-null value
* using {@link #getValueRetrieval()}.
*/
public String getPresenseCheck() {
if (optional == null) {
return identifier + " != null";
} else {
return identifier + ".isPresent()";
}
}

/**
* Returns an expression to get the value of the property, unwrapping it if it is an Optional.
*/
public String getValueRetrieval() {
if (optional == null) {
return identifier;
} else {
return identifier + ".get()";
}
}

public String getAccess() {
return access(method);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,16 @@ private boolean classifyMethods(
}
for (Map.Entry<ExecutableElement, String> getterEntry : getterToPropertyName.entrySet()) {
String property = getterEntry.getValue();
String propertyType = typeSimplifier.simplify(getterEntry.getKey().getReturnType());

TypeMirror propertyTypeMirror = getterEntry.getKey().getReturnType();
// Handle `Optional<Bar>` by replacing the type mirror with the contained type
Optionalish propertyOptionalish = Optionalish.createIfOptional(propertyTypeMirror,
typeSimplifier.simplifyRaw(propertyTypeMirror));
if (propertyOptionalish != null) {
propertyTypeMirror = propertyOptionalish.getContainedType(typeUtils);
}

String propertyType = typeSimplifier.simplify(propertyTypeMirror);
boolean hasSetter = propertyNameToSetter.containsKey(property);
PropertyBuilder propertyBuilder = propertyNameToPropertyBuilder.get(property);
boolean hasBuilder = propertyBuilder != null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ public static class PropertyBuilder {
private final String initDefault;
private final String builtToBuilder;
private final String copyAll;
private final Optionalish optional;

PropertyBuilder(
ExecutableElement propertyBuilderMethod,
Expand All @@ -95,7 +96,8 @@ public static class PropertyBuilder {
String beforeInitDefault,
String initDefault,
String builtToBuilder,
String copyAll) {
String copyAll,
Optionalish optional) {
this.propertyBuilderMethod = propertyBuilderMethod;
this.name = propertyBuilderMethod.getSimpleName() + "$";
this.builderType = builderType;
Expand All @@ -104,6 +106,7 @@ public static class PropertyBuilder {
this.initDefault = initDefault;
this.builtToBuilder = builtToBuilder;
this.copyAll = copyAll;
this.optional = optional;
}

/** The property builder method, for example {@code barBuilder()}. */
Expand Down Expand Up @@ -167,6 +170,24 @@ public String getBuiltToBuilder() {
public String getCopyAll() {
return copyAll;
}

/**
* The {@link Optionalish} that was created for the property, if applicable.
*/
public Optionalish getOptional() {
return optional;
}

/**
* An expression to build the property, wrapping it using Optionalish if needed.
*/
public String getBuild() {
if (optional == null) {
return name + ".build()";
} else {
return optional.getRawType() + ".of(" + name + ".build())";
}
}
}

// Construct this string so it won't be found by Maven shading and renamed, which is not what
Expand All @@ -181,6 +202,8 @@ public String getCopyAll() {
// `BarBuilder` type are:
// (1) It must have an instance method called `build()` that returns `Bar`. If the type of
// `bar()` is `Bar<String>` then the type of `build()` must be `Bar<String>`.
// The property may be `Optional<Bar> bar()`, in which case the type of `build()` must
// still be `Bar`, and it still cannot return `null`.
// (2) `BarBuilder` must have a public no-arg constructor, or `Bar` must have a static method
// `builder()` or `newBuilder()` that returns `BarBuilder`.
// (3) `Bar` must have an instance method `BarBuilder toBuilder()`, or `BarBuilder` must be a
Expand All @@ -202,6 +225,14 @@ Optional<PropertyBuilder> makePropertyBuilder(ExecutableElement method, String p

ExecutableElement barGetter = getterToPropertyName.inverse().get(property);
TypeMirror barTypeMirror = barGetter.getReturnType();

// Handle `Optional<Bar>` by replacing the type mirror with the contained type
Optionalish barOptionalish =
Optionalish.createIfOptional(barTypeMirror, typeSimplifier.simplifyRaw(barTypeMirror));
if (barOptionalish != null) {
barTypeMirror = barOptionalish.getContainedType(typeUtils);
}

if (barTypeMirror.getKind() != TypeKind.DECLARED) {
errorReporter.reportError("Method looks like a property builder, but the type of property "
+ property + " is not a class or interface", method);
Expand Down Expand Up @@ -292,7 +323,8 @@ Optional<PropertyBuilder> makePropertyBuilder(ExecutableElement method, String p
beforeInitDefault,
initDefault,
builtToBuilder,
copyAll);
copyAll,
barOptionalish);
return Optional.of(propertyBuilder);
}

Expand Down
20 changes: 11 additions & 9 deletions value/src/main/java/com/google/auto/value/processor/autovalue.vm
Original file line number Diff line number Diff line change
Expand Up @@ -289,13 +289,10 @@ $a

#else

if ($p == null) {
${propertyBuilder.name} = ${propertyBuilder.initializer};
} else {

if (${p.presenseCheck}) {
#if (${propertyBuilder.builtToBuilder})

${propertyBuilder.name} = ${p}.${propertyBuilder.builtToBuilder}();
${propertyBuilder.name} = ${p.valueRetrieval}.${propertyBuilder.builtToBuilder}();

#else

Expand All @@ -305,6 +302,8 @@ $a
#end

$p = null;
} else {
${propertyBuilder.name} = ${propertyBuilder.initializer};
}

#end
Expand Down Expand Up @@ -341,13 +340,16 @@ $a
#if ($propertyBuilder)

if (${propertyBuilder.name} != null) {
return ${propertyBuilder.name}.build();
return ${propertyBuilder.build};
}
#if (!${p.optional})

if ($p == null) {
${propertyBuilder.beforeInitDefault}
$p = ${propertyBuilder.initDefault};
}

#end
#end

return $p;
Expand All @@ -367,11 +369,11 @@ $a
#if ($propertyBuilder)

if (${propertyBuilder.name} != null) {
this.$p = ${propertyBuilder.name}.build();
} else if (this.$p == null) {
this.$p = ${propertyBuilder.build};
} #if (!${p.optional}) else if (this.$p == null) {
${propertyBuilder.beforeInitDefault}
this.$p = ${propertyBuilder.initDefault};
}
} #end

#end
#end
Expand Down
Loading