Skip to content

Commit

Permalink
Include a @Nullable type annotation on appropriate builder fields i…
Browse files Browse the repository at this point in the history 
…f available.

RELNOTES=Generated builders now include a `@Nullable` type annotation on appropriate builder fields if one is available.
PiperOrigin-RevId: 492071989
  • Loading branch information
@eamonnmcmanus
eamonnmcmanus authored and Google Java Core Libraries committed Dec 1, 2022
1 parent 050f0ac commit 91d5f32
Show file tree
Hide file tree
Showing 11 changed files with 481 additions and 92 deletions.
6 changes: 2 additions & 4 deletions value/src/main/java/com/google/auto/value/processor/AutoAnnotationProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ public AutoAnnotationProcessor() {}

private Elements elementUtils;
private Types typeUtils;
private Nullables nullables;
private TypeMirror javaLangObject;

@Override
Expand All @@ -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();
}

Expand Down Expand Up @@ -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<AnnotationMirror> equalsParameterAnnotations =
nullables
.appropriateNullableGivenMethods(ImmutableSet.of())
Nullables.fromMethods(processingEnv, ImmutableList.of())
.nullableTypeAnnotation()
.map(ImmutableList::of)
.orElse(ImmutableList.of());
return TypeEncoder.encodeWithAnnotations(
Expand Down
31 changes: 24 additions & 7 deletions value/src/main/java/com/google/auto/value/processor/AutoBuilderProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,13 @@ private void processType(TypeElement autoBuilderType, TypeElement ofClass, Strin
ImmutableMap<String, String> 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_");
Expand All @@ -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);
Expand Down Expand Up @@ -278,7 +285,8 @@ private Optional<String> maybeForwardingClass(
private ImmutableSet<Property> propertySet(
Executable executable,
Map<String, String> propertyToGetterName,
ImmutableMap<String, String> builderInitializers) {
ImmutableMap<String, String> 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<VariableElement, String> identifiers =
Expand All @@ -294,7 +302,8 @@ private ImmutableSet<Property> propertySet(
identifiers.get(v),
propertyToGetterName.get(name),
Optional.ofNullable(builderInitializers.get(name)),
executable.isOptional(name));
executable.isOptional(name),
nullables);
})
.collect(toImmutableSet());
}
Expand All @@ -304,7 +313,8 @@ private Property newProperty(
String identifier,
String getterName,
Optional<String> builderInitializer,
boolean hasDefault) {
boolean hasDefault,
Nullables nullables) {
String name = var.getSimpleName().toString();
TypeMirror type = var.asType();
Optional<String> nullableAnnotation = nullableAnnotationFor(var, var.asType());
Expand All @@ -314,6 +324,7 @@ private Property newProperty(
TypeEncoder.encode(type),
type,
nullableAnnotation,
nullables,
getterName,
builderInitializer,
hasDefault);
Expand Down Expand Up @@ -745,18 +756,23 @@ private void buildAnnotation(
}

private ImmutableSet<Property> 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
// don't have setters in the builder. The generated builder implementation will pass the default
// 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(
Expand All @@ -765,6 +781,7 @@ private static Property annotationBuilderProperty(ExecutableElement annotationMe
TypeEncoder.encode(type),
type,
/* nullableAnnotation= */ Optional.empty(),
nullables,
/* getter= */ "",
/* maybeBuilderInitializer= */ Optional.empty(),
/* hasDefault= */ false);
Expand Down
13 changes: 9 additions & 4 deletions value/src/main/java/com/google/auto/value/processor/AutoOneOfProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -257,10 +258,14 @@ private void defineVarsForType(
TypeElement type,
AutoOneOfTemplateVars vars,
ImmutableMap<ExecutableElement, TypeMirror> 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");
Expand Down
20 changes: 16 additions & 4 deletions value/src/main/java/com/google/auto/value/processor/AutoValueProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -422,7 +429,8 @@ private void defineVarsForType(
AutoValueTemplateVars vars,
ImmutableSet<ExecutableElement> toBuilderMethods,
ImmutableMap<ExecutableElement, TypeMirror> propertyMethodsAndTypes,
Optional<BuilderSpec.Builder> maybeBuilder) {
Optional<BuilderSpec.Builder> maybeBuilder,
Nullables nullables) {
ImmutableSet<ExecutableElement> propertyMethods = propertyMethodsAndTypes.keySet();
vars.toBuilderMethods =
toBuilderMethods.stream().map(SimpleMethod::new).collect(toImmutableList());
Expand All @@ -432,7 +440,11 @@ private void defineVarsForType(
ImmutableListMultimap<ExecutableElement, AnnotationMirror> 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 -> {
Expand Down
80 changes: 61 additions & 19 deletions value/src/main/java/com/google/auto/value/processor/AutoValueishProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -166,6 +164,7 @@ public static class Property {
private final String type;
private final TypeMirror typeMirror;
private final Optional<String> nullableAnnotation;
private final Optional<AnnotationMirror> availableNullableTypeAnnotation;
private final Optionalish optional;
private final String getter;
private final String builderInitializer; // empty, or with initial ` = `.
Expand All @@ -177,6 +176,7 @@ public static class Property {
String type,
TypeMirror typeMirror,
Optional<String> nullableAnnotation,
Nullables nullables,
String getter,
Optional<String> maybeBuilderInitializer,
boolean hasDefault) {
Expand All @@ -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;
}
Expand All @@ -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<String> nullableAnnotation) {
if (nullableAnnotation.isPresent()) {
return "";
}
Expand All @@ -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:
*
* <ul>
* <li>the property is not primitive;
* <li>the property is not already nullable;
* <li>there is no explicit initializer (for example {@code Optional} properties start off as
* {@code Optional.empty()});
* <li>we have found a {@code @Nullable} type annotation that can be applied.
* </ul>
*/
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}.
Expand Down Expand Up @@ -302,16 +332,19 @@ public static class GetterProperty extends Property {
String name,
String identifier,
ExecutableElement method,
String type,
TypeMirror typeMirror,
String typeString,
ImmutableList<String> fieldAnnotations,
ImmutableList<String> methodAnnotations,
Optional<String> nullableAnnotation) {
Optional<String> nullableAnnotation,
Nullables nullables) {
super(
name,
identifier,
type,
method.getReturnType(),
typeString,
typeMirror,
nullableAnnotation,
nullables,
method.getSimpleName().toString(),
Optional.empty(),
/* hasDefault= */ false);
Expand Down Expand Up @@ -492,7 +525,8 @@ private void validateType(TypeElement type) {
final ImmutableSet<Property> propertySet(
ImmutableMap<ExecutableElement, TypeMirror> propertyMethodsAndTypes,
ImmutableListMultimap<ExecutableElement, AnnotationMirror> annotatedPropertyFields,
ImmutableListMultimap<ExecutableElement, AnnotationMirror> annotatedPropertyMethods) {
ImmutableListMultimap<ExecutableElement, AnnotationMirror> annotatedPropertyMethods,
Nullables nullables) {
ImmutableBiMap<ExecutableElement, String> methodToPropertyName =
propertyNameToMethodMap(propertyMethodsAndTypes.keySet()).inverse();
Map<ExecutableElement, String> methodToIdentifier = new LinkedHashMap<>(methodToPropertyName);
Expand All @@ -501,7 +535,7 @@ final ImmutableSet<Property> propertySet(
ImmutableSet.Builder<Property> props = ImmutableSet.builder();
propertyMethodsAndTypes.forEach(
(propertyMethod, returnType) -> {
String propertyType =
String propertyTypeString =
TypeEncoder.encodeWithAnnotations(
returnType, ImmutableList.of(), getExcludedAnnotationTypes(propertyMethod));
String propertyName = methodToPropertyName.get(propertyMethod);
Expand All @@ -517,10 +551,12 @@ final ImmutableSet<Property> propertySet(
propertyName,
identifier,
propertyMethod,
propertyType,
returnType,
propertyTypeString,
fieldAnnotations,
methodAnnotations,
nullableAnnotation);
nullableAnnotation,
nullables);
props.add(p);
if (p.isNullable() && returnType.getKind().isPrimitive()) {
errorReporter()
Expand All @@ -535,7 +571,10 @@ final ImmutableSet<Property> propertySet(

/** Defines the template variables that are shared by AutoValue, AutoOneOf, and AutoBuilder. */
final void defineSharedVarsForType(
TypeElement type, ImmutableSet<ExecutableElement> methods, AutoValueishTemplateVars vars) {
TypeElement type,
ImmutableSet<ExecutableElement> methods,
Nullables nullables,
AutoValueishTemplateVars vars) {
vars.pkg = TypeSimplifier.packageNameOf(type);
vars.origClass = TypeSimplifier.classNameOf(type);
vars.simpleClassName = TypeSimplifier.simpleNameOf(vars.origClass);
Expand All @@ -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<AnnotationMirror> nullable = nullables.appropriateNullableGivenMethods(methods);
vars.equalsParameterType = equalsParameterType(methodsToGenerate, nullable);
vars.equalsParameterType =
equalsParameterType(methodsToGenerate, nullables);
vars.serialVersionUID = getSerialVersionUID(type);
}

Expand Down Expand Up @@ -835,7 +874,7 @@ private static Map<ObjectMethod, ExecutableElement> determineObjectMethodsToGene
* @param nullable the type of a {@code @Nullable} type annotation that we have found, if any
*/
static String equalsParameterType(
Map<ObjectMethod, ExecutableElement> methodsToGenerate, Optional<AnnotationMirror> nullable) {
Map<ObjectMethod, ExecutableElement> 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
Expand All @@ -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<AnnotationMirror> nullableTypeAnnotation = nullables.nullableTypeAnnotation();
ImmutableList<AnnotationMirror> 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());
}

/**
Expand Down
Loading

0 comments on commit 91d5f32

Please sign in to comment.