From 7b8cc467b8a65a321d6f5b5de87ef4e77944f9dd Mon Sep 17 00:00:00 2001 From: Guice Team Date: Wed, 13 Nov 2024 09:55:23 -0800 Subject: [PATCH] Add better Guice hints when using classes of the same name from different packages. PiperOrigin-RevId: 696172862 --- .../MissingImplementationErrorHints.java | 73 ++++++++++++- core/test/com/google/inject/errors/BUILD | 1 + .../MissingImplementationErrorTest.java | 25 +++++ ...plementation_with_mismatched_optionals.txt | 26 +++++ .../internal/SimilarLookingTypesTest.java | 103 ++++++++++++++++++ 5 files changed, 226 insertions(+), 2 deletions(-) create mode 100644 core/test/com/google/inject/errors/testdata/missing_implementation_with_mismatched_optionals.txt create mode 100644 core/test/com/google/inject/internal/SimilarLookingTypesTest.java diff --git a/core/src/com/google/inject/internal/MissingImplementationErrorHints.java b/core/src/com/google/inject/internal/MissingImplementationErrorHints.java index e0ca06d8ae..2a70475b5d 100644 --- a/core/src/com/google/inject/internal/MissingImplementationErrorHints.java +++ b/core/src/com/google/inject/internal/MissingImplementationErrorHints.java @@ -12,6 +12,10 @@ import com.google.inject.TypeLiteral; import com.google.inject.spi.BindingSourceRestriction; import com.google.inject.spi.UntargettedBinding; +import java.lang.reflect.GenericArrayType; +import java.lang.reflect.ParameterizedType; +import java.lang.reflect.Type; +import java.lang.reflect.WildcardType; import java.util.ArrayList; import java.util.Formatter; import java.util.List; @@ -40,6 +44,69 @@ private MissingImplementationErrorHints() {} .addAll(Primitives.allWrapperTypes()) .build(); + /** + * Returns whether two types look similar (i.e. if you were to ignore their package). This helps + * users who, for example, have injected the wrong Optional (java.util.Optional vs + * com.google.common.base.Optional). For generic types, the entire structure must match in + * addition to the simple names of the generic arguments (e.g. Optional<String> won't be + * similar to Optional<Integer>). + */ + static boolean areSimilarLookingTypes(Type a, Type b) { + if (a instanceof Class && b instanceof Class) { + return ((Class) a).getSimpleName().equals(((Class) b).getSimpleName()); + } + if (a instanceof ParameterizedType && b instanceof ParameterizedType) { + ParameterizedType aType = (ParameterizedType) a; + ParameterizedType bType = (ParameterizedType) b; + if (!areSimilarLookingTypes(aType.getRawType(), bType.getRawType())) { + return false; + } + Type[] aArgs = aType.getActualTypeArguments(); + Type[] bArgs = bType.getActualTypeArguments(); + if (aArgs.length != bArgs.length) { + return false; + } + for (int i = 0; i < aArgs.length; i++) { + if (!areSimilarLookingTypes(aArgs[i], bArgs[i])) { + return false; + } + } + return true; + } + if (a instanceof GenericArrayType && b instanceof GenericArrayType) { + GenericArrayType aType = (GenericArrayType) a; + GenericArrayType bType = (GenericArrayType) b; + return areSimilarLookingTypes( + aType.getGenericComponentType(), bType.getGenericComponentType()); + } + if (a instanceof WildcardType && b instanceof WildcardType) { + WildcardType aType = (WildcardType) a; + WildcardType bType = (WildcardType) b; + Type[] aLowerBounds = aType.getLowerBounds(); + Type[] bLowerBounds = bType.getLowerBounds(); + if (aLowerBounds.length != bLowerBounds.length) { + return false; + } + for (int i = 0; i < aLowerBounds.length; i++) { + if (!areSimilarLookingTypes(aLowerBounds[i], bLowerBounds[i])) { + return false; + } + } + Type[] aUpperBounds = aType.getUpperBounds(); + Type[] bUpperBounds = bType.getUpperBounds(); + if (aUpperBounds.length != bUpperBounds.length) { + return false; + } + for (int i = 0; i < aUpperBounds.length; i++) { + if (!areSimilarLookingTypes(aUpperBounds[i], bUpperBounds[i])) { + return false; + } + } + return true; + } + return false; + } + static ImmutableList getSuggestions(Key key, Injector injector) { ImmutableList.Builder suggestions = ImmutableList.builder(); TypeLiteral type = key.getTypeLiteral(); @@ -49,9 +116,11 @@ static ImmutableList getSuggestions(Key key, Injector injector) { // Keys which have similar strings as the desired key List possibleMatches = new ArrayList<>(); - ImmutableList> sameTypes = - injector.findBindingsByType(type).stream() + ImmutableList> sameTypes = + injector.getAllBindings().values().stream() .filter(b -> !(b instanceof UntargettedBinding)) // These aren't valid matches + .filter( + b -> areSimilarLookingTypes(b.getKey().getTypeLiteral().getType(), type.getType())) .collect(toImmutableList()); if (!sameTypes.isEmpty()) { suggestions.add("\nDid you mean?"); diff --git a/core/test/com/google/inject/errors/BUILD b/core/test/com/google/inject/errors/BUILD index 11d2414c6d..eae233b1a9 100644 --- a/core/test/com/google/inject/errors/BUILD +++ b/core/test/com/google/inject/errors/BUILD @@ -15,6 +15,7 @@ java_library( deps = [ "//core/src/com/google/inject", "//third_party/java/guava/annotations", + "//third_party/java/guava/base", "//third_party/java/guava/collect", "//third_party/java/guava/io", "//third_party/java/jakarta_inject", diff --git a/core/test/com/google/inject/errors/MissingImplementationErrorTest.java b/core/test/com/google/inject/errors/MissingImplementationErrorTest.java index e3ef0e809f..cff1edbe2a 100644 --- a/core/test/com/google/inject/errors/MissingImplementationErrorTest.java +++ b/core/test/com/google/inject/errors/MissingImplementationErrorTest.java @@ -17,6 +17,7 @@ import com.google.inject.internal.InternalFlags.IncludeStackTraceOption; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; +import java.util.Optional; import jakarta.inject.Provider; import jakarta.inject.Qualifier; import org.junit.Before; @@ -207,4 +208,28 @@ public void missingImplementationWithHints_lazyInjectorUsage() throws Exception assertThat(ex).hasMessageThat().containsMatch("Did you mean?"); assertThat(ex).hasMessageThat().containsMatch("InnerType"); } + + private static final class MismatchedOptionalsModule extends AbstractModule { + @Override + protected void configure() {} + + @Provides + Optional provideString() { + return Optional.of("ignored"); + } + + @Provides + Optional provideInteger(com.google.common.base.Optional dep) { + return Optional.of(123); + } + } + + @Test + public void testMismatchedOptionals() { + CreationException exception = + assertThrows( + CreationException.class, () -> Guice.createInjector(new MismatchedOptionalsModule())); + assertGuiceErrorEqualsIgnoreLineNumber( + exception.getMessage(), "missing_implementation_with_mismatched_optionals.txt"); + } } diff --git a/core/test/com/google/inject/errors/testdata/missing_implementation_with_mismatched_optionals.txt b/core/test/com/google/inject/errors/testdata/missing_implementation_with_mismatched_optionals.txt new file mode 100644 index 0000000000..8e69cf5d77 --- /dev/null +++ b/core/test/com/google/inject/errors/testdata/missing_implementation_with_mismatched_optionals.txt @@ -0,0 +1,26 @@ +Unable to create injector, see the following errors: + +1) [Guice/MissingImplementation]: No implementation for base.Optional was bound. + +Did you mean? + * util.Optional bound at MissingImplementationErrorTest$MismatchedOptionalsModule.provideString(MissingImplementationErrorTest.java:218) + +Requested by: +1 : MissingImplementationErrorTest$MismatchedOptionalsModule.provideInteger(MissingImplementationErrorTest.java:223) + \_ for 1st parameter dep + at MissingImplementationErrorTest$MismatchedOptionalsModule.provideInteger(MissingImplementationErrorTest.java:223) + +Learn more: + https://github.com/google/guice/wiki/MISSING_IMPLEMENTATION + +1 error + +====================== +Full classname legend: +====================== +MissingImplementationErrorTest$MismatchedOptionalsModule: "com.google.inject.errors.MissingImplementationErrorTest$MismatchedOptionalsModule" +base.Optional: "com.google.common.base.Optional" +util.Optional: "java.util.Optional" +======================== +End of classname legend: +======================== diff --git a/core/test/com/google/inject/internal/SimilarLookingTypesTest.java b/core/test/com/google/inject/internal/SimilarLookingTypesTest.java new file mode 100644 index 0000000000..412d2647ed --- /dev/null +++ b/core/test/com/google/inject/internal/SimilarLookingTypesTest.java @@ -0,0 +1,103 @@ +package com.google.inject.internal; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.inject.internal.MissingImplementationErrorHints.areSimilarLookingTypes; + +import com.google.inject.TypeLiteral; +import java.util.List; +import java.util.Optional; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public final class SimilarLookingTypesTest { + + private static class Wrapper { + private static class Foo {} + } + + private static class Foo {} + + @Test + public void similarClasses() { + assertThat(areSimilarLookingTypes(Foo.class, Wrapper.Foo.class)).isTrue(); + } + + @Test + public void differentClasses() { + assertThat(areSimilarLookingTypes(String.class, Integer.class)).isFalse(); + } + + @Test + public void similarGenericTypes() { + assertThat( + areSimilarLookingTypes( + new TypeLiteral>>() {}.getType(), + new TypeLiteral>>() {}.getType())) + .isTrue(); + } + + @Test + public void multipleSimilarGenericTypes() { + assertThat( + areSimilarLookingTypes( + new TypeLiteral>>() {}.getType(), + new TypeLiteral>>() {}.getType())) + .isTrue(); + } + + @Test + public void differentGenericTypes() { + assertThat( + areSimilarLookingTypes( + new TypeLiteral>() {}.getType(), + new TypeLiteral>() {}.getType())) + .isFalse(); + } + + @Test + public void similarGenericArrayTypes() { + assertThat( + areSimilarLookingTypes( + new TypeLiteral[]>() {}.getType(), + new TypeLiteral[]>() {}.getType())) + .isTrue(); + } + + @Test + public void differentGenericArrayTypes() { + assertThat( + areSimilarLookingTypes( + new TypeLiteral[]>() {}.getType(), + new TypeLiteral[][]>() {}.getType())) + .isFalse(); + } + + @Test + public void similarWildcardTypes() { + assertThat( + areSimilarLookingTypes( + new TypeLiteral>() {}.getType(), + new TypeLiteral>() {}.getType())) + .isTrue(); + } + + @Test + public void differentWildcardType_extends() { + assertThat( + areSimilarLookingTypes( + new TypeLiteral>() {}.getType(), + new TypeLiteral>() {}.getType())) + .isFalse(); + } + + @Test + public void differentWildcardType_super() { + assertThat( + areSimilarLookingTypes( + new TypeLiteral>() {}.getType(), + new TypeLiteral>() {}.getType())) + .isFalse(); + } +}