From 216a9d29f5fdc06f2bde2cd9ba593db0a52baf1e Mon Sep 17 00:00:00 2001
From: Brad Corso <bcorso@google.com>
Date: Thu, 18 Jul 2024 08:43:11 -0700
Subject: [PATCH] Fix broken assertion in
 `RequiresResolutionChecker#visitUncachedDependencies()`

Within `visitUncachedDependencies()` we assert that each uncached key is actually uncached before caching it. However, this assertion can fail because during the process of caching, we call `getLocalExplicitBindings()` and `getLocalMultibindingContributions()` which can trigger its own call to `visitUncachedDependencies()` and cache it first.

This CL fixes this issue by avoiding the calls to `getLocalExplicitBindings()` and `getLocalMultibindingContributions()`. In these cases we don't actually need the bindings themselves since we just need to know if the declarations exist or not, so we can just use the declarations directly. This saves us a bit of work because creating the binding requires us to not only resolve the binding itself, but also bindings for dependencies (possibly even transitive dependencies) in the case `@Binds` delegate declarations. It's also nice to keep the assertion because it gaurantees some simplicity to the code.

RELNOTES=N/A
PiperOrigin-RevId: 653633501
---
 .../codegen/binding/BindingGraphFactory.java  | 22 ++++++++++++++-----
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/java/dagger/internal/codegen/binding/BindingGraphFactory.java b/java/dagger/internal/codegen/binding/BindingGraphFactory.java
index 79a93d5b7b9..b55ec0b8b0e 100644
--- a/java/dagger/internal/codegen/binding/BindingGraphFactory.java
+++ b/java/dagger/internal/codegen/binding/BindingGraphFactory.java
@@ -725,7 +725,7 @@ void resolve(Key key) {
                 .anyMatch(binding -> binding.kind() == BindingKind.ASSISTED_INJECTION);
         if (!isAssistedInjectionBinding
                 && !requiresResolution(key)
-                && getLocalExplicitBindings(key).isEmpty()) {
+                && !hasLocalExplicitBindings(key)) {
           /* Cache the inherited parent component's bindings in case resolving at the parent found
            * bindings in some component between this one and the previously-resolved one. */
           resolvedContributionBindings.put(key, previouslyResolvedBindings);
@@ -867,9 +867,12 @@ private boolean hasLocalBindings(ResolvedBindings resolvedBindings) {
      * this component's modules that matches the key.
      */
     private boolean hasLocalMultibindingContributions(Key requestKey) {
-      return keysMatchingRequest(requestKey)
-          .stream()
-          .anyMatch(key -> !getLocalMultibindingContributions(key).isEmpty());
+      return keysMatchingRequest(requestKey).stream()
+          .anyMatch(
+              multibindingKey ->
+                  !declarations.multibindingContributions(multibindingKey).isEmpty()
+                      || !declarations.delegateMultibindingContributions(
+                          keyFactory.unwrapMapValueType(multibindingKey)).isEmpty());
     }
 
     /**
@@ -886,8 +889,7 @@ private boolean hasLocalOptionalBindingContribution(
       if (previouslyResolvedBindings.stream()
           .map(Binding::kind)
           .anyMatch(isEqual(OPTIONAL))) {
-        return !getLocalExplicitBindings(keyFactory.unwrapOptional(key).get())
-            .isEmpty();
+        return hasLocalExplicitBindings(keyFactory.unwrapOptional(key).get());
       } else {
         // If a parent contributes a @Provides Optional<Foo> binding and a child has a
         // @BindsOptionalOf Foo method, the two should conflict, even if there is no binding for
@@ -895,5 +897,13 @@ private boolean hasLocalOptionalBindingContribution(
         return !getOptionalBindingDeclarations(key).isEmpty();
       }
     }
+
+    /**
+     * Returns {@code true} if there is at least one explicit binding that matches the given key.
+     */
+    private boolean hasLocalExplicitBindings(Key requestKey) {
+      return !declarations.bindings(requestKey).isEmpty()
+          || !declarations.delegates(keyFactory.unwrapMapValueType(requestKey)).isEmpty();
+    }
   }
 }