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.

Sign up for GitHub

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

AutoValue support for very large classes. #1642

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

AutoValue support for very large classes. #1642

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions value/src/main/java/com/google/auto/value/processor/AutoValueOrBuilderTemplateVars.java
View file
Open in desktop
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;
}
5 changes: 5 additions & 0 deletions value/src/main/java/com/google/auto/value/processor/AutoValueProcessor.java
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
}
}
69 changes: 26 additions & 43 deletions value/src/test/java/com/google/auto/value/processor/AutoValueCompilationTest.java
View file
Open in desktop
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