Skip to content

Commit

Permalink
Correct the type that appears in new for the generated toBuilder().
Browse files Browse the repository at this point in the history
We want extensions to be able to subclass the nested `Builder` class that AutoValue generates. For example, if we are compiling `@AutoValue abstract class Foo`, and there is one extension, then AutoValue itself will generate `$AutoValue_Foo extends Foo` with a nested class `Builder` and the extension will generate `AutoValue_Foo extends $AutoValue_Foo`. That can contain its own `Builder` class extending `$AutoValue_Foo.Builder`, which will work correctly if `Foo` itself contains `new AutoValue_Foo.Builder()`. But before this change, the implementation of `$AutoValue_Foo.toBuilder()` contained just `new Builder(this)`, which therefore instantiates `$AutoValue_Foo.Builder` instead of `AutoValue_Foo.Builder`. The fix is for the generated `toBuilder()` to contain `new AutoValue_Foo.Builder(this)` instead.

This change requires extensions to generate a "copy constructor" if they generate a `Builder` subclass and if [`BuilderContext.toBuilderMethods()`](https://javadoc.io/static/com.google.auto.value/auto-value/1.10.4/com/google/auto/value/extension/AutoValueExtension.BuilderContext.html#toBuilderMethods()) is not empty.

RELNOTES=The generated `toBuilder()` method now says `new AutoValue_Foo.Builder(this)` rather than just `new Builder(this)`, to do the right thing if an extension generates its own subclass of `Builder`.
PiperOrigin-RevId: 570406040
  • Loading branch information
eamonnmcmanus authored and Google Java Core Libraries committed Oct 3, 2023
1 parent 63ee7b4 commit 324470b
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,26 @@ public Set<ExecutableElement> consumeBuilderMethods(Context context) {
* name. The {@code <constructorParameters>} and {@code <constructorParameterNames>} are typically
* derived from {@link Context#propertyTypes()}.
*
* <p>An extension can also generate a subclass of the nested {@code Builder} class if there is
* one. In that case, it should check if {@link BuilderContext#toBuilderMethods()} is empty. If
* not, the {@code Builder} subclass should include a "copy constructor", like this:
*
* <pre>{@code
* ...
* <finalOrAbstract> class <className> extends <classToExtend> {
* ...
* static class Builder extends <classToExtend>.Builder {
* Builder() {}
* Builder(<autoValueClass> copyFrom) {
* super(copyFrom);
* }
* ...
* }
* }
* }</pre>
*
* <p>Here, {@code <autoValueClass>} is {@link Context#autoValueClass()}.}
*
* @param context The {@link Context} of the code generation for this class.
* @param className The simple name of the resulting class. The returned code will be written to a
* file named accordingly.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ void processType(TypeElement type) {
int subclassDepth = writeExtensions(type, context, applicableExtensions);
String subclass = generatedSubclassName(type, subclassDepth);
vars.subclass = TypeSimplifier.simpleNameOf(subclass);
vars.finalSubclass = finalSubclass;
vars.isFinal = (subclassDepth == 0);
vars.modifiers = vars.isFinal ? "final " : "abstract ";
vars.builderClassModifiers =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,14 @@ class AutoValueTemplateVars extends AutoValueOrBuilderTemplateVars {

/** The simple name of the generated subclass. */
String subclass;

/**
* The simple name of the final subclass. This is the same as {@link #subclass} unless there are
* extensions. If there are extensions, then {@link #subclass} might be something like {@code
* $$AutoValue_Foo} while {@code finalSubclass} will be {@code AutoValue_Foo}.
*/
String finalSubclass;

/**
* The modifiers (for example {@code final} or {@code abstract}) for the generated subclass,
* followed by a space if they are not empty.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ ${modifiers}class $subclass$formalTypes extends $origClass$actualTypes {

@`java.lang.Override`
${m.access}${builderTypeName}${builderActualTypes} ${m.name}() {
return new Builder${builderActualTypes}(this);
return new ${finalSubclass}.Builder${builderActualTypes}(this);
}

#end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1271,7 +1271,7 @@ public void correctBuilder() {
" }",
"",
" @Override public Baz.Builder<T> toBuilder() {",
" return new Builder<T>(this);",
" return new AutoValue_Baz.Builder<T>(this);",
" }",
"",
" static final class Builder<T extends Number> extends Baz.Builder<T> {",
Expand Down Expand Up @@ -1621,7 +1621,7 @@ public void builderWithNullableTypeAnnotation() {
" }",
"",
" @Override public Baz.Builder<T> toBuilder() {",
" return new Builder<T>(this);",
" return new AutoValue_Baz.Builder<T>(this);",
" }",
"",
" static final class Builder<T extends Number> extends Baz.Builder<T> {",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ public void testDoesntRaiseWarningForConsumedProperties() {
}

@Test
public void testDoesntRaiseWarningForToBuilder() {
public void testDoesntRaiseWarningForToBuilder() throws IOException {
JavaFileObject impl =
JavaFileObjects.forSourceLines(
"foo.bar.Baz",
Expand All @@ -228,6 +228,18 @@ public void testDoesntRaiseWarningForToBuilder() {
.withProcessors(new AutoValueProcessor(ImmutableList.of(new FooExtension())))
.compile(impl);
assertThat(compilation).succeededWithoutWarnings();

// $AutoValue_Baz is the source file generated by AutoValue itself, which is then subclassed as
// AutoValue_Baz by the extension. The implementation of toBuilder() should call
// `new AutoValue_Baz.Builder` in case the extension includes its own subclass of the Builder
// class.
Optional<JavaFileObject> generatedSourceFile =
compilation.generatedSourceFile("foo.bar.$AutoValue_Baz");
assertThat(generatedSourceFile).isPresent();
String content =
generatedSourceFile.get().getCharContent(/* ignoreEncodingErrors= */ false).toString();
assertThat(content).doesNotContain("new Builder");
assertThat(content).contains("new AutoValue_Baz.Builder");
}

@Test
Expand Down

0 comments on commit 324470b

Please sign in to comment.