diff --git a/core/src/com/google/inject/internal/MissingImplementationError.java b/core/src/com/google/inject/internal/MissingImplementationError.java index b504b88c27..2e4ab9f379 100644 --- a/core/src/com/google/inject/internal/MissingImplementationError.java +++ b/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,6 +9,7 @@ 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. */ @@ -15,21 +17,26 @@ final class MissingImplementationError extends InternalErrorDetail> { private final Key key; - private final ImmutableList suggestions; + private final Supplier> suggestionsSupplier; public MissingImplementationError(Key key, Injector injector, List 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 key, ImmutableList suggestions, List sources) { + Key key, Supplier> suggestionsSupplier, List 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> mergeableErrors, Formatter formatter) { + ImmutableList suggestions = suggestionsSupplier.get(); if (!suggestions.isEmpty()) { suggestions.forEach(formatter::format); } @@ -65,7 +73,7 @@ public void formatDetail(List> mergeableErrors, Formatter formatt @Override public MissingImplementationError withSources(List newSources) { - return new MissingImplementationError(key, suggestions, newSources); + return new MissingImplementationError(key, suggestionsSupplier, newSources); } /** Omit the key itself in the source list since the information is redundant. */ diff --git a/core/src/com/google/inject/internal/MissingImplementationErrorHints.java b/core/src/com/google/inject/internal/MissingImplementationErrorHints.java index a48ed5561d..e0ca06d8ae 100644 --- a/core/src/com/google/inject/internal/MissingImplementationErrorHints.java +++ b/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 ImmutableList getSuggestions(Key key, Injector injector) { // Keys which have similar strings as the desired key List possibleMatches = new ArrayList<>(); - List> sameTypes = injector.findBindingsByType(type); + ImmutableList> 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 ImmutableList getSuggestions(Key key, Injector injector) { String want = type.toString(); Map, 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 ImmutableList getSuggestions(Key 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 ImmutableList getSuggestions(Key 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(); + } } diff --git a/core/test/com/google/inject/errors/MissingImplementationErrorTest.java b/core/test/com/google/inject/errors/MissingImplementationErrorTest.java index 0eaae65613..1cfb8d1288 100644 --- a/core/test/com/google/inject/errors/MissingImplementationErrorTest.java +++ b/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"); + } } diff --git a/core/test/com/google/inject/errors/testdata/missing_implementation_with_hints.txt b/core/test/com/google/inject/errors/testdata/missing_implementation_with_hints.txt index 3e8d90c814..9d133a8e61 100644 --- a/core/test/com/google/inject/errors/testdata/missing_implementation_with_hints.txt +++ b/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) diff --git a/core/test/com/google/inject/errors/testdata/missing_implementation_with_hints_with_dot_annots.txt b/core/test/com/google/inject/errors/testdata/missing_implementation_with_hints_with_dot_annots.txt index 4ef77d087a..7f8d4fa594 100644 --- a/core/test/com/google/inject/errors/testdata/missing_implementation_with_hints_with_dot_annots.txt +++ b/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) diff --git a/core/test/com/google/inject/internal/OptionalBinderTest.java b/core/test/com/google/inject/internal/OptionalBinderTest.java index aef57e5e44..49f616ef4f 100644 --- a/core/test/com/google/inject/internal/OptionalBinderTest.java +++ b/core/test/com/google/inject/internal/OptionalBinderTest.java @@ -809,6 +809,7 @@ protected void configure() { assertContains( expected.getMessage(), "No implementation for Integer", + "Requested by:", getShortName(module) + ".configure"); } }