From e03251852d9616017bb831508adcb7921e7acb80 Mon Sep 17 00:00:00 2001 From: Guice Team Date: Fri, 15 Nov 2024 11:29:28 -0800 Subject: [PATCH] Add Guice hints when the only difference between two types is when one is a wildcard type and the other is a non-wildcard type that is mentioned in the extends/super clause of the wildcard type (e.g. Optional vs. Optional). PiperOrigin-RevId: 696949702 --- .../MissingImplementationErrorHints.java | 38 ++++++++++++++----- core/test/com/google/inject/errors/BUILD | 18 ++++++++- ...ntation_has_unnecessary_extends_clause.txt | 26 +++++++++++++ ...mentation_has_unnecessary_super_clause.txt | 26 +++++++++++++ ..._implementation_missing_extends_clause.txt | 26 +++++++++++++ ...ng_implementation_missing_super_clause.txt | 26 +++++++++++++ .../internal/SimilarLookingTypesTest.java | 36 ++++++++++++++++++ 7 files changed, 184 insertions(+), 12 deletions(-) create mode 100644 core/test/com/google/inject/errors/testdata/missing_implementation_has_unnecessary_extends_clause.txt create mode 100644 core/test/com/google/inject/errors/testdata/missing_implementation_has_unnecessary_super_clause.txt create mode 100644 core/test/com/google/inject/errors/testdata/missing_implementation_missing_extends_clause.txt create mode 100644 core/test/com/google/inject/errors/testdata/missing_implementation_missing_super_clause.txt diff --git a/core/src/com/google/inject/internal/MissingImplementationErrorHints.java b/core/src/com/google/inject/internal/MissingImplementationErrorHints.java index 2a70475b5d..20ddb265db 100644 --- a/core/src/com/google/inject/internal/MissingImplementationErrorHints.java +++ b/core/src/com/google/inject/internal/MissingImplementationErrorHints.java @@ -47,9 +47,9 @@ private MissingImplementationErrorHints() {} /** * 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>). + * com.google.common.base.Optional). For generic types, the entire structure must mostly match + * (wildcard types of extends and super can be ignored) 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) { @@ -104,6 +104,24 @@ static boolean areSimilarLookingTypes(Type a, Type b) { } return true; } + // The next section handles when one type is a wildcard type and the other is not (e.g. to + // catch cases of `Foo` vs `? extends/super Foo`). + if (a instanceof WildcardType ^ b instanceof WildcardType) { + WildcardType wildcardType = a instanceof WildcardType ? (WildcardType) a : (WildcardType) b; + Type otherType = (wildcardType == a) ? b : a; + Type[] upperBounds = wildcardType.getUpperBounds(); + Type[] lowerBounds = wildcardType.getLowerBounds(); + if (upperBounds.length == 1 && lowerBounds.length == 0) { + // This is the '? extends Foo' case + return areSimilarLookingTypes(upperBounds[0], otherType); + } + if (lowerBounds.length == 1 + && upperBounds.length == 1 + && upperBounds[0].equals(Object.class)) { + // this is the '? super Foo' case + return areSimilarLookingTypes(lowerBounds[0], otherType); + } + } return false; } @@ -116,25 +134,25 @@ static ImmutableList getSuggestions(Key key, Injector injector) { // Keys which have similar strings as the desired key List possibleMatches = new ArrayList<>(); - ImmutableList> sameTypes = + ImmutableList> similarTypes = 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()) { + if (!similarTypes.isEmpty()) { suggestions.add("\nDid you mean?"); - int howMany = min(sameTypes.size(), MAX_MATCHING_TYPES_REPORTED); + int howMany = min(similarTypes.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 + Key bindingKey = similarTypes.get(i).getKey(); + // TODO: Look into a better way to prioritize suggestions. For example, possibly // use levenshtein distance of the given annotation vs actual annotation. suggestions.add( Messages.format( "\n * %s", formatSuggestion(bindingKey, injector.getExistingBinding(bindingKey)))); } - int remaining = sameTypes.size() - MAX_MATCHING_TYPES_REPORTED; + int remaining = similarTypes.size() - MAX_MATCHING_TYPES_REPORTED; if (remaining > 0) { String plural = (remaining == 1) ? "" : "s"; suggestions.add( @@ -178,7 +196,7 @@ static ImmutableList getSuggestions(Key key, Injector injector) { // If where are no possibilities to suggest, then handle the case of missing // annotations on simple types. This is usually a bad idea. - if (sameTypes.isEmpty() + if (similarTypes.isEmpty() && possibleMatches.isEmpty() && key.getAnnotationType() == null && COMMON_AMBIGUOUS_TYPES.contains(key.getTypeLiteral().getRawType())) { diff --git a/core/test/com/google/inject/errors/BUILD b/core/test/com/google/inject/errors/BUILD index eae233b1a9..c0fe9c6696 100644 --- a/core/test/com/google/inject/errors/BUILD +++ b/core/test/com/google/inject/errors/BUILD @@ -5,19 +5,33 @@ package( default_testonly = 1, ) +ERROR_MESSAGE_TEST_UTILS_SRCS = ["ErrorMessageTestUtils.java"] + +java_library( + name = "error_message_test_utils", + srcs = ERROR_MESSAGE_TEST_UTILS_SRCS, + deps = [ + "//third_party/java/guava/io", + "//third_party/java/truth", + ], +) + java_library( name = "tests", - srcs = glob(["*.java"]), + srcs = glob( + ["*.java"], + exclude = ERROR_MESSAGE_TEST_UTILS_SRCS, + ), javacopts = ["-Xep:BetaApi:OFF"], resources = [ ":test_error_files", ], deps = [ + ":error_message_test_utils", "//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", "//third_party/java/junit", "//third_party/java/truth", diff --git a/core/test/com/google/inject/errors/testdata/missing_implementation_has_unnecessary_extends_clause.txt b/core/test/com/google/inject/errors/testdata/missing_implementation_has_unnecessary_extends_clause.txt new file mode 100644 index 0000000000..c9007f7501 --- /dev/null +++ b/core/test/com/google/inject/errors/testdata/missing_implementation_has_unnecessary_extends_clause.txt @@ -0,0 +1,26 @@ +Unable to create injector, see the following errors: + +1) [Guice/MissingImplementation]: No implementation for MissingImplementationErrorKtTest$Producer was bound. + +Did you mean? + * MissingImplementationErrorKtTest$Producer bound at MissingImplementationErrorKtTest$InjectionHasUnnecessaryExtendsClauseModule.provideProducerOfFoo(MissingImplementationErrorKtTest.kt:53) + +Requested by: +1 : MissingImplementationErrorKtTest$InjectionHasUnnecessaryExtendsClauseModule.injectProducerOfFoo(MissingImplementationErrorKtTest.kt:58) + \_ for 1st parameter unused + at MissingImplementationErrorKtTest$InjectionHasUnnecessaryExtendsClauseModule.injectProducerOfFoo(MissingImplementationErrorKtTest.kt:58) + +Learn more: + https://github.com/google/guice/wiki/MISSING_IMPLEMENTATION + +1 error + +====================== +Full classname legend: +====================== +MissingImplementationErrorKtTest$Foo: "com.google.inject.errors.MissingImplementationErrorKtTest$Foo" +MissingImplementationErrorKtTest$InjectionHasUnnecessaryExtendsClauseModule: "com.google.inject.errors.MissingImplementationErrorKtTest$InjectionHasUnnecessaryExtendsClauseModule" +MissingImplementationErrorKtTest$Producer: "com.google.inject.errors.MissingImplementationErrorKtTest$Producer" +======================== +End of classname legend: +======================== diff --git a/core/test/com/google/inject/errors/testdata/missing_implementation_has_unnecessary_super_clause.txt b/core/test/com/google/inject/errors/testdata/missing_implementation_has_unnecessary_super_clause.txt new file mode 100644 index 0000000000..1dd3903e9d --- /dev/null +++ b/core/test/com/google/inject/errors/testdata/missing_implementation_has_unnecessary_super_clause.txt @@ -0,0 +1,26 @@ +Unable to create injector, see the following errors: + +1) [Guice/MissingImplementation]: No implementation for MissingImplementationErrorKtTest$Consumer was bound. + +Did you mean? + * MissingImplementationErrorKtTest$Consumer bound at MissingImplementationErrorKtTest$InjectionHasUnnecessarySuperClauseModule.provideConsumerOfFoo(MissingImplementationErrorKtTest.kt:98) + +Requested by: +1 : MissingImplementationErrorKtTest$InjectionHasUnnecessarySuperClauseModule.injectConsumerOfFoo(MissingImplementationErrorKtTest.kt:103) + \_ for 1st parameter unused + at MissingImplementationErrorKtTest$InjectionHasUnnecessarySuperClauseModule.injectConsumerOfFoo(MissingImplementationErrorKtTest.kt:103) + +Learn more: + https://github.com/google/guice/wiki/MISSING_IMPLEMENTATION + +1 error + +====================== +Full classname legend: +====================== +MissingImplementationErrorKtTest$Consumer: "com.google.inject.errors.MissingImplementationErrorKtTest$Consumer" +MissingImplementationErrorKtTest$Foo: "com.google.inject.errors.MissingImplementationErrorKtTest$Foo" +MissingImplementationErrorKtTest$InjectionHasUnnecessarySuperClauseModule: "com.google.inject.errors.MissingImplementationErrorKtTest$InjectionHasUnnecessarySuperClauseModule" +======================== +End of classname legend: +======================== diff --git a/core/test/com/google/inject/errors/testdata/missing_implementation_missing_extends_clause.txt b/core/test/com/google/inject/errors/testdata/missing_implementation_missing_extends_clause.txt new file mode 100644 index 0000000000..19539aadf2 --- /dev/null +++ b/core/test/com/google/inject/errors/testdata/missing_implementation_missing_extends_clause.txt @@ -0,0 +1,26 @@ +Unable to create injector, see the following errors: + +1) [Guice/MissingImplementation]: No implementation for MissingImplementationErrorKtTest$Producer was bound. + +Did you mean? + * MissingImplementationErrorKtTest$Producer bound at MissingImplementationErrorKtTest$InjectionMissingExtendsClauseModule.configure(MissingImplementationErrorKtTest.kt:118) + +Requested by: +1 : MissingImplementationErrorKtTest$InjectionMissingExtendsClauseModule.injectProducerOfFoo(MissingImplementationErrorKtTest.kt:36) + \_ for 1st parameter unused + at MissingImplementationErrorKtTest$InjectionMissingExtendsClauseModule.injectProducerOfFoo(MissingImplementationErrorKtTest.kt:36) + +Learn more: + https://github.com/google/guice/wiki/MISSING_IMPLEMENTATION + +1 error + +====================== +Full classname legend: +====================== +MissingImplementationErrorKtTest$Foo: "com.google.inject.errors.MissingImplementationErrorKtTest$Foo" +MissingImplementationErrorKtTest$InjectionMissingExtendsClauseModule: "com.google.inject.errors.MissingImplementationErrorKtTest$InjectionMissingExtendsClauseModule" +MissingImplementationErrorKtTest$Producer: "com.google.inject.errors.MissingImplementationErrorKtTest$Producer" +======================== +End of classname legend: +======================== diff --git a/core/test/com/google/inject/errors/testdata/missing_implementation_missing_super_clause.txt b/core/test/com/google/inject/errors/testdata/missing_implementation_missing_super_clause.txt new file mode 100644 index 0000000000..1eac3fde57 --- /dev/null +++ b/core/test/com/google/inject/errors/testdata/missing_implementation_missing_super_clause.txt @@ -0,0 +1,26 @@ +Unable to create injector, see the following errors: + +1) [Guice/MissingImplementation]: No implementation for MissingImplementationErrorKtTest$Consumer was bound. + +Did you mean? + * MissingImplementationErrorKtTest$Consumer bound at MissingImplementationErrorKtTest$InjectionMissingSuperClauseModule.configure(MissingImplementationErrorKtTest.kt:118) + +Requested by: +1 : MissingImplementationErrorKtTest$InjectionMissingSuperClauseModule.injectConsumerOfFoo(MissingImplementationErrorKtTest.kt:81) + \_ for 1st parameter unused + at MissingImplementationErrorKtTest$InjectionMissingSuperClauseModule.injectConsumerOfFoo(MissingImplementationErrorKtTest.kt:81) + +Learn more: + https://github.com/google/guice/wiki/MISSING_IMPLEMENTATION + +1 error + +====================== +Full classname legend: +====================== +MissingImplementationErrorKtTest$Consumer: "com.google.inject.errors.MissingImplementationErrorKtTest$Consumer" +MissingImplementationErrorKtTest$Foo: "com.google.inject.errors.MissingImplementationErrorKtTest$Foo" +MissingImplementationErrorKtTest$InjectionMissingSuperClauseModule: "com.google.inject.errors.MissingImplementationErrorKtTest$InjectionMissingSuperClauseModule" +======================== +End of classname legend: +======================== diff --git a/core/test/com/google/inject/internal/SimilarLookingTypesTest.java b/core/test/com/google/inject/internal/SimilarLookingTypesTest.java index 412d2647ed..0de8150528 100644 --- a/core/test/com/google/inject/internal/SimilarLookingTypesTest.java +++ b/core/test/com/google/inject/internal/SimilarLookingTypesTest.java @@ -83,6 +83,42 @@ public void similarWildcardTypes() { .isTrue(); } + @Test + public void similarExtendsCase() { + assertThat( + areSimilarLookingTypes( + new TypeLiteral>() {}.getType(), + new TypeLiteral>() {}.getType())) + .isTrue(); + } + + @Test + public void differentExtendsCase() { + assertThat( + areSimilarLookingTypes( + new TypeLiteral>() {}.getType(), + new TypeLiteral>() {}.getType())) + .isFalse(); + } + + @Test + public void similarSuperCase() { + assertThat( + areSimilarLookingTypes( + new TypeLiteral>() {}.getType(), + new TypeLiteral>() {}.getType())) + .isTrue(); + } + + @Test + public void differentSuperCase() { + assertThat( + areSimilarLookingTypes( + new TypeLiteral>() {}.getType(), + new TypeLiteral>() {}.getType())) + .isFalse(); + } + @Test public void differentWildcardType_extends() { assertThat(