google / dagger

A fast dependency injector for Android and Java.
https://dagger.dev
Apache License 2.0
17.43k stars 2.01k forks source link

Improve error message for transitive types #2123

Open ansman opened 4 years ago

ansman commented 4 years ago

Say you have 3 modules, :app, :libraryA and :libraryB.

:app depends on :libraryA which in turn depends :libraryB using the implementation configuration. If :libraryB has some dagger modules or components you can get this cryptic error message:

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':app:kaptReleaseKotlin'.
> A failure occurred while executing org.jetbrains.kotlin.gradle.internal.KaptExecution
   > java.lang.reflect.InvocationTargetException (no error message)

Running with --stacktrace gives slightly more information:

java.lang.IllegalArgumentException: com.example.libraryb.SomeType does not represent a declared type
    at dagger.shaded.auto.common.MoreTypes$CastingTypeVisitor.defaultAction(MoreTypes.java:948)
    at dagger.shaded.auto.common.MoreTypes$CastingTypeVisitor.defaultAction(MoreTypes.java:939)
    at jdk.compiler/com.sun.tools.javac.code.Type$ErrorType.accept(Type.java:2386)
    at dagger.shaded.auto.common.MoreTypes.asDeclared(MoreTypes.java:579)
    at dagger.internal.codegen.base.Keys$1.visitDeclared(Keys.java:85)
    at dagger.internal.codegen.base.Keys$1.visitDeclared(Keys.java:62)
    at jdk.compiler/com.sun.tools.javac.code.Type$ClassType.accept(Type.java:1151)
    at dagger.internal.codegen.base.Keys.isValidImplicitProvisionKey(Keys.java:61)
    at dagger.internal.codegen.base.Keys.isValidImplicitProvisionKey(Keys.java:46)
    at dagger.internal.codegen.validation.InjectBindingRegistryImpl.getOrFindProvisionBinding(InjectBindingRegistryImpl.java:300)
    at dagger.internal.codegen.binding.BindingGraphFactory$Resolver.lookUpBindings(BindingGraphFactory.java:410)
    at dagger.internal.codegen.binding.BindingGraphFactory$Resolver.resolve(BindingGraphFactory.java:919)
    at dagger.internal.codegen.binding.BindingGraphFactory$Resolver.resolveDependencies(BindingGraphFactory.java:934)
    at dagger.internal.codegen.binding.BindingGraphFactory$Resolver.resolveMembersInjection(BindingGraphFactory.java:877)
    at dagger.internal.codegen.binding.BindingGraphFactory$Resolver.access$1300(BindingGraphFactory.java:310)
    at dagger.internal.codegen.binding.BindingGraphFactory.lambda$createLegacyBindingGraph$4(BindingGraphFactory.java:214)
    at dagger.internal.codegen.binding.BindingGraphFactory.createLegacyBindingGraph(BindingGraphFactory.java:211)
    at dagger.internal.codegen.binding.BindingGraphFactory.createLegacyBindingGraph(BindingGraphFactory.java:240)
    at dagger.internal.codegen.binding.BindingGraphFactory.create(BindingGraphFactory.java:118)
    at dagger.internal.codegen.ComponentProcessingStep.processRootComponent(ComponentProcessingStep.java:115)
    at dagger.internal.codegen.ComponentProcessingStep.process(ComponentProcessingStep.java:93)
    at dagger.internal.codegen.ComponentProcessingStep.process(ComponentProcessingStep.java:53)
    at dagger.internal.codegen.validation.TypeCheckingProcessingStep.lambda$process$0(TypeCheckingProcessingStep.java:51)
    at com.google.common.collect.RegularImmutableMap.forEach(RegularImmutableMap.java:185)
    at dagger.internal.codegen.validation.TypeCheckingProcessingStep.process(TypeCheckingProcessingStep.java:48)
    at dagger.internal.codegen.validation.TypeCheckingProcessingStep.process(TypeCheckingProcessingStep.java:34)
    at dagger.internal.codegen.statistics.DaggerStatisticsCollectingProcessingStep.process(DaggerStatisticsCollectingProcessingStep.java:52)
    at dagger.shaded.auto.common.BasicAnnotationProcessor$ProcessingStepAsStep.process(BasicAnnotationProcessor.java:495)
    at dagger.shaded.auto.common.BasicAnnotationProcessor.process(BasicAnnotationProcessor.java:228)
    at dagger.shaded.auto.common.BasicAnnotationProcessor.process(BasicAnnotationProcessor.java:208)
    at org.jetbrains.kotlin.kapt3.base.incremental.IncrementalProcessor.process(incrementalProcessors.kt)
    at org.jetbrains.kotlin.kapt3.base.ProcessorWrapper.process(annotationProcessing.kt:161)
    at jdk.compiler/com.sun.tools.javac.processing.JavacProcessingEnvironment.callProcessor(JavacProcessingEnvironment.java:980)
    ... 37 more

This is of course an error in the setup and :app should either depend on :libraryB directly or you should use api but it's very difficult to determine what module is causing issue. You see which type it related too but not really which file is being processed.

In any case I think there is room improvement here.

Kshitij09 commented 3 years ago

What is the solution on this problem? You mean I either app should have direct dependency (adding implementation) on :libraryB or :libraryA have api dependency on :libraryB right?

Don't you think this solution is anti-pattern? what's the point of creating separate modules then?

bcorso commented 3 years ago

You mean I either app should have direct dependency (adding implementation) on :libraryB or :libraryA have api dependency on :libraryB right?

Hi @Kshitij09, yes that's the current solution and we're looking into ways to improve this (see https://github.com/google/dagger/issues/1991#issuecomment-661245887)

Don't you think this solution is anti-pattern? what's the point of creating separate modules then?

Admittedly, the current situation is not great (which is why we're looking into ways to fix this); however, there are other benefits to modules as explained in https://github.com/google/dagger/issues/1991#issuecomment-704950480.

ritesh-singh commented 3 years ago

Any update on improving the error messages for transitive types. I recently came across the same issue, and from stack-trace it's not clear why it's happening in multi-module project.

vassilisimon commented 3 years ago

Would also appreciate it. Currently it's for me not possible to set up end to end testing with espresso because of that problem, which I can't solve.

harshfast commented 3 years ago

What is the solution on this problem? You mean I either app should have direct dependency (adding implementation) on :libraryB or :libraryA have api dependency on :libraryB right?

Don't you think this solution is anti-pattern? what's the point of creating separate modules then?

Yes, even I also faced the same problem. My architecture is :app <- :usecase <- :repository <- datastore and not able add Hilt as DI in this kind of architecture at lower layer modules. According to documentation it says "The Gradle module that compiles your Application class needs to have all Hilt modules and constructor-injected classes in its transitive dependencies."

I need to do workaround as :app module should have all the dependencies of all other modules. Which I also felt it is going antipattern. :app gradle should have :usecase :repository :datastore :anyOtherFeature

bcorso commented 3 years ago

If you're using Hilt (with Gradle) the solution is to use the Hilt Gradle plugin and then enable the aggregating task in your build.gradle modules:

hilt {
    enableAggregatingTask = true
}
harshfast commented 3 years ago

then ena

@bcorso Thanks for the quick resolution. It worked!! now I removed all other dependencies for ;app module. I feel Hilt documentation is misleading as first line itself says "The Gradle module that compiles your Application class needs to have all Hilt modules and constructor-injected classes in its transitive dependencies."

hamid97m commented 3 years ago

then ena

@bcorso Thanks for the quick resolution. It worked!! now I removed all other dependencies for ;app module. I feel Hilt documentation is misleading as first line itself says "The Gradle module that compiles your Application class needs to have all Hilt modules and constructor-injected classes in its transitive dependencies."

Really thanks! saved my day !

melihaksoy commented 2 years ago

Does dagger2 have any flag for aggregation or is it only for hilt ?

mahdizareeii commented 2 years ago

If you're using Hilt (with Gradle) the solution is to use the Hilt Gradle plugin and then enable the aggregating task in your build.gradle modules:

hilt {
    enableAggregatingTask = true
}

thanks, it worked for me