From 2c4a870c40add6f5ee9ff83eb02f7f0777ee3291 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Sat, 22 Jun 2019 11:33:53 +0200 Subject: [PATCH 1/2] Allow nested generic types Dont allow type variables in keys. --- .../com/example/JustInTimeGenericNested.java | 38 +++++++++++++++++++ .../java/com/example/IntegrationTest.java | 6 +++ reflect/src/main/java/dagger/reflect/Key.java | 20 ++++++++++ .../main/java/dagger/reflect/TypeUtil.java | 4 +- .../reflect/UnlinkedJustInTimeBinding.java | 7 +++- 5 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 integration-tests/src/main/java/com/example/JustInTimeGenericNested.java diff --git a/integration-tests/src/main/java/com/example/JustInTimeGenericNested.java b/integration-tests/src/main/java/com/example/JustInTimeGenericNested.java new file mode 100644 index 00000000..f55fb7ea --- /dev/null +++ b/integration-tests/src/main/java/com/example/JustInTimeGenericNested.java @@ -0,0 +1,38 @@ +package com.example; + +import dagger.Component; +import dagger.Module; +import dagger.Provides; +import javax.inject.Inject; +import javax.inject.Provider; + +@Component(modules = JustInTimeGenericNested.Module1.class) +interface JustInTimeGenericNested { + Registry thing(); + + @Module + abstract class Module1 { + @Provides + static String provideString() { + return "foo"; + } + } + + final class Registry { + + @Inject + Registry(JobProvider jobProvider) {} + } + + final class JobProvider { + + @Inject + public JobProvider(Provider> jobProvider) {} + } + + final class Job { + + @Inject + public Job(Provider provider) {} + } +} diff --git a/integration-tests/src/test/java/com/example/IntegrationTest.java b/integration-tests/src/test/java/com/example/IntegrationTest.java index a2c69599..99312356 100644 --- a/integration-tests/src/test/java/com/example/IntegrationTest.java +++ b/integration-tests/src/test/java/com/example/IntegrationTest.java @@ -230,6 +230,12 @@ public void justInTimeGeneric() { assertThat(component.thing()).isNotNull(); } + @Test + public void justInTimeGenericNested() { + JustInTimeGenericNested component = backend.create(JustInTimeGenericNested.class); + assertThat(component.thing()).isNotNull(); + } + @Test public void justInTimeMembersInjection() { JustInTimeMembersInjection component = backend.create(JustInTimeMembersInjection.class); diff --git a/reflect/src/main/java/dagger/reflect/Key.java b/reflect/src/main/java/dagger/reflect/Key.java index d3b6611e..ebc384d2 100644 --- a/reflect/src/main/java/dagger/reflect/Key.java +++ b/reflect/src/main/java/dagger/reflect/Key.java @@ -20,12 +20,17 @@ import com.google.auto.value.AutoValue; import java.lang.annotation.Annotation; +import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; +import java.lang.reflect.TypeVariable; import org.jetbrains.annotations.Nullable; @AutoValue abstract class Key { static Key of(@Nullable Annotation qualifier, Type type) { + if (containsTypeVariable(type)) { + throw new IllegalArgumentException("cannot contain type variable " + type); + } return new AutoValue_Key(qualifier, canonicalize(boxIfNecessary(type))); } @@ -33,6 +38,21 @@ static Key of(@Nullable Annotation qualifier, Type type) { abstract Type type(); + private static boolean containsTypeVariable(Type type) { + if (type instanceof TypeVariable) { + return true; + } + if (type instanceof ParameterizedType) { + Type[] actualTypeArguments = ((ParameterizedType) type).getActualTypeArguments(); + for (Type arg : actualTypeArguments) { + if (containsTypeVariable(arg)) { + return true; + } + } + } + return false; + } + @Override public final String toString() { Annotation qualifier = qualifier(); diff --git a/reflect/src/main/java/dagger/reflect/TypeUtil.java b/reflect/src/main/java/dagger/reflect/TypeUtil.java index 36e98c10..d697ea4d 100644 --- a/reflect/src/main/java/dagger/reflect/TypeUtil.java +++ b/reflect/src/main/java/dagger/reflect/TypeUtil.java @@ -83,7 +83,9 @@ static final class ParameterizedTypeImpl implements ParameterizedType { "unexpected owner type for " + rawType + ": " + ownerType); } } else if (enclosingClass != null) { - throw new IllegalArgumentException("unexpected owner type for " + rawType + ": null"); + // TODO ? + // throw new IllegalArgumentException("unexpected owner type for " + rawType + ": + // null"); } } diff --git a/reflect/src/main/java/dagger/reflect/UnlinkedJustInTimeBinding.java b/reflect/src/main/java/dagger/reflect/UnlinkedJustInTimeBinding.java index 9601e043..61d73aa1 100644 --- a/reflect/src/main/java/dagger/reflect/UnlinkedJustInTimeBinding.java +++ b/reflect/src/main/java/dagger/reflect/UnlinkedJustInTimeBinding.java @@ -65,6 +65,9 @@ private static boolean hasParameterizedTypeVariable(Type parameterType) { if (isTypeVariable(type)) { return true; } + if (hasParameterizedTypeVariable(type)) { + return true; + } } return false; } @@ -78,7 +81,7 @@ private TypeUtil.ParameterizedTypeImpl findKeyForParameterizedType( /** * Find matching concrete types for a list of types. For every TypeVariable like `T` in the array * arg, we lookup the matching type in this class's concrete type arguments. If it is already a - * concrete type, just return the type. Creates a new array to match parameterizd type. + * concrete type, just return the type. Creates a new array to match parameterized type. * * @param typeArguments The Types and TypeVariables to find matching concrete types for. * @return The matching concrete type for the placeholder. @@ -88,6 +91,8 @@ private Type[] matchingParameterizedType(Type[] typeArguments) { for (int i = 0; i < typeArguments.length; i++) { if (isTypeVariable(typeArguments[i])) { matchedTypeArguments[i] = matchTypeToConcreteType((TypeVariable) typeArguments[i]); + } else if (typeArguments[i] instanceof ParameterizedType) { + matchedTypeArguments[i] = findKeyForParameterizedType((ParameterizedType) typeArguments[i]); } else { matchedTypeArguments[i] = typeArguments[i]; } From c1d299deb5bba543c4fbe31796a92b7b20b98aed Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Fri, 5 Jul 2019 10:59:06 +0200 Subject: [PATCH 2/2] Add back exception. Pass correct owner type. --- reflect/src/main/java/dagger/reflect/TypeUtil.java | 4 +--- .../main/java/dagger/reflect/UnlinkedJustInTimeBinding.java | 3 ++- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/reflect/src/main/java/dagger/reflect/TypeUtil.java b/reflect/src/main/java/dagger/reflect/TypeUtil.java index d697ea4d..36e98c10 100644 --- a/reflect/src/main/java/dagger/reflect/TypeUtil.java +++ b/reflect/src/main/java/dagger/reflect/TypeUtil.java @@ -83,9 +83,7 @@ static final class ParameterizedTypeImpl implements ParameterizedType { "unexpected owner type for " + rawType + ": " + ownerType); } } else if (enclosingClass != null) { - // TODO ? - // throw new IllegalArgumentException("unexpected owner type for " + rawType + ": - // null"); + throw new IllegalArgumentException("unexpected owner type for " + rawType + ": null"); } } diff --git a/reflect/src/main/java/dagger/reflect/UnlinkedJustInTimeBinding.java b/reflect/src/main/java/dagger/reflect/UnlinkedJustInTimeBinding.java index 61d73aa1..a547148b 100644 --- a/reflect/src/main/java/dagger/reflect/UnlinkedJustInTimeBinding.java +++ b/reflect/src/main/java/dagger/reflect/UnlinkedJustInTimeBinding.java @@ -75,7 +75,8 @@ private static boolean hasParameterizedTypeVariable(Type parameterType) { private TypeUtil.ParameterizedTypeImpl findKeyForParameterizedType( ParameterizedType parameterType) { Type[] matchingTypes = matchingParameterizedType(parameterType.getActualTypeArguments()); - return new TypeUtil.ParameterizedTypeImpl(null, parameterType.getRawType(), matchingTypes); + return new TypeUtil.ParameterizedTypeImpl( + parameterType.getOwnerType(), parameterType.getRawType(), matchingTypes); } /**