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 #1209.

RELNOTES=n/a
PiperOrigin-RevId: 414779231

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 @@ void processType(TypeElement autoBuilderType) {
     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 @@ void processType(TypeElement autoBuilderType) {
     Optional<BuilderMethodClassifier<VariableElement>> 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;
     }

value/src/main/java/com/google/auto/value/processor/AutoOneOfProcessor.java

@@ -60,7 +60,7 @@
 @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 ImmutableSet<String> getSupportedOptions() {
 
   @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.

value/src/main/java/com/google/auto/value/processor/AutoValueProcessor.java

@@ -18,6 +18,7 @@
 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.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 AutoValueProcessor() {
 
   @VisibleForTesting
   AutoValueProcessor(ClassLoader loaderForExtensions) {
-    super(AUTO_VALUE_NAME);
-    this.extensions = null;
-    this.loaderForExtensions = loaderForExtensions;
+    this(ImmutableList.of(), loaderForExtensions);
   }
 
   @VisibleForTesting
-  public AutoValueProcessor(Iterable<? extends AutoValueExtension> extensions) {
-    super(AUTO_VALUE_NAME);
-    this.extensions = ImmutableList.copyOf(extensions);
-    this.loaderForExtensions = null;
+  public AutoValueProcessor(Iterable<? extends AutoValueExtension> testExtensions) {
+    this(testExtensions, null);
+  }
+
+  private AutoValueProcessor(
+      Iterable<? extends AutoValueExtension> 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<AutoValueExtension> extensions;
   private final ClassLoader loaderForExtensions;
 
@@ -108,7 +111,8 @@ static ImmutableList<AutoValueExtension> extensionsFromLoader(ClassLoader loader
   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 @@ static String generatedSubclassName(TypeElement type, int depth) {
 
   @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 @@ void processType(TypeElement type) {
               "[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.

value/src/main/java/com/google/auto/value/processor/AutoValueishProcessor.java

@@ -29,6 +29,7 @@
 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 @@
  */
 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<String> 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 @@ 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();
+    }
   }
 
   final ErrorReporter errorReporter() {
@@ -132,9 +140,9 @@ final Elements elementUtils() {
   }
 
   /**
-   * 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<String> deferredTypeNames() {
     return ImmutableList.copyOf(deferredTypeNames);
@@ -339,7 +347,6 @@ public int hashCode() {
 
   @Override
   public final boolean process(Set<? extends TypeElement> 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 @@ public final boolean process(Set<? extends TypeElement> annotations, RoundEnviro
                   + " because the annotation class was not found");
       return false;
     }
-    simpleAnnotationName = annotationType.getSimpleName().toString();
     List<TypeElement> deferredTypes =
         deferredTypeNames.stream()
             .map(name -> elementUtils().getTypeElement(name))
@@ -364,9 +370,10 @@ public final boolean process(Set<? extends TypeElement> annotations, RoundEnviro
       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 @@ public final boolean process(Set<? extends TypeElement> annotations, RoundEnviro
     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 @@ public final boolean process(Set<? extends TypeElement> annotations, RoundEnviro
         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 @@ public final boolean process(Set<? extends TypeElement> annotations, RoundEnviro
   }
 
   /**
-   * 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 @@ final ImmutableSet<Property> propertySet(
           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 @@ static ImmutableList<String> annotationStrings(List<? extends AnnotationMirror>
   }
 
   /**
-   * 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 @@ final ImmutableBiMap<String, ExecutableElement> propertyNameToMethodMap(
         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 @@ static boolean hasAnnotationMirror(Element element, String annotationName) {
     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 =

value/src/main/java/com/google/auto/value/processor/BuilderSpec.java

@@ -18,6 +18,7 @@
 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 @@ Optional<Builder> getBuilder() {
     Optional<TypeElement> 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 @@ Optional<Builder> getBuilder() {
     }
   }
 
+  /** Finds why this {@code @AutoValue.Builder} class is bad, if it is bad. */
+  private Optional<String> 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;