From bc250f6f57d190ecd57027b69b8146b912ee8150 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89amonn=20McManus?= Date: Sat, 24 Jul 2021 08:37:57 -0700 Subject: Simplify old GWT-handling code in AutoValue. RELNOTES=n/a PiperOrigin-RevId: 386635339 --- .../auto/value/processor/GwtCompatibility.java | 30 +--------------------- .../auto/value/processor/GwtSerialization.java | 16 +++++------- 2 files changed, 7 insertions(+), 39 deletions(-) (limited to 'value/src/main/java/com/google') diff --git a/value/src/main/java/com/google/auto/value/processor/GwtCompatibility.java b/value/src/main/java/com/google/auto/value/processor/GwtCompatibility.java index fae4e092..35fcbbf0 100644 --- a/value/src/main/java/com/google/auto/value/processor/GwtCompatibility.java +++ b/value/src/main/java/com/google/auto/value/processor/GwtCompatibility.java @@ -15,15 +15,9 @@ */ package com.google.auto.value.processor; -import static java.util.stream.Collectors.joining; - -import java.util.Collections; import java.util.List; -import java.util.Map; import java.util.Optional; import javax.lang.model.element.AnnotationMirror; -import javax.lang.model.element.AnnotationValue; -import javax.lang.model.element.ExecutableElement; import javax.lang.model.element.Name; import javax.lang.model.element.TypeElement; @@ -46,29 +40,7 @@ class GwtCompatibility { return gwtCompatibleAnnotation; } - // Get rid of the misconceived - // in the return type of getElementValues(). - static Map getElementValues(AnnotationMirror annotation) { - return Collections.unmodifiableMap( - annotation.getElementValues()); - } - String gwtCompatibleAnnotationString() { - if (gwtCompatibleAnnotation.isPresent()) { - AnnotationMirror annotation = gwtCompatibleAnnotation.get(); - TypeElement annotationElement = (TypeElement) annotation.getAnnotationType().asElement(); - String annotationArguments; - if (annotation.getElementValues().isEmpty()) { - annotationArguments = ""; - } else { - annotationArguments = - getElementValues(annotation).entrySet().stream() - .map(e -> e.getKey().getSimpleName() + " = " + e.getValue()) - .collect(joining(", ", "(", ")")); - } - return "@" + annotationElement.getQualifiedName() + annotationArguments; - } else { - return ""; - } + return gwtCompatibleAnnotation.map(AnnotationOutput::sourceFormForAnnotation).orElse(""); } } diff --git a/value/src/main/java/com/google/auto/value/processor/GwtSerialization.java b/value/src/main/java/com/google/auto/value/processor/GwtSerialization.java index 30ad0926..8673d3db 100644 --- a/value/src/main/java/com/google/auto/value/processor/GwtSerialization.java +++ b/value/src/main/java/com/google/auto/value/processor/GwtSerialization.java @@ -18,6 +18,7 @@ package com.google.auto.value.processor; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.stream.Collectors.toList; +import com.google.auto.common.AnnotationMirrors; import com.google.auto.value.processor.AutoValueishProcessor.GetterProperty; import com.google.auto.value.processor.PropertyBuilderClassifier.PropertyBuilder; import com.google.common.collect.ImmutableMap; @@ -26,13 +27,10 @@ import com.google.escapevelocity.Template; import java.io.IOException; import java.io.Writer; import java.util.List; -import java.util.Map; import java.util.Optional; import java.util.zip.CRC32; import javax.annotation.processing.ProcessingEnvironment; import javax.lang.model.element.AnnotationMirror; -import javax.lang.model.element.AnnotationValue; -import javax.lang.model.element.ExecutableElement; import javax.lang.model.element.TypeElement; import javax.lang.model.type.TypeMirror; import javax.tools.Diagnostic; @@ -60,13 +58,11 @@ class GwtSerialization { Optional optionalGwtCompatible = gwtCompatibility.gwtCompatibleAnnotation(); if (optionalGwtCompatible.isPresent()) { AnnotationMirror gwtCompatible = optionalGwtCompatible.get(); - for (Map.Entry entry : - GwtCompatibility.getElementValues(gwtCompatible).entrySet()) { - if (entry.getKey().getSimpleName().contentEquals("serializable") - && entry.getValue().getValue().equals(true)) { - return true; - } - } + return AnnotationMirrors.getAnnotationValuesWithDefaults(gwtCompatible).entrySet().stream() + .anyMatch( + e -> + e.getKey().getSimpleName().contentEquals("serializable") + && e.getValue().getValue().equals(true)); } return false; } -- cgit v1.2.3 From 04b2353bf026381c59b8eedcd4ae54e41d19f5ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89amonn=20McManus?= Date: Mon, 16 Aug 2021 11:14:12 -0700 Subject: Avoid `instanceof TypeElement` in Auto code. The documentation for [`Element`](https://docs.oracle.com/en/java/javase/11/docs/api/java.compiler/javax/lang/model/element/Element.html) specifies that `instanceof` is not safe: > Using `instanceof` is _not_ necessarily a reliable idiom for determining the effective class of an object in this modeling hierarchy since an implementation may choose to have a single object implement multiple `Element` subinterfaces. RELNOTES=n/a PiperOrigin-RevId: 391091995 --- .../java/com/google/auto/value/processor/AutoAnnotationProcessor.java | 2 +- .../java/com/google/auto/value/processor/AutoValueishProcessor.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'value/src/main/java/com/google') 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 3acf9332..cc0e62ec 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 @@ -287,7 +287,7 @@ public class AutoAnnotationProcessor extends AbstractProcessor { private String generatedClassName(ExecutableElement method) { TypeElement type = MoreElements.asType(method.getEnclosingElement()); String name = type.getSimpleName().toString(); - while (type.getEnclosingElement() instanceof TypeElement) { + while (MoreElements.isType(type.getEnclosingElement())) { type = MoreElements.asType(type.getEnclosingElement()); name = type.getSimpleName() + "_" + name; } 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 93f2f79e..9b860552 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 @@ -484,7 +484,7 @@ abstract class AutoValueishProcessor extends AbstractProcessor { */ static String generatedClassName(TypeElement type, String prefix) { String name = type.getSimpleName().toString(); - while (type.getEnclosingElement() instanceof TypeElement) { + while (MoreElements.isType(type.getEnclosingElement())) { type = MoreElements.asType(type.getEnclosingElement()); name = type.getSimpleName() + "_" + name; } -- cgit v1.2.3 From fb96c8368254b6b2af06d166a94700d0531f6c18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89amonn=20McManus?= Date: Mon, 6 Sep 2021 10:18:04 -0700 Subject: Use annotation default values with `@AutoBuilder` and `@AutoAnnotation`. You can use `@AutoBuilder` to call an `@AutoAnnotation` method. As usual for `@AutoAnnotation`, each parameter of that method corresponds to an element of the annotation. If that element has a default value, the parameter should default to that same value. Trivial example: ```java class Example { @interface MyAnnotation { String value() default "foo"; } @AutoAnnotation static MyAnnotation myAnnotation(String value) { return new AutoAnnotation_Example_myAnnotation(value); } @AutoBuilder(callMethod = "myAnnotation") interface MyAnnotationBuilder { MyAnnotationBuilder value(String value); MyAnnotation build(); } static MyAnnotationBuilder myAnnotationBuilder() { return new AutoBuilder_Example_MyAnnotationBuilder(); } } ``` In the example, the call `myAnnotationBuilder().build()` has the same effect as `myAnnotationBuilder().value("foo").build()` because of `value() default "foo"`. RELNOTES=AutoBuilder now uses annotation defaults when building a call to an `@AutoAnnotation` method. PiperOrigin-RevId: 395114215 --- .../java/com/google/auto/value/AutoAnnotation.java | 33 +++++++++++ .../auto/value/processor/AnnotationOutput.java | 12 ++-- .../auto/value/processor/AutoBuilderProcessor.java | 65 +++++++++++++++++++--- .../processor/AutoValueOrBuilderTemplateVars.java | 3 +- .../value/processor/AutoValueishProcessor.java | 38 ++++++++++++- .../google/auto/value/processor/BuilderSpec.java | 2 +- .../com/google/auto/value/processor/builder.vm | 4 +- 7 files changed, 138 insertions(+), 19 deletions(-) (limited to 'value/src/main/java/com/google') diff --git a/value/src/main/java/com/google/auto/value/AutoAnnotation.java b/value/src/main/java/com/google/auto/value/AutoAnnotation.java index d36d8e28..c6fab240 100644 --- a/value/src/main/java/com/google/auto/value/AutoAnnotation.java +++ b/value/src/main/java/com/google/auto/value/AutoAnnotation.java @@ -71,6 +71,39 @@ import java.lang.reflect.AnnotatedElement; * parameter corresponding to an array-valued annotation member, and the implementation of each such * member will also return a clone of the array. * + *

If your annotation has many elements, you may consider using {@code @AutoBuilder} to make it + * easier to construct instances. In that case, {@code default} values from the annotation will + * become default values for the parameters of the {@code @AutoAnnotation} method. For example: + * + * 

+ * class Example {
+ *   {@code @interface} MyAnnotation {
+ *     String name() default "foo";
+ *     int number() default 23;
+ *   }
+ *
+ *   {@code @AutoAnnotation}
+ *   static MyAnnotation myAnnotation(String value) {
+ *     return new AutoAnnotation_Example_myAnnotation(value);
+ *   }
+ *
+ *   {@code @AutoBuilder(callMethod = "myAnnotation")}
+ *   interface MyAnnotationBuilder {
+ *     MyAnnotationBuilder name(String name);
+ *     MyAnnotationBuilder number(int number);
+ *     MyAnnotation build();
+ *   }
+ *
+ *   static MyAnnotationBuilder myAnnotationBuilder() {
+ *     return new AutoBuilder_Example_MyAnnotationBuilder();
+ *   }
+ * }
+ *
+ * + * Here, {@code myAnnotationBuilder().build()} is the same as {@code + * myAnnotationBuilder().name("foo").number(23).build()} because those are the defaults in the + * annotation definition. + * * @author emcmanus@google.com (Éamonn McManus) */ @Target(ElementType.METHOD) diff --git a/value/src/main/java/com/google/auto/value/processor/AnnotationOutput.java b/value/src/main/java/com/google/auto/value/processor/AnnotationOutput.java index ed986ab7..0c8b8f0f 100644 --- a/value/src/main/java/com/google/auto/value/processor/AnnotationOutput.java +++ b/value/src/main/java/com/google/auto/value/processor/AnnotationOutput.java @@ -130,13 +130,13 @@ final class AnnotationOutput { private static class InitializerSourceFormVisitor extends SourceFormVisitor { private final ProcessingEnvironment processingEnv; private final String memberName; - private final Element context; + private final Element errorContext; InitializerSourceFormVisitor( - ProcessingEnvironment processingEnv, String memberName, Element context) { + ProcessingEnvironment processingEnv, String memberName, Element errorContext) { this.processingEnv = processingEnv; this.memberName = memberName; - this.context = context; + this.errorContext = errorContext; } @Override @@ -148,7 +148,7 @@ final class AnnotationOutput { "@AutoAnnotation cannot yet supply a default value for annotation-valued member '" + memberName + "'", - context); + errorContext); sb.append("null"); return null; } @@ -209,9 +209,9 @@ final class AnnotationOutput { AnnotationValue annotationValue, ProcessingEnvironment processingEnv, String memberName, - Element context) { + Element errorContext) { SourceFormVisitor visitor = - new InitializerSourceFormVisitor(processingEnv, memberName, context); + new InitializerSourceFormVisitor(processingEnv, memberName, errorContext); StringBuilder sb = new StringBuilder(); visitor.visit(annotationValue, sb); return sb.toString(); 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 b6a578fc..53fe836d 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 @@ -20,6 +20,7 @@ import static com.google.auto.common.MoreElements.getPackage; import static com.google.auto.common.MoreStreams.toImmutableList; import static com.google.auto.common.MoreStreams.toImmutableSet; import static com.google.auto.value.processor.AutoValueProcessor.OMIT_IDENTIFIERS_OPTION; +import static com.google.auto.value.processor.ClassNames.AUTO_ANNOTATION_NAME; import static com.google.auto.value.processor.ClassNames.AUTO_BUILDER_NAME; import static java.util.stream.Collectors.joining; import static java.util.stream.Collectors.toCollection; @@ -38,6 +39,7 @@ import com.google.auto.value.processor.MissingTypes.MissingTypeException; import com.google.common.base.Ascii; import com.google.common.base.VerifyException; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; import java.lang.reflect.Field; @@ -60,6 +62,7 @@ import javax.lang.model.element.Modifier; import javax.lang.model.element.PackageElement; import javax.lang.model.element.TypeElement; import javax.lang.model.element.VariableElement; +import javax.lang.model.type.TypeKind; import javax.lang.model.type.TypeMirror; import net.ltgt.gradle.incap.IncrementalAnnotationProcessor; import net.ltgt.gradle.incap.IncrementalAnnotationProcessorType; @@ -134,7 +137,7 @@ public class AutoBuilderProcessor extends AutoValueishProcessor { Map propertyToGetterName = Maps.transformValues(classifier.builderGetters(), PropertyGetter::getName); AutoBuilderTemplateVars vars = new AutoBuilderTemplateVars(); - vars.props = propertySet(executable, propertyToGetterName); + vars.props = propertySet(autoBuilderType, executable, propertyToGetterName); builder.defineVars(vars, classifier); vars.identifiers = !processingEnv.getOptions().containsKey(OMIT_IDENTIFIERS_OPTION); String generatedClassName = generatedClassName(autoBuilderType, "AutoBuilder_"); @@ -152,7 +155,15 @@ public class AutoBuilderProcessor extends AutoValueishProcessor { } private ImmutableSet propertySet( - ExecutableElement executable, Map propertyToGetterName) { + TypeElement autoBuilderType, + ExecutableElement executable, + Map propertyToGetterName) { + boolean autoAnnotation = + MoreElements.getAnnotationMirror(executable, AUTO_ANNOTATION_NAME).isPresent(); + ImmutableMap builderInitializers = + autoAnnotation + ? autoAnnotationInitializers(autoBuilderType, executable) + : ImmutableMap.of(); // 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 = @@ -161,18 +172,58 @@ public class AutoBuilderProcessor extends AutoValueishProcessor { fixReservedIdentifiers(identifiers); return executable.getParameters().stream() .map( - v -> - newProperty( - v, identifiers.get(v), propertyToGetterName.get(v.getSimpleName().toString()))) + v -> { + String name = v.getSimpleName().toString(); + return newProperty( + v, + identifiers.get(v), + propertyToGetterName.get(name), + Optional.ofNullable(builderInitializers.get(name))); + }) .collect(toImmutableSet()); } - private Property newProperty(VariableElement var, String identifier, String getterName) { + private Property newProperty( + VariableElement var, + String identifier, + String getterName, + Optional builderInitializer) { String name = var.getSimpleName().toString(); TypeMirror type = var.asType(); Optional nullableAnnotation = nullableAnnotationFor(var, var.asType()); return new Property( - name, identifier, TypeEncoder.encode(type), type, nullableAnnotation, getterName); + name, + identifier, + TypeEncoder.encode(type), + type, + nullableAnnotation, + getterName, + builderInitializer); + } + + private ImmutableMap autoAnnotationInitializers( + TypeElement autoBuilderType, ExecutableElement autoAnnotationMethod) { + // We expect the return type of an @AutoAnnotation method to be an annotation type. If it isn't, + // AutoAnnotation will presumably complain, so we don't need to complain further. + TypeMirror returnType = autoAnnotationMethod.getReturnType(); + if (!returnType.getKind().equals(TypeKind.DECLARED)) { + return ImmutableMap.of(); + } + // This might not actually be an annotation (if the code is wrong), but if that's the case we + // just won't see any contained ExecutableElement where getDefaultValue() returns something. + TypeElement annotation = MoreTypes.asTypeElement(returnType); + ImmutableMap.Builder builder = ImmutableMap.builder(); + for (ExecutableElement method : methodsIn(annotation.getEnclosedElements())) { + AnnotationValue defaultValue = method.getDefaultValue(); + if (defaultValue != null) { + String memberName = method.getSimpleName().toString(); + builder.put( + memberName, + AnnotationOutput.sourceFormForInitializer( + defaultValue, processingEnv, memberName, autoBuilderType)); + } + } + return builder.build(); } private ExecutableElement findExecutable( diff --git a/value/src/main/java/com/google/auto/value/processor/AutoValueOrBuilderTemplateVars.java b/value/src/main/java/com/google/auto/value/processor/AutoValueOrBuilderTemplateVars.java index 86cf4974..9fbc1652 100644 --- a/value/src/main/java/com/google/auto/value/processor/AutoValueOrBuilderTemplateVars.java +++ b/value/src/main/java/com/google/auto/value/processor/AutoValueOrBuilderTemplateVars.java @@ -109,7 +109,8 @@ abstract class AutoValueOrBuilderTemplateVars extends AutoValueishTemplateVars { * *
    *
  • it is {@code @Nullable} (in which case it defaults to null); - *
  • it is {@code Optional} (in which case it defaults to empty); + *
  • it has a builder initializer (for example it is {@code Optional}, which will have an + * initializer of {@code Optional.empty()}); *
  • it has a property-builder method (in which case it defaults to empty). *
*/ 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 9b860552..ea085575 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 @@ -160,6 +160,7 @@ abstract class AutoValueishProcessor extends AbstractProcessor { private final Optional nullableAnnotation; private final Optionalish optional; private final String getter; + private final String builderInitializer; // empty, or with initial ` = `. Property( String name, @@ -167,16 +168,40 @@ abstract class AutoValueishProcessor extends AbstractProcessor { String type, TypeMirror typeMirror, Optional nullableAnnotation, - String getter) { + String getter, + Optional maybeBuilderInitializer) { this.name = name; this.identifier = identifier; this.type = type; this.typeMirror = typeMirror; this.nullableAnnotation = nullableAnnotation; this.optional = Optionalish.createIfOptional(typeMirror); + this.builderInitializer = + maybeBuilderInitializer.isPresent() + ? " = " + maybeBuilderInitializer.get() + : builderInitializer(); this.getter = getter; } + /** + * Returns the appropriate initializer for a builder property. Builder properties are never + * primitive; if the built property is an {@code int} the builder property will be an {@code + * Integer}. So the default value for a builder property will be null unless there is an + * initializer. The caller of the constructor may have supplied an initializer, but otherwise we + * supply one only if 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() { + if (nullableAnnotation.isPresent()) { + return ""; + } + Optionalish optional = Optionalish.createIfOptional(typeMirror); + if (optional == null) { + return ""; + } + return " = " + optional.getEmpty(); + } + /** * 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}. @@ -218,6 +243,14 @@ abstract class AutoValueishProcessor extends AbstractProcessor { return optional; } + /** + * Returns a string to be used as an initializer for a builder field for this property, + * including the leading {@code =}, or an empty string if there is no explicit initializer. + */ + public String getBuilderInitializer() { + return builderInitializer; + } + /** * Returns the string to use as a method annotation to indicate the nullability of this * property. It is either the empty string, if the property is not nullable, or an annotation @@ -266,7 +299,8 @@ abstract class AutoValueishProcessor extends AbstractProcessor { type, method.getReturnType(), nullableAnnotation, - method.getSimpleName().toString()); + method.getSimpleName().toString(), + Optional.empty()); this.method = method; this.fieldAnnotations = fieldAnnotations; this.methodAnnotations = methodAnnotations; diff --git a/value/src/main/java/com/google/auto/value/processor/BuilderSpec.java b/value/src/main/java/com/google/auto/value/processor/BuilderSpec.java index 9f45d172..dfdbb8c7 100644 --- a/value/src/main/java/com/google/auto/value/processor/BuilderSpec.java +++ b/value/src/main/java/com/google/auto/value/processor/BuilderSpec.java @@ -333,7 +333,7 @@ class BuilderSpec { vars.builderRequiredProperties = vars.props.stream() .filter(p -> !p.isNullable()) - .filter(p -> p.getOptional() == null) + .filter(p -> p.getBuilderInitializer().isEmpty()) .filter(p -> !vars.builderPropertyBuilders.containsKey(p.getName())) .collect(toImmutableSet()); } 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 630330ca..950f9838 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 @@ -40,7 +40,7 @@ class ${builderName}${builderFormalTypes} ## #if ($p.kind.primitive) - private $types.boxedClass($p.typeMirror).simpleName $p; + private $types.boxedClass($p.typeMirror).simpleName $p $p.builderInitializer; #else @@ -54,7 +54,7 @@ class ${builderName}${builderFormalTypes} ## #end - private $p.type $p #if ($p.optional && !$p.nullable) = $p.optional.empty #end ; + private $p.type $p $p.builderInitializer; #end #end -- cgit v1.2.3 From 8ad800edde0e8c743666a5d814e86ecd6c6f39c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89amonn=20McManus?= Date: Tue, 5 Oct 2021 15:34:20 -0700 Subject: Ensure the order of copied annotations is deterministic. Fixes https://github.com/google/auto/issues/1176. RELNOTES=The order of annotations copied into generated code is now deterministic. PiperOrigin-RevId: 401086698 --- .../com/google/auto/value/processor/AutoValueishProcessor.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'value/src/main/java/com/google') 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 ea085575..55a94a71 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 @@ -501,9 +501,9 @@ abstract class AutoValueishProcessor extends AbstractProcessor { /** Returns the spelling to be used in the generated code for the given list of annotations. */ static ImmutableList annotationStrings(List annotations) { - // TODO(b/68008628): use ImmutableList.toImmutableList() when that works. return annotations.stream() .map(AnnotationOutput::sourceFormForAnnotation) + .sorted() // ensures deterministic order .collect(toImmutableList()); } @@ -623,8 +623,9 @@ abstract class AutoValueishProcessor extends AbstractProcessor { List elementAnnotations = element.getAnnotationMirrors(); OptionalInt nullableAnnotationIndex = nullableAnnotationIndex(elementAnnotations); if (nullableAnnotationIndex.isPresent()) { - ImmutableList annotations = annotationStrings(elementAnnotations); - return Optional.of(annotations.get(nullableAnnotationIndex.getAsInt()) + " "); + AnnotationMirror annotation = elementAnnotations.get(nullableAnnotationIndex.getAsInt()); + String annotationString = AnnotationOutput.sourceFormForAnnotation(annotation); + return Optional.of(annotationString + " "); } else { return Optional.empty(); } -- cgit v1.2.3 From e0740327d830597e17273946418f6adc976bc619 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89amonn=20McManus?= Date: Tue, 19 Oct 2021 10:52:20 -0700 Subject: Handle missing type when copying annotations. If you have `@CopyAnnotations` and `@Foo(Bar.class)`, and if `Bar` is undefined, we want to defer processing to give some other annotation processor a chance to define `Bar`. This turns out to be quite difficult, because javac essentially makes it look as if you have written `@Foo("")` in this case. So we check to see if an annotation element that _should_ be a class is something else (a string in this case). RELNOTES=We now handle better the case where an annotation being copied references a missing class. PiperOrigin-RevId: 404307632 --- .../auto/value/processor/AnnotationOutput.java | 52 ++++++++++++++++++++++ 1 file changed, 52 insertions(+) (limited to 'value/src/main/java/com/google') diff --git a/value/src/main/java/com/google/auto/value/processor/AnnotationOutput.java b/value/src/main/java/com/google/auto/value/processor/AnnotationOutput.java index 0c8b8f0f..ed6abaa6 100644 --- a/value/src/main/java/com/google/auto/value/processor/AnnotationOutput.java +++ b/value/src/main/java/com/google/auto/value/processor/AnnotationOutput.java @@ -15,6 +15,8 @@ */ package com.google.auto.value.processor; +import com.google.auto.common.MoreTypes; +import com.google.auto.value.processor.MissingTypes.MissingTypeException; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import java.util.List; @@ -24,8 +26,10 @@ import javax.annotation.processing.ProcessingEnvironment; import javax.lang.model.element.AnnotationMirror; import javax.lang.model.element.AnnotationValue; import javax.lang.model.element.Element; +import javax.lang.model.element.ElementKind; import javax.lang.model.element.ExecutableElement; import javax.lang.model.element.VariableElement; +import javax.lang.model.type.TypeKind; import javax.lang.model.type.TypeMirror; import javax.lang.model.util.SimpleAnnotationValueVisitor8; import javax.tools.Diagnostic; @@ -222,11 +226,59 @@ final class AnnotationOutput { * Java source file to reproduce the annotation in source form. */ static String sourceFormForAnnotation(AnnotationMirror annotationMirror) { + // If a value in the annotation is a reference to a class constant and that class constant is + // undefined, javac unhelpfully converts it into a string "" and visits that instead. We + // want to catch this case and defer processing to allow the class to be defined by another + // annotation processor. So we look for annotation elements whose type is Class but whose + // reported value is a string. Unfortunately we can't extract the ErrorType corresponding to the + // missing class portably. With javac, the AttributeValue is a + // com.sun.tools.javac.code.Attribute.UnresolvedClass, which has a public field classType that + // is the ErrorType we need, but obviously that's nonportable and fragile. + validateClassValues(annotationMirror); StringBuilder sb = new StringBuilder(); new AnnotationSourceFormVisitor().visitAnnotation(annotationMirror, sb); return sb.toString(); } + /** + * Throws an exception if this annotation contains a value for a Class element that is not + * actually a type. The assumption is that the value is the string {@code ""} which javac + * presents when a Class value is an undefined type. + */ + private static void validateClassValues(AnnotationMirror annotationMirror) { + // A class literal can appear in three places: + // * for an element of type Class, for example @SomeAnnotation(Foo.class); + // * for an element of type Class[], for example @SomeAnnotation({Foo.class, Bar.class}); + // * inside a nested annotation, for example @SomeAnnotation(@Nested(Foo.class)). + // These three possibilities are the three branches of the if/else chain below. + annotationMirror + .getElementValues() + .forEach( + (method, value) -> { + TypeMirror type = method.getReturnType(); + if (isJavaLangClass(type) && !(value.getValue() instanceof TypeMirror)) { + throw new MissingTypeException(null); + } else if (type.getKind().equals(TypeKind.ARRAY) + && isJavaLangClass(MoreTypes.asArray(type).getComponentType()) + && value.getValue() instanceof List) { + @SuppressWarnings("unchecked") // a List can only be a List here + List values = (List) value.getValue(); + if (values.stream().anyMatch(av -> !(av.getValue() instanceof TypeMirror))) { + throw new MissingTypeException(null); + } + } else if (type.getKind().equals(TypeKind.DECLARED) + && MoreTypes.asElement(type).getKind().equals(ElementKind.ANNOTATION_TYPE) + && value.getValue() instanceof AnnotationMirror) { + validateClassValues((AnnotationMirror) value.getValue()); + } + }); + } + + private static boolean isJavaLangClass(TypeMirror type) { + return type.getKind().equals(TypeKind.DECLARED) + && MoreTypes.asTypeElement(type).getQualifiedName().contentEquals("java.lang.Class"); + } + private static StringBuilder appendQuoted(StringBuilder sb, String s) { sb.append('"'); for (int i = 0; i < s.length(); i++) { -- cgit v1.2.3 From 8abfb9cdd873457a0696df08c38cb7f04d7208f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89amonn=20McManus?= Date: Tue, 19 Oct 2021 17:35:10 -0700 Subject: Use `Objects.requireNonNull(x)` instead of `x.getClass()` in no-identifiers mode. RELNOTES=n/a PiperOrigin-RevId: 404405947 --- value/src/main/java/com/google/auto/value/processor/autovalue.vm | 6 +----- value/src/main/java/com/google/auto/value/processor/builder.vm | 6 +----- 2 files changed, 2 insertions(+), 10 deletions(-) (limited to 'value/src/main/java/com/google') diff --git a/value/src/main/java/com/google/auto/value/processor/autovalue.vm b/value/src/main/java/com/google/auto/value/processor/autovalue.vm index 86cfe493..18ca827a 100644 --- a/value/src/main/java/com/google/auto/value/processor/autovalue.vm +++ b/value/src/main/java/com/google/auto/value/processor/autovalue.vm @@ -75,15 +75,11 @@ ${modifiers}class $subclass$formalTypes extends $origClass$actualTypes { ## the constructor is called from the extension code. #if ($identifiers) - if ($p == null) { throw new NullPointerException("Null $p.name"); } #else - ## Just throw NullPointerException with no message if it's null. - ## The Object cast has no effect on the code but silences an ErrorProne warning. - - ((`java.lang.Object`) ${p}).getClass(); + `java.util.Objects`.requireNonNull($p); #end #end 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 950f9838..b1787f25 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 @@ -94,15 +94,11 @@ class ${builderName}${builderFormalTypes} ## #if (!$setter.primitiveParameter && !$p.nullable && ${setter.copy($p)} == $p) #if ($identifiers) - if ($p == null) { throw new NullPointerException("Null $p.name"); } #else - ## Just throw NullPointerException with no message if it's null. - ## The Object cast has no effect on the code but silences an ErrorProne warning. - - ((`java.lang.Object`) ${p}).getClass(); + `java.util.Objects`.requireNonNull($p); #end #end -- cgit v1.2.3 From 0820e2e2a460bb42a15de190e4825e066a77f5ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89amonn=20McManus?= Date: Tue, 2 Nov 2021 13:15:09 -0700 Subject: Make it easier to make a step-builder for AutoValue. We no longer require each setter method in an `@AutoValue.Builder` class `Builder` to return `Builder`. A setter can return some supertype, in particular a step interface. That removes the need to override each inherited setter method just to change its return type. Also add documentation showing how this works. Previously we just linked to an example in a GitHub issue. RELNOTES=Making a step-builder for AutoValue is now easier because the inherited setters don't need to be restated in the `Builder` class so that they return `Builder`. PiperOrigin-RevId: 407161124 --- .../com/google/auto/value/processor/BuilderMethodClassifier.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'value/src/main/java/com/google') diff --git a/value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifier.java b/value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifier.java index 51773e6f..a4336f5e 100644 --- a/value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifier.java +++ b/value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifier.java @@ -386,14 +386,17 @@ abstract class BuilderMethodClassifier { DeclaredType builderTypeMirror = MoreTypes.asDeclared(builderType.asType()); ExecutableType methodMirror = MoreTypes.asExecutable(typeUtils.asMemberOf(builderTypeMirror, method)); - if (TYPE_EQUIVALENCE.equivalent(methodMirror.getReturnType(), builderType.asType())) { + TypeMirror returnType = methodMirror.getReturnType(); + if (typeUtils.isSubtype(builderType.asType(), returnType) + && !MoreTypes.isTypeOf(Object.class, returnType)) { + // We allow the return type to be a supertype (other than Object), to support step builders. TypeMirror parameterType = Iterables.getOnlyElement(methodMirror.getParameterTypes()); propertyNameToSetters.put( propertyName, new PropertySetter(method, parameterType, function.get())); } else { errorReporter.reportError( method, - "[%sBuilderRet] Setter methods must return %s", + "[%sBuilderRet] Setter methods must return %s or a supertype", autoWhat(), builderType.asType()); } -- cgit v1.2.3 From 1f8d7f26bf4b0613a8602085cae77f15693ab086 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89amonn=20McManus?= Date: Mon, 6 Dec 2021 13:15:26 -0800 Subject: Make `@AutoBuilder` available unconditionally. RELNOTES=The `@AutoBuilder` annotation documented [here](https://github.com/google/auto/blob/master/value/userguide/autobuilder.md) is now fully stable and supported. PiperOrigin-RevId: 414523614 --- .../com/google/auto/value/processor/AutoBuilderProcessor.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'value/src/main/java/com/google') 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 53fe836d..79bccafd 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 @@ -98,12 +98,8 @@ public class AutoBuilderProcessor extends AutoValueishProcessor { @Override void processType(TypeElement autoBuilderType) { - if (!processingEnv.getOptions().containsKey(ALLOW_OPTION)) { - errorReporter() - .abortWithError( - autoBuilderType, - "Compile with -A%s to enable this UNSUPPORTED AND UNSTABLE prototype", - ALLOW_OPTION); + if (processingEnv.getOptions().containsKey(ALLOW_OPTION)) { + errorReporter().reportWarning(autoBuilderType, "The -A%s option is obsolete", ALLOW_OPTION); } if (autoBuilderType.getKind() != ElementKind.CLASS && autoBuilderType.getKind() != ElementKind.INTERFACE) { -- cgit v1.2.3 From a74508b7eac6254f8e00cf5d3634a10c690e058d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89amonn=20McManus?= Date: Tue, 7 Dec 2021 11:23:33 -0800 Subject: Validate that `@AutoValue` (etc) classes have appropriate constructors. The class must have a non-private no-arg constructor so it can be subclassed in the generated code. Previously, if it didn't, we would generate code anyway and rely on the somewhat obscure compiler error message to signal the problem to the user. Now we check for the situation explicitly and produce a specific error message. Fixes https://github.com/google/auto/issues/1209. RELNOTES=n/a PiperOrigin-RevId: 414779231 --- .../auto/value/processor/AutoBuilderProcessor.java | 12 +-- .../auto/value/processor/AutoOneOfProcessor.java | 9 +-- .../auto/value/processor/AutoValueProcessor.java | 31 ++++---- .../value/processor/AutoValueishProcessor.java | 88 +++++++++++++++++----- .../google/auto/value/processor/BuilderSpec.java | 32 +++++--- 5 files changed, 108 insertions(+), 64 deletions(-) (limited to 'value/src/main/java/com/google') 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 79bccafd..fc0d8b3e 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 @@ -80,7 +80,7 @@ public class AutoBuilderProcessor extends AutoValueishProcessor { private static final String ALLOW_OPTION = "com.google.auto.value.AutoBuilderIsUnstable"; public AutoBuilderProcessor() { - super(AUTO_BUILDER_NAME); + super(AUTO_BUILDER_NAME, /* appliesToInterfaces= */ true); } @Override @@ -101,14 +101,6 @@ public class AutoBuilderProcessor extends AutoValueishProcessor { if (processingEnv.getOptions().containsKey(ALLOW_OPTION)) { errorReporter().reportWarning(autoBuilderType, "The -A%s option is obsolete", ALLOW_OPTION); } - if (autoBuilderType.getKind() != ElementKind.CLASS - && autoBuilderType.getKind() != ElementKind.INTERFACE) { - errorReporter() - .abortWithError( - autoBuilderType, - "[AutoBuilderWrongType] @AutoBuilder only applies to classes and interfaces"); - } - checkModifiersIfNested(autoBuilderType); // The annotation is guaranteed to be present by the contract of Processor#process AnnotationMirror autoBuilderAnnotation = getAnnotationMirror(autoBuilderType, AUTO_BUILDER_NAME).get(); @@ -125,7 +117,7 @@ public class AutoBuilderProcessor extends AutoValueishProcessor { Optional> maybeClassifier = BuilderMethodClassifierForAutoBuilder.classify( methods, errorReporter(), processingEnv, executable, builtType, autoBuilderType); - if (!maybeClassifier.isPresent()) { + if (!maybeClassifier.isPresent() || errorReporter().errorCount() > 0) { // We've already output one or more error messages. return; } 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 711b138c..4d19d216 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 @@ -60,7 +60,7 @@ import net.ltgt.gradle.incap.IncrementalAnnotationProcessorType; @IncrementalAnnotationProcessor(IncrementalAnnotationProcessorType.ISOLATING) public class AutoOneOfProcessor extends AutoValueishProcessor { public AutoOneOfProcessor() { - super(AUTO_ONE_OF_NAME); + super(AUTO_ONE_OF_NAME, /* appliesToInterfaces= */ false); } @Override @@ -75,13 +75,6 @@ public class AutoOneOfProcessor extends AutoValueishProcessor { @Override void processType(TypeElement autoOneOfType) { - if (autoOneOfType.getKind() != ElementKind.CLASS) { - errorReporter() - .abortWithError( - autoOneOfType, - "[AutoOneOfNotClass] @" + AUTO_ONE_OF_NAME + " only applies to classes"); - } - checkModifiersIfNested(autoOneOfType); DeclaredType kindMirror = mirrorForKindType(autoOneOfType); // We are going to classify the methods of the @AutoOneOf class into several categories. 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 ab7da924..4479a056 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 @@ -18,6 +18,7 @@ package com.google.auto.value.processor; import static com.google.auto.common.MoreElements.getLocalAndInheritedMethods; import static com.google.auto.common.MoreStreams.toImmutableList; import static com.google.auto.value.processor.ClassNames.AUTO_VALUE_NAME; +import static com.google.common.base.Preconditions.checkState; import static com.google.common.collect.Sets.difference; import static com.google.common.collect.Sets.intersection; import static java.util.Comparator.naturalOrder; @@ -45,7 +46,6 @@ import javax.annotation.processing.ProcessingEnvironment; import javax.annotation.processing.Processor; import javax.annotation.processing.SupportedAnnotationTypes; import javax.lang.model.element.AnnotationMirror; -import javax.lang.model.element.ElementKind; import javax.lang.model.element.ExecutableElement; import javax.lang.model.element.TypeElement; import javax.lang.model.type.TypeKind; @@ -79,21 +79,24 @@ public class AutoValueProcessor extends AutoValueishProcessor { @VisibleForTesting AutoValueProcessor(ClassLoader loaderForExtensions) { - super(AUTO_VALUE_NAME); - this.extensions = null; - this.loaderForExtensions = loaderForExtensions; + this(ImmutableList.of(), loaderForExtensions); } @VisibleForTesting - public AutoValueProcessor(Iterable extensions) { - super(AUTO_VALUE_NAME); - this.extensions = ImmutableList.copyOf(extensions); - this.loaderForExtensions = null; + public AutoValueProcessor(Iterable testExtensions) { + this(testExtensions, null); + } + + private AutoValueProcessor( + Iterable testExtensions, ClassLoader loaderForExtensions) { + super(AUTO_VALUE_NAME, /* appliesToInterfaces= */ false); + this.extensions = ImmutableList.copyOf(testExtensions); + this.loaderForExtensions = loaderForExtensions; } // Depending on how this AutoValueProcessor was constructed, we might already have a list of - // extensions when init() is run, or, if `extensions` is null, we have a ClassLoader that will be - // used to get the list using the ServiceLoader API. + // extensions when init() is run, or, if `loaderForExtensions` is not null, it is a ClassLoader + // that will be used to get the list using the ServiceLoader API. private ImmutableList extensions; private final ClassLoader loaderForExtensions; @@ -108,7 +111,8 @@ public class AutoValueProcessor extends AutoValueishProcessor { public synchronized void init(ProcessingEnvironment processingEnv) { super.init(processingEnv); - if (extensions == null) { + if (loaderForExtensions != null) { + checkState(extensions.isEmpty()); try { extensions = extensionsFromLoader(loaderForExtensions); } catch (RuntimeException | Error e) { @@ -165,10 +169,6 @@ public class AutoValueProcessor extends AutoValueishProcessor { @Override void processType(TypeElement type) { - if (type.getKind() != ElementKind.CLASS) { - errorReporter() - .abortWithError(type, "[AutoValueNotClass] @AutoValue only applies to classes"); - } if (ancestorIsAutoValue(type)) { errorReporter() .abortWithError(type, "[AutoValueExtend] One @AutoValue class may not extend another"); @@ -180,7 +180,6 @@ public class AutoValueProcessor extends AutoValueishProcessor { "[AutoValueImplAnnotation] @AutoValue may not be used to implement an annotation" + " interface; try using @AutoAnnotation instead"); } - checkModifiersIfNested(type); // We are going to classify the methods of the @AutoValue class into several categories. // This covers the methods in the class itself and the ones it inherits from supertypes. 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 55a94a71..56e8a98a 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 @@ -29,6 +29,7 @@ import static java.util.stream.Collectors.joining; import static java.util.stream.Collectors.toCollection; import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toSet; +import static javax.lang.model.util.ElementFilter.constructorsIn; import com.google.auto.common.MoreElements; import com.google.auto.common.MoreTypes; @@ -92,19 +93,22 @@ import javax.tools.JavaFileObject; */ abstract class AutoValueishProcessor extends AbstractProcessor { private final String annotationClassName; + private final boolean appliesToInterfaces; /** - * Qualified names of {@code @AutoValue} or {@code AutoOneOf} classes that we attempted to process - * but had to abandon because we needed other types that they referenced and those other types - * were missing. + * Qualified names of {@code @AutoValue} (etc) classes that we attempted to process but had to + * abandon because we needed other types that they referenced and those other types were missing. */ private final List deferredTypeNames = new ArrayList<>(); - AutoValueishProcessor(String annotationClassName) { + AutoValueishProcessor(String annotationClassName, boolean appliesToInterfaces) { this.annotationClassName = annotationClassName; + this.appliesToInterfaces = appliesToInterfaces; } - /** The annotation we are processing, {@code AutoValue} or {@code AutoOneOf}. */ + /** + * The annotation we are processing, for example {@code AutoValue} or {@code AutoBuilder}. + */ private TypeElement annotationType; /** The simple name of {@link #annotationType}. */ private String simpleAnnotationName; @@ -117,6 +121,10 @@ abstract class AutoValueishProcessor extends AbstractProcessor { super.init(processingEnv); errorReporter = new ErrorReporter(processingEnv); nullables = new Nullables(processingEnv); + annotationType = elementUtils().getTypeElement(annotationClassName); + if (annotationType != null) { + simpleAnnotationName = annotationType.getSimpleName().toString(); + } } final ErrorReporter errorReporter() { @@ -132,9 +140,9 @@ abstract class AutoValueishProcessor extends AbstractProcessor { } /** - * Qualified names of {@code @AutoValue} or {@code AutoOneOf} classes that we attempted to process - * but had to abandon because we needed other types that they referenced and those other types - * were missing. This is used by tests. + * Qualified names of {@code @AutoValue} (etc) classes that we attempted to process but had to + * abandon because we needed other types that they referenced and those other types were missing. + * This is used by tests. */ final ImmutableList deferredTypeNames() { return ImmutableList.copyOf(deferredTypeNames); @@ -339,7 +347,6 @@ abstract class AutoValueishProcessor extends AbstractProcessor { @Override public final boolean process(Set annotations, RoundEnvironment roundEnv) { - annotationType = elementUtils().getTypeElement(annotationClassName); if (annotationType == null) { // This should not happen. If the annotation type is not found, how did the processor get // triggered? @@ -352,7 +359,6 @@ abstract class AutoValueishProcessor extends AbstractProcessor { + " because the annotation class was not found"); return false; } - simpleAnnotationName = annotationType.getSimpleName().toString(); List deferredTypes = deferredTypeNames.stream() .map(name -> elementUtils().getTypeElement(name)) @@ -364,9 +370,10 @@ abstract class AutoValueishProcessor extends AbstractProcessor { for (TypeElement type : deferredTypes) { errorReporter.reportError( type, - "[AutoValueUndefined] Did not generate @%s class for %s because it references" + "[%sUndefined] Did not generate @%s class for %s because it references" + " undefined types", simpleAnnotationName, + simpleAnnotationName, type.getQualifiedName()); } return false; @@ -381,6 +388,7 @@ abstract class AutoValueishProcessor extends AbstractProcessor { deferredTypeNames.clear(); for (TypeElement type : types) { try { + validateType(type); processType(type); } catch (AbortProcessingException e) { // We abandoned this type; continue with the next. @@ -396,7 +404,8 @@ abstract class AutoValueishProcessor extends AbstractProcessor { String trace = Throwables.getStackTraceAsString(e); errorReporter.reportError( type, - "[AutoValueException] @%s processor threw an exception: %s", + "[%sException] @%s processor threw an exception: %s", + simpleAnnotationName, simpleAnnotationName, trace); throw e; @@ -406,8 +415,37 @@ abstract class AutoValueishProcessor extends AbstractProcessor { } /** - * Analyzes a single {@code @AutoValue} or {@code @AutoOneOf} class, and outputs the corresponding - * implementation class or classes. + * Validations common to all the subclasses. An {@code @AutoFoo} type must be a class, or possibly + * an interface for {@code @AutoBuilder}. If it is a class then it must have a non-private no-arg + * constructor. + */ + private void validateType(TypeElement type) { + ElementKind kind = type.getKind(); + boolean kindOk = + kind.equals(ElementKind.CLASS) + || (appliesToInterfaces && kind.equals(ElementKind.INTERFACE)); + if (!kindOk) { + String appliesTo = appliesToInterfaces ? "classes and interfaces" : "classes"; + errorReporter.abortWithError( + type, + "[%sWrongType] @%s only applies to %s", + simpleAnnotationName, + simpleAnnotationName, + appliesTo); + } + checkModifiersIfNested(type); + if (!hasVisibleNoArgConstructor(type)) { + errorReporter.reportError( + type, + "[%sConstructor] @%s class must have a non-private no-arg constructor", + simpleAnnotationName, + simpleAnnotationName); + } + } + + /** + * Analyzes a single {@code @AutoValue} (etc) class, and outputs the corresponding implementation + * class or classes. * * @param type the class with the {@code @AutoValue} or {@code @AutoOneOf} annotation. */ @@ -469,7 +507,9 @@ abstract class AutoValueishProcessor extends AbstractProcessor { if (p.isNullable() && returnType.getKind().isPrimitive()) { errorReporter() .reportError( - propertyMethod, "[AutoValueNullPrimitive] Primitive types cannot be @Nullable"); + propertyMethod, + "[%sNullPrimitive] Primitive types cannot be @Nullable", + simpleAnnotationName); } }); return props.build(); @@ -508,11 +548,10 @@ abstract class AutoValueishProcessor extends AbstractProcessor { } /** - * Returns the name of the generated {@code @AutoValue} or {@code @AutoOneOf} class, for example - * {@code AutoOneOf_TaskResult} or {@code $$AutoValue_SimpleMethod}. + * Returns the name of the generated {@code @AutoValue} (etc) class, for example {@code + * AutoOneOf_TaskResult} or {@code $$AutoValue_SimpleMethod}. * - * @param type the name of the type bearing the {@code @AutoValue} or {@code @AutoOneOf} - * annotation. + * @param type the name of the type bearing the {@code @AutoValue} (etc) annotation. * @param prefix the prefix to use in the generated class. This may start with one or more dollar * signs, for an {@code @AutoValue} implementation where there are AutoValue extensions. */ @@ -589,7 +628,8 @@ abstract class AutoValueishProcessor extends AbstractProcessor { for (ExecutableElement context : contexts) { errorReporter.reportError( context, - "[AutoValueDupProperty] More than one @%s property called %s", + "[%sDupProperty] More than one @%s property called %s", + simpleAnnotationName, simpleAnnotationName, name); } @@ -1187,6 +1227,14 @@ abstract class AutoValueishProcessor extends AbstractProcessor { return getAnnotationMirror(element, annotationName).isPresent(); } + /** True if the type is a class with a non-private no-arg constructor, or is an interface. */ + static boolean hasVisibleNoArgConstructor(TypeElement type) { + return type.getKind().isInterface() + || constructorsIn(type.getEnclosedElements()).stream() + .anyMatch( + c -> c.getParameters().isEmpty() && !c.getModifiers().contains(Modifier.PRIVATE)); + } + final void writeSourceFile(String className, String text, TypeElement originatingType) { try { JavaFileObject sourceFile = diff --git a/value/src/main/java/com/google/auto/value/processor/BuilderSpec.java b/value/src/main/java/com/google/auto/value/processor/BuilderSpec.java index dfdbb8c7..b612c104 100644 --- a/value/src/main/java/com/google/auto/value/processor/BuilderSpec.java +++ b/value/src/main/java/com/google/auto/value/processor/BuilderSpec.java @@ -18,6 +18,7 @@ package com.google.auto.value.processor; import static com.google.auto.common.MoreElements.getLocalAndInheritedMethods; import static com.google.auto.common.MoreStreams.toImmutableSet; import static com.google.auto.value.processor.AutoValueishProcessor.hasAnnotationMirror; +import static com.google.auto.value.processor.AutoValueishProcessor.hasVisibleNoArgConstructor; import static com.google.auto.value.processor.AutoValueishProcessor.nullableAnnotationFor; import static com.google.auto.value.processor.ClassNames.AUTO_VALUE_BUILDER_NAME; import static com.google.common.collect.Sets.immutableEnumSet; @@ -86,16 +87,9 @@ class BuilderSpec { Optional builderTypeElement = Optional.empty(); for (TypeElement containedClass : typesIn(autoValueClass.getEnclosedElements())) { if (hasAnnotationMirror(containedClass, AUTO_VALUE_BUILDER_NAME)) { - if (!CLASS_OR_INTERFACE.contains(containedClass.getKind())) { - errorReporter.reportError( - containedClass, - "[AutoValueBuilderClass] @AutoValue.Builder can only apply to a class or an" - + " interface"); - } else if (!containedClass.getModifiers().contains(Modifier.STATIC)) { - errorReporter.reportError( - containedClass, - "[AutoValueInnerBuilder] @AutoValue.Builder cannot be applied to a non-static class"); - } else if (builderTypeElement.isPresent()) { + findBuilderError(containedClass) + .ifPresent(error -> errorReporter.reportError(containedClass, "%s", error)); + if (builderTypeElement.isPresent()) { errorReporter.reportError( containedClass, "[AutoValueTwoBuilders] %s already has a Builder: %s", @@ -114,6 +108,24 @@ class BuilderSpec { } } + /** Finds why this {@code @AutoValue.Builder} class is bad, if it is bad. */ + private Optional findBuilderError(TypeElement builderTypeElement) { + if (!CLASS_OR_INTERFACE.contains(builderTypeElement.getKind())) { + return Optional.of( + "[AutoValueBuilderClass] @AutoValue.Builder can only apply to a class or an" + + " interface"); + } else if (!builderTypeElement.getModifiers().contains(Modifier.STATIC)) { + return Optional.of( + "[AutoValueInnerBuilder] @AutoValue.Builder cannot be applied to a non-static class"); + } else if (builderTypeElement.getKind().equals(ElementKind.CLASS) + && !hasVisibleNoArgConstructor(builderTypeElement)) { + return Optional.of( + "[AutoValueBuilderConstructor] @AutoValue.Builder class must have a non-private no-arg" + + " constructor"); + } + return Optional.empty(); + } + /** Representation of an {@code AutoValue.Builder} class or interface. */ class Builder implements AutoValueExtension.BuilderContext { private final TypeElement builderTypeElement; -- cgit v1.2.3 From 07f37b2be9e060f57eed3e3f5cdccab6a53f0927 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89amonn=20McManus?= Date: Mon, 13 Dec 2021 07:55:03 -0800 Subject: Disallow `@AutoValue final class`. We will otherwise try to generate a subclass for it which obviously won't compile. This is pretty unlikely anyway since a non-trivial `@AutoValue` class has to be abstract and a class can't be both final and abstract. Fixes https://github.com/google/auto/issues/1215. RELNOTES=n/a PiperOrigin-RevId: 416042702 --- .../com/google/auto/value/processor/AutoValueishProcessor.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'value/src/main/java/com/google') 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 56e8a98a..31f1ec1c 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 @@ -417,7 +417,7 @@ abstract class AutoValueishProcessor extends AbstractProcessor { /** * Validations common to all the subclasses. An {@code @AutoFoo} type must be a class, or possibly * an interface for {@code @AutoBuilder}. If it is a class then it must have a non-private no-arg - * constructor. + * constructor. And, since we'll be generating a subclass, it can't be final. */ private void validateType(TypeElement type) { ElementKind kind = type.getKind(); @@ -441,6 +441,13 @@ abstract class AutoValueishProcessor extends AbstractProcessor { simpleAnnotationName, simpleAnnotationName); } + if (type.getModifiers().contains(Modifier.FINAL)) { + errorReporter.abortWithError( + type, + "[%sFinal] @%s class must not be final", + simpleAnnotationName, + simpleAnnotationName); + } } /** -- cgit v1.2.3