Skip to content

Commit

Permalink
Add better Guice hints when using classes of the same name from diffe…
Browse files Browse the repository at this point in the history
…rent packages.

PiperOrigin-RevId: 696172862
  • Loading branch information
java-team-github-bot authored and Guice Team committed Nov 13, 2024
1 parent d83f677 commit 7b8cc46
Show file tree
Hide file tree
Showing 5 changed files with 226 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 <T> ImmutableList<String> getSuggestions(Key<T> key, Injector injector) {
ImmutableList.Builder<String> suggestions = ImmutableList.builder();
TypeLiteral<T> type = key.getTypeLiteral();
Expand All @@ -49,9 +116,11 @@ 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<T>> sameTypes =
injector.findBindingsByType(type).stream()
ImmutableList<Binding<?>> 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?");
Expand Down
1 change: 1 addition & 0 deletions core/test/com/google/inject/errors/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String> provideString() {
return Optional.of("ignored");
}

@Provides
Optional<Integer> provideInteger(com.google.common.base.Optional<String> 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");
}
}
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 base.Optional<String> was bound.

Did you mean?
* util.Optional<String> 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:
========================
103 changes: 103 additions & 0 deletions core/test/com/google/inject/internal/SimilarLookingTypesTest.java
Original file line number Diff line number Diff line change
@@ -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<List<com.google.common.base.Optional<String>>>() {}.getType(),
new TypeLiteral<List<Optional<String>>>() {}.getType()))
.isTrue();
}

@Test
public void multipleSimilarGenericTypes() {
assertThat(
areSimilarLookingTypes(
new TypeLiteral<Optional<com.google.common.base.Optional<String>>>() {}.getType(),
new TypeLiteral<com.google.common.base.Optional<Optional<String>>>() {}.getType()))
.isTrue();
}

@Test
public void differentGenericTypes() {
assertThat(
areSimilarLookingTypes(
new TypeLiteral<List<String>>() {}.getType(),
new TypeLiteral<List<Integer>>() {}.getType()))
.isFalse();
}

@Test
public void similarGenericArrayTypes() {
assertThat(
areSimilarLookingTypes(
new TypeLiteral<com.google.common.base.Optional<String>[]>() {}.getType(),
new TypeLiteral<Optional<String>[]>() {}.getType()))
.isTrue();
}

@Test
public void differentGenericArrayTypes() {
assertThat(
areSimilarLookingTypes(
new TypeLiteral<List<String>[]>() {}.getType(),
new TypeLiteral<List<String>[][]>() {}.getType()))
.isFalse();
}

@Test
public void similarWildcardTypes() {
assertThat(
areSimilarLookingTypes(
new TypeLiteral<com.google.common.base.Optional<?>>() {}.getType(),
new TypeLiteral<Optional<?>>() {}.getType()))
.isTrue();
}

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

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

0 comments on commit 7b8cc46

Please sign in to comment.