Don't call a dependency overshot if it's already declared in that source set

Author: RBusarow

modulecheck-core/src/main/kotlin/modulecheck/core/context/OverShotDependencies.kt

@@ -21,7 +21,10 @@ import modulecheck.parsing.gradle.ConfigurationName
 import modulecheck.project.ConfiguredProjectDependency
 import modulecheck.project.McProject
 import modulecheck.project.ProjectContext
+import modulecheck.project.toSourceSetDependency
 import modulecheck.utils.SafeCache
+import modulecheck.utils.mapToSet
+import modulecheck.utils.unsafeLazy
 
 data class OverShotDependencies(
   private val delegate: SafeCache<ConfigurationName, List<OverShotDependencyFinding>>,
@@ -39,12 +42,19 @@ data class OverShotDependencies(
         .flatMap { unused ->
 
           val unusedCpd = unused.cpd()
+          val unusedSsd =
+            unusedCpd.toSourceSetDependency(unused.configurationName.toSourceSetName())
           val unusedSourceSetName = unused.configurationName.toSourceSetName()
 
           val allUsedByConfigName = unusedSourceSetName
             .withDownStream(project)
             .mapNotNull { sourceSetName ->
 
+              val existingDependencies by unsafeLazy {
+                project.projectDependencies[sourceSetName]
+                  .mapToSet { it.toSourceSetDependency(sourceSetName) }
+              }
+
               val configName = sourceSetName.implementationConfig()
 
               val seed = when {
@@ -69,10 +79,10 @@ data class OverShotDependencies(
                   )
                 }
                 .filterNot {
-                  it.isTestFixture == unused.cpd().isTestFixture &&
-                    it.configurationName.toSourceSetName() == unused.cpd()
-                    .configurationName
-                    .toSourceSetName()
+
+                  val asSsd = it.toSourceSetDependency(sourceSetName)
+
+                  asSsd == unusedSsd || existingDependencies.contains(asSsd)
                 }
                 .firstNotNullOfOrNull { cpd ->
 

modulecheck-core/src/main/kotlin/modulecheck/core/rule/InheritedDependencyRule.kt

@@ -22,12 +22,13 @@ import modulecheck.api.settings.ChecksSettings
 import modulecheck.core.InheritedDependencyFinding
 import modulecheck.core.context.asApiOrImplementation
 import modulecheck.core.internal.uses
-import modulecheck.parsing.gradle.ProjectPath.StringProjectPath
 import modulecheck.parsing.gradle.SourceSetName
 import modulecheck.parsing.gradle.sortedByInheritance
 import modulecheck.project.ConfiguredProjectDependency
 import modulecheck.project.McProject
+import modulecheck.project.SourceSetDependency
 import modulecheck.project.TransitiveProjectDependency
+import modulecheck.project.toSourceSetDependency
 import modulecheck.utils.flatMapToSet
 import modulecheck.utils.mapAsync
 
@@ -39,22 +40,6 @@ class InheritedDependencyRule : ModuleCheckRule<InheritedDependencyFinding> {
 
   override suspend fun check(project: McProject): List<InheritedDependencyFinding> {
 
-    data class SourceSetDependency(
-      val sourceSetName: SourceSetName,
-      val path: StringProjectPath,
-      val isTestFixture: Boolean
-    )
-
-    fun ConfiguredProjectDependency.toSourceSetDependency(
-      sourceSetName: SourceSetName = configurationName.toSourceSetName(),
-      path: StringProjectPath = this@toSourceSetDependency.path,
-      isTestFixture: Boolean = this@toSourceSetDependency.isTestFixture
-    ) = SourceSetDependency(
-      sourceSetName = sourceSetName,
-      path = path,
-      isTestFixture = isTestFixture
-    )
-
     // For each source set, the set of all module paths and whether they're test fixtures
     val dependencyPathCache = mutableMapOf<SourceSetName, Set<SourceSetDependency>>()
 

modulecheck-core/src/test/kotlin/modulecheck/core/OverShotDependenciesTest.kt

@@ -815,6 +815,107 @@ class OverShotDependenciesTest : RunnerTest() {
     )
   }
 
+  // https://github.com/RBusarow/ModuleCheck/issues/520
+  @Test
+  fun `overshot as debugApi from invisible dependency with a visible unrelated debugApi project dependency`() {
+
+    val lib1 = kotlinProject(":lib1") {
+      addKotlinSource(
+        """
+        package com.modulecheck.lib1
+
+        open class Lib1Class
+        """.trimIndent()
+      )
+    }
+
+    val lib2 = androidLibrary(":lib2", "com.modulecheck.lib2") {
+      // lib1 is added as a dependency, but it's not in the build file.
+      // This is intentional, because it mimics the behavior of a convention plugin
+      // which adds a dependency without any visible declaration in the build file
+      addDependency(ConfigurationName.api, lib1)
+
+      buildFile {
+        """
+        plugins {
+          kotlin("jvm")
+        }
+
+        dependencies {
+          debugApi(project(path = ":lib4"))
+        }
+        """
+      }
+      addKotlinSource(
+        """
+        package com.modulecheck.lib2
+
+        import com.modulecheck.lib1.Lib1Class
+
+        val clazz = Lib1Class()
+        """.trimIndent(),
+        SourceSetName.DEBUG
+      )
+    }
+
+    run().isSuccess shouldBe false
+
+    lib2.buildFile shouldHaveText """
+        plugins {
+          kotlin("jvm")
+        }
+
+        dependencies {
+          debugApi(project(path = ":lib1"))
+          debugApi(project(path = ":lib4"))
+        }
+    """
+
+    logger.parsedReport() shouldBe listOf(
+      ":lib2" to listOf(
+        overshot(
+          fixed = true,
+          configuration = "debugApi",
+          dependency = ":lib1",
+          position = null
+        ),
+        unusedDependency(
+          fixed = false,
+          configuration = "api",
+          dependency = ":lib1",
+          position = null
+        )
+      )
+    )
+
+    logger.clear()
+
+    // this second run should not have an overshot finding, and shouldn't modify the build file
+    run().isSuccess shouldBe false
+
+    lib2.buildFile shouldHaveText """
+        plugins {
+          kotlin("jvm")
+        }
+
+        dependencies {
+          debugApi(project(path = ":lib1"))
+          debugApi(project(path = ":lib4"))
+        }
+    """
+
+    logger.parsedReport() shouldBe listOf(
+      ":lib2" to listOf(
+        unusedDependency(
+          fixed = false,
+          configuration = "api",
+          dependency = ":lib1",
+          position = null
+        )
+      )
+    )
+  }
+
   @Test
   fun `overshot as testImplementation from invisible dependency with a visible unrelated implementation project dependency`() {
 

modulecheck-project/api/src/main/kotlin/modulecheck/project/ConfiguredProjectDependency.kt

@@ -16,6 +16,7 @@
 package modulecheck.project
 
 import modulecheck.parsing.gradle.ConfigurationName
+import modulecheck.parsing.gradle.ProjectPath.StringProjectPath
 import modulecheck.parsing.gradle.SourceSetName
 
 data class ConfiguredProjectDependency(
@@ -77,3 +78,19 @@ data class DownstreamDependency(
   val dependentProject: McProject,
   val configuredProjectDependency: ConfiguredProjectDependency
 )
+
+data class SourceSetDependency(
+  val sourceSetName: SourceSetName,
+  val path: StringProjectPath,
+  val isTestFixture: Boolean
+)
+
+fun ConfiguredProjectDependency.toSourceSetDependency(
+  sourceSetName: SourceSetName = configurationName.toSourceSetName(),
+  path: StringProjectPath = this@toSourceSetDependency.path,
+  isTestFixture: Boolean = this@toSourceSetDependency.isTestFixture
+) = SourceSetDependency(
+  sourceSetName = sourceSetName,
+  path = path,
+  isTestFixture = isTestFixture
+)