From 324470ba28e85482c7b359a3679601583323d9ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89amonn=20McManus?= Date: Tue, 3 Oct 2023 09:10:11 -0700 Subject: [PATCH] Correct the type that appears in `new` for the generated `toBuilder()`. 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 --- .../value/extension/AutoValueExtension.java | 20 +++++++++++++++++++ .../value/processor/AutoValueProcessor.java | 1 + .../processor/AutoValueTemplateVars.java | 8 ++++++++ .../google/auto/value/processor/autovalue.vm | 2 +- .../processor/AutoValueCompilationTest.java | 4 ++-- .../auto/value/processor/ExtensionTest.java | 14 ++++++++++++- 6 files changed, 45 insertions(+), 4 deletions(-) diff --git a/value/src/main/java/com/google/auto/value/extension/AutoValueExtension.java b/value/src/main/java/com/google/auto/value/extension/AutoValueExtension.java index be76470191..7b5e40f7ec 100644 --- a/value/src/main/java/com/google/auto/value/extension/AutoValueExtension.java +++ b/value/src/main/java/com/google/auto/value/extension/AutoValueExtension.java @@ -507,6 +507,26 @@ public Set consumeBuilderMethods(Context context) { * name. The {@code } and {@code } are typically * derived from {@link Context#propertyTypes()}. * + *

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: + * + * 

{@code
+   * ...
+   *  class  extends  {
+   *   ...
+   *   static class Builder extends .Builder {
+   *     Builder() {}
+   *     Builder( copyFrom) {
+   *       super(copyFrom);
+   *     }
+   *     ...
+   *   }
+   * }
+   * }
+ * + *

Here, {@code } 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. 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 3630b6caba..0206dcd725 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 @@ -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 = diff --git a/value/src/main/java/com/google/auto/value/processor/AutoValueTemplateVars.java b/value/src/main/java/com/google/auto/value/processor/AutoValueTemplateVars.java index e53b92e930..9e811c1fb6 100644 --- a/value/src/main/java/com/google/auto/value/processor/AutoValueTemplateVars.java +++ b/value/src/main/java/com/google/auto/value/processor/AutoValueTemplateVars.java @@ -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. 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 18ca827aea..ec0910c2a0 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 @@ -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 diff --git a/value/src/test/java/com/google/auto/value/processor/AutoValueCompilationTest.java b/value/src/test/java/com/google/auto/value/processor/AutoValueCompilationTest.java index d07019834d..e4224913cf 100644 --- a/value/src/test/java/com/google/auto/value/processor/AutoValueCompilationTest.java +++ b/value/src/test/java/com/google/auto/value/processor/AutoValueCompilationTest.java @@ -1271,7 +1271,7 @@ public void correctBuilder() { " }", "", " @Override public Baz.Builder toBuilder() {", - " return new Builder(this);", + " return new AutoValue_Baz.Builder(this);", " }", "", " static final class Builder extends Baz.Builder {", @@ -1621,7 +1621,7 @@ public void builderWithNullableTypeAnnotation() { " }", "", " @Override public Baz.Builder toBuilder() {", - " return new Builder(this);", + " return new AutoValue_Baz.Builder(this);", " }", "", " static final class Builder extends Baz.Builder {", diff --git a/value/src/test/java/com/google/auto/value/processor/ExtensionTest.java b/value/src/test/java/com/google/auto/value/processor/ExtensionTest.java index ec85c2ee7c..b1caf939a2 100644 --- a/value/src/test/java/com/google/auto/value/processor/ExtensionTest.java +++ b/value/src/test/java/com/google/auto/value/processor/ExtensionTest.java @@ -206,7 +206,7 @@ public void testDoesntRaiseWarningForConsumedProperties() { } @Test - public void testDoesntRaiseWarningForToBuilder() { + public void testDoesntRaiseWarningForToBuilder() throws IOException { JavaFileObject impl = JavaFileObjects.forSourceLines( "foo.bar.Baz", @@ -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 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