dependency-analysis-gradle-plugin icon indicating copy to clipboard operation
dependency-analysis-gradle-plugin copied to clipboard

CompileOnly false positive with annotations used at runtime

Open JordanLongstaff opened this issue 1 year ago • 1 comments
trafficstars

Plugin version 1.32.0

Gradle version 8.7

JDK version 17

(Optional) Kotlin and Kotlin Gradle Plugin (KGP) version 2.0.0

(Optional) Android Gradle Plugin (AGP) version AGP 8.5.0

(Optional) reason output for bugs relating to incorrect advice

> Task :IAN:konsist:reason

----------------------------------------
You asked about the dependency ':IAN:annotations'.
You have been advised to change this dependency to 'testCompileOnly' from 'testImplementation'.
----------------------------------------

There is no path from :IAN:konsist to ArtemisAgent.IAN:annotations for compileClasspath


There is no path from :IAN:konsist to ArtemisAgent.IAN:annotations for runtimeClasspath


Shortest path from :IAN:konsist to ArtemisAgent.IAN:annotations for testCompileClasspath:
:IAN:konsist
\--- ArtemisAgent.IAN:annotations

Shortest path from :IAN:konsist to ArtemisAgent.IAN:annotations for testRuntimeClasspath:
:IAN:konsist
\--- ArtemisAgent.IAN:annotations

Source: main
------------
(no usages)

Source: test
------------
* Provides compile-time annotations (implies testCompileOnly).
* Uses 2 classes: com.walkertribe.ian.protocol.PacketSubtype, com.walkertribe.ian.protocol.PacketType (implies testCompileOnly).

Describe the bug The plugin is giving me advice to change a dependency to compileOnly or testCompileOnly. The dependency is on a module that contains only annotation classes, so that is the reasoning given for it. However, the classes are being used at runtime, which makes it impossible to make the dependency compile-only; if I do, I get a NoClassDefFoundError at runtime.

To Reproduce Steps to reproduce the behavior:

  1. Make a multi-module project, with one of the modules containing only annotation definitions.
  2. Add the annotation module to the main module as an implementation or api dependency.
  3. In the main module, write some code that uses the annotation classes for some purpose other than as annotations (e.g. Annotation::class).
  4. When running buildHealth or projectHealth, the plugin will tell you that the dependency can be changed to compile-only.

Expected behavior The plugin shouldn't be telling me that the dependency is compile-only, because it obviously isn't; the classes are being used at runtime. The advice to make it compile-only should not be there.

JordanLongstaff avatar Jul 06 '24 16:07 JordanLongstaff

Thanks for the issue. The plugin uses a heuristic where it checks the retention policy on the annotations. If they're all CLASS or SOURCE, then it says the dependency can be compileOnly. Your analysis seems correct -- I think the plugin is simply missing the case where someone might have a class reference to an annotation class outside of the context of using it like an annotation.

autonomousapps avatar Jul 13 '24 05:07 autonomousapps

This is going to be pretty hard to resolve, given current assumptions and implementation. Due to type erasure, when you have a reference like Class<MyAnnotation>, the bytecode only contains the java/lang/Class reference itself. DAGP can only tell that the MyAnnotation type is present (as something other than an annotation) because it parses import statements, so it'll see import com.example.MyAnnotation and know that that type is used, but not how. It tries to be conservative, but since the dependency in question only contains CLASS- or SOURCE-level annotations (as described above), it still tries to move that to compileOnly.

Currently your best option is to just exclude the advice saying to change this dependency. A proper fix would be a lot of work, which I don't have time for right now. I am open to sponsorships, however.

Note to self for future reference: more comprehensive source-code parsing would resolve this (vs just grabbing import statements).

autonomousapps avatar Sep 13 '24 23:09 autonomousapps

I actually found a way to do this. The associated PR should resolve this issue 👍

autonomousapps avatar Sep 17 '24 16:09 autonomousapps