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 b2aad04109..664f0a414a 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 @@ -153,4 +153,10 @@ abstract class AutoValueOrBuilderTemplateVars extends AutoValueishTemplateVars { * subclass, followed by a space if they are not empty. */ String builderClassModifiers = ""; + + /** + * Set if the code should generate a constructor that takes a Builder as an argument instead of + * the usual per-field constructor. + */ + Boolean shouldGenerateBuilderConstructor = false; } 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 0206dcd725..a4800d3a8f 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 @@ -45,6 +45,7 @@ import javax.annotation.processing.ProcessingEnvironment; import javax.annotation.processing.Processor; import javax.annotation.processing.SupportedAnnotationTypes; +import javax.lang.model.SourceVersion; import javax.lang.model.element.AnnotationMirror; import javax.lang.model.element.ExecutableElement; import javax.lang.model.element.TypeElement; @@ -289,6 +290,10 @@ void processType(TypeElement type) { vars.subclass = TypeSimplifier.simpleNameOf(subclass); vars.finalSubclass = finalSubclass; vars.isFinal = (subclassDepth == 0); + vars.shouldGenerateBuilderConstructor = + applicableExtensions.isEmpty() + && builder.isPresent() + && processingEnv.getSourceVersion().compareTo(SourceVersion.RELEASE_8) > 0; vars.modifiers = vars.isFinal ? "final " : "abstract "; vars.builderClassModifiers = consumedBuilderMethods.isEmpty() 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 ec0910c2a0..ae112051d8 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 @@ -59,34 +59,42 @@ ${modifiers}class $subclass$formalTypes extends $origClass$actualTypes { ## Constructor +#if ($shouldGenerateBuilderConstructor) + @SuppressWarnings("nullness") // Null checks happen during "build" method. +#end #if ($isFinal && $builderTypeName != "") private ## #end +#if ($shouldGenerateBuilderConstructor) + $subclass(${builderName}${actualTypes} builder) { + #foreach ($p in $props) + this.$p = builder.$p; + #end + } +#else $subclass( -#foreach ($p in $props) - - ${p.nullableAnnotation}$p.type $p #if ($foreach.hasNext) , #end -#end ) { -#foreach ($p in $props) - #if (!$p.kind.primitive && !$p.nullable && ($builderTypeName == "" || !$isFinal)) - ## We don't need a null check if the type is primitive or @Nullable. We also don't need it - ## if there is a builder, since the build() method will check for us. However, if there is a - ## builder but there are also extensions (!$isFinal) then we can't omit the null check because - ## the constructor is called from the extension code. + #foreach ($p in $props) + ${p.nullableAnnotation}$p.type $p #if ($foreach.hasNext) , #end + #end) { + #foreach ($p in $props) + #if (!$p.kind.primitive && !$p.nullable && ($builderTypeName == "" || !$isFinal)) + ## We don't need a null check if the type is primitive or @Nullable. We also don't need it + ## if there is a builder, since the build() method will check for us. However, if there is a + ## builder but there are also extensions (!$isFinal) then we can't omit the null check because + ## the constructor is called from the extension code. - #if ($identifiers) + #if ($identifiers) if ($p == null) { throw new NullPointerException("Null $p.name"); } - #else + #else `java.util.Objects`.requireNonNull($p); + #end #end - - #end - this.$p = $p; -#end + #end } +#end ## Property getters 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 30989b7843..e5313386c5 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 @@ -274,11 +274,15 @@ ${builderClassModifiers}class ${builderName}${builderFormalTypes} ## #end - #if ($builtType != "void") return #end ${build}( -#foreach ($p in $props) - - this.$p #if ($foreach.hasNext) , #end -#end - $builderRequiredProperties.defaultedBitmaskParameters ); + #if ($builtType != "void") return #end + #if ($shouldGenerateBuilderConstructor) + ${build}(this); + #else + ${build}( + #foreach ($p in $props) + this.$p #if ($foreach.hasNext) , #end + #end + $builderRequiredProperties.defaultedBitmaskParameters ); + #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 e4224913cf..47f50b9dd1 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 @@ -61,6 +61,11 @@ public class AutoValueCompilationTest { private boolean typeAnnotationsWork = Double.parseDouble(JAVA_SPECIFICATION_VERSION.value()) >= 9.0; + // Sadly we can't rely on JDK 8 to handle accessing the private methods of the builder. + // So skip the test unless we are on at least JDK 9. + private boolean constructorUsesBuilderFields = + Double.parseDouble(JAVA_SPECIFICATION_VERSION.value()) >= 9.0; + @Test public void simpleSuccess() { // Positive test case that ensures we generate the expected code for at least one case. @@ -1066,6 +1071,7 @@ public void nullablePrimitive() { @Test public void correctBuilder() { + assume().that(constructorUsesBuilderFields).isTrue(); JavaFileObject javaFileObject = JavaFileObjects.forSourceLines( "foo.bar.Baz", @@ -1170,21 +1176,15 @@ public void correctBuilder() { " private final Optional anOptionalString;", " private final NestedAutoValue aNestedAutoValue;", "", - " private AutoValue_Baz(", - " int anInt,", - " byte[] aByteArray,", - " @Nullable int[] aNullableIntArray,", - " List aList,", - " ImmutableMap anImmutableMap,", - " Optional anOptionalString,", - " NestedAutoValue aNestedAutoValue) {", - " this.anInt = anInt;", - " this.aByteArray = aByteArray;", - " this.aNullableIntArray = aNullableIntArray;", - " this.aList = aList;", - " this.anImmutableMap = anImmutableMap;", - " this.anOptionalString = anOptionalString;", - " this.aNestedAutoValue = aNestedAutoValue;", + " @SuppressWarnings(\"nullness\") // Null checks happen during \"build\" method.", + " private AutoValue_Baz(Builder builder) {", + " this.anInt = builder.anInt;", + " this.aByteArray = builder.aByteArray;", + " this.aNullableIntArray = builder.aNullableIntArray;", + " this.aList = builder.aList;", + " this.anImmutableMap = builder.anImmutableMap;", + " this.anOptionalString = builder.anOptionalString;", + " this.aNestedAutoValue = builder.aNestedAutoValue;", " }", "", " @Override public int anInt() {", @@ -1439,14 +1439,7 @@ public void correctBuilder() { " }", " throw new IllegalStateException(\"Missing required properties:\" + missing);", " }", - " return new AutoValue_Baz(", - " this.anInt,", - " this.aByteArray,", - " this.aNullableIntArray,", - " this.aList,", - " this.anImmutableMap,", - " this.anOptionalString,", - " this.aNestedAutoValue);", + " return new AutoValue_Baz(this);", " }", " }", "}"); @@ -1465,6 +1458,7 @@ public void correctBuilder() { @Test public void builderWithNullableTypeAnnotation() { assume().that(typeAnnotationsWork).isTrue(); + assume().that(constructorUsesBuilderFields).isTrue(); JavaFileObject javaFileObject = JavaFileObjects.forSourceLines( "foo.bar.Baz", @@ -1531,19 +1525,14 @@ public void builderWithNullableTypeAnnotation() { " private final ImmutableMap anImmutableMap;", " private final Optional anOptionalString;", "", - " private AutoValue_Baz(", - " int anInt,", - " byte[] aByteArray,", - " int @Nullable [] aNullableIntArray,", - " List aList,", - " ImmutableMap anImmutableMap,", - " Optional anOptionalString) {", - " this.anInt = anInt;", - " this.aByteArray = aByteArray;", - " this.aNullableIntArray = aNullableIntArray;", - " this.aList = aList;", - " this.anImmutableMap = anImmutableMap;", - " this.anOptionalString = anOptionalString;", + " @SuppressWarnings(\"nullness\") // Null checks happen during \"build\" method.", + " private AutoValue_Baz(Builder builder) {", + " this.anInt = builder.anInt;", + " this.aByteArray = builder.aByteArray;", + " this.aNullableIntArray = builder.aNullableIntArray;", + " this.aList = builder.aList;", + " this.anImmutableMap = builder.anImmutableMap;", + " this.anOptionalString = builder.anOptionalString;", " }", "", " @Override public int anInt() {", @@ -1733,13 +1722,7 @@ public void builderWithNullableTypeAnnotation() { " }", " throw new IllegalStateException(\"Missing required properties:\" + missing);", " }", - " return new AutoValue_Baz(", - " this.anInt,", - " this.aByteArray,", - " this.aNullableIntArray,", - " this.aList,", - " this.anImmutableMap,", - " this.anOptionalString);", + " return new AutoValue_Baz(this);", " }", " }", "}");