Skip to content

Commit

Permalink
Add Guice hints when the only difference between two types is when on…
Browse files Browse the repository at this point in the history
…e 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<Foo> vs. Optional<? extends Foo>).

PiperOrigin-RevId: 696949702
  • Loading branch information
java-team-github-bot authored and Guice Team committed Nov 15, 2024
1 parent 7b8cc46 commit e032518
Show file tree
Hide file tree
Showing 7 changed files with 184 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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&lt;String&gt; won't be
* similar to Optional&lt;Integer&gt;).
* 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&lt;String&gt; won't be similar to Optional&lt;Integer&gt;).
*/
static boolean areSimilarLookingTypes(Type a, Type b) {
if (a instanceof Class && b instanceof Class) {
Expand Down Expand Up @@ -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;
}

Expand All @@ -116,25 +134,25 @@ static <T> ImmutableList<String> getSuggestions(Key<T> key, Injector injector) {

// Keys which have similar strings as the desired key
List<String> possibleMatches = new ArrayList<>();
ImmutableList<Binding<?>> sameTypes =
ImmutableList<Binding<?>> 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(
Expand Down Expand Up @@ -178,7 +196,7 @@ static <T> ImmutableList<String> getSuggestions(Key<T> 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())) {
Expand Down
18 changes: 16 additions & 2 deletions core/test/com/google/inject/errors/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
Unable to create injector, see the following errors:

1) [Guice/MissingImplementation]: No implementation for MissingImplementationErrorKtTest$Producer<? extends MissingImplementationErrorKtTest$Foo> was bound.

Did you mean?
* MissingImplementationErrorKtTest$Producer<MissingImplementationErrorKtTest$Foo> 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:
========================
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
Unable to create injector, see the following errors:

1) [Guice/MissingImplementation]: No implementation for MissingImplementationErrorKtTest$Consumer<? super MissingImplementationErrorKtTest$Foo> was bound.

Did you mean?
* MissingImplementationErrorKtTest$Consumer<MissingImplementationErrorKtTest$Foo> 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:
========================
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
Unable to create injector, see the following errors:

1) [Guice/MissingImplementation]: No implementation for MissingImplementationErrorKtTest$Producer<MissingImplementationErrorKtTest$Foo> was bound.

Did you mean?
* MissingImplementationErrorKtTest$Producer<? extends MissingImplementationErrorKtTest$Foo> 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:
========================
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
Unable to create injector, see the following errors:

1) [Guice/MissingImplementation]: No implementation for MissingImplementationErrorKtTest$Consumer<MissingImplementationErrorKtTest$Foo> was bound.

Did you mean?
* MissingImplementationErrorKtTest$Consumer<? super MissingImplementationErrorKtTest$Foo> 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:
========================
36 changes: 36 additions & 0 deletions core/test/com/google/inject/internal/SimilarLookingTypesTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,42 @@ public void similarWildcardTypes() {
.isTrue();
}

@Test
public void similarExtendsCase() {
assertThat(
areSimilarLookingTypes(
new TypeLiteral<Optional<Foo>>() {}.getType(),
new TypeLiteral<Optional<? extends Foo>>() {}.getType()))
.isTrue();
}

@Test
public void differentExtendsCase() {
assertThat(
areSimilarLookingTypes(
new TypeLiteral<Optional<String>>() {}.getType(),
new TypeLiteral<Optional<? extends Foo>>() {}.getType()))
.isFalse();
}

@Test
public void similarSuperCase() {
assertThat(
areSimilarLookingTypes(
new TypeLiteral<Optional<Foo>>() {}.getType(),
new TypeLiteral<Optional<? super Foo>>() {}.getType()))
.isTrue();
}

@Test
public void differentSuperCase() {
assertThat(
areSimilarLookingTypes(
new TypeLiteral<Optional<String>>() {}.getType(),
new TypeLiteral<Optional<? super Foo>>() {}.getType()))
.isFalse();
}

@Test
public void differentWildcardType_extends() {
assertThat(
Expand Down

0 comments on commit e032518

Please sign in to comment.