fix: don't suggest removing runtime-required annotation libraries.

Resolves issue 1290

src/functionalTest/groovy/com/autonomousapps/jvm/AnnotationsImplementationSpec.groovy

@@ -2,6 +2,8 @@ package com.autonomousapps.jvm
 
 import com.autonomousapps.jvm.projects.AnnotationsCompileOnlyProject
 import com.autonomousapps.jvm.projects.AnnotationsImplementationProject
+import com.autonomousapps.jvm.projects.AnnotationsImplementationProject2
+import com.autonomousapps.utils.Colors
 
 import static com.autonomousapps.utils.Runner.build
 import static com.google.common.truth.Truth.assertThat
@@ -23,6 +25,58 @@ final class AnnotationsImplementationSpec extends AbstractJvmSpec {
     gradleVersion << gradleVersions()
   }
 
+  // https://github.com/autonomousapps/dependency-analysis-gradle-plugin/issues/1290
+  def "runtime-retained annotations are implementation (#gradleVersion)"() {
+    given:
+    def project = new AnnotationsImplementationProject2()
+    gradleProject = project.gradleProject
+
+    when:
+    build(gradleVersion, gradleProject.rootDir, 'buildHealth')
+
+    then:
+    assertThat(project.actualBuildHealth()).containsExactlyElementsIn(project.expectedBuildHealth)
+
+    when:
+    def result = build(gradleVersion, gradleProject.rootDir, 'consumer:reason', '--id', 'org.cthing:cthing-annotations')
+
+    then:
+    assertThat(Colors.decolorize(result.output)).contains(
+      '''\
+        ------------------------------------------------------------
+        You asked about the dependency 'org.cthing:cthing-annotations:1.0.0'.
+        There is no advice regarding this dependency.
+        ------------------------------------------------------------
+        
+        Shortest path from :consumer to org.cthing:cthing-annotations:1.0.0 for compileClasspath:
+        :consumer
+        \\--- org.cthing:cthing-annotations:1.0.0
+        
+        Shortest path from :consumer to org.cthing:cthing-annotations:1.0.0 for runtimeClasspath:
+        :consumer
+        \\--- org.cthing:cthing-annotations:1.0.0
+        
+        Shortest path from :consumer to org.cthing:cthing-annotations:1.0.0 for testCompileClasspath:
+        :consumer
+        \\--- org.cthing:cthing-annotations:1.0.0
+        
+        Shortest path from :consumer to org.cthing:cthing-annotations:1.0.0 for testRuntimeClasspath:
+        :consumer
+        \\--- org.cthing:cthing-annotations:1.0.0
+        
+        Source: main
+        ------------
+        * Uses (in an annotation) 1 class: org.cthing.annotations.PackageNonnullByDefault (implies implementation, sometimes).
+        
+        Source: test
+        ------------
+        (no usages)'''.stripIndent()
+    )
+
+    where:
+    gradleVersion << gradleVersions()
+  }
+
   def "classes used in compile-retained annotations are compileOnly (#gradleVersion)"() {
     given:
     def project = new AnnotationsCompileOnlyProject()

src/functionalTest/groovy/com/autonomousapps/jvm/projects/AnnotationsImplementationProject2.groovy

@@ -0,0 +1,67 @@
+package com.autonomousapps.jvm.projects
+
+import com.autonomousapps.AbstractProject
+import com.autonomousapps.kit.GradleProject
+import com.autonomousapps.kit.Source
+import com.autonomousapps.kit.gradle.Java
+import com.autonomousapps.model.ProjectAdvice
+
+import static com.autonomousapps.AdviceHelper.actualProjectAdvice
+import static com.autonomousapps.AdviceHelper.emptyProjectAdviceFor
+import static com.autonomousapps.kit.gradle.Dependency.implementation
+
+final class AnnotationsImplementationProject2 extends AbstractProject {
+
+  final GradleProject gradleProject
+
+  AnnotationsImplementationProject2() {
+    this.gradleProject = build()
+  }
+
+  private GradleProject build() {
+    return newGradleProjectBuilder()
+      .withSubproject('consumer') { s ->
+        s.sources = SOURCE_CONSUMER
+        s.withBuildScript { bs ->
+          bs.plugins = javaLibrary
+          bs.dependencies(
+            implementation('org.cthing:cthing-annotations:1.0.0'),
+          )
+
+          // "cthing" uses Java 17+
+          bs.java = Java.of(17)
+        }
+      }
+      .write()
+  }
+
+  private static final List<Source> SOURCE_CONSUMER = [
+    Source.java(
+      '''\
+        @PackageNonnullByDefault
+        package com.example.consumer;
+        
+        import org.cthing.annotations.PackageNonnullByDefault;
+      '''
+    )
+      .withPath('com.example.consumer', 'package-info')
+      .build(),
+    Source.java(
+      '''\
+      package com.example.consumer;
+      
+      public class Consumer {}
+    '''
+    )
+      .withPath('com.example.consumer', 'Consumer')
+      .build()
+  ]
+
+  Set<ProjectAdvice> actualBuildHealth() {
+    return actualProjectAdvice(gradleProject)
+  }
+
+  final Set<ProjectAdvice> expectedBuildHealth = [
+    emptyProjectAdviceFor(':consumer'),
+  ]
+}

src/main/kotlin/com/autonomousapps/model/intermediates/Reason.kt

@@ -111,7 +111,7 @@ internal sealed class Reason(open val reason: String) {
     )
 
     // TODO: ugh.
-    override val configurationName: String = "implementation (sometimes)"
+    override val configurationName: String = "implementation, sometimes"
   }
 
   @TypeLabel("imported")

src/main/kotlin/com/autonomousapps/tasks/ComputeUsagesTask.kt

@@ -121,6 +121,7 @@ private class GraphVisitor(
     var isUnusedCandidate = false
     var isLintJar = false
     var isCompileOnlyCandidate = false
+    var isRequiredAnnotationCandidate = false
     var isRuntimeAndroid = false
     var usesTestInstrumentationRunner = false
     var usesResBySource = false
@@ -166,7 +167,7 @@ private class GraphVisitor(
           } else if (isImported(dependencyCoordinates, capability, context)) {
             isImplByImportCandidate = true
           } else if (usesAnnotation(dependencyCoordinates, capability, context)) {
-            // TODO: Nothing to actually do I think? But this does avoid setting isUnusedCandidate to true.
+            isRequiredAnnotationCandidate = true
           } else {
             isUnusedCandidate = true
           }
@@ -245,6 +246,10 @@ private class GraphVisitor(
       reportBuilder[dependencyCoordinates, Kind.DEPENDENCY] = Bucket.COMPILE_ONLY
     } else if (isImplByImportCandidate) {
       reportBuilder[dependencyCoordinates, Kind.DEPENDENCY] = Bucket.IMPL
+    } else if (isRequiredAnnotationCandidate) {
+      // We detected an annotation, but it's a RUNTIME annotation, so we can't suggest it be moved to compileOnly.
+      // Don't suggest removing it!
+      reportBuilder[dependencyCoordinates, Kind.DEPENDENCY] = Bucket.IMPL
     } else if (noRealCapabilities(dependency)) {
       isUnusedCandidate = true
     }
@@ -342,8 +347,6 @@ private class GraphVisitor(
     }.toSortedSet()
 
     return if (annoClasses.isNotEmpty()) {
-      // TODO(tsr): this reason is correct, but maybe we could offer a bit more detail now that we know it's used as an
-      //  annotation.
       reportBuilder[coordinates, Kind.DEPENDENCY] = Reason.Annotation(annoClasses)
       true
     } else {