diff --git a/value/src/it/functional/src/test/java/com/google/auto/value/CompileWithEclipseTest.java b/value/src/it/functional/src/test/java/com/google/auto/value/CompileWithEclipseTest.java index ead5843f37..e49091216b 100644 --- a/value/src/it/functional/src/test/java/com/google/auto/value/CompileWithEclipseTest.java +++ b/value/src/it/functional/src/test/java/com/google/auto/value/CompileWithEclipseTest.java @@ -58,7 +58,8 @@ public class CompileWithEclipseTest { @BeforeClass public static void setSourceRoot() { assertWithMessage("basedir property must be set - test must be run from Maven") - .that(SOURCE_ROOT).isNotNull(); + .that(SOURCE_ROOT) + .isNotNull(); } public @Rule TemporaryFolder tmp = new TemporaryFolder(); @@ -69,14 +70,16 @@ public static void setSourceRoot() { private static final Predicate JAVA_FILE = f -> f.getName().endsWith(".java") && !IGNORED_TEST_FILES.contains(f.getName()); - private static final Predicate JAVA8_TEST = - f -> f.getName().equals("AutoValueJava8Test.java") - || f.getName().equals("AutoOneOfJava8Test.java") - || f.getName().equals("EmptyExtension.java"); + private static final ImmutableSet JAVA8_FILE_NAMES = + ImmutableSet.of( + "AutoOneOfJava8Test.java", + "AutoValueJava8Test.java", + "EclipseNullable.java", + "EmptyExtension.java"); @Test public void compileWithEclipseJava6() throws Exception { - compileWithEclipse("6", JAVA_FILE.and(JAVA8_TEST.negate())); + compileWithEclipse("6", JAVA_FILE.and(f -> !JAVA8_FILE_NAMES.contains(f.getName()))); } @Test @@ -103,10 +106,11 @@ private void compileWithEclipse(String version, Predicate predicate) throw // fileManager.getLocation(SYSTEM_MODULES). File rtJar = new File(JAVA_HOME.value() + "/lib/rt.jar"); if (rtJar.exists()) { - List bootClassPath = ImmutableList.builder() - .add(rtJar) - .addAll(fileManager.getLocation(StandardLocation.PLATFORM_CLASS_PATH)) - .build(); + List bootClassPath = + ImmutableList.builder() + .add(rtJar) + .addAll(fileManager.getLocation(StandardLocation.PLATFORM_CLASS_PATH)) + .build(); fileManager.setLocation(StandardLocation.PLATFORM_CLASS_PATH, bootClassPath); } Iterable sourceFileObjects = @@ -122,7 +126,16 @@ private void compileWithEclipse(String version, Predicate predicate) throw version, "-target", version, - "-warn:-warningToken,-intfAnnotation"); + "-warn:-warningToken,-intfAnnotation,+nullAnnot(" + + "com.google.auto.value.nullness.Nullable|" + + "com.google.auto.value.nullness.NonNull|" + + "com.google.auto.value.nullness.NullMarked)"); + // +nullAnnot turns on nullability analysis, and the class-names in parentheses specify what + // the annotations are for nullable, not-nullable, and default-non-null, respectively. We will + // switch to using the JSpecify annotations when available. + // The purpose of turning on +nullAnnot is just to see what warnings it produces. We don't + // currently try to avoid all those warnings, which appears impossible anyway because of Eclipse + // bugs. JavaCompiler.CompilationTask task = compiler.getTask(null, fileManager, null, options, null, sourceFileObjects); // Explicitly supply an empty list of extensions for AutoValueProcessor, because otherwise this diff --git a/value/src/it/functional/src/test/java/com/google/auto/value/eclipse/EclipseNullable.java b/value/src/it/functional/src/test/java/com/google/auto/value/eclipse/EclipseNullable.java new file mode 100644 index 0000000000..4977264da4 --- /dev/null +++ b/value/src/it/functional/src/test/java/com/google/auto/value/eclipse/EclipseNullable.java @@ -0,0 +1,71 @@ +/* + * Copyright 2021 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.auto.value.eclipse; + +import com.google.auto.value.AutoValue; +import com.google.auto.value.nullness.Nullable; +import com.google.common.collect.ImmutableList; +import java.util.List; +import java.util.Map; + +/** + * A class that exercises Eclipse's null analysis. In {@code CompileWithEclipseTest}, we configure + * the compiler to use nullness annotations that are not Eclipse's own but custom alternatives + * defined here. Eventually we will use the JSpecify annotations + * when those are public. + */ +public final class EclipseNullable { + @AutoValue + abstract static class Nullables { + abstract int primitiveInt(); + + abstract Integer nonNullableInteger(); + + abstract @Nullable Integer nullableInteger(); + + abstract List<@Nullable Integer> listOfNullableIntegers(); + + abstract Map.@Nullable Entry nullableMapEntry(); + + @SuppressWarnings("mutable") + abstract int @Nullable [] nullableArrayOfInts(); + + abstract ImmutableList immutableListOfStrings(); + + @AutoValue.Builder + interface Builder { + Builder setPrimitiveInt(int x); + + Builder setNonNullableInteger(Integer x); + + Builder setNullableInteger(@Nullable Integer x); + + Builder setListOfNullableIntegers(List<@Nullable Integer> x); + + Builder setNullableMapEntry(Map.@Nullable Entry x); + + Builder setNullableArrayOfInts(int @Nullable [] x); + + Builder setImmutableListOfStrings(ImmutableList x); + + ImmutableList.Builder immutableListOfStringsBuilder(); + + Nullables build(); + } + } + + private EclipseNullable() {} +} diff --git a/value/src/it/functional/src/test/java/com/google/auto/value/eclipse/package-info.java b/value/src/it/functional/src/test/java/com/google/auto/value/eclipse/package-info.java new file mode 100644 index 0000000000..c68d99aacb --- /dev/null +++ b/value/src/it/functional/src/test/java/com/google/auto/value/eclipse/package-info.java @@ -0,0 +1,19 @@ +/* + * Copyright 2021 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +@NullMarked +package com.google.auto.value.eclipse; + +import com.google.auto.value.nullness.NullMarked; diff --git a/value/src/it/functional/src/test/java/com/google/auto/value/nullness/NonNull.java b/value/src/it/functional/src/test/java/com/google/auto/value/nullness/NonNull.java new file mode 100644 index 0000000000..36eb848c67 --- /dev/null +++ b/value/src/it/functional/src/test/java/com/google/auto/value/nullness/NonNull.java @@ -0,0 +1,25 @@ +/* + * Copyright 2021 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.auto.value.nullness; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Target(ElementType.TYPE_USE) +@Retention(RetentionPolicy.RUNTIME) +public @interface NonNull {} diff --git a/value/src/it/functional/src/test/java/com/google/auto/value/nullness/NullMarked.java b/value/src/it/functional/src/test/java/com/google/auto/value/nullness/NullMarked.java new file mode 100644 index 0000000000..247c289acf --- /dev/null +++ b/value/src/it/functional/src/test/java/com/google/auto/value/nullness/NullMarked.java @@ -0,0 +1,23 @@ +/* + * Copyright 2021 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.auto.value.nullness; + +/** + * A temporary annotation that we use in the {@code package-info.java} for the {@code + * EclipseNullable} test class. Once JSpecify annotations become + * available, we can switch to using them and delete this file. + */ +public @interface NullMarked {} diff --git a/value/src/it/functional/src/test/java/com/google/auto/value/nullness/Nullable.java b/value/src/it/functional/src/test/java/com/google/auto/value/nullness/Nullable.java new file mode 100644 index 0000000000..662700a2ce --- /dev/null +++ b/value/src/it/functional/src/test/java/com/google/auto/value/nullness/Nullable.java @@ -0,0 +1,25 @@ +/* + * Copyright 2021 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.auto.value.nullness; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Target(ElementType.TYPE_USE) +@Retention(RetentionPolicy.RUNTIME) +public @interface Nullable {} diff --git a/value/src/main/java/com/google/auto/value/processor/AutoOneOfProcessor.java b/value/src/main/java/com/google/auto/value/processor/AutoOneOfProcessor.java index bce7c93d62..c5276e1e36 100644 --- a/value/src/main/java/com/google/auto/value/processor/AutoOneOfProcessor.java +++ b/value/src/main/java/com/google/auto/value/processor/AutoOneOfProcessor.java @@ -113,8 +113,10 @@ void processType(TypeElement autoOneOfType) { AutoOneOfTemplateVars vars = new AutoOneOfTemplateVars(); vars.generatedClass = TypeSimplifier.simpleNameOf(subclass); vars.propertyToKind = propertyToKind; - defineSharedVarsForType(autoOneOfType, methods, vars); - defineVarsForType(autoOneOfType, vars, propertyMethodsAndTypes, kindGetter); + Optional nullableAnnotationType = Nullables.nullableMentionedInMethods(methods); + defineSharedVarsForType(autoOneOfType, methods, vars, nullableAnnotationType); + defineVarsForType( + autoOneOfType, vars, propertyMethodsAndTypes, kindGetter, nullableAnnotationType); String text = vars.toText(); text = TypeEncoder.decode(text, processingEnv, vars.pkg, autoOneOfType.asType()); @@ -264,9 +266,11 @@ private void defineVarsForType( TypeElement type, AutoOneOfTemplateVars vars, ImmutableMap propertyMethodsAndTypes, - ExecutableElement kindGetter) { + ExecutableElement kindGetter, + Optional nullableAnnotationType) { vars.props = propertySet( - propertyMethodsAndTypes, ImmutableListMultimap.of(), ImmutableListMultimap.of()); + propertyMethodsAndTypes, ImmutableListMultimap.of(), ImmutableListMultimap.of(), + nullableAnnotationType); vars.kindGetter = kindGetter.getSimpleName().toString(); vars.kindType = TypeEncoder.encode(kindGetter.getReturnType()); TypeElement javaIoSerializable = elementUtils().getTypeElement("java.io.Serializable"); diff --git a/value/src/main/java/com/google/auto/value/processor/AutoValueOrOneOfProcessor.java b/value/src/main/java/com/google/auto/value/processor/AutoValueOrOneOfProcessor.java index 4b545bc003..9bc7946e00 100644 --- a/value/src/main/java/com/google/auto/value/processor/AutoValueOrOneOfProcessor.java +++ b/value/src/main/java/com/google/auto/value/processor/AutoValueOrOneOfProcessor.java @@ -152,6 +152,7 @@ public static class Property { private final String identifier; private final ExecutableElement method; private final String type; + private final String builderFieldType; private final ImmutableList fieldAnnotations; private final ImmutableList methodAnnotations; private final Optional nullableAnnotation; @@ -162,6 +163,7 @@ public static class Property { String identifier, ExecutableElement method, String type, + String builderFieldType, ImmutableList fieldAnnotations, ImmutableList methodAnnotations, Optional nullableAnnotation) { @@ -169,6 +171,7 @@ public static class Property { this.identifier = identifier; this.method = method; this.type = type; + this.builderFieldType = builderFieldType; this.fieldAnnotations = fieldAnnotations; this.methodAnnotations = methodAnnotations; this.nullableAnnotation = nullableAnnotation; @@ -258,6 +261,15 @@ public boolean isNullable() { return nullableAnnotation.isPresent(); } + /** + * The spelling for a builder field to set this property. If the property has primitive type, + * this will be the corresponding boxed type. If the property is not already {@code @Nullable}, + * and a {@code @Nullable} is available, it will be applied to the (possibly boxed) type. + */ + public String getBuilderFieldType() { + return builderFieldType; + } + public String getAccess() { return SimpleMethod.access(method); } @@ -369,11 +381,13 @@ public final boolean process(Set annotations, RoundEnviro * specifically. Type annotations do not appear because they are considered part of the return * type and will appear when that is spelled out. Annotations that are excluded by {@code * AutoValue.CopyAnnotations} also do not appear here. + * @param nullableAnnotationType */ final ImmutableSet propertySet( ImmutableMap propertyMethodsAndTypes, ImmutableListMultimap annotatedPropertyFields, - ImmutableListMultimap annotatedPropertyMethods) { + ImmutableListMultimap annotatedPropertyMethods, + Optional nullableAnnotationType) { ImmutableBiMap methodToPropertyName = propertyNameToMethodMap(propertyMethodsAndTypes.keySet()).inverse(); Map methodToIdentifier = new LinkedHashMap<>(methodToPropertyName); @@ -382,9 +396,23 @@ final ImmutableSet propertySet( ImmutableSet.Builder props = ImmutableSet.builder(); propertyMethodsAndTypes.forEach( (propertyMethod, returnType) -> { + Set excludedAnnotationTypes = getExcludedAnnotationTypes(propertyMethod); String propertyType = TypeEncoder.encodeWithAnnotations( - returnType, ImmutableList.of(), getExcludedAnnotationTypes(propertyMethod)); + returnType, ImmutableList.of(), excludedAnnotationTypes); + Optional nullableAnnotation = nullableAnnotationForMethod(propertyMethod); + // Use the boxed type for builder fields (if the type is primitive) and add a + // @Nullable annotation if one is available and the type is not already @Nullable + // or Optional. + ImmutableList extraAnnotations = + (nullableAnnotation.isPresent() + || !nullableAnnotationType.isPresent() + || Optionalish.isOptional(returnType)) + ? ImmutableList.of() + : ImmutableList.of(annotationMirror(nullableAnnotationType.get())); + String builderFieldType = + TypeEncoder.encodeWithAnnotations( + box(returnType), extraAnnotations, excludedAnnotationTypes); String propertyName = methodToPropertyName.get(propertyMethod); String identifier = methodToIdentifier.get(propertyMethod); ImmutableList fieldAnnotations = @@ -392,13 +420,13 @@ final ImmutableSet propertySet( ImmutableList methodAnnotationMirrors = annotatedPropertyMethods.get(propertyMethod); ImmutableList methodAnnotations = annotationStrings(methodAnnotationMirrors); - Optional nullableAnnotation = nullableAnnotationForMethod(propertyMethod); Property p = new Property( propertyName, identifier, propertyMethod, propertyType, + builderFieldType, fieldAnnotations, methodAnnotations, nullableAnnotation); @@ -412,11 +440,18 @@ final ImmutableSet propertySet( return props.build(); } + private TypeMirror box(TypeMirror type) { + return type.getKind().isPrimitive() + ? typeUtils().boxedClass(MoreTypes.asPrimitiveType(type)).asType() + : type; + } + /** Defines the template variables that are shared by AutoValue and AutoOneOf. */ final void defineSharedVarsForType( TypeElement type, ImmutableSet methods, - AutoValueOrOneOfTemplateVars vars) { + AutoValueOrOneOfTemplateVars vars, + Optional nullableAnnotationType) { vars.pkg = TypeSimplifier.packageNameOf(type); vars.origClass = TypeSimplifier.classNameOf(type); vars.simpleClassName = TypeSimplifier.simpleNameOf(vars.origClass); @@ -433,8 +468,7 @@ final void defineSharedVarsForType( vars.toString = methodsToGenerate.containsKey(ObjectMethod.TO_STRING); vars.equals = methodsToGenerate.containsKey(ObjectMethod.EQUALS); vars.hashCode = methodsToGenerate.containsKey(ObjectMethod.HASH_CODE); - Optional nullable = Nullables.nullableMentionedInMethods(methods); - vars.equalsParameterType = equalsParameterType(methodsToGenerate, nullable); + vars.equalsParameterType = equalsParameterType(methodsToGenerate, nullableAnnotationType); vars.serialVersionUID = getSerialVersionUID(type); } @@ -1128,6 +1162,7 @@ static boolean hasAnnotationMirror(Element element, String annotationName) { } final void writeSourceFile(String className, String text, TypeElement originatingType) { + if (className.contains("EclipseNullable")) System.err.println(text); try { JavaFileObject sourceFile = processingEnv.getFiler().createSourceFile(className, originatingType); 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 13b5d1d9ef..3cdbb03fba 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 @@ -49,6 +49,7 @@ import javax.lang.model.element.ElementKind; import javax.lang.model.element.ExecutableElement; import javax.lang.model.element.TypeElement; +import javax.lang.model.type.DeclaredType; import javax.lang.model.type.TypeKind; import javax.lang.model.type.TypeMirror; import net.ltgt.gradle.incap.IncrementalAnnotationProcessor; @@ -244,8 +245,10 @@ void processType(TypeElement type) { vars.finalSubclass = TypeSimplifier.simpleNameOf(finalSubclass); vars.types = processingEnv.getTypeUtils(); vars.identifiers = !processingEnv.getOptions().containsKey(OMIT_IDENTIFIERS_OPTION); - defineSharedVarsForType(type, methods, vars); - defineVarsForType(type, vars, toBuilderMethods, propertyMethodsAndTypes, builder); + Optional nullableAnnotationType = Nullables.nullableMentionedInMethods(methods); + defineSharedVarsForType(type, methods, vars, nullableAnnotationType); + defineVarsForType(type, vars, toBuilderMethods, propertyMethodsAndTypes, builder, + nullableAnnotationType); // If we've encountered problems then we might end up invoking extensions with inconsistent // state. Anyway we probably don't want to generate code which is likely to provoke further @@ -420,7 +423,8 @@ private void defineVarsForType( AutoValueTemplateVars vars, ImmutableSet toBuilderMethods, ImmutableMap propertyMethodsAndTypes, - Optional maybeBuilder) { + Optional maybeBuilder, + Optional nullableAnnotationType) { ImmutableSet propertyMethods = propertyMethodsAndTypes.keySet(); // We can't use ImmutableList.toImmutableList() for obscure Google-internal reasons. vars.toBuilderMethods = @@ -430,7 +434,11 @@ private void defineVarsForType( ImmutableListMultimap annotatedPropertyMethods = propertyMethodAnnotationMap(type, propertyMethods); vars.props = - propertySet(propertyMethodsAndTypes, annotatedPropertyFields, annotatedPropertyMethods); + propertySet( + propertyMethodsAndTypes, + annotatedPropertyFields, + annotatedPropertyMethods, + nullableAnnotationType); // Check for @AutoValue.Builder and add appropriate variables if it is present. maybeBuilder.ifPresent( builder -> { diff --git a/value/src/main/java/com/google/auto/value/processor/TypeEncoder.java b/value/src/main/java/com/google/auto/value/processor/TypeEncoder.java index 56a69a3286..17d39c59a2 100644 --- a/value/src/main/java/com/google/auto/value/processor/TypeEncoder.java +++ b/value/src/main/java/com/google/auto/value/processor/TypeEncoder.java @@ -116,16 +116,15 @@ static String encodeWithAnnotations( ImmutableList extraAnnotations, Set excludedAnnotationTypes) { StringBuilder sb = new StringBuilder(); + ImmutableList annotationsOnType = + ImmutableList.builder() + .addAll(type.getAnnotationMirrors()) + .addAll(extraAnnotations) + .build(); // A function that is equivalent to t.getAnnotationMirrors() except when the t in question is // our starting type. In that case we also add extraAnnotations to the result. Function> getTypeAnnotations = - t -> - (t == type) - ? ImmutableList.builder() - .addAll(t.getAnnotationMirrors()) - .addAll(extraAnnotations) - .build() - : t.getAnnotationMirrors(); + t -> (t == type) ? annotationsOnType : t.getAnnotationMirrors(); return new AnnotatedEncodingTypeVisitor(excludedAnnotationTypes, getTypeAnnotations) .visit2(type, sb) .toString(); 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 04663a8359..2b1d5c2484 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 @@ -35,26 +35,16 @@ static #if ($isFinal) final #end class Builder${builderFormalTypes} ## ${builderTypeName}${builderActualTypes} { #foreach ($p in $props) - - #if ($p.kind.primitive) - - private $types.boxedClass($p.typeMirror).simpleName $p; - - #else - - #if ($builderPropertyBuilders[$p.name]) - ## If you have ImmutableList.Builder stringsBuilder() then we define two fields: - ## private ImmutableList.Builder stringsBuilder$; - ## private ImmutableList strings; - + #if ($builderPropertyBuilders[$p.name]) + ## If you have ImmutableList.Builder stringsBuilder() then we define two fields: + ## private ImmutableList.Builder stringsBuilder$; + ## private @Nullable ImmutableList strings; private ${builderPropertyBuilders[$p.name].builderType} ## ${builderPropertyBuilders[$p.name].name}; + #end - #end - - private $p.type $p #if ($p.optional && !$p.nullable) = $p.optional.empty #end ; + private $p.builderFieldType $p #if ($p.optional && !$p.nullable) = $p.optional.empty #end ; - #end #end Builder() { @@ -234,38 +224,58 @@ static #if ($isFinal) final #end class Builder${builderFormalTypes} ## #end #end -#if (!$builderRequiredProperties.empty) - #if ($identifiers) ## build a friendly message showing all missing properties +#if ($builderRequiredProperties.empty) + return new ${finalSubclass}${actualTypes}( + #foreach ($p in $props) - `java.lang.String` missing = ""; + this.$p #if ($foreach.hasNext) , #end + #end ); +#else + + ## Copy the required properties into local variables so nullness analysis + ## will know they haven't changed between the null check and the constructor + ## call. We have to use the original type, possibly boxed, because we don't + ## want a @Nullable annotation on a local variable. #foreach ($p in $builderRequiredProperties) + #if ($p.kind.primitive) `$types.boxedClass($p.typeMirror)` #else $p.type #end $p = this.$p; + #end - if (this.$p == null) { - missing += " $p.name"; - } + if (#foreach ($p in $builderRequiredProperties)## + $p == null## + #if ($foreach.hasNext) - #end + || #end + #end) { + #if ($identifiers) ## build a friendly message showing all missing properties + #if ($builderRequiredProperties.size() == 1) - if (!missing.isEmpty()) { - throw new IllegalStateException("Missing required properties:" + missing); - } + String missing$ = " $builderRequiredProperties.iterator().next()"; - #else ## just throw an exception if anything is missing + #else - if (#foreach ($p in $builderRequiredProperties)## - this.$p == null## - #if ($foreach.hasNext) || #end - #end) { + `java.lang.StringBuilder` missing$ = new `java.lang.StringBuilder`(); + #foreach ($p in $builderRequiredProperties) + if ($p == null) { + missing$.append(" $p.name"); + } + #end + + #end + throw new IllegalStateException("Missing required properties:" + missing$); + #else ## just throw an exception if anything is missing throw new IllegalStateException(); - } #end -#end + } + ## Now use either $p or this.$p depending on whether we copied into a local + ## variable above. return new ${finalSubclass}${actualTypes}( -#foreach ($p in $props) + #foreach ($p in $props) - this.$p #if ($foreach.hasNext) , #end -#end ); + #if (!$builderRequiredProperties.contains($p))this.#end$p## + #if ($foreach.hasNext) , #end + #end ); +#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 9ce85b6e50..2ea96f4009 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 @@ -53,6 +53,12 @@ public class AutoValueCompilationTest { @Rule public final Expect expect = Expect.create(); + // PLEASE DON'T ADD NEW POSITIVE TEST CASES HERE! + // Well, sometimes it's appropriate, but usually you should add them to AutoValueTest.java. + // We don't care as much about what the code looks like as that it does what it is supposed to do, + // and that's what AutoValueTest checks. The more positive test cases we have in this file, the + // more strings we have to update here when some detail of the generated code changes. + @Test public void simpleSuccess() { // Positive test case that ensures we generate the expected code for at least one case. @@ -959,6 +965,17 @@ public void nullablePrimitive() { @Test public void correctBuilder() { + // Also tests behaviour when @org.jspecify.nullness.Nullable is available. + JavaFileObject jspecifyNullable = + JavaFileObjects.forSourceLines( + "org.jspecify.nullness.Nullable", + "package org.jspecify.nullness;", + "", + "import java.lang.annotation.ElementType;", + "import java.lang.annotation.Target;", + "", + "@Target(ElementType.TYPE_USE)", + "public @interface Nullable {}"); JavaFileObject javaFileObject = JavaFileObjects.forSourceLines( "foo.bar.Baz", @@ -970,7 +987,7 @@ public void correctBuilder() { "", "import java.util.ArrayList;", "import java.util.List;", - "import javax.annotation.Nullable;", + "import org.jspecify.nullness.Nullable;", "", "@AutoValue", "public abstract class Baz {", @@ -978,7 +995,7 @@ public void correctBuilder() { " @SuppressWarnings(\"mutable\")", " public abstract byte[] aByteArray();", " @SuppressWarnings(\"mutable\")", - " @Nullable public abstract int[] aNullableIntArray();", + " public abstract int @Nullable [] aNullableIntArray();", " public abstract List aList();", " public abstract ImmutableList anImmutableList();", " public abstract Optional anOptionalString();", @@ -990,7 +1007,7 @@ public void correctBuilder() { " public abstract static class Builder {", " public abstract Builder anInt(int x);", " public abstract Builder aByteArray(byte[] x);", - " public abstract Builder aNullableIntArray(@Nullable int[] x);", + " public abstract Builder aNullableIntArray(int @Nullable [] x);", " public abstract Builder aList(List x);", " public abstract Builder anImmutableList(List x);", " public abstract ImmutableList.Builder anImmutableListBuilder();", @@ -1048,13 +1065,13 @@ public void correctBuilder() { "import java.util.List;", sorted( GeneratedImport.importGeneratedAnnotationType(), - "import javax.annotation.Nullable;"), + "import org.jspecify.nullness.Nullable;"), "", "@Generated(\"" + AutoValueProcessor.class.getName() + "\")", "final class AutoValue_Baz extends Baz {", " private final int anInt;", " private final byte[] aByteArray;", - " private final int[] aNullableIntArray;", + " private final int @Nullable [] aNullableIntArray;", " private final List aList;", " private final ImmutableList anImmutableList;", " private final Optional anOptionalString;", @@ -1063,7 +1080,7 @@ public void correctBuilder() { " private AutoValue_Baz(", " int anInt,", " byte[] aByteArray,", - " @Nullable int[] aNullableIntArray,", + " int @Nullable [] aNullableIntArray,", " List aList,", " ImmutableList anImmutableList,", " Optional anOptionalString,", @@ -1087,8 +1104,7 @@ public void correctBuilder() { " }", "", " @SuppressWarnings(\"mutable\")", - " @Nullable", - " @Override public int[] aNullableIntArray() {", + " @Override public int @Nullable [] aNullableIntArray() {", " return aNullableIntArray;", " }", "", @@ -1120,7 +1136,7 @@ public void correctBuilder() { " + \"}\";", " }", "", - " @Override public boolean equals(Object o) {", + " @Override public boolean equals(@Nullable Object o) {", " if (o == this) {", " return true;", " }", @@ -1165,15 +1181,15 @@ public void correctBuilder() { " }", "", " static final class Builder extends Baz.Builder {", - " private Integer anInt;", - " private byte[] aByteArray;", - " private int[] aNullableIntArray;", - " private List aList;", + " private @Nullable Integer anInt;", + " private byte @Nullable [] aByteArray;", + " private int @Nullable [] aNullableIntArray;", + " private @Nullable List aList;", " private ImmutableList.Builder anImmutableListBuilder$;", - " private ImmutableList anImmutableList;", + " private @Nullable ImmutableList anImmutableList;", " private Optional anOptionalString = Optional.absent();", " private NestedAutoValue.Builder aNestedAutoValueBuilder$;", - " private NestedAutoValue aNestedAutoValue;", + " private @Nullable NestedAutoValue aNestedAutoValue;", "", " Builder() {", " }", @@ -1213,7 +1229,7 @@ public void correctBuilder() { " }", "", " @Override", - " public Baz.Builder aNullableIntArray(@Nullable int[] aNullableIntArray) {", + " public Baz.Builder aNullableIntArray(int @Nullable [] aNullableIntArray) {", " this.aNullableIntArray = aNullableIntArray;", " return this;", " }", @@ -1312,24 +1328,29 @@ public void correctBuilder() { + "NestedAutoValue.builder();", " this.aNestedAutoValue = aNestedAutoValue$builder.build();", " }", - " String missing = \"\";", - " if (this.anInt == null) {", - " missing += \" anInt\";", - " }", - " if (this.aByteArray == null) {", - " missing += \" aByteArray\";", - " }", - " if (this.aList == null) {", - " missing += \" aList\";", - " }", - " if (!missing.isEmpty()) {", - " throw new IllegalStateException(\"Missing required properties:\" + missing);", + " Integer anInt = this.anInt;", + " byte[] aByteArray = this.aByteArray;", + " List aList = this.aList;", + " if (anInt == null", + " || aByteArray == null", + " || aList == null) {", + " StringBuilder missing$ = new StringBuilder();", + " if (anInt == null) {", + " missing$.append(\" anInt\");", + " }", + " if (aByteArray == null) {", + " missing$.append(\" aByteArray\");", + " }", + " if (aList == null) {", + " missing$.append(\" aList\");", + " }", + " throw new IllegalStateException(\"Missing required properties:\" + missing$);", " }", " return new AutoValue_Baz(", - " this.anInt,", - " this.aByteArray,", + " anInt,", + " aByteArray,", " this.aNullableIntArray,", - " this.aList,", + " aList,", " this.anImmutableList,", " this.anOptionalString,", " this.aNestedAutoValue);", @@ -1340,7 +1361,7 @@ public void correctBuilder() { javac() .withProcessors(new AutoValueProcessor()) .withOptions("-Xlint:-processing", "-implicit:none") - .compile(javaFileObject, nestedJavaFileObject); + .compile(javaFileObject, nestedJavaFileObject, jspecifyNullable); assertThat(compilation).succeededWithoutWarnings(); assertThat(compilation) .generatedSourceFile("foo.bar.AutoValue_Baz")