Improve MissingImplementationError to lazily calculate suggestions and standardize its output.

The error will now generate suggestions only when the message is used (as opposed to on instantiation). This should significantly improve the performance of any code that happens to call `injector.get{Instance,Provider,Binding}` and catches/ignores a `ProvisionException` (with the intention of falling back to code that deals with the type not being bound). While this isn't a great pattern for code to use, and there are far better ways of doing it... there's enough code that does this that it's worthwhile improving.

This pattern in particular was very problematic with the introduction of the jakarta.inject.Provider variants for Multibinders/MapBinders & OptionalBinders, because we have *lots* of those. By adding N more bindings to each multi/map/optionalbinder, we added a very large number of bindings to the overall total, which resulted in code using this pattern getting much slower.

This code also fixes suggestions to skip "UntargettedBindings", which are bindings like `bind(SomeInterface.class)` and aren't a valid suggestion (because they're missing a .to(..)). By standardizing, this otherwise broke a test in BinderTest that was asserting we failed those with proper error messages (showing their source). We would have otherwise started showing their source twice, once in a "Did you mean?" section and again in the normal "bound here" section. But really we shouldn't show these in "Did you mean?" at all, because it's just a repetition (or incorrect suggestion) of the wrong binding.

PiperOrigin-RevId: 531062151

Author: sameb

core/src/com/google/inject/internal/MissingImplementationError.java

@@ -1,5 +1,6 @@
 package com.google.inject.internal;
 
+import com.google.common.base.Suppliers;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import com.google.inject.Injector;
@@ -8,28 +9,34 @@
 import java.util.ArrayList;
 import java.util.Formatter;
 import java.util.List;
+import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
 /** Error reported by Guice when a key is not bound in the injector. */
 final class MissingImplementationError<T>
     extends InternalErrorDetail<MissingImplementationError<T>> {
 
   private final Key<T> key;
-  private final ImmutableList<String> suggestions;
+  private final Supplier<ImmutableList<String>> suggestionsSupplier;
 
   public MissingImplementationError(Key<T> key, Injector injector, List<Object> sources) {
-    this(key, MissingImplementationErrorHints.getSuggestions(key, injector), sources);
+    this(
+        key,
+        // Defer building suggestions until messages are requested, to avoid the work associated
+        // with iterating bindings in scenarios where the exceptions are discarded.
+        Suppliers.memoize(() -> MissingImplementationErrorHints.getSuggestions(key, injector)),
+        sources);
   }
 
   private MissingImplementationError(
-      Key<T> key, ImmutableList<String> suggestions, List<Object> sources) {
+      Key<T> key, Supplier<ImmutableList<String>> suggestionsSupplier, List<Object> sources) {
     super(
         ErrorId.MISSING_IMPLEMENTATION,
         String.format("No implementation for %s was bound.", Messages.convert(key)),
         sources,
         null);
     this.key = key;
-    this.suggestions = suggestions;
+    this.suggestionsSupplier = suggestionsSupplier;
   }
 
   @Override
@@ -40,6 +47,7 @@ public boolean isMergeable(ErrorDetail<?> otherError) {
 
   @Override
   public void formatDetail(List<ErrorDetail<?>> mergeableErrors, Formatter formatter) {
+    ImmutableList<String> suggestions = suggestionsSupplier.get();
     if (!suggestions.isEmpty()) {
       suggestions.forEach(formatter::format);
     }
@@ -65,7 +73,7 @@ public void formatDetail(List<ErrorDetail<?>> mergeableErrors, Formatter formatt
 
   @Override
   public MissingImplementationError<T> withSources(List<Object> newSources) {
-    return new MissingImplementationError<T>(key, suggestions, newSources);
+    return new MissingImplementationError<T>(key, suggestionsSupplier, newSources);
   }
 
   /** Omit the key itself in the source list since the information is redundant. */

core/src/com/google/inject/internal/MissingImplementationErrorHints.java

@@ -1,5 +1,6 @@
 package com.google.inject.internal;
 
+import static com.google.common.collect.ImmutableList.toImmutableList;
 import static java.lang.Math.min;
 
 import com.google.common.collect.ImmutableList;
@@ -10,6 +11,7 @@
 import com.google.inject.Key;
 import com.google.inject.TypeLiteral;
 import com.google.inject.spi.BindingSourceRestriction;
+import com.google.inject.spi.UntargettedBinding;
 import java.util.ArrayList;
 import java.util.Formatter;
 import java.util.List;
@@ -47,22 +49,29 @@ static <T> ImmutableList<String> getSuggestions(Key<T> key, Injector injector) {
 
     // Keys which have similar strings as the desired key
     List<String> possibleMatches = new ArrayList<>();
-    List<Binding<T>> sameTypes = injector.findBindingsByType(type);
+    ImmutableList<Binding<T>> sameTypes =
+        injector.findBindingsByType(type).stream()
+            .filter(b -> !(b instanceof UntargettedBinding)) // These aren't valid matches
+            .collect(toImmutableList());
     if (!sameTypes.isEmpty()) {
       suggestions.add("\nDid you mean?");
       int howMany = min(sameTypes.size(), MAX_MATCHING_TYPES_REPORTED);
       for (int i = 0; i < howMany; ++i) {
+        Key<?> bindingKey = sameTypes.get(i).getKey();
         // TODO: Look into a better way to prioritize suggestions. For example, possbily
         // use levenshtein distance of the given annotation vs actual annotation.
-        suggestions.add(Messages.format("\n    * %s", sameTypes.get(i).getKey()));
+        suggestions.add(
+            Messages.format(
+                "\n    * %s",
+                formatSuggestion(bindingKey, injector.getExistingBinding(bindingKey))));
       }
       int remaining = sameTypes.size() - MAX_MATCHING_TYPES_REPORTED;
       if (remaining > 0) {
         String plural = (remaining == 1) ? "" : "s";
         suggestions.add(
-            Messages.format("\n    %d more binding%s with other annotations.", remaining, plural));
+            Messages.format(
+                "\n    * %d more binding%s with other annotations.", remaining, plural));
       }
-      suggestions.add("\n");
     } else {
       // For now, do a simple substring search for possibilities. This can help spot
       // issues when there are generics being used (such as a wrapper class) and the
@@ -73,14 +82,14 @@ static <T> ImmutableList<String> getSuggestions(Key<T> key, Injector injector) {
       String want = type.toString();
       Map<Key<?>, Binding<?>> bindingMap = injector.getAllBindings();
       for (Key<?> bindingKey : bindingMap.keySet()) {
+        Binding<?> binding = bindingMap.get(bindingKey);
+        // Ignore untargeted bindings, those aren't valid matches.
+        if (binding instanceof UntargettedBinding) {
+          continue;
+        }
         String have = bindingKey.getTypeLiteral().toString();
         if (have.contains(want) || want.contains(have)) {
-          Formatter fmt = new Formatter();
-          fmt.format("%s bound ", Messages.convert(bindingKey));
-          new SourceFormatter(
-                  bindingMap.get(bindingKey).getSource(), fmt, /* omitPreposition= */ false)
-              .format();
-          possibleMatches.add(fmt.toString());
+          possibleMatches.add(formatSuggestion(bindingKey, bindingMap.get(bindingKey)));
           // TODO: Consider a check that if there are more than some number of results,
           // don't suggest any.
           if (possibleMatches.size() > MAX_RELATED_TYPES_REPORTED) {
@@ -93,7 +102,7 @@ static <T> ImmutableList<String> getSuggestions(Key<T> key, Injector injector) {
       if (!possibleMatches.isEmpty() && (possibleMatches.size() <= MAX_RELATED_TYPES_REPORTED)) {
         suggestions.add("\nDid you mean?");
         for (String possibleMatch : possibleMatches) {
-          suggestions.add(Messages.format("\n    %s", possibleMatch));
+          suggestions.add(Messages.format("\n    * %s", possibleMatch));
         }
       }
     }
@@ -110,4 +119,11 @@ static <T> ImmutableList<String> getSuggestions(Key<T> key, Injector injector) {
 
     return suggestions.build();
   }
+
+  private static String formatSuggestion(Key<?> key, Binding<?> binding) {
+    Formatter fmt = new Formatter();
+    fmt.format("%s bound ", Messages.convert(key));
+    new SourceFormatter(binding.getSource(), fmt, /* omitPreposition= */ false).format();
+    return fmt.toString();
+  }
 }

core/test/com/google/inject/errors/MissingImplementationErrorTest.java

@@ -1,14 +1,17 @@
 package com.google.inject.errors;
 
+import static com.google.common.truth.Truth.assertThat;
 import static com.google.inject.errors.ErrorMessageTestUtils.assertGuiceErrorEqualsIgnoreLineNumber;
 import static java.lang.annotation.RetentionPolicy.RUNTIME;
 import static org.junit.Assert.assertThrows;
 import static org.junit.Assume.assumeTrue;
 
 import com.google.inject.AbstractModule;
+import com.google.inject.ConfigurationException;
 import com.google.inject.CreationException;
 import com.google.inject.Guice;
 import com.google.inject.Inject;
+import com.google.inject.Injector;
 import com.google.inject.Provides;
 import com.google.inject.internal.InternalFlags;
 import com.google.inject.internal.InternalFlags.IncludeStackTraceOption;
@@ -170,4 +173,38 @@ public void missingImplementationWithHints() throws Exception {
     assertGuiceErrorEqualsIgnoreLineNumber(
         exception.getMessage(), "missing_implementation_with_hints" + GOLDEN_SUFFIX);
   }
+
+  private static interface CustomType {
+    static class InnerType {}
+  }
+
+  @Test
+  public void missingImplementationWithHints_memoizesSuggestion() throws Exception {
+    Injector injector = Guice.createInjector();
+    ConfigurationException ex =
+        assertThrows(ConfigurationException.class, () -> injector.getInstance(CustomType.class));
+    // Ensure that the message doesn't contain a "Did you mean?" by default,
+    // because there's no other type that fits.
+    assertThat(ex).hasMessageThat().doesNotContain("Did you mean?");
+    // And even after we insert another type that fits, we don't redo the suggestions.
+    injector.getInstance(CustomType.InnerType.class);
+    assertThat(ex).hasMessageThat().doesNotContain("Did you mean?");
+  }
+
+  @Test
+  public void missingImplementationWithHints_lazyInjectorUsage() throws Exception {
+    // Note: this test is extremely contrived. This scenario is unlikely to happen for real, but
+    // it's a very convenient way to assert that usage of the injector is lazy.
+    // By adding a type into the injector after the exception is thrown but before we
+    // call getMessage, we're validating that the suggestions are populated only on getMessage
+    // usage.
+    // This test works in tandem with the above one which asserts that by default,
+    // the message *will not* have suggestions.
+    Injector injector = Guice.createInjector();
+    ConfigurationException ex =
+        assertThrows(ConfigurationException.class, () -> injector.getInstance(CustomType.class));
+    injector.getInstance(CustomType.InnerType.class);
+    assertThat(ex).hasMessageThat().containsMatch("Did you mean?");
+    assertThat(ex).hasMessageThat().containsMatch("InnerType");
+  }
 }

core/test/com/google/inject/errors/testdata/missing_implementation_with_hints.txt

@@ -3,7 +3,7 @@ Unable to create injector, see the following errors:
 1) [Guice/MissingImplementation]: No implementation for MissingImplementationErrorTest$Klass annotated with @MissingImplementationErrorTest$B() was bound.
 
 Did you mean?
-    MissingImplementationErrorTest$Klass annotated with @MissingImplementationErrorTest$A() bound at MissingImplementationErrorTest$HintsModule.provideKlass(MissingImplementationErrorTest.java:117)
+    * MissingImplementationErrorTest$Klass annotated with @MissingImplementationErrorTest$A() bound at MissingImplementationErrorTest$HintsModule.provideKlass(MissingImplementationErrorTest.java:117)
 
 Requested by:
 1  : MissingImplementationErrorTest$HintsModule.provideString(MissingImplementationErrorTest.java:123)
@@ -16,7 +16,7 @@ Learn more:
 2) [Guice/MissingImplementation]: No implementation for MissingImplementationErrorTest$Klass2 annotated with @MissingImplementationErrorTest$A() was bound.
 
 Did you mean?
-    MissingImplementationErrorTest$Klass annotated with @MissingImplementationErrorTest$A() bound at MissingImplementationErrorTest$HintsModule.provideKlass(MissingImplementationErrorTest.java:117)
+    * MissingImplementationErrorTest$Klass annotated with @MissingImplementationErrorTest$A() bound at MissingImplementationErrorTest$HintsModule.provideKlass(MissingImplementationErrorTest.java:117)
 
 Requested by:
 1  : MissingImplementationErrorTest$HintsModule.provideAString(MissingImplementationErrorTest.java:130)

core/test/com/google/inject/errors/testdata/missing_implementation_with_hints_with_dot_annots.txt

@@ -3,7 +3,7 @@ Unable to create injector, see the following errors:
 1) [Guice/MissingImplementation]: No implementation for MissingImplementationErrorTest$Klass annotated with @MissingImplementationErrorTest.B() was bound.
 
 Did you mean?
-    MissingImplementationErrorTest$Klass annotated with @MissingImplementationErrorTest.A() bound at MissingImplementationErrorTest$HintsModule.provideKlass(MissingImplementationErrorTest.java:149)
+    * MissingImplementationErrorTest$Klass annotated with @MissingImplementationErrorTest.A() bound at MissingImplementationErrorTest$HintsModule.provideKlass(MissingImplementationErrorTest.java:149)
 
 Requested by:
 1  : MissingImplementationErrorTest$HintsModule.provideString(MissingImplementationErrorTest.java:155)
@@ -16,7 +16,7 @@ Learn more:
 2) [Guice/MissingImplementation]: No implementation for MissingImplementationErrorTest$Klass2 annotated with @MissingImplementationErrorTest.A() was bound.
 
 Did you mean?
-    MissingImplementationErrorTest$Klass annotated with @MissingImplementationErrorTest.A() bound at MissingImplementationErrorTest$HintsModule.provideKlass(MissingImplementationErrorTest.java:149)
+    * MissingImplementationErrorTest$Klass annotated with @MissingImplementationErrorTest.A() bound at MissingImplementationErrorTest$HintsModule.provideKlass(MissingImplementationErrorTest.java:149)
 
 Requested by:
 1  : MissingImplementationErrorTest$HintsModule.provideAString(MissingImplementationErrorTest.java:162)

core/test/com/google/inject/internal/OptionalBinderTest.java

@@ -775,6 +775,7 @@ protected void configure() {
       assertContains(
           expected.getMessage(),
           "No implementation for Integer",
+          "Requested by:",
           getShortName(module) + ".configure");
     }
   }