Consolidate logic for whether we generate MembersInjector in one place.

The logic for whether we generate a MembersInjector was duplicated in `InjectBindingRegistryImpl` and `MembersInjectorGenerator`. In particular, there are cases where `InjectBindingRegistryImpl` decided that a MembersInjector should be generated, but additional logic in `MembersInjectorGenerator` overrides that decision and decides not to generate the MembersInjector (e.g. if the class has no local injection sites).

This led to duplicating of logic in `InjectBindingRegistryImpl` when we're trying to decide if we should output a warning for the case where a MembersInjector should be generated in an upstream library because we needed to repeat the logic in `MembersInjectorGenerator` to know if the MembersInjector will actually be generated.

This CL moves the decision on whether to generate a MembersInjector completely into `InjectBindingRegistryImpl`, which removes the duplicated logic and also makes it easier to understand when the MembersInjector will actually be generated since we only need to look in one location.

This CL also removes `warnIfNotAlreadyGenerated` since `isCalledFromInjectProcessor`  contains all the information we need to decide whether to warn in this case.

RELNOTES=N/A
PiperOrigin-RevId: 569528883

java/dagger/internal/codegen/validation/InjectBindingRegistryImpl.java

@@ -118,10 +118,7 @@ B getBinding(Key key) {
     }
 
     /** Caches the binding and generates it if it needs generation. */
-    void tryRegisterBinding(
-        B binding,
-        boolean warnIfNotAlreadyGenerated,
-        boolean isCalledFromInjectProcessor) {
+    void tryRegisterBinding(B binding, boolean isCalledFromInjectProcessor) {
       if (processingEnv.getBackend() == XProcessingEnv.Backend.KSP) {
         Origin origin =
             toKS(closestEnclosingTypeElement(binding.bindingElement().get())).getOrigin();
@@ -143,21 +140,18 @@ void tryRegisterBinding(
       @SuppressWarnings("unchecked")
       B maybeUnresolved =
           binding.unresolved().isPresent() ? (B) binding.unresolved().get() : binding;
-      tryToGenerateBinding(maybeUnresolved, warnIfNotAlreadyGenerated, isCalledFromInjectProcessor);
+      tryToGenerateBinding(maybeUnresolved, isCalledFromInjectProcessor);
     }
 
     /**
      * Tries to generate a binding, not generating if it already is generated. For resolved
      * bindings, this will try to generate the unresolved version of the binding.
      */
-    void tryToGenerateBinding(
-        B binding,
-        boolean warnIfNotAlreadyGenerated,
-        boolean isCalledFromInjectProcessor) {
+    void tryToGenerateBinding(B binding, boolean isCalledFromInjectProcessor) {
       if (shouldGenerateBinding(binding)) {
         bindingsRequiringGeneration.offer(binding);
         if (compilerOptions.warnIfInjectionFactoryNotGeneratedUpstream()
-            && warnIfNotAlreadyGenerated) {
+                && !isCalledFromInjectProcessor) {
           messager.printMessage(
               Kind.NOTE,
               String.format(
@@ -172,6 +166,22 @@ void tryToGenerateBinding(
 
     /** Returns true if the binding needs to be generated. */
     private boolean shouldGenerateBinding(B binding) {
+      if (binding instanceof MembersInjectionBinding) {
+        MembersInjectionBinding membersInjectionBinding = (MembersInjectionBinding) binding;
+        // Empty members injection bindings are special and don't need source files.
+        if (membersInjectionBinding.injectionSites().isEmpty()) {
+          return false;
+        }
+        // Members injectors for classes with no local injection sites and no @Inject
+        // constructor are unused.
+        boolean hasInjectConstructor =
+            !(injectedConstructors(membersInjectionBinding.membersInjectedType()).isEmpty()
+                && assistedInjectedConstructors(
+                    membersInjectionBinding.membersInjectedType()).isEmpty());
+        if (!membersInjectionBinding.hasLocalInjectionSites() && !hasInjectConstructor) {
+          return false;
+        }
+      }
       return !binding.unresolved().isPresent()
           && !materializedBindingKeys.contains(binding.key())
           && !bindingsRequiringGeneration.contains(binding)
@@ -225,64 +235,19 @@ public void generateSourcesForRequiredBindings(
     membersInjectionBindings.generateBindings(membersInjectorGenerator);
   }
 
-  /**
-   * Registers the binding for generation and later lookup. If the binding is resolved, we also
-   * attempt to register an unresolved version of it.
-   */
-  private void registerBinding(
-      ProvisionBinding binding,
-      boolean warnIfNotAlreadyGenerated,
-      boolean isCalledFromInjectProcessor) {
-    provisionBindings.tryRegisterBinding(
-        binding, warnIfNotAlreadyGenerated, isCalledFromInjectProcessor);
-  }
-
-  /**
-   * Registers the binding for generation and later lookup. If the binding is resolved, we also
-   * attempt to register an unresolved version of it.
-   */
-  private void registerBinding(
-      MembersInjectionBinding binding,
-      boolean warnIfNotAlreadyGenerated,
-      boolean isCalledFromInjectProcessor) {
-    /*
-     * We generate MembersInjector classes for types with @Inject constructors only if they have any
-     * injection sites.
-     *
-     * We generate MembersInjector classes for types without @Inject constructors only if they have
-     * local (non-inherited) injection sites.
-     *
-     * Warn only when registering bindings post-hoc for those types.
-     */
-    if (warnIfNotAlreadyGenerated) {
-      boolean hasInjectConstructor =
-          !(injectedConstructors(binding.membersInjectedType()).isEmpty()
-              && assistedInjectedConstructors(binding.membersInjectedType()).isEmpty());
-      warnIfNotAlreadyGenerated =
-          hasInjectConstructor
-              ? !binding.injectionSites().isEmpty()
-              : binding.hasLocalInjectionSites();
-    }
-
-    membersInjectionBindings
-        .tryRegisterBinding(binding, warnIfNotAlreadyGenerated, isCalledFromInjectProcessor);
-  }
-
   @Override
   public Optional<ProvisionBinding> tryRegisterInjectConstructor(
       XConstructorElement constructorElement) {
     return tryRegisterConstructor(
         constructorElement,
         Optional.empty(),
-        /* warnIfNotAlreadyGenerated= */ false,
         /* isCalledFromInjectProcessor= */ true);
   }
 
   @CanIgnoreReturnValue
   private Optional<ProvisionBinding> tryRegisterConstructor(
       XConstructorElement constructorElement,
       Optional<XType> resolvedType,
-      boolean warnIfNotAlreadyGenerated,
       boolean isCalledFromInjectProcessor) {
     XTypeElement typeElement = constructorElement.getEnclosingElement();
 
@@ -301,10 +266,9 @@ private Optional<ProvisionBinding> tryRegisterConstructor(
     }
 
     ProvisionBinding binding = bindingFactory.injectionBinding(constructorElement, resolvedType);
-    registerBinding(binding, warnIfNotAlreadyGenerated, isCalledFromInjectProcessor);
+    provisionBindings.tryRegisterBinding(binding, isCalledFromInjectProcessor);
     if (!binding.injectionSites().isEmpty()) {
-      tryRegisterMembersInjectedType(
-          typeElement, resolvedType, warnIfNotAlreadyGenerated, isCalledFromInjectProcessor);
+      tryRegisterMembersInjectedType(typeElement, resolvedType, isCalledFromInjectProcessor);
     }
     return Optional.of(binding);
   }
@@ -322,7 +286,6 @@ public Optional<MembersInjectionBinding> tryRegisterInjectField(XFieldElement fi
     return tryRegisterMembersInjectedType(
         asTypeElement(fieldElement.getEnclosingElement()),
         Optional.empty(),
-        /* warnIfNotAlreadyGenerated= */ false,
         /* isCalledFromInjectProcessor= */ true);
   }
 
@@ -339,15 +302,13 @@ public Optional<MembersInjectionBinding> tryRegisterInjectMethod(XMethodElement
     return tryRegisterMembersInjectedType(
         asTypeElement(methodElement.getEnclosingElement()),
         Optional.empty(),
-        /* warnIfNotAlreadyGenerated= */ false,
         /* isCalledFromInjectProcessor= */ true);
   }
 
   @CanIgnoreReturnValue
   private Optional<MembersInjectionBinding> tryRegisterMembersInjectedType(
       XTypeElement typeElement,
       Optional<XType> resolvedType,
-      boolean warnIfNotAlreadyGenerated,
       boolean isCalledFromInjectProcessor) {
     // Validating here shouldn't have a performance penalty because the validator caches its reports
     ValidationReport report = injectValidator.validateForMembersInjection(typeElement);
@@ -364,7 +325,7 @@ private Optional<MembersInjectionBinding> tryRegisterMembersInjectedType(
     }
 
     MembersInjectionBinding binding = bindingFactory.membersInjectionBinding(type, resolvedType);
-    registerBinding(binding, warnIfNotAlreadyGenerated, isCalledFromInjectProcessor);
+    membersInjectionBindings.tryRegisterBinding(binding, isCalledFromInjectProcessor);
     for (Optional<XType> supertype = nonObjectSuperclass(type);
         supertype.isPresent();
         supertype = nonObjectSuperclass(supertype.get())) {
@@ -404,7 +365,6 @@ public Optional<ProvisionBinding> getOrFindProvisionBinding(Key key) {
                 tryRegisterConstructor(
                     constructor,
                     Optional.of(type),
-                    /* warnIfNotAlreadyGenerated= */ true,
                     /* isCalledFromInjectProcessor= */ false));
   }
 
@@ -421,7 +381,6 @@ public Optional<MembersInjectionBinding> getOrFindMembersInjectionBinding(Key ke
     return tryRegisterMembersInjectedType(
         key.type().xprocessing().getTypeElement(),
         Optional.of(key.type().xprocessing()),
-        /* warnIfNotAlreadyGenerated= */ true,
         /* isCalledFromInjectProcessor= */ false);
   }
 

java/dagger/internal/codegen/writing/MembersInjectorGenerator.java

@@ -20,8 +20,6 @@
 import static com.squareup.javapoet.MethodSpec.constructorBuilder;
 import static com.squareup.javapoet.MethodSpec.methodBuilder;
 import static com.squareup.javapoet.TypeSpec.classBuilder;
-import static dagger.internal.codegen.binding.AssistedInjectionAnnotations.assistedInjectedConstructors;
-import static dagger.internal.codegen.binding.InjectionAnnotations.injectedConstructors;
 import static dagger.internal.codegen.binding.SourceFiles.bindingTypeElementTypeVariableNames;
 import static dagger.internal.codegen.binding.SourceFiles.generateBindingFieldsForDependencies;
 import static dagger.internal.codegen.binding.SourceFiles.membersInjectorNameForType;
@@ -90,19 +88,6 @@ public XElement originatingElement(MembersInjectionBinding binding) {
 
   @Override
   public ImmutableList<TypeSpec.Builder> topLevelTypes(MembersInjectionBinding binding) {
-    // Empty members injection bindings are special and don't need source files.
-    if (binding.injectionSites().isEmpty()) {
-      return ImmutableList.of();
-    }
-
-    // Members injectors for classes with no local injection sites and no @Inject
-    // constructor are unused.
-    if (!binding.hasLocalInjectionSites()
-        && injectedConstructors(binding.membersInjectedType()).isEmpty()
-        && assistedInjectedConstructors(binding.membersInjectedType()).isEmpty()) {
-      return ImmutableList.of();
-    }
-
 
     // We don't want to write out resolved bindings -- we want to write out the generic version.
     checkState(

javatests/dagger/internal/codegen/ComponentProcessorTest.java

@@ -16,7 +16,13 @@
 
 package dagger.internal.codegen;
 
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
+import static org.junit.Assert.assertThrows;
+
+import androidx.room.compiler.processing.util.CompilationResultSubject;
 import androidx.room.compiler.processing.util.Source;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.squareup.javapoet.AnnotationSpec;
 import com.squareup.javapoet.MethodSpec;
@@ -27,6 +33,7 @@
 import dagger.testing.golden.GoldenFileRule;
 import java.util.Collection;
 import javax.inject.Inject;
+import javax.tools.Diagnostic;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -1380,21 +1387,60 @@ public void unprocessedMembersInjectorNotes() {
             subject -> {
               subject.hasErrorCount(0);
               subject.hasWarningCount(0);
-              subject.hasNoteContaining(
-                  String.format(
-                      "Generating a MembersInjector for dagger.internal.codegen.%s.",
-                      "ComponentProcessorTestClasses.LocalInjectMemberNoConstructor"));
-              subject.hasNoteContaining(
-                  String.format(
-                      "Generating a MembersInjector for dagger.internal.codegen.%s.",
-                      "ComponentProcessorTestClasses.LocalInjectMemberWithConstructor"));
-              subject.hasNoteContaining(
-                  String.format(
-                      "Generating a MembersInjector for dagger.internal.codegen.%s.",
-                      "ComponentProcessorTestClasses.ParentInjectMemberWithConstructor"));
+
+              String generatedFileTemplate =
+                  "dagger/internal/codegen/ComponentProcessorTestClasses_%s_MembersInjector.java";
+              String noteTemplate =
+                  "Generating a MembersInjector for "
+                      + "dagger.internal.codegen.ComponentProcessorTestClasses.%s.";
+
+              // Assert that we generate sources and notes for the following classes.
+              ImmutableList.of(
+                      "LocalInjectMemberNoConstructor",
+                      "LocalInjectMemberWithConstructor",
+                      "ParentInjectMemberWithConstructor")
+                  .forEach(
+                      className -> {
+                        subject.generatedSourceFileWithPath(
+                            String.format(generatedFileTemplate, className));
+                        subject.hasNoteContaining(String.format(noteTemplate, className));
+                      });
+
+              // Assert that we **do not** generate sources and notes for the following classes.
+              ImmutableList.of(
+                      "ParentInjectMemberNoConstructor",
+                      "NoInjectMemberNoConstructor",
+                      "NoInjectMemberWithConstructor")
+                  .forEach(
+                      className -> {
+                        assertFileNotGenerated(
+                            subject, String.format(generatedFileTemplate, className));
+                        assertDoesNotHaveNoteContaining(
+                            subject, String.format(noteTemplate, className));
+                      });
             });
   }
 
+  private void assertFileNotGenerated(CompilationResultSubject subject, String filePath) {
+    // TODO(b/303653163): replace this with better XProcessing API once we have the ability to get a
+    // list of all generated sources.
+    AssertionError error =
+        assertThrows(
+            AssertionError.class,
+            () -> subject.generatedSourceFileWithPath(filePath));
+    assertThat(error).hasMessageThat().contains("Didn't generate file");
+  }
+
+  private void assertDoesNotHaveNoteContaining(CompilationResultSubject subject, String content) {
+    assertThat(
+        subject
+            .getCompilationResult()
+            .getDiagnostics()
+            .get(Diagnostic.Kind.NOTE)
+            .stream()
+            .filter(diagnostic -> diagnostic.getMsg().contains(content))).isEmpty();
+  }
+
   @Test
   public void scopeAnnotationOnInjectConstructorNotValid() {
     Source aScope =