Skip to content

Commit

Permalink
AutoValue support for very large classes.
Browse files Browse the repository at this point in the history
This change makes it so that if the AutoValue class defines a Builder method, the builder will be passed to the constructor. Effectively working around the java 255 method-arg limit.

This is instead of defining a constructor where every field in the autovalue is an argument in it.

This change will only apply to autovalues that don't have extensions: in which case the constructor is private and we can be assured that updating it shouldn't be problematic for current cases.

RELNOTES=Support for AutoValues with +255 properties.
PiperOrigin-RevId: 582096324
  • Loading branch information
alasiads authored and Google Java Core Libraries committed Nov 21, 2023
1 parent 99d964c commit 2af3791
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 65 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
Expand Down
40 changes: 24 additions & 16 deletions value/src/main/java/com/google/auto/value/processor/autovalue.vm
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
16 changes: 10 additions & 6 deletions value/src/main/java/com/google/auto/value/processor/builder.vm
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -1066,6 +1071,7 @@ public void nullablePrimitive() {

@Test
public void correctBuilder() {
assume().that(constructorUsesBuilderFields).isTrue();
JavaFileObject javaFileObject =
JavaFileObjects.forSourceLines(
"foo.bar.Baz",
Expand Down Expand Up @@ -1170,21 +1176,15 @@ public void correctBuilder() {
" private final Optional<String> anOptionalString;",
" private final NestedAutoValue<T> aNestedAutoValue;",
"",
" private AutoValue_Baz(",
" int anInt,",
" byte[] aByteArray,",
" @Nullable int[] aNullableIntArray,",
" List<T> aList,",
" ImmutableMap<T, String> anImmutableMap,",
" Optional<String> anOptionalString,",
" NestedAutoValue<T> 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<T> 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() {",
Expand Down Expand Up @@ -1439,14 +1439,7 @@ public void correctBuilder() {
" }",
" throw new IllegalStateException(\"Missing required properties:\" + missing);",
" }",
" return new AutoValue_Baz<T>(",
" this.anInt,",
" this.aByteArray,",
" this.aNullableIntArray,",
" this.aList,",
" this.anImmutableMap,",
" this.anOptionalString,",
" this.aNestedAutoValue);",
" return new AutoValue_Baz<T>(this);",
" }",
" }",
"}");
Expand All @@ -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",
Expand Down Expand Up @@ -1531,19 +1525,14 @@ public void builderWithNullableTypeAnnotation() {
" private final ImmutableMap<T, String> anImmutableMap;",
" private final Optional<String> anOptionalString;",
"",
" private AutoValue_Baz(",
" int anInt,",
" byte[] aByteArray,",
" int @Nullable [] aNullableIntArray,",
" List<T> aList,",
" ImmutableMap<T, String> anImmutableMap,",
" Optional<String> 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<T> 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() {",
Expand Down Expand Up @@ -1733,13 +1722,7 @@ public void builderWithNullableTypeAnnotation() {
" }",
" throw new IllegalStateException(\"Missing required properties:\" + missing);",
" }",
" return new AutoValue_Baz<T>(",
" this.anInt,",
" this.aByteArray,",
" this.aNullableIntArray,",
" this.aList,",
" this.anImmutableMap,",
" this.anOptionalString);",
" return new AutoValue_Baz<T>(this);",
" }",
" }",
"}");
Expand Down

0 comments on commit 2af3791

Please sign in to comment.