Migrate more tests to XProcessing testing APIs.

This CL also fixes some naming of golden files. RELNOTES=N/A PiperOrigin-RevId: 474618766
google · Sep 15, 2022 · 85af407 · 85af407
1 parent e11fa81
commit 85af407
Show file tree

Hide file tree

Showing 9 changed files with 703 additions and 556 deletions.
diff --git a/java/dagger/internal/codegen/binding/ErrorMessages.java b/java/dagger/internal/codegen/binding/ErrorMessages.java
@@ -239,8 +239,8 @@ private static final class BuilderMessages extends ComponentCreatorMessages {
     @Override
     public String missingFactoryMethod() {
       return process(
-          "@Component.Builder types must have exactly one no-args method that "
-              + " returns the @Component type");
+          "@Component.Builder types must have exactly one no-args method that returns the "
+              + "@Component type");
     }
 
     @Override

diff --git a/java/dagger/internal/codegen/bindinggraphvalidation/SubcomponentFactoryMethodValidator.java b/java/dagger/internal/codegen/bindinggraphvalidation/SubcomponentFactoryMethodValidator.java
@@ -20,12 +20,12 @@
 import static dagger.internal.codegen.binding.ComponentRequirement.componentCanMakeNewInstances;
 import static dagger.internal.codegen.extension.DaggerStreams.instancesOf;
 import static dagger.internal.codegen.extension.DaggerStreams.toImmutableSet;
+import static java.util.stream.Collectors.joining;
 import static javax.tools.Diagnostic.Kind.ERROR;
 
 import androidx.room.compiler.processing.XExecutableType;
 import androidx.room.compiler.processing.XType;
 import androidx.room.compiler.processing.XTypeElement;
-import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 import com.google.common.collect.Sets.SetView;
@@ -146,6 +146,8 @@ private void reportMissingModuleParameters(
             .currentComponent()
             .className()
             .canonicalName(),
-        Joiner.on(", ").join(missingModules));
+        missingModules.stream()
+            .map(XTypeElement::getQualifiedName)
+            .collect(joining(", ")));
   }
 }
diff --git a/java/dagger/internal/codegen/validation/ComponentCreatorValidator.java b/java/dagger/internal/codegen/validation/ComponentCreatorValidator.java
@@ -129,8 +129,12 @@ private ElementValidator(
     /** Validates the creator type. */
     final ValidationReport validate() {
       XTypeElement enclosingType = creator.getEnclosingTypeElement();
-      if (enclosingType == null || !enclosingType.hasAnnotation(annotation.componentAnnotation())) {
-        report.addError(messages.mustBeInComponent());
+
+      // If the type isn't enclosed in a component don't validate anything else since the rest of
+      // the messages will be bogus.
+      if (enclosingType == null
+              || !enclosingType.hasAnnotation(annotation.componentAnnotation())) {
+        return report.addError(messages.mustBeInComponent()).build();
       }
 
       // If the type isn't a class or interface, don't validate anything else since the rest of the
@@ -139,7 +143,12 @@ final ValidationReport validate() {
         return report.build();
       }
 
-      validateTypeRequirements();
+      // If the type isn't a valid creator type, don't validate anything else since the rest of the
+      // messages will be bogus.
+      if (!validateTypeRequirements()) {
+        return report.build();
+      }
+
       switch (annotation.creatorKind()) {
         case FACTORY:
           validateFactory();
@@ -181,21 +190,26 @@ private void validateConstructor() {
     }
 
     /** Validates basic requirements about the type that are common to both creator kinds. */
-    private void validateTypeRequirements() {
+    private boolean validateTypeRequirements() {
+      boolean isClean = true;
       if (hasTypeParameters(creator)) {
         report.addError(messages.generics());
+        isClean = false;
       }
-
       if (creator.isPrivate()) {
         report.addError(messages.isPrivate());
+        isClean = false;
       }
       if (!creator.isStatic()) {
         report.addError(messages.mustBeStatic());
+        isClean = false;
       }
       // Note: Must be abstract, so no need to check for final.
       if (!creator.isAbstract()) {
         report.addError(messages.mustBeAbstract());
+        isClean = false;
       }
+      return isClean;
     }
 
     private void validateBuilder() {

diff --git a/java/dagger/internal/codegen/validation/ComponentValidator.java b/java/dagger/internal/codegen/validation/ComponentValidator.java
@@ -70,6 +70,7 @@
 import dagger.internal.codegen.javapoet.TypeNames;
 import dagger.internal.codegen.kotlin.KotlinMetadataUtil;
 import dagger.internal.codegen.xprocessing.XTypeElements;
+import dagger.internal.codegen.xprocessing.XTypes;
 import dagger.spi.model.DependencyRequest;
 import dagger.spi.model.Key;
 import java.util.ArrayDeque;
@@ -362,7 +363,7 @@ private void validateSubcomponentFactoryMethod(ComponentAnnotation subcomponentA
             report.addError(
                 String.format(
                     "Subcomponent factory methods may only accept modules, but %s is not.",
-                    parameterType),
+                    XTypes.toStableString(parameterType)),
                 parameter);
           }
         }
@@ -482,7 +483,12 @@ private void validateSubcomponentReferences() {
           .forEach(
               (subcomponent, methods) ->
                   report.addError(
-                      String.format(moreThanOneRefToSubcomponent(), subcomponent, methods),
+                      String.format(
+                          moreThanOneRefToSubcomponent(),
+                          subcomponent.getQualifiedName(),
+                          methods.stream()
+                              .map(methodSignatureFormatter::formatWithoutReturnType)
+                              .collect(toImmutableSet())),
                       component));
     }
 

diff --git a/javatests/dagger/internal/codegen/ComponentCreatorTest.java b/javatests/dagger/internal/codegen/ComponentCreatorTest.java
@@ -267,7 +267,7 @@ public void testCreatorNotInComponentFails() {
         .withProcessingOptions(compilerOptions)
         .compile(
             subject -> {
-              subject.hasErrorCount(3);
+              subject.hasErrorCount(1);
               subject.hasErrorContaining(messages.mustBeInComponent()).onSource(builder);
             });
   }
@@ -401,7 +401,7 @@ public void testPrivateCreatorFails() {
         .withProcessingOptions(compilerOptions)
         .compile(
             subject -> {
-              subject.hasErrorCount(2);
+              subject.hasErrorCount(1);
               subject.hasErrorContaining(messages.isPrivate()).onSource(componentFile);
             });
   }
@@ -425,7 +425,7 @@ public void testNonStaticCreatorFails() {
         .withProcessingOptions(compilerOptions)
         .compile(
             subject -> {
-              subject.hasErrorCount(2);
+              subject.hasErrorCount(1);
               subject.hasErrorContaining(messages.mustBeStatic()).onSource(componentFile);
             });
   }
@@ -449,7 +449,7 @@ public void testNonAbstractCreatorFails() {
         .withProcessingOptions(compilerOptions)
         .compile(
             subject -> {
-              subject.hasErrorCount(2);
+              subject.hasErrorCount(1);
               subject.hasErrorContaining(messages.mustBeAbstract()).onSource(componentFile);
             });
   }