From 523a454a3f852c49e1e7c13d110bb3dee483a922 Mon Sep 17 00:00:00 2001 From: Eric Chang Date: Wed, 15 Jan 2025 14:44:42 -0800 Subject: [PATCH] Fix issue with `jakarta.inject.Provider` support where in certain cases requests for a `Map>` would fail to compile. Similarly, fix support for `@LazyClassKey` with `jakarta.inject.Provider`. Fixes #4572. RELNOTES=Fixes #4572. Fix issue with `jakarta.inject.Provider` support where in certain cases requests for a `Map>` would fail to compile. PiperOrigin-RevId: 715952799 --- .../dagger/internal/codegen/base/MapType.java | 12 ++++-- .../internal/codegen/binding/SourceFiles.java | 3 +- .../DependencyCycleValidator.java | 3 +- .../writing/MapFactoryCreationExpression.java | 2 +- .../jakarta/JakartaProviderTest.java | 37 ++++++++++++++++++- 5 files changed, 48 insertions(+), 9 deletions(-) diff --git a/java/dagger/internal/codegen/base/MapType.java b/java/dagger/internal/codegen/base/MapType.java index ba4b1e79274..52294fc18e6 100644 --- a/java/dagger/internal/codegen/base/MapType.java +++ b/java/dagger/internal/codegen/base/MapType.java @@ -86,6 +86,11 @@ public boolean valuesAreFrameworkType() { return valueRequestKind() != RequestKind.INSTANCE; } + /** Returns {@code true} if the raw type of {@link #valueType()} is a provider type.*/ + public boolean valuesAreProvider() { + return valuesAreTypeOf(TypeNames.PROVIDER) || valuesAreTypeOf(TypeNames.JAKARTA_PROVIDER); + } + /** * Returns the map's {@link #valueType()} without any wrapping framework type, if one exists. * @@ -124,19 +129,20 @@ public RequestKind valueRequestKind() { } } - /** {@code true} if {@code type} is a {@link java.util.Map} type. */ + /** Returns {@code true} if {@code type} is a {@link java.util.Map} type. */ public static boolean isMap(XType type) { return isTypeOf(type, TypeNames.MAP); } - /** {@code true} if {@code key.type()} is a {@link java.util.Map} type. */ + /** Returns {@code true} if {@code key.type()} is a {@link java.util.Map} type. */ public static boolean isMap(Key key) { return isMap(key.type().xprocessing()); } + /** Returns {@code true} if the given type is a {@code Map>}. */ public static boolean isMapOfProvider(XType keyType) { if (MapType.isMap(keyType)) { - return MapType.from(keyType).valuesAreTypeOf(TypeNames.PROVIDER); + return MapType.from(keyType).valuesAreProvider(); } return false; } diff --git a/java/dagger/internal/codegen/binding/SourceFiles.java b/java/dagger/internal/codegen/binding/SourceFiles.java index 8319181bca5..de353dee05d 100644 --- a/java/dagger/internal/codegen/binding/SourceFiles.java +++ b/java/dagger/internal/codegen/binding/SourceFiles.java @@ -24,7 +24,6 @@ import static com.google.common.base.Verify.verify; import static dagger.internal.codegen.javapoet.TypeNames.DOUBLE_CHECK; import static dagger.internal.codegen.javapoet.TypeNames.PRODUCER; -import static dagger.internal.codegen.javapoet.TypeNames.PROVIDER; import static dagger.internal.codegen.javapoet.TypeNames.PROVIDER_OF_LAZY; import static dagger.internal.codegen.model.BindingKind.ASSISTED_INJECTION; import static dagger.internal.codegen.model.BindingKind.INJECTION; @@ -280,7 +279,7 @@ public static XClassName mapFactoryClassName(MultiboundMapBinding binding) { MapType mapType = MapType.from(binding.key()); switch (binding.bindingType()) { case PROVISION: - return mapType.valuesAreTypeOf(PROVIDER) + return mapType.valuesAreProvider() ? XTypeNames.MAP_PROVIDER_FACTORY : XTypeNames.MAP_FACTORY; case PRODUCTION: return mapType.valuesAreFrameworkType() diff --git a/java/dagger/internal/codegen/bindinggraphvalidation/DependencyCycleValidator.java b/java/dagger/internal/codegen/bindinggraphvalidation/DependencyCycleValidator.java index 0ed664184a3..386c40aa74a 100644 --- a/java/dagger/internal/codegen/bindinggraphvalidation/DependencyCycleValidator.java +++ b/java/dagger/internal/codegen/bindinggraphvalidation/DependencyCycleValidator.java @@ -43,7 +43,6 @@ import dagger.internal.codegen.base.MapType; import dagger.internal.codegen.base.OptionalType; import dagger.internal.codegen.binding.DependencyRequestFormatter; -import dagger.internal.codegen.javapoet.TypeNames; import dagger.internal.codegen.model.Binding; import dagger.internal.codegen.model.BindingGraph; import dagger.internal.codegen.model.BindingGraph.ComponentNode; @@ -242,7 +241,7 @@ private boolean breaksCycle(XType requestedType, RequestKind requestKind) { case INSTANCE: if (MapType.isMap(requestedType)) { - return MapType.from(requestedType).valuesAreTypeOf(TypeNames.PROVIDER); + return MapType.from(requestedType).valuesAreProvider(); } // fall through diff --git a/java/dagger/internal/codegen/writing/MapFactoryCreationExpression.java b/java/dagger/internal/codegen/writing/MapFactoryCreationExpression.java index 0ac7212283e..be9c637e917 100644 --- a/java/dagger/internal/codegen/writing/MapFactoryCreationExpression.java +++ b/java/dagger/internal/codegen/writing/MapFactoryCreationExpression.java @@ -101,7 +101,7 @@ private static ClassName lazyMapFactoryClassName(MultiboundMapBinding binding) { MapType mapType = MapType.from(binding.key()); switch (binding.bindingType()) { case PROVISION: - return mapType.valuesAreTypeOf(TypeNames.PROVIDER) + return mapType.valuesAreProvider() ? TypeNames.LAZY_CLASS_KEY_MAP_PROVIDER_FACTORY : TypeNames.LAZY_CLASS_KEY_MAP_FACTORY; case PRODUCTION: diff --git a/javatests/dagger/functional/jakarta/JakartaProviderTest.java b/javatests/dagger/functional/jakarta/JakartaProviderTest.java index 0c228619fb3..3e74d35f0ba 100644 --- a/javatests/dagger/functional/jakarta/JakartaProviderTest.java +++ b/javatests/dagger/functional/jakarta/JakartaProviderTest.java @@ -26,6 +26,7 @@ import dagger.Module; import dagger.Provides; import dagger.multibindings.IntoMap; +import dagger.multibindings.LazyClassKey; import dagger.multibindings.StringKey; import jakarta.inject.Inject; import jakarta.inject.Provider; @@ -75,6 +76,10 @@ interface TestComponent { Map> getJavaxProviderMap(); + Map, Provider> getJakartaProviderClassMap(); + + Map, javax.inject.Provider> getJavaxProviderClassMap(); + Map>> getJakartaProviderLazyMap(); Map>> getJavaxProviderLazyMap(); @@ -103,10 +108,17 @@ public static final class Bar { Bar() {} } + // Scoped as this forces the generated code to use a Provider instead of inlining + // in default mode + @TestScope public static final class InjectUsages { Provider jakartaBar; Provider jakartaQualifiedBar; javax.inject.Provider javaxQualifiedBar; + Map> javaxProviderMap; + Map> jakartaProviderMap; + Map, javax.inject.Provider> javaxProviderClassMap; + Map, Provider> jakartaProviderClassMap; @Inject InjectUsages(Provider jakartaBar) { @@ -117,9 +129,18 @@ public static final class InjectUsages { @Inject void injectBar( - Provider jakartaQualifiedBar, javax.inject.Provider javaxQualifiedBar) { + Provider jakartaQualifiedBar, + javax.inject.Provider javaxQualifiedBar, + Map> javaxProviderMap, + Map> jakartaProviderMap, + Map, javax.inject.Provider> javaxProviderClassMap, + Map, Provider> jakartaProviderClassMap) { this.jakartaQualifiedBar = jakartaQualifiedBar; this.javaxQualifiedBar = javaxQualifiedBar; + this.javaxProviderMap = javaxProviderMap; + this.jakartaProviderMap = jakartaProviderMap; + this.javaxProviderClassMap = javaxProviderClassMap; + this.jakartaProviderClassMap = jakartaProviderClassMap; } } @@ -146,6 +167,11 @@ static Bar provideBar( @StringKey("bar") abstract Bar bindBarIntoMap(Bar bar); + @Binds + @IntoMap + @LazyClassKey(Bar.class) + abstract Bar bindBarIntoClassMap(Bar bar); + // TODO(b/65118638): Use @Binds @IntoMap Lazy once that works properly. @Provides @IntoMap @@ -210,9 +236,18 @@ public void testJakartaProviders() { assertThat(testComponent.getJakartaProviderMap().get("bar").get()).isSameInstanceAs( testComponent.getJavaxProviderMap().get("bar").get()); + assertThat(testComponent.getJakartaProviderClassMap().get(Bar.class).get()).isSameInstanceAs( + testComponent.getJavaxProviderClassMap().get(Bar.class).get()); + assertThat(testComponent.getJakartaProviderLazyMap().get("bar").get().get()).isSameInstanceAs( testComponent.getJavaxProviderLazyMap().get("bar").get().get()); + assertThat(injectUsages.jakartaProviderMap.get("bar").get()).isSameInstanceAs( + injectUsages.javaxProviderMap.get("bar").get()); + + assertThat(injectUsages.jakartaProviderClassMap.get(Bar.class).get()).isSameInstanceAs( + injectUsages.javaxProviderClassMap.get(Bar.class).get()); + Map> manualJakartaMap = testComponent.getManuallyProvidedJakartaMap(); assertThat(manualJakartaMap.keySet()).containsExactly(9L);