Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugin fails to analyze Dagger usages #479

Closed
gabrielittner opened this issue Sep 19, 2021 · 7 comments
Closed

Plugin fails to analyze Dagger usages #479

gabrielittner opened this issue Sep 19, 2021 · 7 comments
Labels
bug Something isn't working
Milestone

Comments

@gabrielittner
Copy link
Contributor

Build scan link

Reproducer kapt-dagger.zip

Plugin version

0.78.0

Gradle version

7.0 and 7.2

Describe the bug

When Dagger is used with kapt the plugin will print Could not reflectively access processor class dagger.internal.codegen.ComponentProcessor and will then go ahead to suggest

Plugin advice:
- kotlin-kapt: this project has the kotlin-kapt (org.jetbrains.kotlin.kapt) plugin applied, but no annotation processors (or no used annotation processors), which is redundant.

To Reproduce
Steps to reproduce the behavior:

  1. Add Dagger as a kapt dependency

Expected behavior

Plugin does not fail to access the Dagger processor and does not suggest to remove kapt.

Additional context

I wonder if the behavior should generally be different if the access to a processor class fails. Maybe a different message for the plugin advice? In bigger builds it's easy to miss the "Could not reflectively access ..." message. I can open a feature request for that if you think it makes sense.

@autonomousapps autonomousapps added the bug Something isn't working label Sep 19, 2021
@autonomousapps autonomousapps added this to the 1.0 milestone Sep 19, 2021
@autonomousapps
Copy link
Owner

Thanks for the reproducer. I actually just noticed this myself the other day but didn't have time to investigate. This makes it so much easier.

Regarding "Could not reflectively access...", what do you think would be a better solution? I'm open to improving this use-case. Warnings-as-errors? Something else?

@gabrielittner
Copy link
Contributor Author

I was thinking that the plugin advice would say something like "Could not determine whether kapt is used because processor ... could not be accessed" instead of recommending the removal of kapt. That way even if you miss the log beforehand it's clear from the actual output. Currently you'd probably try removing kapt and then realize it was actually needed when things don't build anymore. It could still use the plugin advice severity

@autonomousapps
Copy link
Owner

That might be feasible. Would you mind filing a separate feature request for that?

@autonomousapps
Copy link
Owner

autonomousapps commented Sep 22, 2021

This is pretty interesting. The dagger AP has a pretty complex initialization compared to other APs I've looked at. It even uses Dagger itself (sigh). Here's the top of the stacktrace from the failing init() call:

> Task :settings:findDeclaredProcsDebug                                                                                                                                      
Trying to initialize annotation processor with type dagger.internal.codegen.ComponentProcessor                                   
Could not initialize dagger.internal.codegen.ComponentProcessor. May not be able to get supported annotation types.          
java.lang.NoSuchMethodError: 'java.lang.Object dagger.internal.Preconditions.checkNotNullFromProvides(java.lang.Object)'                                                     
        at dagger.internal.codegen.ProcessingEnvironmentModule_DaggerElementsFactory.daggerElements(ProcessingEnvironmentModule_DaggerElementsFactory.java:39)
        at dagger.internal.codegen.ProcessingEnvironmentModule_DaggerElementsFactory.get(ProcessingEnvironmentModule_DaggerElementsFactory.java:30)
        at dagger.internal.codegen.ProcessingEnvironmentModule_DaggerElementsFactory.get(ProcessingEnvironmentModule_DaggerElementsFactory.java:11)
        at dagger.internal.DoubleCheck.get(DoubleCheck.java:47)                                                                                                              
        at dagger.internal.codegen.validation.InjectBindingRegistryImpl_Factory.get(InjectBindingRegistryImpl_Factory.java:54)              
        at dagger.internal.codegen.validation.InjectBindingRegistryImpl_Factory.get(InjectBindingRegistryImpl_Factory.java:14)                  
        at dagger.internal.DoubleCheck.get(DoubleCheck.java:47)                                                                                                              
        at dagger.internal.codegen.DaggerComponentProcessor_ProcessorComponent.injectComponentProcessor(DaggerComponentProcessor_ProcessorComponent.java:741)
        at dagger.internal.codegen.DaggerComponentProcessor_ProcessorComponent.inject(DaggerComponentProcessor_ProcessorComponent.java:736)
        at dagger.internal.codegen.ComponentProcessor.processingSteps(ComponentProcessor.java:127)                 
        at dagger.shaded.androidx.room.compiler.processing.javac.JavacBasicAnnotationProcessor.steps(JavacBasicAnnotationProcessor.kt:41)
        at dagger.shaded.auto.common.BasicAnnotationProcessor.init(BasicAnnotationProcessor.java:121)                                    
        at com.autonomousapps.tasks.FindDeclaredProcsTask.tryInit(FindDeclaredProcsTask.kt:166)

So, at the very least, it's choking on the NoSuchMethodError because it can't find dagger.internal.Preconditions.checkNotNullFromProvides. This makes sense, because I'm loading the AP class dynamically at runtime, in a classloader that doesn't also have this other required class. I dug through the decompiled bytecode a bit (for some reason, IDEA can't download the source jars?), and in this case, it is required to call init() in order to then call getSupportedAnnotationTypes().

So there are a couple of options:

  1. Punt and create a list of known annotation types that dagger supports. Meh.
  2. Treat dagger a bit specially, which may be warranted given its prominence, and try to appease it by creating a custom classloader and loading more stuff at runtime.
  3. Something else??

Turns out that getting the supported annotation types out of dagger requires two additional dependencies on the classpath:

  1. com.google.dagger:dagger:2.38.1
  2. com.squareup:javapoet:1.13.0

The versions would obviously have to be variable -- wouldn't want to hardcode that. Gradle workers with isolated classloaders seem in order.

And these are the types if I decide to punt:

0 = "dagger.MapKey"
1 = "javax.inject.Inject"
2 = "dagger.assisted.AssistedInject"
3 = "dagger.assisted.AssistedFactory"
4 = "dagger.assisted.Assisted"
5 = "dagger.producers.ProductionComponent"
6 = "dagger.producers.ProductionSubcomponent"
7 = "dagger.multibindings.IntoSet"
8 = "dagger.multibindings.ElementsIntoSet"
9 = "dagger.multibindings.IntoMap"
10 = "dagger.BindsInstance"
11 = "dagger.Module"
12 = "dagger.producers.ProducerModule"
13 = "dagger.Component"
14 = "dagger.Subcomponent"
15 = "dagger.Component.Builder"
16 = "dagger.Component.Factory"
17 = "dagger.Subcomponent.Builder"
18 = "dagger.Subcomponent.Factory"
19 = "dagger.producers.ProductionComponent.Builder"
20 = "dagger.producers.ProductionComponent.Factory"
21 = "dagger.producers.ProductionSubcomponent.Builder"
22 = "dagger.producers.ProductionSubcomponent.Factory"
23 = "dagger.Binds"
24 = "dagger.multibindings.Multibinds"
25 = "dagger.Provides"
26 = "dagger.producers.Produces"
27 = "dagger.BindsOptionalOf"

Actually it turns out the problem is somehow more complicated. There already is a dagger.internal.Preconditions class on the classpath, but it's from a parent classloader and is missing the required method. Presumably something similar for JavaPoet?

A-ha, found it: https://scans.gradle.com/s/ihmwblmcab6y4/build-dependencies?dependencies=dagger&expandAll&focusedDependency=WzAsMCwzNzksWzAsMCxbMF1dXQ&focusedDependencyView=versions

Screenshot from 2021-09-22 00-18-50

This project's dependency (not the plugin's!) on AGP brings along an older version of dagger. Because java classloading delegates to the parent first, when trying to load the dagger AP for analysis, when it needs to access the Preconditions class, it finds it first in the parent, which is an older version missing a required method.

@autonomousapps
Copy link
Owner

@gabrielittner you can workaround this right now by updating your build.gradle:

buildscript {
  repositories {
    mavenCentral()
  }
  dependencies {
    classpath("com.google.dagger:dagger:2.38.1")
    classpath("com.squareup:javapoet:1.13.0")
  }
}

The annotation processor analysis is failing because it can't initialize the dagger AP due to classes from these dependencies missing from the build classpath. I will be working on resolving this transparently.

@gabrielittner
Copy link
Contributor Author

Thanks for the workaround, that's helpful 👍

I've opened the extra feature request for our message discussion.

@autonomousapps
Copy link
Owner

Resolved, will be in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants