fix auto-correct when using a non-standard config name

fixes #360

Author: RBusarow

modulecheck-api/src/main/kotlin/modulecheck/api/context/Declarations.kt

@@ -29,7 +29,6 @@ import modulecheck.utils.LazySet.DataSource.Priority.HIGH
 import modulecheck.utils.SafeCache
 import modulecheck.utils.dataSource
 import modulecheck.utils.lazySet
-import modulecheck.utils.mapToSet
 
 data class Declarations(
   private val delegate: SafeCache<SourceSetName, LazySet<DeclarationName>>,
@@ -42,13 +41,10 @@ data class Declarations(
   suspend fun get(sourceSetName: SourceSetName): LazySet<DeclarationName> {
     return delegate.getOrPut(sourceSetName) {
 
-      val inheritedSourceSetsNames = sourceSetName.javaConfigurationNames()
-        .flatMapTo(mutableSetOf(sourceSetName)) { configName ->
-          project.configurations[configName]
-            ?.inherited
-            ?.mapToSet { inherited -> inherited.name.toSourceSetName() }
-            .orEmpty()
-        }
+      val inheritedSourceSetsNames = sourceSetName.inheritedSourceSetNames(
+        project,
+        includeSelf = true
+      )
 
       val rNameOrNull = (project as? AndroidMcProject)?.androidRFqNameOrNull
 

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

@@ -22,8 +22,12 @@ import modulecheck.api.settings.ChecksSettings
 import modulecheck.core.InheritedDependencyFinding
 import modulecheck.core.context.mustBeApiIn
 import modulecheck.core.internal.uses
+import modulecheck.parsing.gradle.ConfigurationName
 import modulecheck.parsing.gradle.SourceSetName
+import modulecheck.parsing.gradle.inheritingConfigurations
+import modulecheck.project.ConfiguredProjectDependency
 import modulecheck.project.McProject
+import modulecheck.project.TransitiveProjectDependency
 import modulecheck.utils.mapAsync
 
 class InheritedDependencyRule : ModuleCheckRule<InheritedDependencyFinding> {
@@ -34,47 +38,91 @@ class InheritedDependencyRule : ModuleCheckRule<InheritedDependencyFinding> {
 
   override suspend fun check(project: McProject): List<InheritedDependencyFinding> {
 
-    val mainDirectDependencies = project.projectDependencies.main()
-      .map { it.project to it.isTestFixture }
-      .toSet()
+    // For each source set, the set of all module paths and whether they're test fixtures
+    val dependencyPathCache = mutableMapOf<SourceSetName, Set<Pair<String, Boolean>>>()
+
+    // Returns true if the dependency is already declared in this exact configuration, **or** if
+    // it's declared in an upstream configuration.
+    //
+    // For example, this function will return true for a `testImplementation` configured dependency
+    // which is already declared in the main source set (such as with `api` or `implementation`).
+    fun ConfiguredProjectDependency.alreadyInClasspath(): Boolean {
+      fun dependencyPathsForSourceSet(sourceSetName: SourceSetName): Set<Pair<String, Boolean>> {
+        return dependencyPathCache.getOrPut(sourceSetName) {
+          project.projectDependencies[sourceSetName]
+            .map { it.project.path to it.isTestFixture }
+            .toSet()
+        }
+      }
+
+      return configurationName.toSourceSetName()
+        // Check the receiver's configuration first, but if the dependency isn't used there, also
+        // check the upstream configurations.
+        .inheritedSourceSetNames(project, includeSelf = true)
+        .any { sourceSet ->
+          dependencyPathsForSourceSet(sourceSet)
+            .contains(this.project.path to isTestFixture)
+        }
+    }
+
+    // Returns the list of all transitive dependencies where the contributed dependency is used,
+    // filtering out any configuration which would be redundant.  For instance, if a dependency is
+    // used in `main`, the function will stop there instead of returning a list of `main`, `debug`,
+    // `test`, etc.
+    suspend fun List<TransitiveProjectDependency>.allUsed(): List<TransitiveProjectDependency> {
+      return fold(listOf()) { acc, transitiveProjectDependency ->
+
+        val contributedSourceSet = transitiveProjectDependency.contributed
+          .configurationName
+          .toSourceSetName()
+
+        val alreadyUsedUpstream = acc.any {
+          val usedSourceSet = it.contributed.configurationName.toSourceSetName()
+          contributedSourceSet.inheritsFrom(usedSourceSet, project)
+        }
+
+        when {
+          alreadyUsedUpstream -> acc
+          project.uses(transitiveProjectDependency.contributed) -> {
+            acc + transitiveProjectDependency
+          }
+          else -> acc
+        }
+      }
+    }
 
     val used = project.classpathDependencies().all()
-      .filterNot { mainDirectDependencies.contains(it.contributed.project to it.contributed.isTestFixture) }
       .distinctBy { it.contributed.project.path to it.contributed.isTestFixture }
-      .filter { project.uses(it) }
+      .flatMap { transitive ->
 
-    val dependencyPathCache = mutableMapOf<SourceSetName, Set<Pair<String, Boolean>>>()
-    fun pathsForSourceSet(sourceSetName: SourceSetName): Set<Pair<String, Boolean>> {
-      return dependencyPathCache.getOrPut(sourceSetName) {
-        project.projectDependencies[sourceSetName]
-          .map { it.project.path to it.isTestFixture }
-          .toSet()
+        // If a transitive dependency is used in the same configuration as its source, then that's
+        // the configuration which should be used and we're done.  However, that transitive
+        // dependency is also providing the dependency to other source sets which depend upon it.
+        // So, check the inheriting dependencies as well.
+        transitive.withContributedConfiguration(transitive.source.configurationName)
+          .withInheritingVariants(project)
+          .filterNot { it.contributed.alreadyInClasspath() }
+          .toList()
+          .sortedByInheritance(project)
+          .allUsed()
       }
-    }
 
     return used.asSequence()
-      .filterNot {
-        pathsForSourceSet(it.source.configurationName.toSourceSetName())
-          .contains((it.contributed.project.path to it.contributed.isTestFixture))
-      }
       .distinct()
       // Projects shouldn't inherit themselves.  This false-positive can happen if a test
       // fixture/utilities module depends upon a module, and that module uses the test module in
       // testImplementation.
       .filterNot { transitive -> transitive.contributed.project == project }
-      .mapAsync { transitive ->
-
-        val source = transitive.source
-        val inherited = transitive.contributed
+      .mapAsync { (source, inherited) ->
 
         val mustBeApi = source.configurationName
           .toSourceSetName() == SourceSetName.MAIN && inherited.project
           .mustBeApiIn(project, inherited.isTestFixture)
 
         val newConfig = if (mustBeApi) {
-          source.configurationName.apiVariant()
+          inherited.configurationName.apiVariant()
         } else {
-          source.configurationName
+          inherited.configurationName.implementationVariant()
         }
 
         InheritedDependencyFinding(
@@ -93,6 +141,46 @@ class InheritedDependencyRule : ModuleCheckRule<InheritedDependencyFinding> {
       .flatten()
   }
 
+  private fun TransitiveProjectDependency.withContributedConfiguration(
+    configurationName: ConfigurationName
+  ): TransitiveProjectDependency {
+    val newContributed = contributed.copy(configurationName = configurationName)
+    return copy(contributed = newContributed)
+  }
+
+  // Returns a sequence starting with the receiver's configuration, then all **downstream**
+  // configurations.  This is useful because when we're checking to see if a transitive dependency
+  // is used in `main` (for instance), we should also check whether it's used in the source sets
+  // which inherit from `main` (like `debug`, `release`, `androidTest`, `test`, etc.).
+  private fun TransitiveProjectDependency.withInheritingVariants(
+    project: McProject
+  ): Sequence<TransitiveProjectDependency> {
+    return sequenceOf(this) + project.inheritingConfigurations(source.configurationName)
+      .asSequence()
+      .map { config -> config.name }
+      .filter { name -> name.isImplementation() }
+      .map { configName -> withContributedConfiguration(configName) }
+  }
+
+  private fun List<TransitiveProjectDependency>.sortedByInheritance(
+    project: McProject
+  ): List<TransitiveProjectDependency> {
+    val sorted = toMutableList()
+
+    sorted.sortWith { o1, o2 ->
+      val o1SourceSet = o1.contributed.configurationName.toSourceSetName()
+      val o2SourceSet = o2.contributed.configurationName.toSourceSetName()
+
+      val inherits = o1SourceSet.inheritsFrom(o2SourceSet, project)
+      when {
+        inherits -> -1
+        o1SourceSet == o2SourceSet -> 0
+        else -> 1
+      }
+    }
+    return sorted
+  }
+
   override fun shouldApply(checksSettings: ChecksSettings): Boolean {
     return checksSettings.inheritedDependency
   }