diff options
author | Eric Chang <erichang@google.com> | 2021-04-21 15:03:10 -0700 |
---|---|---|
committer | Dagger Team <dagger-dev+copybara@google.com> | 2021-04-21 15:04:21 -0700 |
commit | 115eaac88bf53c81dd9600041968074ac8db0e52 (patch) | |
tree | a465e4e19c624c83d0d609b3551f1721d80a53cd | |
parent | ffb045ae7aa2edbb6446439424ff07910343ae86 (diff) | |
download | dagger2-115eaac88bf53c81dd9600041968074ac8db0e52.tar.gz |
Fix an issue where internal Kotlin objects were not properly being processed. Consolidate logic to figure out if a module requires an instance.
RELNOTES=Fixes an issue where internal Kotlin object modules were incorrectly depended on directly by the component.
PiperOrigin-RevId: 369743121
5 files changed, 22 insertions, 25 deletions
diff --git a/java/dagger/hilt/processor/internal/Processors.java b/java/dagger/hilt/processor/internal/Processors.java index f33462dad..8740bd6a0 100644 --- a/java/dagger/hilt/processor/internal/Processors.java +++ b/java/dagger/hilt/processor/internal/Processors.java @@ -925,7 +925,10 @@ public final class Processors { return ElementFilter.methodsIn(elements.getAllMembers(module)).stream() .filter(Processors::isBindingMethod) .map(ExecutableElement::getModifiers) - .anyMatch(modifiers -> !modifiers.contains(ABSTRACT) && !modifiers.contains(STATIC)); + .anyMatch(modifiers -> !modifiers.contains(ABSTRACT) && !modifiers.contains(STATIC)) + // TODO(erichang): Getting a new KotlinMetadataUtil each time isn't great here, but until + // we have some sort of dependency management it will be difficult to share the instance. + && !KotlinMetadataUtils.getMetadataUtil().isObjectOrCompanionObjectClass(module); } private static boolean isBindingMethod(ExecutableElement method) { diff --git a/java/dagger/hilt/processor/internal/aggregateddeps/AggregatedDepsProcessor.java b/java/dagger/hilt/processor/internal/aggregateddeps/AggregatedDepsProcessor.java index ff66e5904..c45b373cb 100644 --- a/java/dagger/hilt/processor/internal/aggregateddeps/AggregatedDepsProcessor.java +++ b/java/dagger/hilt/processor/internal/aggregateddeps/AggregatedDepsProcessor.java @@ -37,10 +37,8 @@ import com.squareup.javapoet.ClassName; import dagger.hilt.processor.internal.BaseProcessor; import dagger.hilt.processor.internal.ClassNames; import dagger.hilt.processor.internal.Components; -import dagger.hilt.processor.internal.KotlinMetadataUtils; import dagger.hilt.processor.internal.ProcessorErrors; import dagger.hilt.processor.internal.Processors; -import dagger.internal.codegen.kotlin.KotlinMetadataUtil; import java.util.HashSet; import java.util.List; import java.util.Optional; @@ -167,7 +165,10 @@ public final class AggregatedDepsProcessor extends BaseProcessor { // Check that if Dagger needs an instance of the module, Hilt can provide it automatically by // calling a visible empty constructor. ProcessorErrors.checkState( - !daggerRequiresModuleInstance(module) || hasVisibleEmptyConstructor(module), + // Skip ApplicationContextModule, since Hilt manages this module internally. + ClassNames.APPLICATION_CONTEXT_MODULE.equals(ClassName.get(module)) + || !Processors.requiresModuleInstance(getElementUtils(), module) + || hasVisibleEmptyConstructor(module), module, "Modules that need to be instantiated by Hilt must have a visible, empty constructor."); @@ -444,27 +445,6 @@ public final class AggregatedDepsProcessor extends BaseProcessor { || name.contentEquals("javax.annotation.processing.Generated"); } - private static boolean daggerRequiresModuleInstance(TypeElement module) { - return !module.getModifiers().contains(ABSTRACT) - && !hasOnlyStaticProvides(module) - // Skip ApplicationContextModule, since Hilt manages this module internally. - && !ClassNames.APPLICATION_CONTEXT_MODULE.equals(ClassName.get(module)) - // Skip Kotlin object modules since all their provision methods are static - && !isKotlinObject(module); - } - - private static boolean isKotlinObject(TypeElement type) { - KotlinMetadataUtil metadataUtil = KotlinMetadataUtils.getMetadataUtil(); - return metadataUtil.isObjectClass(type) || metadataUtil.isCompanionObjectClass(type); - } - - private static boolean hasOnlyStaticProvides(TypeElement module) { - // TODO(erichang): Check for @Produces too when we have a producers story - return ElementFilter.methodsIn(module.getEnclosedElements()).stream() - .filter(method -> Processors.hasAnnotation(method, ClassNames.PROVIDES)) - .allMatch(method -> method.getModifiers().contains(STATIC)); - } - private static boolean hasVisibleEmptyConstructor(TypeElement type) { List<ExecutableElement> constructors = ElementFilter.constructorsIn(type.getEnclosedElements()); return constructors.isEmpty() diff --git a/java/dagger/internal/codegen/kotlin/KotlinMetadataUtil.java b/java/dagger/internal/codegen/kotlin/KotlinMetadataUtil.java index cf80f1a96..980ff3438 100644 --- a/java/dagger/internal/codegen/kotlin/KotlinMetadataUtil.java +++ b/java/dagger/internal/codegen/kotlin/KotlinMetadataUtil.java @@ -100,6 +100,11 @@ public final class KotlinMetadataUtil { && metadataFactory.create(typeElement).classMetadata().flags(IS_COMPANION_OBJECT); } + /** Returns {@code true} if this type element is a Kotlin object or companion object. */ + public boolean isObjectOrCompanionObjectClass(TypeElement typeElement) { + return isObjectClass(typeElement) || isCompanionObjectClass(typeElement); + } + /* Returns {@code true} if this type element has a Kotlin Companion Object. */ public boolean hasEnclosedCompanionObject(TypeElement typeElement) { return hasMetadata(typeElement) diff --git a/javatests/dagger/hilt/android/InternalKtModuleTest.java b/javatests/dagger/hilt/android/InternalKtModuleTest.java index b58309313..0b489896e 100644 --- a/javatests/dagger/hilt/android/InternalKtModuleTest.java +++ b/javatests/dagger/hilt/android/InternalKtModuleTest.java @@ -37,6 +37,7 @@ public final class InternalKtModuleTest { @Rule public final HiltAndroidRule rule = new HiltAndroidRule(this); @Inject String string; + @Inject Integer intValue; @Before public void setUp() { @@ -46,5 +47,6 @@ public final class InternalKtModuleTest { @Test public void testBindingFromInternalKtModule() { assertThat(string).isEqualTo("expected_string_value"); + assertThat(intValue).isEqualTo(9); } } diff --git a/javatests/dagger/hilt/testmodules/InternalKtTestModule.kt b/javatests/dagger/hilt/testmodules/InternalKtTestModule.kt index c78f3347f..0a71e445b 100644 --- a/javatests/dagger/hilt/testmodules/InternalKtTestModule.kt +++ b/javatests/dagger/hilt/testmodules/InternalKtTestModule.kt @@ -32,3 +32,10 @@ internal abstract class InternalKtTestModule private constructor() { fun provideString(): String = "expected_string_value" } } + +@Module +@InstallIn(SingletonComponent::class) +internal object InternalKtObjectModule { + @Provides + fun provideString(): Int = 9 +} |