fix: compileOnly dependencies are not visible to the test compile cla…

…sspath.

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

@@ -5,6 +5,7 @@ package com.autonomousapps.jvm
 import com.autonomousapps.jvm.projects.CompileOnlyJarProject
 import com.autonomousapps.jvm.projects.CompileOnlyProject
 import com.autonomousapps.jvm.projects.CompileOnlyProject2
+import com.autonomousapps.jvm.projects.CompileOnlyTestImplementationProject
 import com.autonomousapps.jvm.projects.WarTestProject
 
 import static com.autonomousapps.utils.Runner.build
@@ -58,6 +59,28 @@ final class CompileOnlySpec extends AbstractJvmSpec {
     gradleVersion << gradleVersions()
   }
 
+  def "compileOnly is not propagated to testImplementation (#gradleVersion)"() {
+    given:
+    def project = new CompileOnlyTestImplementationProject()
+    gradleProject = project.gradleProject
+
+    when:
+    def result = build(
+      gradleVersion, gradleProject.rootDir,
+      'buildHealth',
+      ':lib:reason', '--id', 'org.apache.commons:commons-collections4',
+    )
+
+    then: 'advice is correct'
+    assertThat(project.actualBuildHealth()).containsExactlyElementsIn(project.expectedBuildHealth)
+
+    and: 'reason makes sense'
+    assertThat(result.output).contains('There is no advice regarding this dependency.')
+
+    where:
+    gradleVersion << gradleVersions()
+  }
+
   // The plugin cannot decide if something that is required for compilation is only needed at compile time.
   // Currently, such dependencies produce no advice at all. In the future the plugin could:
   // - Give an advice if one of these dependencies can be removed completely

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

@@ -0,0 +1,85 @@
+package com.autonomousapps.jvm.projects
+
+import com.autonomousapps.AbstractProject
+import com.autonomousapps.kit.GradleProject
+import com.autonomousapps.kit.Source
+import com.autonomousapps.model.ProjectAdvice
+
+import static com.autonomousapps.AdviceHelper.actualProjectAdvice
+import static com.autonomousapps.AdviceHelper.emptyProjectAdviceFor
+import static com.autonomousapps.kit.gradle.dependencies.Dependencies.commonsCollections
+
+/**
+ * The {@code testImplementation} configuration does not extend from the {@code compileOnly} configuration. So, it is
+ * inaccurate to suggest removing {@code testImplementation} dependencies just because they're on the compile classpath.
+ *
+ * @see <a href="https://docs.gradle.org/current/userguide/java_plugin.html#resolvable_configurations">Java configurations</a>
+ */
+final class CompileOnlyTestImplementationProject extends AbstractProject {
+
+  final GradleProject gradleProject
+
+  CompileOnlyTestImplementationProject() {
+    this.gradleProject = build()
+  }
+
+  private GradleProject build() {
+    return newGradleProjectBuilder()
+      .withSubproject('lib') { s ->
+        s.sources = SOURCES
+        s.withBuildScript { bs ->
+          bs.plugins = javaLibrary
+          bs.dependencies = [
+            // These two configurations are independent!
+            commonsCollections('compileOnly'),
+            commonsCollections('testImplementation'),
+          ]
+        }
+      }
+      .write()
+  }
+
+  private static final List<Source> SOURCES = [
+    Source.java(
+      '''\
+        package com.example.lib;
+        
+        import org.apache.commons.collections4.Bag;
+        import org.apache.commons.collections4.bag.HashBag;
+        
+        public class Lib {
+          private Bag<String> newBag() {
+            return new HashBag<>();
+          }
+        }
+      '''
+    )
+      .withPath('com.example.lib', 'Lib')
+      .build(),
+    Source.java(
+      '''\
+        package com.example.lib;
+        
+        import org.apache.commons.collections4.Bag;
+        import org.apache.commons.collections4.bag.HashBag;
+        
+        public class LibTest {
+          private void test() {
+            Bag<String> bag = new HashBag<>();
+          }
+        }
+      '''
+    )
+      .withPath('com.example.lib', 'LibTest')
+      .withSourceSet('test')
+      .build(),
+  ]
+
+  Set<ProjectAdvice> actualBuildHealth() {
+    return actualProjectAdvice(gradleProject)
+  }
+
+  final Set<ProjectAdvice> expectedBuildHealth = [
+    emptyProjectAdviceFor(':lib'),
+  ]
+}

src/main/kotlin/com/autonomousapps/model/declaration/internal/Bucket.kt

@@ -2,13 +2,19 @@
 // SPDX-License-Identifier: Apache-2.0
 package com.autonomousapps.model.declaration.internal
 
+import com.autonomousapps.internal.utils.reallyAll
+import com.autonomousapps.model.internal.intermediates.Usage
 import com.squareup.moshi.JsonClass
 
-/** Standard user-facing dependency buckets (such as **implementation** and **api**), [variant][Variant]-agnostic. */
+/**
+ *  Standard user-facing dependency buckets (such as **implementation** and **api**),
+ * [variant][com.autonomousapps.model.declaration.Variant]-agnostic.
+ */
 @JsonClass(generateAdapter = false)
 internal enum class Bucket(val value: String) {
   API("api"),
   IMPL("implementation"),
+
   // These configurations go into the compileOnly bucket: '...CompileOny', '...CompileOnlyApi', 'providedCompile'
   COMPILE_ONLY("compileOnly"),
   RUNTIME_ONLY("runtimeOnly"),
@@ -37,9 +43,30 @@ internal enum class Bucket(val value: String) {
     }
 
     /**
-     * [Declarations][Declaration] in these buckets are visible from [SourceSetKind.MAIN] to [SourceSetKind.TEST] and
-     * [SourceSetKind.ANDROID_TEST]. This is necessary for correct advice relating to test source.
+     * [Declarations][Declaration] in these buckets are visible from
+     * [SourceSetKind.MAIN][com.autonomousapps.model.declaration.SourceSetKind.MAIN] to
+     * [SourceSetKind.TEST][com.autonomousapps.model.declaration.SourceSetKind.TEST] and
+     * [SourceSetKind.ANDROID_TEST][com.autonomousapps.model.declaration.SourceSetKind.ANDROID_TEST]. This is necessary
+     * for correct advice relating to test source.
+     *
+     * TODO(tsr): wait, is this actually true for ANNOTATION_PROCESSOR as well? That seems like an error. Oh, maybe it
+     *  was true for an older version of Gradle?
      */
-    val VISIBLE_TO_TEST_SOURCE = listOf(API, IMPL, ANNOTATION_PROCESSOR)
+    private val VISIBLE_TO_TEST_SOURCE = listOf(API, IMPL, ANNOTATION_PROCESSOR)
+
+    /**
+     * A dependency is visible from main to test source iff it is in the correct bucket ([VISIBLE_TO_TEST_SOURCE]) _and_
+     * if it is declared on either the [API] or [IMPL] configurations.
+     *
+     * Note that the `compileOnly` configuration _is not_ visible to the `testImplementation` configuration.
+     *
+     * @see <a href="https://docs.gradle.org/current/userguide/java_plugin.html#resolvable_configurations.">Java configurations</a>
+     */
+    fun isVisibleToTestSource(usages: Set<Usage>, declarations: Set<Declaration>): Boolean {
+      return usages.reallyAll { usage ->
+        VISIBLE_TO_TEST_SOURCE.any { it == usage.bucket }
+          && declarations.any { API.matches(it) || IMPL.matches(it) }
+      }
+    }
   }
 }

src/main/kotlin/com/autonomousapps/transform/StandardTransform.kt

@@ -42,7 +42,7 @@ internal class StandardTransform(
       { it.variant.kind == SourceSetKind.MAIN },
       { it.variant.kind == SourceSetKind.TEST },
       { it.variant.kind == SourceSetKind.ANDROID_TEST },
-      { it.variant.kind == SourceSetKind.CUSTOM_JVM }
+      { it.variant.kind == SourceSetKind.CUSTOM_JVM },
     )
 
     val hasCustomSourceSets = hasCustomSourceSets(usages)
@@ -51,17 +51,15 @@ internal class StandardTransform(
         { it.variant(supportedSourceSets, hasCustomSourceSets)?.kind == SourceSetKind.MAIN },
         { it.variant(supportedSourceSets, hasCustomSourceSets)?.kind == SourceSetKind.TEST },
         { it.variant(supportedSourceSets, hasCustomSourceSets)?.kind == SourceSetKind.ANDROID_TEST },
-        { it.variant(supportedSourceSets, hasCustomSourceSets)?.kind == SourceSetKind.CUSTOM_JVM }
+        { it.variant(supportedSourceSets, hasCustomSourceSets)?.kind == SourceSetKind.CUSTOM_JVM },
       )
 
     /*
      * Main usages.
      */
 
     val singleVariant = mainUsages.size == 1
-    val isMainVisibleDownstream = mainUsages.reallyAll { usage ->
-      Bucket.VISIBLE_TO_TEST_SOURCE.any { it == usage.bucket }
-    }
+    val isMainVisibleDownstream = Bucket.isVisibleToTestSource(mainUsages, mainDeclarations)
 
     mainUsages = reduceUsages(mainUsages)
     computeAdvice(advice, mainUsages, mainDeclarations, singleVariant)

src/test/kotlin/com/autonomousapps/transform/StandardTransformTest.kt

@@ -1093,7 +1093,7 @@ internal class StandardTransformTest {
       )
     }
 
-    @Test fun `should be declared on implementation, not testImplementation`() {
+    @Test fun `cannot be removed from testImplementation`() {
       val id = "com.foo:bar"
       val usages = setOf(
         usage(bucket = Bucket.IMPL, variant = "debug", kind = SourceSetKind.MAIN),
@@ -1102,6 +1102,11 @@ internal class StandardTransformTest {
         usage(bucket = Bucket.IMPL, variant = "release", kind = SourceSetKind.TEST),
       )
       val declarations = setOf(
+        Declaration(
+          identifier = id,
+          configurationName = "compileOnly",
+          gradleVariantIdentification = emptyGVI
+        ),
         Declaration(
           identifier = id,
           configurationName = "testImplementation",
@@ -1119,12 +1124,10 @@ internal class StandardTransformTest {
         isKaptApplied = false,
       ).reduce(usages)
 
-      assertThat(actual).containsExactly(
-        Advice.ofChange(ModuleCoordinates(id, "1.0", emptyGVI), "testImplementation", "implementation"),
-      )
+      assertThat(actual).isEmpty()
     }
 
-    @Test fun `should be declared on implementation, not androidTestImplementation`() {
+    @Test fun `cannot be removed from androidTestImplementation`() {
       val id = "com.foo:bar"
       val usages = setOf(
         usage(bucket = Bucket.IMPL, variant = "debug", kind = SourceSetKind.MAIN),
@@ -1133,6 +1136,11 @@ internal class StandardTransformTest {
         usage(bucket = Bucket.IMPL, variant = "release", kind = SourceSetKind.ANDROID_TEST),
       )
       val declarations = setOf(
+        Declaration(
+          identifier = id,
+          configurationName = "compileOnly",
+          gradleVariantIdentification = emptyGVI
+        ),
         Declaration(
           identifier = id,
           configurationName = "androidTestImplementation",
@@ -1150,9 +1158,7 @@ internal class StandardTransformTest {
         isKaptApplied = false,
       ).reduce(usages)
 
-      assertThat(actual).containsExactly(
-        Advice.ofChange(ModuleCoordinates(id, "1.0", emptyGVI), "androidTestImplementation", "implementation"),
-      )
+      assertThat(actual).isEmpty()
     }
 
     @Test fun `should be debugRuntimeOnly`() {