From 30a3114f48ff43b61779d646bbf95bd85816ef74 Mon Sep 17 00:00:00 2001 From: Eric Chang Date: Thu, 26 Dec 2024 16:03:40 -0800 Subject: [PATCH] Support injecting `jakarta.inject.Provider` type in the places `javax.inject.Provider` can be used. RELNOTES=Support injecting `jakarta.inject.Provider` PiperOrigin-RevId: 709898053 --- .../internal/codegen/base/FrameworkTypes.java | 17 +- .../dagger/internal/codegen/base/MapType.java | 21 +- .../internal/codegen/base/RequestKinds.java | 9 +- .../binding/ComponentDeclarations.java | 13 +- .../internal/codegen/binding/KeyFactory.java | 11 +- javatests/dagger/functional/jakarta/BUILD | 13 + .../jakarta/JakartaProviderTest.java | 228 ++++++++++++++++++ .../codegen/ModuleFactoryGeneratorTest.java | 6 + 8 files changed, 289 insertions(+), 29 deletions(-) create mode 100644 javatests/dagger/functional/jakarta/JakartaProviderTest.java diff --git a/java/dagger/internal/codegen/base/FrameworkTypes.java b/java/dagger/internal/codegen/base/FrameworkTypes.java index 2a56e1d8bba..e607f766461 100644 --- a/java/dagger/internal/codegen/base/FrameworkTypes.java +++ b/java/dagger/internal/codegen/base/FrameworkTypes.java @@ -16,7 +16,6 @@ package dagger.internal.codegen.base; -import static dagger.internal.codegen.extension.DaggerStreams.toImmutableSet; import static dagger.internal.codegen.xprocessing.XTypes.isTypeOf; import androidx.room.compiler.processing.XType; @@ -32,7 +31,11 @@ public final class FrameworkTypes { // TODO(erichang): Add the Jakarta Provider here private static final ImmutableSet PROVISION_TYPES = - ImmutableSet.of(TypeNames.PROVIDER, TypeNames.LAZY, TypeNames.MEMBERS_INJECTOR); + ImmutableSet.of( + TypeNames.PROVIDER, + TypeNames.JAKARTA_PROVIDER, + TypeNames.LAZY, + TypeNames.MEMBERS_INJECTOR); // NOTE(beder): ListenableFuture is not considered a producer framework type because it is not // defined by the framework, so we can't treat it specially in ordinary Dagger. @@ -42,13 +45,15 @@ public final class FrameworkTypes { private static final ImmutableSet ALL_FRAMEWORK_TYPES = ImmutableSet.builder().addAll(PROVISION_TYPES).addAll(PRODUCTION_TYPES).build(); - private static final ImmutableSet SET_VALUE_FRAMEWORK_TYPES = + public static final ImmutableSet SET_VALUE_FRAMEWORK_TYPES = ImmutableSet.of(TypeNames.PRODUCED); public static final ImmutableSet MAP_VALUE_FRAMEWORK_TYPES = - MapType.VALID_FRAMEWORK_REQUEST_KINDS.stream() - .map(RequestKinds::frameworkClassName) - .collect(toImmutableSet()); + ImmutableSet.of( + TypeNames.PRODUCED, + TypeNames.PRODUCER, + TypeNames.PROVIDER, + TypeNames.JAKARTA_PROVIDER); /** Returns true if the type represents a producer-related framework type. */ public static boolean isProducerType(XType type) { diff --git a/java/dagger/internal/codegen/base/MapType.java b/java/dagger/internal/codegen/base/MapType.java index f9c6304084a..ba4b1e79274 100644 --- a/java/dagger/internal/codegen/base/MapType.java +++ b/java/dagger/internal/codegen/base/MapType.java @@ -35,8 +35,10 @@ @AutoValue public abstract class MapType { // TODO(b/28555349): support PROVIDER_OF_LAZY here too + // TODO(b/376124787): We could consolidate this with a similar list in FrameworkTypes + // if we had a better way to go from RequestKind to framework ClassName or vice versa /** The valid framework request kinds allowed on a multibinding map value. */ - public static final ImmutableSet VALID_FRAMEWORK_REQUEST_KINDS = + private static final ImmutableSet VALID_FRAMEWORK_REQUEST_KINDS = ImmutableSet.of(RequestKind.PROVIDER, RequestKind.PRODUCER, RequestKind.PRODUCED); private XType type; @@ -107,12 +109,19 @@ public XType unwrappedFrameworkValueType() { */ public RequestKind valueRequestKind() { checkArgument(!isRawType()); - for (RequestKind frameworkRequestKind : VALID_FRAMEWORK_REQUEST_KINDS) { - if (valuesAreTypeOf(RequestKinds.frameworkClassName(frameworkRequestKind))) { - return frameworkRequestKind; - } + RequestKind requestKind = RequestKinds.getRequestKind(valueType()); + if (VALID_FRAMEWORK_REQUEST_KINDS.contains(requestKind)) { + return requestKind; + } else if (requestKind == RequestKind.PROVIDER_OF_LAZY) { + // This is kind of a weird case. We don't support Map>, so we also don't support + // Map>> directly. However, if the user bound that themselves, we don't + // want that to get confused as a normal instance request, so return PROVIDER here. + return RequestKind.PROVIDER; + } else { + // Not all RequestKinds are supported, so if there's a map value that matches an unsupported + // RequestKind, just treat it like it is a normal instance request. + return RequestKind.INSTANCE; } - return RequestKind.INSTANCE; } /** {@code true} if {@code type} is a {@link java.util.Map} type. */ diff --git a/java/dagger/internal/codegen/base/RequestKinds.java b/java/dagger/internal/codegen/base/RequestKinds.java index 4fb39186b95..fdf1c2020ed 100644 --- a/java/dagger/internal/codegen/base/RequestKinds.java +++ b/java/dagger/internal/codegen/base/RequestKinds.java @@ -97,6 +97,8 @@ public static TypeName requestTypeName(RequestKind requestKind, TypeName keyType private static final ImmutableMap FRAMEWORK_CLASSES = ImmutableMap.of( + // Default to the javax Provider since that is what is used for the binding graph + // representation. PROVIDER, TypeNames.PROVIDER, LAZY, TypeNames.LAZY, PRODUCER, TypeNames.PRODUCER, @@ -111,10 +113,15 @@ public static RequestKind getRequestKind(XType type) { return RequestKind.INSTANCE; } - if (isTypeOf(type, TypeNames.PROVIDER) && isTypeOf(unwrapType(type), TypeNames.LAZY)) { + if ((isTypeOf(type, TypeNames.PROVIDER) || isTypeOf(type, TypeNames.JAKARTA_PROVIDER)) + && isTypeOf(unwrapType(type), TypeNames.LAZY)) { return RequestKind.PROVIDER_OF_LAZY; } + if (isTypeOf(type, TypeNames.JAKARTA_PROVIDER)) { + return RequestKind.PROVIDER; + } + return FRAMEWORK_CLASSES.keySet().stream() .filter(kind -> isTypeOf(type, FRAMEWORK_CLASSES.get(kind))) .collect(toOptional()) diff --git a/java/dagger/internal/codegen/binding/ComponentDeclarations.java b/java/dagger/internal/codegen/binding/ComponentDeclarations.java index 3bf35cdf018..28557220ce6 100644 --- a/java/dagger/internal/codegen/binding/ComponentDeclarations.java +++ b/java/dagger/internal/codegen/binding/ComponentDeclarations.java @@ -26,10 +26,12 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSetMultimap; import com.google.common.collect.Multimaps; +import com.squareup.javapoet.ClassName; import com.squareup.javapoet.ParameterizedTypeName; import com.squareup.javapoet.TypeName; import com.squareup.javapoet.WildcardTypeName; import dagger.internal.codegen.base.DaggerSuperficialValidation; +import dagger.internal.codegen.base.FrameworkTypes; import dagger.internal.codegen.javapoet.TypeNames; import dagger.internal.codegen.model.DaggerAnnotation; import dagger.internal.codegen.model.Key; @@ -39,11 +41,6 @@ /** Stores the bindings and declarations of a component by key. */ final class ComponentDeclarations { - private static final ImmutableSet MAP_FRAMEWORK_TYPENAMES = - ImmutableSet.of(TypeNames.PROVIDER, TypeNames.PRODUCER, TypeNames.PRODUCED); - private static final ImmutableSet SET_FRAMEWORK_TYPENAMES = - ImmutableSet.of(TypeNames.PRODUCED); - private final KeyFactory keyFactory; private final ImmutableSetMultimap bindings; private final ImmutableSetMultimap delegates; @@ -322,14 +319,14 @@ private static TypeName unwrapMultibindingTypeName(TypeName typeName) { return ParameterizedTypeName.get( mapTypeName.rawType, mapKeyTypeName, - unwrapFrameworkTypeName(mapValueTypeName, MAP_FRAMEWORK_TYPENAMES)); + unwrapFrameworkTypeName(mapValueTypeName, FrameworkTypes.MAP_VALUE_FRAMEWORK_TYPES)); } if (isValidSetMultibindingTypeName(typeName)) { ParameterizedTypeName setTypeName = (ParameterizedTypeName) typeName; TypeName setValueTypeName = getOnlyElement(setTypeName.typeArguments); return ParameterizedTypeName.get( setTypeName.rawType, - unwrapFrameworkTypeName(setValueTypeName, SET_FRAMEWORK_TYPENAMES)); + unwrapFrameworkTypeName(setValueTypeName, FrameworkTypes.SET_VALUE_FRAMEWORK_TYPES)); } return typeName; } @@ -356,7 +353,7 @@ private static boolean isValidSetMultibindingTypeName(TypeName typeName) { } private static TypeName unwrapFrameworkTypeName( - TypeName typeName, ImmutableSet frameworkTypeNames) { + TypeName typeName, ImmutableSet frameworkTypeNames) { if (typeName instanceof ParameterizedTypeName) { ParameterizedTypeName parameterizedTypeName = (ParameterizedTypeName) typeName; if (frameworkTypeNames.contains(parameterizedTypeName.rawType)) { diff --git a/java/dagger/internal/codegen/binding/KeyFactory.java b/java/dagger/internal/codegen/binding/KeyFactory.java index e03814899bb..693382e4908 100644 --- a/java/dagger/internal/codegen/binding/KeyFactory.java +++ b/java/dagger/internal/codegen/binding/KeyFactory.java @@ -38,6 +38,7 @@ import dagger.Binds; import dagger.BindsOptionalOf; import dagger.internal.codegen.base.ContributionType; +import dagger.internal.codegen.base.FrameworkTypes; import dagger.internal.codegen.base.MapType; import dagger.internal.codegen.base.OptionalType; import dagger.internal.codegen.base.RequestKinds; @@ -97,10 +98,7 @@ private XType optionalOf(XType type) { /** Returns {@code Map>}. */ private XType mapOfFrameworkType(XType keyType, ClassName frameworkClassName, XType valueType) { - checkArgument( - MapType.VALID_FRAMEWORK_REQUEST_KINDS.stream() - .map(RequestKinds::frameworkClassName) - .anyMatch(frameworkClassName::equals)); + checkArgument(FrameworkTypes.MAP_VALUE_FRAMEWORK_TYPES.contains(frameworkClassName)); return mapOf( keyType, processingEnv.getDeclaredType( @@ -317,10 +315,7 @@ public Key unwrapMapValueType(Key key) { * type. */ private Key wrapMapValue(Key key, ClassName frameworkClassName) { - checkArgument( - MapType.VALID_FRAMEWORK_REQUEST_KINDS.stream() - .map(RequestKinds::frameworkClassName) - .anyMatch(frameworkClassName::equals)); + checkArgument(FrameworkTypes.MAP_VALUE_FRAMEWORK_TYPES.contains(frameworkClassName)); if (MapType.isMap(key)) { MapType mapType = MapType.from(key); if (!mapType.isRawType() && !mapType.valuesAreTypeOf(frameworkClassName)) { diff --git a/javatests/dagger/functional/jakarta/BUILD b/javatests/dagger/functional/jakarta/BUILD index 9feb5e20928..8d98d86d7d7 100644 --- a/javatests/dagger/functional/jakarta/BUILD +++ b/javatests/dagger/functional/jakarta/BUILD @@ -32,3 +32,16 @@ GenJavaTests( "@maven//:jakarta_inject_jakarta_inject_api", ], ) + +GenJavaTests( + name = "JakartaProviderTest", + srcs = ["JakartaProviderTest.java"], + javacopts = DOCLINT_HTML_AND_SYNTAX, + deps = [ + "//:dagger_with_compiler", + "//third_party/java/guava/base", + "//third_party/java/junit", + "//third_party/java/truth", + "@maven//:jakarta_inject_jakarta_inject_api", + ], +) diff --git a/javatests/dagger/functional/jakarta/JakartaProviderTest.java b/javatests/dagger/functional/jakarta/JakartaProviderTest.java new file mode 100644 index 00000000000..0c228619fb3 --- /dev/null +++ b/javatests/dagger/functional/jakarta/JakartaProviderTest.java @@ -0,0 +1,228 @@ +/* + * Copyright (C) 2024 The Dagger Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package dagger.functional.jakarta; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.base.Optional; +import dagger.Binds; +import dagger.BindsOptionalOf; +import dagger.Component; +import dagger.Lazy; +import dagger.Module; +import dagger.Provides; +import dagger.multibindings.IntoMap; +import dagger.multibindings.StringKey; +import jakarta.inject.Inject; +import jakarta.inject.Provider; +import jakarta.inject.Qualifier; +import jakarta.inject.Scope; +import java.util.HashMap; +import java.util.Map; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public final class JakartaProviderTest { + + @Scope + public @interface TestScope {} + + @Qualifier + public @interface TestQualifier {} + + @TestScope + @Component(modules = TestModule.class) + interface TestComponent { + Provider getJakartaFoo(); + + javax.inject.Provider getJavaxFoo(); + + Provider getJakartaBar(); + + javax.inject.Provider getJavaxBar(); + + @TestQualifier + Provider getJakartaQualifiedFoo(); + + @TestQualifier + javax.inject.Provider getJavaxQualifiedFoo(); + + @TestQualifier + Provider getJakartaQualifiedBar(); + + @TestQualifier + javax.inject.Provider getJavaxQualifiedBar(); + + InjectUsages injectUsages(); + + Map> getJakartaProviderMap(); + + Map> getJavaxProviderMap(); + + Map>> getJakartaProviderLazyMap(); + + Map>> getJavaxProviderLazyMap(); + + Map> getManuallyProvidedJakartaMap(); + + Map> getManuallyProvidedJavaxMap(); + + Optional> getPresentOptionalJakartaProvider(); + + Optional> getPresentOptionalJavaxProvider(); + + Optional> getEmptyOptionalJakartaProvider(); + + Optional> getEmptyOptionalJavaxProvider(); + } + + public static final class Foo { + @Inject + Foo() {} + } + + @TestScope + public static final class Bar { + @Inject + Bar() {} + } + + public static final class InjectUsages { + Provider jakartaBar; + Provider jakartaQualifiedBar; + javax.inject.Provider javaxQualifiedBar; + + @Inject + InjectUsages(Provider jakartaBar) { + this.jakartaBar = jakartaBar; + } + + @Inject javax.inject.Provider javaxBar; + + @Inject + void injectBar( + Provider jakartaQualifiedBar, javax.inject.Provider javaxQualifiedBar) { + this.jakartaQualifiedBar = jakartaQualifiedBar; + this.javaxQualifiedBar = javaxQualifiedBar; + } + } + + @Module + abstract static class TestModule { + @Provides + @TestQualifier + static Foo provideFoo( + Provider fooProvider, javax.inject.Provider unusedOtherFooProvider) { + return fooProvider.get(); + } + + @Provides + @TestQualifier + @TestScope + static Bar provideBar( + Provider unusedBarProvider, javax.inject.Provider otherBarProvider) { + // Use the other one in this case just to vary it from Foo + return otherBarProvider.get(); + } + + @Binds + @IntoMap + @StringKey("bar") + abstract Bar bindBarIntoMap(Bar bar); + + // TODO(b/65118638): Use @Binds @IntoMap Lazy once that works properly. + @Provides + @IntoMap + @StringKey("bar") + static Lazy provideLazyIntoMap(Lazy bar) { + return bar; + } + + // Manually provide two Provider maps to make sure they don't conflict. + @Provides + static Map> manuallyProvidedJakartaMap() { + Map> map = new HashMap<>(); + map.put(9L, null); + return map; + } + + @Provides + static Map> manuallyProvidedJavaxMap() { + Map> map = new HashMap<>(); + map.put(0L, null); + return map; + } + + @BindsOptionalOf + abstract String bindOptionalString(); + + @Provides + static String provideString() { + return "present"; + } + + @BindsOptionalOf + abstract Long bindOptionalLong(); + } + + @Test + public void testJakartaProviders() { + TestComponent testComponent = DaggerJakartaProviderTest_TestComponent.create(); + + assertThat(testComponent.getJakartaFoo().get()).isNotNull(); + assertThat(testComponent.getJavaxFoo().get()).isNotNull(); + + assertThat(testComponent.getJakartaBar().get()) + .isSameInstanceAs(testComponent.getJavaxBar().get()); + + assertThat(testComponent.getJakartaQualifiedFoo().get()).isNotNull(); + assertThat(testComponent.getJavaxQualifiedFoo().get()).isNotNull(); + + assertThat(testComponent.getJakartaQualifiedBar().get()) + .isSameInstanceAs(testComponent.getJavaxQualifiedBar().get()); + assertThat(testComponent.getJakartaBar().get()) + .isSameInstanceAs(testComponent.getJakartaQualifiedBar().get()); + + InjectUsages injectUsages = testComponent.injectUsages(); + + assertThat(injectUsages.jakartaBar.get()).isSameInstanceAs(injectUsages.javaxBar.get()); + assertThat(injectUsages.jakartaQualifiedBar.get()) + .isSameInstanceAs(injectUsages.javaxQualifiedBar.get()); + assertThat(injectUsages.jakartaBar.get()) + .isSameInstanceAs(injectUsages.jakartaQualifiedBar.get()); + + assertThat(testComponent.getJakartaProviderMap().get("bar").get()).isSameInstanceAs( + testComponent.getJavaxProviderMap().get("bar").get()); + + assertThat(testComponent.getJakartaProviderLazyMap().get("bar").get().get()).isSameInstanceAs( + testComponent.getJavaxProviderLazyMap().get("bar").get().get()); + + Map> manualJakartaMap = testComponent.getManuallyProvidedJakartaMap(); + assertThat(manualJakartaMap.keySet()).containsExactly(9L); + + Map> manualJavaxMap = + testComponent.getManuallyProvidedJavaxMap(); + assertThat(manualJavaxMap.keySet()).containsExactly(0L); + + assertThat(testComponent.getPresentOptionalJakartaProvider().get().get()).isEqualTo("present"); + assertThat(testComponent.getPresentOptionalJavaxProvider().get().get()).isEqualTo("present"); + assertThat(testComponent.getEmptyOptionalJakartaProvider().isPresent()).isFalse(); + assertThat(testComponent.getEmptyOptionalJavaxProvider().isPresent()).isFalse(); + } +} diff --git a/javatests/dagger/internal/codegen/ModuleFactoryGeneratorTest.java b/javatests/dagger/internal/codegen/ModuleFactoryGeneratorTest.java index 0d7d2f02c15..d315b77e0d3 100644 --- a/javatests/dagger/internal/codegen/ModuleFactoryGeneratorTest.java +++ b/javatests/dagger/internal/codegen/ModuleFactoryGeneratorTest.java @@ -71,6 +71,12 @@ public void providesMethodReturnsProvider() { .hasError("@Provides methods must not return framework types"); } + @Test + public void providesMethodReturnsJakartaProvider() { + assertThatModuleMethod("@Provides jakarta.inject.Provider provideProvider() {}") + .hasError("@Provides methods must not return framework types"); + } + @Test public void providesMethodReturnsLazy() { assertThatModuleMethod("@Provides Lazy provideLazy() {}")