Skip to content

Commit

Permalink
Fix an issue where internal Kotlin objects were not properly being pr…
Browse files Browse the repository at this point in the history
…ocessed. 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
  • Loading branch information
Chang-Eric authored and Dagger Team committed Apr 21, 2021
1 parent ffb045a commit 115eaac
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 25 deletions.
5 changes: 4 additions & 1 deletion java/dagger/hilt/processor/internal/Processors.java
Original file line number Diff line number Diff line change
Expand Up @@ -925,7 +925,10 @@ public static boolean requiresModuleInstance(Elements elements, TypeElement modu
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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,8 @@
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;
Expand Down Expand Up @@ -167,7 +165,10 @@ private void processModule(
// 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.");

Expand Down Expand Up @@ -444,27 +445,6 @@ private static boolean isGenerated(AnnotationMirror annotationMirror) {
|| 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()
Expand Down
5 changes: 5 additions & 0 deletions java/dagger/internal/codegen/kotlin/KotlinMetadataUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,11 @@ public boolean isCompanionObjectClass(TypeElement typeElement) {
&& 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)
Expand Down
2 changes: 2 additions & 0 deletions javatests/dagger/hilt/android/InternalKtModuleTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -46,5 +47,6 @@ public void setUp() {
@Test
public void testBindingFromInternalKtModule() {
assertThat(string).isEqualTo("expected_string_value");
assertThat(intValue).isEqualTo(9);
}
}
7 changes: 7 additions & 0 deletions javatests/dagger/hilt/testmodules/InternalKtTestModule.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

0 comments on commit 115eaac

Please sign in to comment.