diff --git a/value/src/main/java/com/google/auto/value/processor/AutoAnnotationProcessor.java b/value/src/main/java/com/google/auto/value/processor/AutoAnnotationProcessor.java index d8c96a3d79..e3aa826cfa 100644 --- a/value/src/main/java/com/google/auto/value/processor/AutoAnnotationProcessor.java +++ b/value/src/main/java/com/google/auto/value/processor/AutoAnnotationProcessor.java @@ -82,7 +82,6 @@ public AutoAnnotationProcessor() {} private Elements elementUtils; private Types typeUtils; - private Nullables nullables; private TypeMirror javaLangObject; @Override @@ -100,7 +99,6 @@ public synchronized void init(ProcessingEnvironment processingEnv) { super.init(processingEnv); this.elementUtils = processingEnv.getElementUtils(); this.typeUtils = processingEnv.getTypeUtils(); - this.nullables = new Nullables(processingEnv); this.javaLangObject = elementUtils.getTypeElement("java.lang.Object").asType(); } @@ -201,8 +199,8 @@ private String equalsParameterType() { // Unlike AutoValue, we don't currently try to guess a @Nullable based on the methods in your // class. It's the default one or nothing. ImmutableList equalsParameterAnnotations = - nullables - .appropriateNullableGivenMethods(ImmutableSet.of()) + Nullables.fromMethods(processingEnv, ImmutableList.of()) + .nullableTypeAnnotation() .map(ImmutableList::of) .orElse(ImmutableList.of()); return TypeEncoder.encodeWithAnnotations( diff --git a/value/src/main/java/com/google/auto/value/processor/AutoBuilderProcessor.java b/value/src/main/java/com/google/auto/value/processor/AutoBuilderProcessor.java index 4cc9a3537a..511356c4da 100644 --- a/value/src/main/java/com/google/auto/value/processor/AutoBuilderProcessor.java +++ b/value/src/main/java/com/google/auto/value/processor/AutoBuilderProcessor.java @@ -205,7 +205,13 @@ private void processType(TypeElement autoBuilderType, TypeElement ofClass, Strin ImmutableMap propertyToGetterName = propertyToGetterName(executable, autoBuilderType); AutoBuilderTemplateVars vars = new AutoBuilderTemplateVars(); - vars.props = propertySet(executable, propertyToGetterName, propertyInitializers); + Nullables nullables = Nullables.fromMethods(processingEnv, methods); + vars.props = + propertySet( + executable, + propertyToGetterName, + propertyInitializers, + nullables); builder.defineVars(vars, classifier); vars.identifiers = !processingEnv.getOptions().containsKey(OMIT_IDENTIFIERS_OPTION); String generatedClassName = generatedClassName(autoBuilderType, "AutoBuilder_"); @@ -219,7 +225,8 @@ private void processType(TypeElement autoBuilderType, TypeElement ofClass, Strin .orElseGet(executable::invoke); vars.toBuilderConstructor = !propertyToGetterName.isEmpty(); vars.toBuilderMethods = ImmutableList.of(); - defineSharedVarsForType(autoBuilderType, ImmutableSet.of(), vars); + defineSharedVarsForType( + autoBuilderType, ImmutableSet.of(), nullables, vars); String text = vars.toText(); text = TypeEncoder.decode(text, processingEnv, vars.pkg, autoBuilderType.asType()); text = Reformatter.fixup(text); @@ -278,7 +285,8 @@ private Optional maybeForwardingClass( private ImmutableSet propertySet( Executable executable, Map propertyToGetterName, - ImmutableMap builderInitializers) { + ImmutableMap builderInitializers, + Nullables nullables) { // Fix any parameter names that are reserved words in Java. Java source code can't have // such parameter names, but Kotlin code might, for example. Map identifiers = @@ -294,7 +302,8 @@ private ImmutableSet propertySet( identifiers.get(v), propertyToGetterName.get(name), Optional.ofNullable(builderInitializers.get(name)), - executable.isOptional(name)); + executable.isOptional(name), + nullables); }) .collect(toImmutableSet()); } @@ -304,7 +313,8 @@ private Property newProperty( String identifier, String getterName, Optional builderInitializer, - boolean hasDefault) { + boolean hasDefault, + Nullables nullables) { String name = var.getSimpleName().toString(); TypeMirror type = var.asType(); Optional nullableAnnotation = nullableAnnotationFor(var, var.asType()); @@ -314,6 +324,7 @@ private Property newProperty( TypeEncoder.encode(type), type, nullableAnnotation, + nullables, getterName, builderInitializer, hasDefault); @@ -745,6 +756,9 @@ private void buildAnnotation( } private ImmutableSet annotationBuilderPropertySet(TypeElement annotationType) { + // Annotation methods can't have their own annotations so there's nowhere for us to discover + // a user @Nullable. We can only use our default @Nullable type annotation. + Nullables nullables = Nullables.fromMethods(processingEnv, ImmutableList.of()); // Translate the annotation elements into fake Property instances. We're really only interested // in the name and type, so we can use them to declare a parameter of the generated // @AutoAnnotation method. We'll generate a parameter for every element, even elements that @@ -752,11 +766,13 @@ private ImmutableSet annotationBuilderPropertySet(TypeElement annotati // value from the annotation to those parameters. return methodsIn(annotationType.getEnclosedElements()).stream() .filter(m -> m.getParameters().isEmpty() && !m.getModifiers().contains(Modifier.STATIC)) - .map(AutoBuilderProcessor::annotationBuilderProperty) + .map(method -> annotationBuilderProperty(method, nullables)) .collect(toImmutableSet()); } - private static Property annotationBuilderProperty(ExecutableElement annotationMethod) { + private static Property annotationBuilderProperty( + ExecutableElement annotationMethod, + Nullables nullables) { String name = annotationMethod.getSimpleName().toString(); TypeMirror type = annotationMethod.getReturnType(); return new Property( @@ -765,6 +781,7 @@ private static Property annotationBuilderProperty(ExecutableElement annotationMe TypeEncoder.encode(type), type, /* nullableAnnotation= */ Optional.empty(), + nullables, /* getter= */ "", /* maybeBuilderInitializer= */ Optional.empty(), /* hasDefault= */ false); 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 4d19d21654..d04f26f647 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 @@ -111,8 +111,9 @@ 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); + Nullables nullables = Nullables.fromMethods(processingEnv, methods); + defineSharedVarsForType(autoOneOfType, methods, nullables, vars); + defineVarsForType(autoOneOfType, vars, propertyMethodsAndTypes, kindGetter, nullables); String text = vars.toText(); text = TypeEncoder.decode(text, processingEnv, vars.pkg, autoOneOfType.asType()); @@ -257,10 +258,14 @@ private void defineVarsForType( TypeElement type, AutoOneOfTemplateVars vars, ImmutableMap propertyMethodsAndTypes, - ExecutableElement kindGetter) { + ExecutableElement kindGetter, + Nullables nullables) { vars.props = propertySet( - propertyMethodsAndTypes, ImmutableListMultimap.of(), ImmutableListMultimap.of()); + propertyMethodsAndTypes, + /* annotatedPropertyFields= */ ImmutableListMultimap.of(), + /* annotatedPropertyMethods= */ ImmutableListMultimap.of(), + nullables); 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/AutoValueProcessor.java b/value/src/main/java/com/google/auto/value/processor/AutoValueProcessor.java index e69587913e..821db9a8f0 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 @@ -244,8 +244,15 @@ void processType(TypeElement type) { String finalSubclass = TypeSimplifier.simpleNameOf(generatedSubclassName(type, 0)); AutoValueTemplateVars vars = new AutoValueTemplateVars(); vars.identifiers = !processingEnv.getOptions().containsKey(OMIT_IDENTIFIERS_OPTION); - defineSharedVarsForType(type, methods, vars); - defineVarsForType(type, vars, toBuilderMethods, propertyMethodsAndTypes, builder); + Nullables nullables = Nullables.fromMethods(processingEnv, methods); + defineSharedVarsForType(type, methods, nullables, vars); + defineVarsForType( + type, + vars, + toBuilderMethods, + propertyMethodsAndTypes, + builder, + nullables); vars.builtType = vars.origClass + vars.actualTypes; vars.build = "new " + finalSubclass + vars.actualTypes; @@ -422,7 +429,8 @@ private void defineVarsForType( AutoValueTemplateVars vars, ImmutableSet toBuilderMethods, ImmutableMap propertyMethodsAndTypes, - Optional maybeBuilder) { + Optional maybeBuilder, + Nullables nullables) { ImmutableSet propertyMethods = propertyMethodsAndTypes.keySet(); vars.toBuilderMethods = toBuilderMethods.stream().map(SimpleMethod::new).collect(toImmutableList()); @@ -432,7 +440,11 @@ private void defineVarsForType( ImmutableListMultimap annotatedPropertyMethods = propertyMethodAnnotationMap(type, propertyMethods); vars.props = - propertySet(propertyMethodsAndTypes, annotatedPropertyFields, annotatedPropertyMethods); + propertySet( + propertyMethodsAndTypes, + annotatedPropertyFields, + annotatedPropertyMethods, + nullables); // 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/AutoValueishProcessor.java b/value/src/main/java/com/google/auto/value/processor/AutoValueishProcessor.java index 23456477e2..0e4940cd93 100644 --- a/value/src/main/java/com/google/auto/value/processor/AutoValueishProcessor.java +++ b/value/src/main/java/com/google/auto/value/processor/AutoValueishProcessor.java @@ -114,13 +114,11 @@ abstract class AutoValueishProcessor extends AbstractProcessor { private String simpleAnnotationName; private ErrorReporter errorReporter; - private Nullables nullables; @Override public synchronized void init(ProcessingEnvironment processingEnv) { super.init(processingEnv); errorReporter = new ErrorReporter(processingEnv); - nullables = new Nullables(processingEnv); annotationType = elementUtils().getTypeElement(annotationClassName); if (annotationType != null) { simpleAnnotationName = annotationType.getSimpleName().toString(); @@ -166,6 +164,7 @@ public static class Property { private final String type; private final TypeMirror typeMirror; private final Optional nullableAnnotation; + private final Optional availableNullableTypeAnnotation; private final Optionalish optional; private final String getter; private final String builderInitializer; // empty, or with initial ` = `. @@ -177,6 +176,7 @@ public static class Property { String type, TypeMirror typeMirror, Optional nullableAnnotation, + Nullables nullables, String getter, Optional maybeBuilderInitializer, boolean hasDefault) { @@ -185,11 +185,12 @@ public static class Property { this.type = type; this.typeMirror = typeMirror; this.nullableAnnotation = nullableAnnotation; + this.availableNullableTypeAnnotation = nullables.nullableTypeAnnotation(); this.optional = Optionalish.createIfOptional(typeMirror); this.builderInitializer = maybeBuilderInitializer.isPresent() ? " = " + maybeBuilderInitializer.get() - : builderInitializer(); + : builderInitializer(typeMirror, nullableAnnotation); this.getter = getter; this.hasDefault = hasDefault; } @@ -200,7 +201,8 @@ public static class Property { * this property is an {@code Optional} and is not {@code @Nullable}. In that case the * initializer sets it to {@code Optional.empty()}. */ - private String builderInitializer() { + private static String builderInitializer( + TypeMirror typeMirror, Optional nullableAnnotation) { if (nullableAnnotation.isPresent()) { return ""; } @@ -211,6 +213,34 @@ private String builderInitializer() { return " = " + optional.getEmpty(); } + /** + * Returns the appropriate type for a builder field that will eventually be assigned to this + * property. This is the same as the final property type, except that it may have an additional + * {@code @Nullable} annotation. Some builder fields start off null and then acquire a value + * when the corresponding setter is called. Builder fields should have an extra + * {@code @Nullable} if all of the following conditions are met: + * + *
    + *
  • the property is not primitive; + *
  • the property is not already nullable; + *
  • there is no explicit initializer (for example {@code Optional} properties start off as + * {@code Optional.empty()}); + *
  • we have found a {@code @Nullable} type annotation that can be applied. + *
+ */ + public String getBuilderFieldType() { + if (typeMirror.getKind().isPrimitive() + || nullableAnnotation.isPresent() + || !builderInitializer.isEmpty() + || !availableNullableTypeAnnotation.isPresent()) { + return type; + } + return TypeEncoder.encodeWithAnnotations( + typeMirror, + ImmutableList.of(availableNullableTypeAnnotation.get()), + /* excludedAnnotationTypes= */ ImmutableSet.of()); + } + /** * Returns the name of the property as it should be used when declaring identifiers (fields and * parameters). If the original getter method was {@code foo()} then this will be {@code foo}. @@ -302,16 +332,19 @@ public static class GetterProperty extends Property { String name, String identifier, ExecutableElement method, - String type, + TypeMirror typeMirror, + String typeString, ImmutableList fieldAnnotations, ImmutableList methodAnnotations, - Optional nullableAnnotation) { + Optional nullableAnnotation, + Nullables nullables) { super( name, identifier, - type, - method.getReturnType(), + typeString, + typeMirror, nullableAnnotation, + nullables, method.getSimpleName().toString(), Optional.empty(), /* hasDefault= */ false); @@ -492,7 +525,8 @@ private void validateType(TypeElement type) { final ImmutableSet propertySet( ImmutableMap propertyMethodsAndTypes, ImmutableListMultimap annotatedPropertyFields, - ImmutableListMultimap annotatedPropertyMethods) { + ImmutableListMultimap annotatedPropertyMethods, + Nullables nullables) { ImmutableBiMap methodToPropertyName = propertyNameToMethodMap(propertyMethodsAndTypes.keySet()).inverse(); Map methodToIdentifier = new LinkedHashMap<>(methodToPropertyName); @@ -501,7 +535,7 @@ final ImmutableSet propertySet( ImmutableSet.Builder props = ImmutableSet.builder(); propertyMethodsAndTypes.forEach( (propertyMethod, returnType) -> { - String propertyType = + String propertyTypeString = TypeEncoder.encodeWithAnnotations( returnType, ImmutableList.of(), getExcludedAnnotationTypes(propertyMethod)); String propertyName = methodToPropertyName.get(propertyMethod); @@ -517,10 +551,12 @@ final ImmutableSet propertySet( propertyName, identifier, propertyMethod, - propertyType, + returnType, + propertyTypeString, fieldAnnotations, methodAnnotations, - nullableAnnotation); + nullableAnnotation, + nullables); props.add(p); if (p.isNullable() && returnType.getKind().isPrimitive()) { errorReporter() @@ -535,7 +571,10 @@ final ImmutableSet propertySet( /** Defines the template variables that are shared by AutoValue, AutoOneOf, and AutoBuilder. */ final void defineSharedVarsForType( - TypeElement type, ImmutableSet methods, AutoValueishTemplateVars vars) { + TypeElement type, + ImmutableSet methods, + Nullables nullables, + AutoValueishTemplateVars vars) { vars.pkg = TypeSimplifier.packageNameOf(type); vars.origClass = TypeSimplifier.classNameOf(type); vars.simpleClassName = TypeSimplifier.simpleNameOf(vars.origClass); @@ -552,8 +591,8 @@ 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.appropriateNullableGivenMethods(methods); - vars.equalsParameterType = equalsParameterType(methodsToGenerate, nullable); + vars.equalsParameterType = + equalsParameterType(methodsToGenerate, nullables); vars.serialVersionUID = getSerialVersionUID(type); } @@ -835,7 +874,7 @@ private static Map determineObjectMethodsToGene * @param nullable the type of a {@code @Nullable} type annotation that we have found, if any */ static String equalsParameterType( - Map methodsToGenerate, Optional nullable) { + Map methodsToGenerate, Nullables nullables) { ExecutableElement equals = methodsToGenerate.get(ObjectMethod.EQUALS); if (equals == null) { return ""; // this will not be referenced because no equals method will be generated @@ -844,11 +883,14 @@ static String equalsParameterType( // Add @Nullable if we know one and the parameter doesn't already have one. // The @Nullable we add will be a type annotation, but if the parameter already has @Nullable // then that might be a type annotation or an annotation on the parameter. + Optional nullableTypeAnnotation = nullables.nullableTypeAnnotation(); ImmutableList extraAnnotations = - nullable.isPresent() && !nullableAnnotationFor(equals, parameterType).isPresent() - ? ImmutableList.of(nullable.get()) + nullableTypeAnnotation.isPresent() + && !nullableAnnotationFor(equals, parameterType).isPresent() + ? ImmutableList.of(nullableTypeAnnotation.get()) : ImmutableList.of(); - return TypeEncoder.encodeWithAnnotations(parameterType, extraAnnotations, ImmutableSet.of()); + return TypeEncoder.encodeWithAnnotations( + parameterType, extraAnnotations, /* excludedAnnotationTypes= */ ImmutableSet.of()); } /** diff --git a/value/src/main/java/com/google/auto/value/processor/Nullables.java b/value/src/main/java/com/google/auto/value/processor/Nullables.java index c07656f1f4..f7925ea4d4 100644 --- a/value/src/main/java/com/google/auto/value/processor/Nullables.java +++ b/value/src/main/java/com/google/auto/value/processor/Nullables.java @@ -18,7 +18,6 @@ import static java.util.stream.Collectors.toList; import com.google.auto.common.MoreTypes; -import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -52,19 +51,56 @@ class Nullables { // We write this using .concat in order to hide it from rewriting rules. private static final String DEFAULT_NULLABLE = "org".concat(".jspecify.nullness.Nullable"); - private final Optional defaultNullable; + private final Optional nullableTypeAnnotation; - Nullables(ProcessingEnvironment processingEnv) { + private Nullables(Optional nullableTypeAnnotation) { + this.nullableTypeAnnotation = nullableTypeAnnotation; + } + + /** + * Make an instance where the default {@code @Nullable} type annotation is discovered by looking + * for methods whose parameter or return types have such an annotation. If there are none, use a + * default {@code @Nullable} type annotation if it is available. + * + * @param methods the methods to examine + * @param processingEnv the {@link ProcessingEnvironment}, or null if one is unavailable + * (typically in tests) + */ + static Nullables fromMethods( + /* @Nullable */ ProcessingEnvironment processingEnv, Collection methods) { + Optional nullableTypeAnnotation = + methods.stream() + .flatMap( + method -> + Stream.concat( + Stream.of(method.getReturnType()), + method.getParameters().stream().map(Element::asType))) + .map(Nullables::nullableIn) + .filter(Optional::isPresent) + .findFirst() + .orElseGet(() -> defaultNullableTypeAnnotation(processingEnv)); + return new Nullables(nullableTypeAnnotation); + } + + /** Returns an appropriate {@code @Nullable} type-annotation, if one is known. */ + Optional nullableTypeAnnotation() { + return nullableTypeAnnotation; + } + + private static Optional defaultNullableTypeAnnotation( + /* @Nullable */ ProcessingEnvironment processingEnv) { + if (processingEnv == null) { + return Optional.empty(); + } // -Afoo without `=` sets "foo" to null in the getOptions() map. String nullableOption = Strings.nullToEmpty( processingEnv.getOptions().getOrDefault(NULLABLE_OPTION, DEFAULT_NULLABLE)); - this.defaultNullable = - (!nullableOption.isEmpty() - && processingEnv.getSourceVersion().ordinal() >= SourceVersion.RELEASE_8.ordinal()) - ? Optional.ofNullable(processingEnv.getElementUtils().getTypeElement(nullableOption)) - .map(t -> annotationMirrorOf(MoreTypes.asDeclared(t.asType()))) - : Optional.empty(); + return (!nullableOption.isEmpty() + && processingEnv.getSourceVersion().ordinal() >= SourceVersion.RELEASE_8.ordinal()) + ? Optional.ofNullable(processingEnv.getElementUtils().getTypeElement(nullableOption)) + .map(t -> annotationMirrorOf(MoreTypes.asDeclared(t.asType()))) + : Optional.empty(); } private static AnnotationMirror annotationMirrorOf(DeclaredType annotationType) { @@ -82,38 +118,6 @@ public DeclaredType getAnnotationType() { }; } - /** - * Returns the type of a {@code @Nullable} type-annotation, if one is found anywhere in the - * signatures of the given methods. - */ - @VisibleForTesting - static Optional nullableMentionedInMethods( - Collection methods) { - return methods.stream() - .flatMap( - method -> - Stream.concat( - Stream.of(method.getReturnType()), - method.getParameters().stream().map(Element::asType))) - .map(Nullables::nullableIn) - .filter(Optional::isPresent) - .findFirst() - .orElse(Optional.empty()); - } - - /** - * Returns the type of an appropriate {@code @Nullable} type-annotation, given a set of methods - * that are known to be in the same compilation as the code being generated. If one of those - * methods contains an appropriate {@code @Nullable} annotation on a parameter or return type, - * this method will return that. Otherwise, if the JSpecify - * {@code @Nullable} is available, this method will return it. Otherwise, this method will return - * an empty {@code Optional}. - */ - Optional appropriateNullableGivenMethods( - Collection methods) { - return nullableMentionedInMethods(methods).map(Optional::of).orElse(defaultNullable); - } - private static Optional nullableIn(TypeMirror type) { return new NullableFinder().visit(type); } 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 d032cda98f..8a106b87b7 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 @@ -47,7 +47,7 @@ class ${builderName}${builderFormalTypes} ## #end - private $p.type $p $p.builderInitializer; + private $p.builderFieldType $p $p.builderInitializer; #end diff --git a/value/src/test/java/com/google/auto/value/processor/AutoBuilderCompilationTest.java b/value/src/test/java/com/google/auto/value/processor/AutoBuilderCompilationTest.java index 0fe4a10ed0..77ec4c75ab 100644 --- a/value/src/test/java/com/google/auto/value/processor/AutoBuilderCompilationTest.java +++ b/value/src/test/java/com/google/auto/value/processor/AutoBuilderCompilationTest.java @@ -19,6 +19,8 @@ import static com.google.common.truth.TruthJUnit.assume; import static com.google.testing.compile.CompilationSubject.assertThat; import static com.google.testing.compile.Compiler.javac; +import static java.util.Arrays.stream; +import static java.util.stream.Collectors.joining; import com.google.testing.compile.Compilation; import com.google.testing.compile.JavaFileObjects; @@ -34,12 +36,14 @@ public final class AutoBuilderCompilationTest { "foo.bar.AutoBuilder_Baz_Builder", "package foo.bar;", "", - GeneratedImport.importGeneratedAnnotationType(), + sorted( + GeneratedImport.importGeneratedAnnotationType(), + "import org.checkerframework.checker.nullness.qual.Nullable;"), "", "@Generated(\"" + AutoBuilderProcessor.class.getName() + "\")", "class AutoBuilder_Baz_Builder implements Baz.Builder {", " private int anInt;", - " private String aString;", + " private @Nullable String aString;", " private byte set$0;", "", " AutoBuilder_Baz_Builder() {}", @@ -123,6 +127,7 @@ public void simpleSuccess() { Compilation compilation = javac() .withProcessors(new AutoBuilderProcessor()) + .withOptions("-A" + Nullables.NULLABLE_OPTION + "=org.checkerframework.checker.nullness.qual.Nullable") .compile(javaFileObject); assertThat(compilation) .generatedSourceFile("foo.bar.AutoBuilder_Baz_Builder") @@ -994,4 +999,8 @@ public void annotationWithCallMethod() { .inFile(javaFileObject) .onLineContaining("interface Builder"); } + + private static String sorted(String... imports) { + return stream(imports).sorted().collect(joining("\n")); + } } 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 8e3eafea62..c651769385 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 @@ -15,10 +15,13 @@ */ package com.google.auto.value.processor; +import static com.google.common.base.StandardSystemProperty.JAVA_SPECIFICATION_VERSION; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.TruthJUnit.assume; import static com.google.testing.compile.CompilationSubject.assertThat; import static com.google.testing.compile.CompilationSubject.compilations; import static com.google.testing.compile.Compiler.javac; +import static java.util.Arrays.stream; import static java.util.stream.Collectors.joining; import com.google.common.collect.ImmutableList; @@ -33,7 +36,6 @@ import java.io.Writer; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; -import java.util.Arrays; import java.util.Set; import javax.annotation.processing.AbstractProcessor; import javax.annotation.processing.RoundEnvironment; @@ -1286,19 +1288,19 @@ public void correctBuilder() { " this.anImmutableMap = source.anImmutableMap();", " this.anOptionalString = source.anOptionalString();", " this.aNestedAutoValue = source.aNestedAutoValue();", - " set$0 = (byte) 0x1;", + " set$0 = (byte) 1;", " }", "", " @Override", " public Baz.Builder anInt(int anInt) {", " this.anInt = anInt;", - " set$0 |= (byte) 0x1", + " set$0 |= (byte) 1;", " return this;", " }", "", " @Override", " public Optional anInt() {", - " if ((set$0 & 0x1) == 0) {", + " if ((set$0 & 1) == 0) {", " return Optional.absent();", " }", " return Optional.of(anInt);", @@ -1413,11 +1415,11 @@ public void correctBuilder() { + "NestedAutoValue.builder();", " this.aNestedAutoValue = aNestedAutoValue$builder.build();", " }", - " if (set$0 != 0x1", + " if (set$0 != 1", " || this.aByteArray == null", " || this.aList == null) {", " StringBuilder missing = new StringBuilder();", - " if ((set$0 & 0x1) == 0) {", + " if ((set$0 & 1) == 0) {", " missing.append(\" anInt\");", " }", " if (this.aByteArray == null) {", @@ -1451,6 +1453,304 @@ public void correctBuilder() { .hasSourceEquivalentTo(expectedOutput); } + @Test + public void builderWithNullableTypeAnnotation() { + // Sadly we can't rely on JDK 8 to handle type annotations correctly. + // Some versions do, some don't. So skip the test unless we are on at least JDK 9. + double javaVersion = Double.parseDouble(JAVA_SPECIFICATION_VERSION.value()); + assume().that(javaVersion).isAtLeast(9.0); + JavaFileObject javaFileObject = + JavaFileObjects.forSourceLines( + "foo.bar.Baz", + "package foo.bar;", + "", + "import com.google.auto.value.AutoValue;", + "import com.google.common.base.Optional;", + "import com.google.common.collect.ImmutableMap;", + "", + "import java.util.ArrayList;", + "import java.util.List;", + "import java.util.Map;", + "import org.checkerframework.checker.nullness.qual.Nullable;", + "", + "@AutoValue", + "public abstract class Baz {", + " public abstract int anInt();", + " @SuppressWarnings(\"mutable\")", + " public abstract byte[] aByteArray();", + " @SuppressWarnings(\"mutable\")", + " public abstract int @Nullable [] aNullableIntArray();", + " public abstract List aList();", + " public abstract ImmutableMap anImmutableMap();", + " public abstract Optional anOptionalString();", + "", + " public abstract Builder toBuilder();", + "", + " @AutoValue.Builder", + " public abstract static class Builder {", + " public abstract Builder anInt(int x);", + " public abstract Builder aByteArray(byte[] x);", + " public abstract Builder aNullableIntArray(int @Nullable [] x);", + " public abstract Builder aList(List x);", + " public abstract Builder anImmutableMap(Map x);", + " public abstract ImmutableMap.Builder anImmutableMapBuilder();", + " public abstract Builder anOptionalString(Optional s);", + " public abstract Baz build();", + " }", + "", + " public static Builder builder() {", + " return AutoValue_Baz.builder();", + " }", + "}"); + JavaFileObject expectedOutput = + JavaFileObjects.forSourceLines( + "foo.bar.AutoValue_Baz", + "package foo.bar;", + "", + "import com.google.common.base.Optional;", + "import com.google.common.collect.ImmutableMap;", + "import java.util.Arrays;", + "import java.util.List;", + "import java.util.Map;", + sorted( + GeneratedImport.importGeneratedAnnotationType(), + "import org.checkerframework.checker.nullness.qual.Nullable;"), + "", + "@Generated(\"" + AutoValueProcessor.class.getName() + "\")", + "final class AutoValue_Baz extends Baz {", + " private final int anInt;", + " private final byte[] aByteArray;", + " private final int @Nullable [] aNullableIntArray;", + " private final List aList;", + " 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;", + " }", + "", + " @Override public int anInt() {", + " return anInt;", + " }", + "", + " @SuppressWarnings(\"mutable\")", + " @Override public byte[] aByteArray() {", + " return aByteArray;", + " }", + "", + " @SuppressWarnings(\"mutable\")", + " @Override public int @Nullable [] aNullableIntArray() {", + " return aNullableIntArray;", + " }", + "", + " @Override public List aList() {", + " return aList;", + " }", + "", + " @Override public ImmutableMap anImmutableMap() {", + " return anImmutableMap;", + " }", + "", + " @Override public Optional anOptionalString() {", + " return anOptionalString;", + " }", + "", + " @Override public String toString() {", + " return \"Baz{\"", + " + \"anInt=\" + anInt + \", \"", + " + \"aByteArray=\" + Arrays.toString(aByteArray) + \", \"", + " + \"aNullableIntArray=\" + Arrays.toString(aNullableIntArray) + \", \"", + " + \"aList=\" + aList + \", \"", + " + \"anImmutableMap=\" + anImmutableMap + \", \"", + " + \"anOptionalString=\" + anOptionalString", + " + \"}\";", + " }", + "", + " @Override public boolean equals(@Nullable Object o) {", + " if (o == this) {", + " return true;", + " }", + " if (o instanceof Baz) {", + " Baz that = (Baz) o;", + " return this.anInt == that.anInt()", + " && Arrays.equals(this.aByteArray, " + + "(that instanceof AutoValue_Baz) " + + "? ((AutoValue_Baz) that).aByteArray : that.aByteArray())", + " && Arrays.equals(this.aNullableIntArray, " + + "(that instanceof AutoValue_Baz) " + + "? ((AutoValue_Baz) that).aNullableIntArray : that.aNullableIntArray())", + " && this.aList.equals(that.aList())", + " && this.anImmutableMap.equals(that.anImmutableMap())", + " && this.anOptionalString.equals(that.anOptionalString());", + " }", + " return false;", + " }", + "", + " @Override public int hashCode() {", + " int h$ = 1;", + " h$ *= 1000003;", + " h$ ^= anInt;", + " h$ *= 1000003;", + " h$ ^= Arrays.hashCode(aByteArray);", + " h$ *= 1000003;", + " h$ ^= Arrays.hashCode(aNullableIntArray);", + " h$ *= 1000003;", + " h$ ^= aList.hashCode();", + " h$ *= 1000003;", + " h$ ^= anImmutableMap.hashCode();", + " h$ *= 1000003;", + " h$ ^= anOptionalString.hashCode();", + " return h$;", + " }", + "", + " @Override public Baz.Builder toBuilder() {", + " return new Builder(this);", + " }", + "", + " static final class Builder extends Baz.Builder {", + " private int anInt;", + " private byte @Nullable [] aByteArray;", + " private int @Nullable [] aNullableIntArray;", + " private @Nullable List aList;", + " private ImmutableMap.@Nullable Builder anImmutableMapBuilder$;", + " private @Nullable ImmutableMap anImmutableMap;", + " private Optional anOptionalString = Optional.absent();", + " private byte set$0;", + "", + " Builder() {", + " }", + "", + " private Builder(Baz source) {", + " this.anInt = source.anInt();", + " this.aByteArray = source.aByteArray();", + " this.aNullableIntArray = source.aNullableIntArray();", + " this.aList = source.aList();", + " this.anImmutableMap = source.anImmutableMap();", + " this.anOptionalString = source.anOptionalString();", + " set$0 = (byte) 1;", + " }", + "", + " @Override", + " public Baz.Builder anInt(int anInt) {", + " this.anInt = anInt;", + " set$0 |= (byte) 1;", + " return this;", + " }", + "", + " @Override", + " public Baz.Builder aByteArray(byte[] aByteArray) {", + " if (aByteArray == null) {", + " throw new NullPointerException(\"Null aByteArray\");", + " }", + " this.aByteArray = aByteArray;", + " return this;", + " }", + "", + " @Override", + " public Baz.Builder aNullableIntArray(int @Nullable [] aNullableIntArray) {", + " this.aNullableIntArray = aNullableIntArray;", + " return this;", + " }", + "", + " @Override", + " public Baz.Builder aList(List aList) {", + " if (aList == null) {", + " throw new NullPointerException(\"Null aList\");", + " }", + " this.aList = aList;", + " return this;", + " }", + "", + " @Override", + " public Baz.Builder anImmutableMap(Map anImmutableMap) {", + " if (anImmutableMapBuilder$ != null) {", + " throw new IllegalStateException(" + + "\"Cannot set anImmutableMap after calling anImmutableMapBuilder()\");", + " }", + " this.anImmutableMap = ImmutableMap.copyOf(anImmutableMap);", + " return this;", + " }", + "", + " @Override", + " public ImmutableMap.Builder anImmutableMapBuilder() {", + " if (anImmutableMapBuilder$ == null) {", + " if (anImmutableMap == null) {", + " anImmutableMapBuilder$ = ImmutableMap.builder();", + " } else {", + " anImmutableMapBuilder$ = ImmutableMap.builder();", + " anImmutableMapBuilder$.putAll(anImmutableMap);", + " anImmutableMap = null;", + " }", + " }", + " return anImmutableMapBuilder$;", + " }", + "", + " @Override", + " public Baz.Builder anOptionalString(Optional anOptionalString) {", + " if (anOptionalString == null) {", + " throw new NullPointerException(\"Null anOptionalString\");", + " }", + " this.anOptionalString = anOptionalString;", + " return this;", + " }", + "", + " @Override", + " public Baz build() {", + " if (anImmutableMapBuilder$ != null) {", + " this.anImmutableMap = anImmutableMapBuilder$.buildOrThrow();", + " } else if (this.anImmutableMap == null) {", + " this.anImmutableMap = ImmutableMap.of();", + " }", + " if (set$0 != 1", + " || this.aByteArray == null", + " || this.aList == null) {", + " StringBuilder missing = new StringBuilder();", + " if ((set$0 & 1) == 0) {", + " missing.append(\" anInt\");", + " }", + " if (this.aByteArray == null) {", + " missing.append(\" aByteArray\");", + " }", + " if (this.aList == null) {", + " missing.append(\" aList\");", + " }", + " throw new IllegalStateException(\"Missing required properties:\" + missing);", + " }", + " return new AutoValue_Baz(", + " this.anInt,", + " this.aByteArray,", + " this.aNullableIntArray,", + " this.aList,", + " this.anImmutableMap,", + " this.anOptionalString);", + " }", + " }", + "}"); + Compilation compilation = + javac() + .withProcessors(new AutoValueProcessor()) + .withOptions( + "-Xlint:-processing", + "-implicit:none", + "-A" + Nullables.NULLABLE_OPTION + "=org.checkerframework.checker.nullness.qual.Nullable") + .compile(javaFileObject); + assertThat(compilation).succeededWithoutWarnings(); + assertThat(compilation) + .generatedSourceFile("foo.bar.AutoValue_Baz") + .hasSourceEquivalentTo(expectedOutput); + } + @Test public void autoValueBuilderOnTopLevelClass() { JavaFileObject javaFileObject = @@ -3671,7 +3971,7 @@ public void kotlinMetadataAnnotationsAreImplicitlyExcludedFromCopying() { .doesNotContain("kotlin.Metadata"); } - private String sorted(String... imports) { - return Arrays.stream(imports).sorted().collect(joining("\n")); + private static String sorted(String... imports) { + return stream(imports).sorted().collect(joining("\n")); } } diff --git a/value/src/test/java/com/google/auto/value/processor/BuilderRequiredPropertiesTest.java b/value/src/test/java/com/google/auto/value/processor/BuilderRequiredPropertiesTest.java index d8286e1f9d..fe7ad321ca 100644 --- a/value/src/test/java/com/google/auto/value/processor/BuilderRequiredPropertiesTest.java +++ b/value/src/test/java/com/google/auto/value/processor/BuilderRequiredPropertiesTest.java @@ -343,6 +343,7 @@ private Property fakeProperty(String name, TypeMirror type, boolean hasDefault) /* type= */ type.toString(), /* typeMirror= */ type, /* nullableAnnotation= */ Optional.empty(), + /* nullables= */ Nullables.fromMethods(null, ImmutableList.of()), /* getter= */ name, /* maybeBuilderInitializer= */ Optional.empty(), /* hasDefault= */ hasDefault); diff --git a/value/src/test/java/com/google/auto/value/processor/NullablesTest.java b/value/src/test/java/com/google/auto/value/processor/NullablesTest.java index 9e345f53f2..4d542625ab 100644 --- a/value/src/test/java/com/google/auto/value/processor/NullablesTest.java +++ b/value/src/test/java/com/google/auto/value/processor/NullablesTest.java @@ -148,7 +148,7 @@ public boolean process(Set annotations, RoundEnvironment expect .about(optionals()) - .that(Nullables.nullableMentionedInMethods(notNullableMethods)) + .that(Nullables.fromMethods(null, notNullableMethods).nullableTypeAnnotation()) .isEmpty(); TypeElement nullableElement = @@ -167,7 +167,8 @@ public boolean process(Set annotations, RoundEnvironment .withMessage("method %s should have @Nullable", nullableMethod) .about(optionals()) .that( - Nullables.nullableMentionedInMethods(notNullablePlusNullable) + Nullables.fromMethods(null, notNullablePlusNullable) + .nullableTypeAnnotation() .map(AnnotationMirror::getAnnotationType)) .hasValue(nullableType); }