From 93201e0f1702e982eaf557f3f240ef41c2a3b9ad Mon Sep 17 00:00:00 2001 From: Brad Corso Date: Thu, 8 Aug 2024 13:38:38 -0700 Subject: [PATCH] Refactor `BindingGraphFactory` to remove the need for `keysMatchingRequest` and corresponding cache. This CL refactors `BindingGraphFactory` so that we no longer need to loop through all variations of a multibindings key when getting the `@Multibindings` declarations or `@IntoMap`/`@IntoSet` contributions from `ComponentDeclarations`. Instead, `ComponentDeclarations` now indexes these declarations by the unwrapped multibinding key (i.e. `Map` or `Set`) which we can use to get all declarations with a single key. In addition, the unwrapping of the key is done within `ComponentDeclarations` so that each caller doesn't have to remember to unwrap the key. RELNOTES=Fixes missing variance in KSP for multibindings which can break KSP users who forgot the `@JvmSuppressWildcards`. PiperOrigin-RevId: 660963882 --- .../codegen/DelegateComponentProcessor.java | 5 - .../codegen/binding/BindingGraphFactory.java | 73 +----- .../binding/ComponentDeclarations.java | 248 +++++++++++++++--- .../internal/codegen/binding/KeyFactory.java | 39 --- .../internal/codegen/MultibindingTest.java | 126 ++++++++- 5 files changed, 342 insertions(+), 149 deletions(-) diff --git a/java/dagger/internal/codegen/DelegateComponentProcessor.java b/java/dagger/internal/codegen/DelegateComponentProcessor.java index 404186341f7..754b4d94f12 100644 --- a/java/dagger/internal/codegen/DelegateComponentProcessor.java +++ b/java/dagger/internal/codegen/DelegateComponentProcessor.java @@ -33,7 +33,6 @@ import dagger.internal.codegen.base.SourceFileGenerationException; import dagger.internal.codegen.base.SourceFileGenerator; import dagger.internal.codegen.base.SourceFileHjarGenerator; -import dagger.internal.codegen.binding.BindingGraphFactory; import dagger.internal.codegen.binding.ComponentDescriptor; import dagger.internal.codegen.binding.ContributionBinding; import dagger.internal.codegen.binding.InjectBindingRegistry; @@ -184,10 +183,6 @@ interface ProcessingRoundCacheModule { @IntoSet ClearableCache monitoringModules(MonitoringModules cache); - @Binds - @IntoSet - ClearableCache bindingGraphFactory(BindingGraphFactory cache); - @Binds @IntoSet ClearableCache componentValidator(ComponentValidator cache); diff --git a/java/dagger/internal/codegen/binding/BindingGraphFactory.java b/java/dagger/internal/codegen/binding/BindingGraphFactory.java index e74efa8a0a1..84640db875d 100644 --- a/java/dagger/internal/codegen/binding/BindingGraphFactory.java +++ b/java/dagger/internal/codegen/binding/BindingGraphFactory.java @@ -37,7 +37,6 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import dagger.Reusable; -import dagger.internal.codegen.base.ClearableCache; import dagger.internal.codegen.base.ContributionType; import dagger.internal.codegen.base.Keys; import dagger.internal.codegen.base.MapType; @@ -64,11 +63,9 @@ import java.util.Queue; import java.util.Set; import javax.inject.Inject; -import javax.inject.Singleton; /** A factory for {@link BindingGraph} objects. */ -@Singleton -public final class BindingGraphFactory implements ClearableCache { +public final class BindingGraphFactory { private final InjectBindingRegistry injectBindingRegistry; private final KeyFactory keyFactory; @@ -76,7 +73,6 @@ public final class BindingGraphFactory implements ClearableCache { private final BindingNode.Factory bindingNodeFactory; private final ComponentDeclarations.Factory componentDeclarationsFactory; private final BindingGraphConverter bindingGraphConverter; - private final Map> keysMatchingRequestCache = new HashMap<>(); private final CompilerOptions compilerOptions; @Inject @@ -158,11 +154,6 @@ private LegacyBindingGraph createLegacyBindingGraph( return new LegacyBindingGraph(requestResolver, subgraphs.build()); } - @Override - public void clearCache() { - keysMatchingRequestCache.clear(); - } - /** Represents a fully resolved binding graph. */ static final class LegacyBindingGraph { private final Resolver resolver; @@ -278,20 +269,15 @@ ResolvedBindings lookUpBindings(Key requestKey) { Set subcomponentDeclarations = new LinkedHashSet<>(); // Gather all bindings, multibindings, optional, and subcomponent declarations/contributions. - ImmutableSet multibindingKeysMatchingRequest = - multibindingKeysMatchingRequest(requestKey); for (Resolver resolver : getResolverLineage()) { bindings.addAll(resolver.getLocalExplicitBindings(requestKey)); + multibindingContributions.addAll(resolver.getLocalMultibindingContributions(requestKey)); + multibindingDeclarations.addAll(resolver.declarations.multibindings(requestKey)); subcomponentDeclarations.addAll(resolver.declarations.subcomponents(requestKey)); // The optional binding declarations are keyed by the unwrapped type. keyFactory.unwrapOptional(requestKey) .map(resolver.declarations::optionalBindings) .ifPresent(optionalBindingDeclarations::addAll); - - for (Key key : multibindingKeysMatchingRequest) { - multibindingContributions.addAll(resolver.getLocalMultibindingContributions(key)); - multibindingDeclarations.addAll(resolver.declarations.multibindings(key)); - } } // Add synthetic multibinding @@ -410,41 +396,6 @@ private void addSubcomponentToOwningResolver(ContributionBinding subcomponentCre owningResolver.componentDescriptor.getChildComponentWithBuilderType(builderType)); } - /** - * Profiling has determined that computing the keys matching {@code requestKey} has measurable - * performance impact. It is called repeatedly (at least 3 times per key resolved per {@link - * BindingGraph}. {@code javac}'s name-checking performance seems suboptimal (converting byte - * strings to Strings repeatedly), and the matching keys creations relies on that. This also - * ensures that the resulting keys have their hash codes cached on successive calls to this - * method. - * - *

This caching may become obsolete if: - * - *

    - *
  • We decide to intern all {@link Key} instances - *
  • We fix javac's name-checking peformance (though we may want to keep this for older - * javac users) - *
- */ - private ImmutableSet multibindingKeysMatchingRequest(Key requestKey) { - return keysMatchingRequestCache.computeIfAbsent( - requestKey, this::multibindingKeysMatchingRequestUncached); - } - - private ImmutableSet multibindingKeysMatchingRequestUncached(Key requestKey) { - ImmutableSet.Builder keys = ImmutableSet.builder(); - keys.add(requestKey); - keyFactory.unwrapSetKey(requestKey, TypeNames.PRODUCED).ifPresent(keys::add); - keyFactory - .rewrapMapKey(requestKey, TypeNames.PRODUCER, TypeNames.PROVIDER) - .ifPresent(keys::add); - keyFactory - .rewrapMapKey(requestKey, TypeNames.PROVIDER, TypeNames.PRODUCER) - .ifPresent(keys::add); - keys.addAll(keyFactory.implicitFrameworkMapKeys(requestKey)); - return keys.build(); - } - private ImmutableSet createDelegateBindings( ImmutableSet delegateDeclarations) { ImmutableSet.Builder builder = ImmutableSet.builder(); @@ -641,15 +592,7 @@ private ImmutableSet getLocalExplicitBindings(Key key) { private ImmutableSet getLocalMultibindingContributions(Key key) { return ImmutableSet.builder() .addAll(declarations.multibindingContributions(key)) - // @Binds @IntoMap declarations have key Map, unlike @Provides @IntoMap or @Produces - // @IntoMap, which have Map> keys. So unwrap the key's type's - // value type if it's a Map> before looking in - // delegate declarations. createDelegateBindings() will create bindings with the properly - // wrapped key type. - .addAll( - createDelegateBindings( - declarations.delegateMultibindingContributions( - keyFactory.unwrapMapValueType(key)))) + .addAll(createDelegateBindings(declarations.delegateMultibindingContributions(key))) .build(); } @@ -868,12 +811,8 @@ private boolean hasLocalBindings(ResolvedBindings resolvedBindings) { * this component's modules that matches the key. */ private boolean hasLocalMultibindingContributions(Key requestKey) { - return multibindingKeysMatchingRequest(requestKey).stream() - .anyMatch( - multibindingKey -> - !declarations.multibindingContributions(multibindingKey).isEmpty() - || !declarations.delegateMultibindingContributions( - keyFactory.unwrapMapValueType(multibindingKey)).isEmpty()); + return !declarations.multibindingContributions(requestKey).isEmpty() + || !declarations.delegateMultibindingContributions(requestKey).isEmpty(); } /** diff --git a/java/dagger/internal/codegen/binding/ComponentDeclarations.java b/java/dagger/internal/codegen/binding/ComponentDeclarations.java index 92fa71bf932..af5acee7cd1 100644 --- a/java/dagger/internal/codegen/binding/ComponentDeclarations.java +++ b/java/dagger/internal/codegen/binding/ComponentDeclarations.java @@ -16,42 +16,57 @@ package dagger.internal.codegen.binding; +import static com.google.common.collect.Iterables.getOnlyElement; import static dagger.internal.codegen.binding.SourceFiles.generatedMonitoringModuleName; +import static dagger.internal.codegen.extension.DaggerStreams.toImmutableSet; import androidx.room.compiler.processing.XProcessingEnv; +import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSetMultimap; import com.google.common.collect.Multimaps; +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.javapoet.TypeNames; +import dagger.internal.codegen.model.DaggerAnnotation; import dagger.internal.codegen.model.Key; +import dagger.internal.codegen.model.Key.MultibindingContributionIdentifier; import java.util.Optional; import javax.inject.Inject; /** 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 ImmutableSetMultimap bindings; private final ImmutableSetMultimap delegates; - private final ImmutableSetMultimap multibindings; private final ImmutableSetMultimap optionalBindings; private final ImmutableSetMultimap subcomponents; - private final ImmutableSetMultimap multibindingContributions; - private final ImmutableSetMultimap delegateMultibindingContributions; + private final ImmutableSetMultimap multibindings; + private final ImmutableSetMultimap multibindingContributions; + private final ImmutableSetMultimap + delegateMultibindingContributions; private ComponentDeclarations( ImmutableSetMultimap bindings, ImmutableSetMultimap delegates, - ImmutableSetMultimap multibindings, ImmutableSetMultimap optionalBindings, - ImmutableSetMultimap subcomponents) { + ImmutableSetMultimap subcomponents, + ImmutableSetMultimap multibindings, + ImmutableSetMultimap multibindingContributions, + ImmutableSetMultimap delegateMultibindingContributions) { this.bindings = bindings; this.delegates = delegates; - this.multibindings = multibindings; this.optionalBindings = optionalBindings; this.subcomponents = subcomponents; - this.multibindingContributions = multibindingContributionsByMultibindingKey(bindings.values()); - this.delegateMultibindingContributions = - multibindingContributionsByMultibindingKey(delegates.values()); + this.multibindings = multibindings; + this.multibindingContributions = multibindingContributions; + this.delegateMultibindingContributions = delegateMultibindingContributions; } ImmutableSet bindings(Key key) { @@ -62,16 +77,76 @@ ImmutableSet delegates(Key key) { return delegates.get(key); } + /** + * Returns the delegate multibinding contributions (e.g. {@code @Binds @IntoMap}) for the given + * {@code key}, or an empty set if none exist. + * + *

For map multibindings, the following request keys represent the same underlying binding and + * will return the same results: + *

    + *
  • {@code Map} + *
  • {@code Map>} + *
  • {@code Map>} + *
  • {@code Map>} + *
+ * + *

For set multibindings, the following request keys represent the same underlying binding and + * will return the same results: + *

    + *
  • {@code Set} + *
  • {@code Set>} + *
+ */ ImmutableSet delegateMultibindingContributions(Key key) { - return delegateMultibindingContributions.get(key); + return delegateMultibindingContributions.get(unwrapMultibindingKey(key)); } + /** + * Returns the multibinding declarations (i.e. {@code @Multibinds}) for the given {@code key}, or + * an empty set if none exists. + * + *

For map multibindings, the following request keys represent the same underlying binding and + * will return the same results: + *

    + *
  • {@code Map} + *
  • {@code Map>} + *
  • {@code Map>} + *
  • {@code Map>} + *
+ * + *

For set multibindings, the following request keys represent the same underlying binding and + * will return the same results: + *

    + *
  • {@code Set} + *
  • {@code Set>} + *
+ */ ImmutableSet multibindings(Key key) { - return multibindings.get(key); + return multibindings.get(unwrapMultibindingKey(key)); } + /** + * Returns the multibinding contributions (e.g. {@code @Provides @IntoMap}) for the given + * {@code key}, or an empty set if none exists. + * + *

For map multibindings, the following request keys represent the same underlying binding and + * will return the same results: + *

    + *
  • {@code Map} + *
  • {@code Map>} + *
  • {@code Map>} + *
  • {@code Map>} + *
+ * + *

For set multibindings, the following request keys represent the same underlying binding and + * will return the same results: + *

    + *
  • {@code Set} + *
  • {@code Set>} + *
+ */ ImmutableSet multibindingContributions(Key key) { - return multibindingContributions.get(key); + return multibindingContributions.get(unwrapMultibindingKey(key)); } ImmutableSet optionalBindings(Key key) { @@ -92,22 +167,6 @@ ImmutableSet allDeclarations() { .build(); } - /** - * A multimap of those {@code declarations} that are multibinding contribution declarations, - * indexed by the key of the set or map to which they contribute. - */ - private static - ImmutableSetMultimap multibindingContributionsByMultibindingKey( - Iterable declarations) { - ImmutableSetMultimap.Builder builder = ImmutableSetMultimap.builder(); - for (T declaration : declarations) { - if (declaration.key().multibindingContributionIdentifier().isPresent()) { - builder.put(declaration.key().withoutMultibindingContributionIdentifier(), declaration); - } - } - return builder.build(); - } - static final class Factory { private final XProcessingEnv processingEnv; private final ModuleDescriptor.Factory moduleDescriptorFactory; @@ -147,9 +206,15 @@ ComponentDeclarations create( return new ComponentDeclarations( indexDeclarationsByKey(bindings.build()), indexDeclarationsByKey(delegates.build()), - indexDeclarationsByKey(multibindings.build()), indexDeclarationsByKey(optionalBindings.build()), - indexDeclarationsByKey(subcomponents.build())); + indexDeclarationsByKey(subcomponents.build()), + // The @Multibinds declarations and @IntoSet/@IntoMap multibinding contributions are all + // indexed by their "unwrapped" multibinding key (i.e. Map or Set) so that we + // don't have to check multiple different keys to gather all of the contributions. + indexDeclarationsByUnwrappedMultibindingKey(multibindings.build()), + indexDeclarationsByUnwrappedMultibindingKey(multibindingContributions(bindings.build())), + indexDeclarationsByUnwrappedMultibindingKey( + multibindingContributions(delegates.build()))); } /** @@ -182,5 +247,128 @@ private static boolean shouldIncludeImplicitProductionModules( ImmutableSetMultimap indexDeclarationsByKey(Iterable declarations) { return ImmutableSetMultimap.copyOf(Multimaps.index(declarations, BindingDeclaration::key)); } + + /** Indexes {@code bindingDeclarations} by the unwrapped multibinding key. */ + private ImmutableSetMultimap + indexDeclarationsByUnwrappedMultibindingKey(Iterable declarations) { + return ImmutableSetMultimap.copyOf( + Multimaps.index( + declarations, + declaration -> + unwrapMultibindingKey( + declaration.key().withoutMultibindingContributionIdentifier()))); + } + + private static ImmutableSet multibindingContributions( + ImmutableSet declarations) { + return declarations.stream() + .filter(declaration -> declaration.key().multibindingContributionIdentifier().isPresent()) + .collect(toImmutableSet()); + } + } + + /** + * Returns a {@link TypeNameKey} with the same qualifiers and multibinding identifier as the + * original key, but with an unwrapped typed. + * + *

In this case, an unwrapped type is a map or set where the value type has been stripped of a + * leading framework type. If the given type is neither a map nor set type, then the original type + * is returned. + * + *

The following map types have an unwrapped type equal to {@code Map}: + *

    + *
  • {@code Map} + *
  • {@code Map>} + *
  • {@code Map>} + *
  • {@code Map>} + *
+ * + *

The following set types have an unwrapped type equal to {@code Set}: + *

    + *
  • {@code Set} + *
  • {@code Set>} + *
+ */ + private static TypeNameKey unwrapMultibindingKey(Key multibindingKey) { + return TypeNameKey.from( + multibindingKey.multibindingContributionIdentifier(), + multibindingKey.qualifier(), + unwrapMultibindingTypeName(multibindingKey.type().xprocessing().getTypeName())); + } + + private static TypeName unwrapMultibindingTypeName(TypeName typeName) { + if (isValidMapMultibindingTypeName(typeName)) { + ParameterizedTypeName mapTypeName = (ParameterizedTypeName) typeName; + TypeName mapKeyTypeName = mapTypeName.typeArguments.get(0); + TypeName mapValueTypeName = mapTypeName.typeArguments.get(1); + return ParameterizedTypeName.get( + mapTypeName.rawType, + mapKeyTypeName, + unwrapFrameworkTypeName(mapValueTypeName, MAP_FRAMEWORK_TYPENAMES)); + } + if (isValidSetMultibindingTypeName(typeName)) { + ParameterizedTypeName setTypeName = (ParameterizedTypeName) typeName; + TypeName setValueTypeName = getOnlyElement(setTypeName.typeArguments); + return ParameterizedTypeName.get( + setTypeName.rawType, + unwrapFrameworkTypeName(setValueTypeName, SET_FRAMEWORK_TYPENAMES)); + } + return typeName; + } + + private static boolean isValidMapMultibindingTypeName(TypeName typeName) { + if (!(typeName instanceof ParameterizedTypeName)) { + return false; + } + ParameterizedTypeName parameterizedTypeName = (ParameterizedTypeName) typeName; + return parameterizedTypeName.rawType.equals(TypeNames.MAP) + && parameterizedTypeName.typeArguments.size() == 2 + && !(parameterizedTypeName.typeArguments.get(0) instanceof WildcardTypeName) + && !(parameterizedTypeName.typeArguments.get(1) instanceof WildcardTypeName); + } + + private static boolean isValidSetMultibindingTypeName(TypeName typeName) { + if (!(typeName instanceof ParameterizedTypeName)) { + return false; + } + ParameterizedTypeName parameterizedTypeName = (ParameterizedTypeName) typeName; + return parameterizedTypeName.rawType.equals(TypeNames.SET) + && parameterizedTypeName.typeArguments.size() == 1 + && !(getOnlyElement(parameterizedTypeName.typeArguments) instanceof WildcardTypeName); + } + + private static TypeName unwrapFrameworkTypeName( + TypeName typeName, ImmutableSet frameworkTypeNames) { + if (typeName instanceof ParameterizedTypeName) { + ParameterizedTypeName parameterizedTypeName = (ParameterizedTypeName) typeName; + if (frameworkTypeNames.contains(parameterizedTypeName.rawType)) { + typeName = getOnlyElement(parameterizedTypeName.typeArguments); + } + } + return typeName; + } + + /** + * Represents a class similar to {@link Key} but uses {@link TypeName} rather than {@code XType}. + * + *

We use {@code TypeName} rather than {@code XType} here because we can lose variance + * information when unwrapping an {@code XType} in KSP (b/352142595), and using {@code TypeName} + * avoids this issue. + */ + @AutoValue + abstract static class TypeNameKey { + static TypeNameKey from( + Optional multibindingContributionIdentifier, + Optional qualifier, + TypeName typeName) { + return new AutoValue_ComponentDeclarations_TypeNameKey( + multibindingContributionIdentifier, qualifier, typeName); + } + + abstract Optional multibindingContributionIdentifier(); + + abstract Optional qualifier(); + + abstract TypeName type(); } } diff --git a/java/dagger/internal/codegen/binding/KeyFactory.java b/java/dagger/internal/codegen/binding/KeyFactory.java index d7f78c16746..5b9265af1d3 100644 --- a/java/dagger/internal/codegen/binding/KeyFactory.java +++ b/java/dagger/internal/codegen/binding/KeyFactory.java @@ -24,8 +24,6 @@ import static dagger.internal.codegen.base.RequestKinds.extractKeyType; import static dagger.internal.codegen.binding.MapKeys.getMapKey; import static dagger.internal.codegen.extension.DaggerStreams.toImmutableList; -import static dagger.internal.codegen.extension.DaggerStreams.toImmutableSet; -import static dagger.internal.codegen.extension.Optionals.firstPresent; import static dagger.internal.codegen.javapoet.TypeNames.isFutureType; import static dagger.internal.codegen.xprocessing.XTypes.isDeclared; import static dagger.internal.codegen.xprocessing.XTypes.unwrapType; @@ -37,7 +35,6 @@ import androidx.room.compiler.processing.XProcessingEnv; import androidx.room.compiler.processing.XType; import androidx.room.compiler.processing.XTypeElement; -import com.google.common.collect.ImmutableSet; import com.squareup.javapoet.ClassName; import dagger.Binds; import dagger.BindsOptionalOf; @@ -56,9 +53,7 @@ import dagger.internal.codegen.model.RequestKind; import dagger.internal.codegen.xprocessing.XAnnotations; import dagger.multibindings.Multibinds; -import java.util.Map; import java.util.Optional; -import java.util.stream.Stream; import javax.inject.Inject; /** A factory for {@link Key}s. */ @@ -267,40 +262,6 @@ public Key forProductionComponentMonitor() { return forType(processingEnv.requireType(TypeNames.PRODUCTION_COMPONENT_MONITOR)); } - /** - * If {@code requestKey} is for a {@code Map} or {@code Map>}, returns keys - * for {@code Map>} and {@code Map>} (if Dagger-Producers is on the - * classpath). - */ - ImmutableSet implicitFrameworkMapKeys(Key requestKey) { - return Stream.of(implicitMapProviderKeyFrom(requestKey), implicitMapProducerKeyFrom(requestKey)) - .filter(Optional::isPresent) - .map(Optional::get) - .collect(toImmutableSet()); - } - - /** - * Optionally extract a {@link Key} for the underlying provision binding(s) if such a valid key - * can be inferred from the given key. Specifically, if the key represents a {@link Map}{@code } or {@code Map>}, a key of {@code Map>} will be returned. - */ - Optional implicitMapProviderKeyFrom(Key possibleMapKey) { - return firstPresent( - rewrapMapKey(possibleMapKey, TypeNames.PRODUCED, TypeNames.PROVIDER), - wrapMapKey(possibleMapKey, TypeNames.PROVIDER)); - } - - /** - * Optionally extract a {@link Key} for the underlying production binding(s) if such a valid key - * can be inferred from the given key. Specifically, if the key represents a {@link Map}{@code } or {@code Map>}, a key of {@code Map>} will be returned. - */ - Optional implicitMapProducerKeyFrom(Key possibleMapKey) { - return firstPresent( - rewrapMapKey(possibleMapKey, TypeNames.PRODUCED, TypeNames.PRODUCER), - wrapMapKey(possibleMapKey, TypeNames.PRODUCER)); - } - /** * If {@code key}'s type is {@code Map>}, {@code Map>}, or {@code * Map>}, returns a key with the same qualifier and {@link diff --git a/javatests/dagger/internal/codegen/MultibindingTest.java b/javatests/dagger/internal/codegen/MultibindingTest.java index 60cbfeff896..e9b403f6896 100644 --- a/javatests/dagger/internal/codegen/MultibindingTest.java +++ b/javatests/dagger/internal/codegen/MultibindingTest.java @@ -16,7 +16,6 @@ package dagger.internal.codegen; -import androidx.room.compiler.processing.XProcessingEnv; import androidx.room.compiler.processing.util.Source; import dagger.testing.compile.CompilerTests; import org.junit.Test; @@ -351,18 +350,129 @@ public void testMultibindingMapWithKotlinSource() { CompilerTests.daggerCompiler(parent, parentModule, myInterface, usage) .compile( subject -> { - // TODO(b/284207175): Due to a bug in our KSP implementation, KSP and Javac behave - // differently. Rather than cause a breaking change by fixing this bug directly, we've - // decided to wait until b/284207175 is implemented. - if (CompilerTests.backend(subject) == XProcessingEnv.Backend.KSP) { - subject.hasErrorCount(0); - } else { subject.hasErrorCount(1); subject.hasErrorContaining("Map cannot be provided"); - } }); } + // Regression test for b/352142595. + @Test + public void testMultibindingMapWithOutVarianceKotlinSource_succeeds() { + Source parent = + CompilerTests.kotlinSource( + "test.Parent.kt", + "package test", + "", + "import dagger.Component", + "", + "@Component(modules = [ParentModule::class])", + "interface Parent {", + " fun usage(): Usage", + "}"); + Source usage = + CompilerTests.kotlinSource( + "test.Usage.kt", + "package test", + "", + "import javax.inject.Inject", + "", + "class Usage @Inject constructor(", + " map: Map>", + ")"); + Source parentModule = + CompilerTests.kotlinSource( + "test.ParentModule.kt", + "@file:Suppress(\"INLINE_FROM_HIGHER_PLATFORM\")", // Required to use TODO() + "package test", + "", + "import dagger.Module", + "import dagger.Provides", + "import dagger.multibindings.IntoMap", + "import dagger.multibindings.StringKey", + "", + "@Module", + "class ParentModule {", + " @Provides", + " @IntoMap", + " @StringKey(\"key\")", + " fun provideMyInterface(): MyGenericInterface = TODO()", + "}"); + Source myGenericInterface = + CompilerTests.kotlinSource( + "test.MyGenericInterface.kt", + "package test", + "", + "interface MyGenericInterface"); + Source myInterface = + CompilerTests.kotlinSource( + "test.MyInterface.kt", + "package test", + "", + "interface MyInterface"); + + CompilerTests.daggerCompiler(parent, parentModule, myGenericInterface, myInterface, usage) + .compile(subject -> subject.hasErrorCount(0)); + } + + // Regression test for b/352142595. + @Test + public void testMultibindingMapWithJvmWildcardsKotlinSource_succeeds() { + Source parent = + CompilerTests.kotlinSource( + "test.Parent.kt", + "package test", + "", + "import dagger.Component", + "", + "@Component(modules = [ParentModule::class])", + "interface Parent {", + " fun usage(): Usage", + "}"); + Source usage = + CompilerTests.kotlinSource( + "test.Usage.kt", + "package test", + "", + "import javax.inject.Inject", + "", + "class Usage @Inject constructor(", + " map: Map>", + ")"); + Source parentModule = + CompilerTests.kotlinSource( + "test.ParentModule.kt", + "@file:Suppress(\"INLINE_FROM_HIGHER_PLATFORM\")", // Required to use TODO() + "package test", + "", + "import dagger.Module", + "import dagger.Provides", + "import dagger.multibindings.IntoMap", + "import dagger.multibindings.StringKey", + "", + "@Module", + "class ParentModule {", + " @Provides", + " @IntoMap", + " @StringKey(\"key\")", + " fun provideMyInterface(): MyGenericInterface<@JvmWildcard MyInterface> = TODO()", + "}"); + Source myGenericInterface = + CompilerTests.kotlinSource( + "test.MyGenericInterface.kt", + "package test", + "", + "interface MyGenericInterface"); + Source myInterface = + CompilerTests.kotlinSource( + "test.MyInterface.kt", + "package test", + "", + "interface MyInterface"); + + CompilerTests.daggerCompiler(parent, parentModule, myGenericInterface, myInterface, usage) + .compile(subject -> subject.hasErrorCount(0)); + } + // Regression test for b/352142595. @Test public void testMultibindingMapProviderWithKotlinSource() {