Repository: RBusarow/ModuleCheck (kotlin)

Issue:
An overshot dependency used in multiple downstream source sets is only added to one of them
I'm using an Android project as an example because it has a compatible SourceSet hierarchy by default, but this isn't an Android-specific bug.

Assume Android project has the dependency `implementation(project(":lib1"))`.

It doesn't use `:lib1` in `main`, or `debug`/`release`, but it does use `:lib1` in both `androidTest` and `test`.

Currently, ModuleCheck will find that it's overshot and should be both `androidTestImplementation` and `testImplementation`, but will only report one of them.  Which one is reported is random.

This is happening because it's de-duplicating the findings based upon the new dependency's project path.  It should be looking at both the path and the configuration.

Pull request:
fixes #809

Patch:
diff --git a/modulecheck-core/src/main/kotlin/modulecheck/core/context/OverShotDependencies.kt b/modulecheck-core/src/main/kotlin/modulecheck/core/context/OverShotDependencies.kt
index 4eb55c19e4..865654dbea 100644
--- a/modulecheck-core/src/main/kotlin/modulecheck/core/context/OverShotDependencies.kt
+++ b/modulecheck-core/src/main/kotlin/modulecheck/core/context/OverShotDependencies.kt
@@ -125,7 +125,10 @@ data class OverShotDependencies(
           )
         }
         .sortedBy { it.newDependency.identifier.name }
-        .distinctBy { it.newDependency.identifier }
+        // Only report each new path/configuration pair once.  So we can add multiple dependencies
+        // for the `testImplementation` config, or multiple configurations of the `:lib1` project,
+        // but we'll only add `testImplementation(project(":lib1"))` once.
+        .distinctBy { it.newDependency.identifier to it.newDependency.configurationName }
     }
   }
 


Test patch:
diff --git a/modulecheck-core/src/test/kotlin/modulecheck/core/OverShotDependenciesTest.kt b/modulecheck-core/src/test/kotlin/modulecheck/core/OverShotDependenciesTest.kt
index 752aa68a42..ea92731865 100644
--- a/modulecheck-core/src/test/kotlin/modulecheck/core/OverShotDependenciesTest.kt
+++ b/modulecheck-core/src/test/kotlin/modulecheck/core/OverShotDependenciesTest.kt
@@ -18,6 +18,7 @@ package modulecheck.core
 import modulecheck.model.dependency.ConfigurationName
 import modulecheck.model.sourceset.SourceSetName
 import modulecheck.runtime.test.ProjectFindingReport.overshot
+import modulecheck.runtime.test.ProjectFindingReport.unsortedDependencies
 import modulecheck.runtime.test.ProjectFindingReport.unusedDependency
 import modulecheck.runtime.test.RunnerTest
 import org.junit.jupiter.api.Test
@@ -531,6 +532,99 @@ class OverShotDependenciesTest : RunnerTest() {
     )
   }
 
+  @Test
+  fun `unused in main but used in both test and androidTest should be added to both`() {
+
+    settings.checks.sortDependencies = true
+
+    val lib1 = kotlinProject(":lib1") {
+      addKotlinSource(
+        """
+        package com.modulecheck.lib1
+
+        interface Lib1Interface
+        """.trimIndent()
+      )
+    }
+
+    val lib2 = androidLibrary(":lib2", "com.modulecheck.lib2") {
+
+      addDependency(ConfigurationName.api, lib1)
+
+      buildFile {
+        """
+        plugins {
+          id("com.android.library")
+          kotlin("android")
+        }
+
+        dependencies {
+          api(project(path = ":lib1"))
+        }
+        """
+      }
+      addKotlinSource(
+        """
+        package com.modulecheck.lib2.debug
+
+        import com.modulecheck.lib1.Lib1Interface
+
+        class Lib2ClassAndroidTest : Lib1Interface
+        """.trimIndent(),
+        SourceSetName.ANDROID_TEST
+      )
+      addKotlinSource(
+        """
+        package com.modulecheck.lib2.test
+
+        import com.modulecheck.lib1.Lib1Interface
+
+        class Lib2ClassTest : Lib1Interface
+        """.trimIndent(),
+        SourceSetName.TEST
+      )
+    }
+
+    run().isSuccess shouldBe true
+
+    lib2.buildFile shouldHaveText """
+      plugins {
+        id("com.android.library")
+        kotlin("android")
+      }
+
+      dependencies {
+        androidTestImplementation(project(path = ":lib1"))
+
+        testImplementation(project(path = ":lib1"))
+      }
+      """
+
+    logger.parsedReport() shouldBe listOf(
+      ":lib2" to listOf(
+        overshot(
+          fixed = true,
+          configuration = "androidTestImplementation",
+          dependency = ":lib1",
+          position = "7, 3"
+        ),
+        overshot(
+          fixed = true,
+          configuration = "testImplementation",
+          dependency = ":lib1",
+          position = "7, 3"
+        ),
+        unsortedDependencies(fixed = true),
+        unusedDependency(
+          fixed = true,
+          configuration = "api",
+          dependency = ":lib1",
+          position = "7, 3"
+        )
+      )
+    )
+  }
+
   @Test
   fun `overshot as api with config block and comment with auto-correct should be fixed`() {
 
diff --git a/modulecheck-runtime/testing/src/main/kotlin/modulecheck/runtime/test/RunnerTest.kt b/modulecheck-runtime/testing/src/main/kotlin/modulecheck/runtime/test/RunnerTest.kt
index 8009ef9161..bd798dd6a6 100644
--- a/modulecheck-runtime/testing/src/main/kotlin/modulecheck/runtime/test/RunnerTest.kt
+++ b/modulecheck-runtime/testing/src/main/kotlin/modulecheck/runtime/test/RunnerTest.kt
@@ -144,7 +144,15 @@ abstract class RunnerTest : ProjectTest() {
   }
 
   private fun List<Pair<String, List<ProjectFindingReport>>>.sorted() = sortedBy { it.first }
-    .map { it.first to it.second.sortedBy { it::class.java.simpleName } }
+    .map { (path, findings) ->
+      path to findings.sortedBy { findingReport ->
+
+        val findingName = findingReport::class.java.simpleName
+        val config = findingReport.configuration ?: "-"
+
+        "$findingName$config${findingReport.position}"
+      }
+    }
 
   infix fun List<Pair<String, List<ProjectFindingReport>>>.shouldBe(
     expected: List<Pair<String, List<ProjectFindingReport>>>