Repository: RBusarow/ModuleCheck (kotlin)

Issue:
false negative for `unused-kapt-plugin` if there are no kapt processor dependencies
The unused kapt rule is currently looking for unused kapt processors, then saying that the plugin is unused if all processors are unused.  But if there are no processors at all, then the plugin is allowed to stick around.

Pull request:
fixes #602

Patch:
diff --git a/modulecheck-core/src/main/kotlin/modulecheck/core/rule/UnusedKaptPluginRule.kt b/modulecheck-core/src/main/kotlin/modulecheck/core/rule/UnusedKaptPluginRule.kt
index a4c0aab7b8..bcfe6c7ab8 100644
--- a/modulecheck-core/src/main/kotlin/modulecheck/core/rule/UnusedKaptPluginRule.kt
+++ b/modulecheck-core/src/main/kotlin/modulecheck/core/rule/UnusedKaptPluginRule.kt
@@ -22,7 +22,6 @@ import modulecheck.config.KaptMatcher
 import modulecheck.config.ModuleCheckSettings
 import modulecheck.config.asMap
 import modulecheck.core.UnusedPluginFinding
-import modulecheck.core.kapt.UnusedKaptProcessorFinding
 import modulecheck.core.kapt.defaultKaptMatchers
 import modulecheck.finding.Finding
 import modulecheck.finding.FindingName
@@ -42,60 +41,43 @@ class UnusedKaptPluginRule(
   override val description = "Warns if the kapt plugin is applied, but unused"
 
   override suspend fun check(project: McProject): List<Finding> {
+    if (!project.hasKapt) return emptyList()
+
     val matchers = kaptMatchers.asMap()
 
     val kaptDependencies = project.kaptDependencies()
-    val allKaptDependencies = kaptDependencies.all()
 
-    return project
+    val usedProcessors = project
       .configurations
       .keys
       .filter { it.value.startsWith("kapt") }
       .flatMap { configName ->
 
-        val processors = kaptDependencies.get(configName)
-
         val references = project.referencesForSourceSetName(configName.toSourceSetName())
 
-        // unused means that none of the processor's annotations are used in any import
-        val unusedProcessors = processors
-          .filterNot {
-            val matcher = matchers[it.name] ?: return@filterNot true
+        kaptDependencies.get(configName)
+          .filter {
+            val matcher = matchers[it.name] ?: return@filter false
 
             matcher.matchedIn(references)
           }
-
-        val unusedProcessorFindings = unusedProcessors
-          .map { processor ->
-            UnusedKaptProcessorFinding(
-              findingName = name,
-              dependentProject = project,
-              dependentPath = project.path,
-              buildFile = project.buildFile,
-              oldDependency = processor,
-              configurationName = configName
-            )
-          }
-
-        val pluginIsUnused = allKaptDependencies.size == unusedProcessorFindings.size &&
-          project.hasKapt && unusedProcessorFindings.isNotEmpty()
-
-        if (pluginIsUnused) {
-          listOf(
-            UnusedPluginFinding(
-              dependentProject = project,
-              dependentPath = project.path,
-              buildFile = project.buildFile,
-              findingName = name,
-              pluginId = KAPT_PLUGIN_ID,
-              alternatePluginId = KAPT_ALTERNATE_PLUGIN_ID,
-              kotlinPluginFunction = KAPT_PLUGIN_FUN
-            )
-          )
-        } else {
-          emptyList()
-        }
       }
+
+    return if (usedProcessors.isEmpty()) {
+      listOf(
+        UnusedPluginFinding(
+          dependentProject = project,
+          dependentPath = project.path,
+          buildFile = project.buildFile,
+          findingName = name,
+          pluginId = KAPT_PLUGIN_ID,
+          alternatePluginId = KAPT_ALTERNATE_PLUGIN_ID,
+          kotlinPluginFunction = KAPT_PLUGIN_FUN
+        )
+      )
+    } else {
+      emptyList()
+    }
   }
 
   override fun shouldApply(checksSettings: ChecksSettings): Boolean {


Test patch:
diff --git a/modulecheck-core/src/test/kotlin/modulecheck/core/UnusedKaptPluginTest.kt b/modulecheck-core/src/test/kotlin/modulecheck/core/UnusedKaptPluginTest.kt
new file mode 100644
index 0000000000..e9f37322a1
--- /dev/null
+++ b/modulecheck-core/src/test/kotlin/modulecheck/core/UnusedKaptPluginTest.kt
@@ -0,0 +1,108 @@
+/*
+ * Copyright (C) 2021-2022 Rick Busarow
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package modulecheck.core
+
+import modulecheck.parsing.gradle.model.ConfigurationName
+import modulecheck.runtime.test.ProjectFindingReport.unusedKaptPlugin
+import modulecheck.runtime.test.RunnerTest
+import org.junit.jupiter.api.Test
+
+class UnusedKaptPluginTest : RunnerTest() {
+
+  val dagger = "com.google.dagger:dagger-compiler:2.40.5"
+
+  @Test
+  fun `plugin applied but with processor in non-kapt configuration without autoCorrect should remove plugin`() {
+
+    val app = kotlinProject(":app") {
+      hasKapt = true
+
+      addExternalDependency(ConfigurationName.api, dagger)
+
+      buildFile {
+        """
+        plugins {
+          kotlin("jvm")
+          kotlin("kapt")
+        }
+
+        dependencies {
+          api("$dagger")
+        }
+        """
+      }
+    }
+
+    run().isSuccess shouldBe true
+
+    app.buildFile shouldHaveText """
+        plugins {
+          kotlin("jvm")
+          // kotlin("kapt")  // ModuleCheck finding [unused-kapt-plugin]
+        }
+
+        dependencies {
+          api("$dagger")
+        }
+    """
+
+    logger.parsedReport() shouldBe listOf(
+      ":app" to listOf(
+        unusedKaptPlugin(
+          fixed = true,
+          dependency = "org.jetbrains.kotlin.kapt",
+          position = "3, 3"
+        )
+      )
+    )
+  }
+
+  @Test
+  fun `unused with main kapt no other processors should remove plugin`() {
+
+    val app = kotlinProject(":app") {
+      hasKapt = true
+
+      buildFile {
+        """
+        plugins {
+          id("kotlin-jvm")
+          id("kotlin-kapt")
+        }
+        """
+      }
+    }
+
+    run().isSuccess shouldBe true
+
+    app.buildFile shouldHaveText """
+      plugins {
+        id("kotlin-jvm")
+        // id("kotlin-kapt")  // ModuleCheck finding [unused-kapt-plugin]
+      }
+    """
+
+    logger.parsedReport() shouldBe listOf(
+      ":app" to listOf(
+        unusedKaptPlugin(
+          fixed = true,
+          dependency = "org.jetbrains.kotlin.kapt",
+          position = "3, 3"
+        )
+      )
+    )
+  }
+}
diff --git a/modulecheck-core/src/test/kotlin/modulecheck/core/UnusedKaptProcessorTest.kt b/modulecheck-core/src/test/kotlin/modulecheck/core/UnusedKaptProcessorTest.kt
index 3d5725283b..e19f637162 100644
--- a/modulecheck-core/src/test/kotlin/modulecheck/core/UnusedKaptProcessorTest.kt
+++ b/modulecheck-core/src/test/kotlin/modulecheck/core/UnusedKaptProcessorTest.kt
@@ -136,10 +136,9 @@ class UnusedKaptProcessorTest : RunnerTest() {
   }
 
   @Test
-  fun `unused from non-kapt configuration without autoCorrect should pass without changes`() {
+  fun `processor in non-kapt configuration without autoCorrect should pass without changes`() {
 
     val app = kotlinProject(":app") {
-      hasKapt = true
 
       addExternalDependency(ConfigurationName.api, dagger)
 
@@ -147,7 +146,6 @@ class UnusedKaptProcessorTest : RunnerTest() {
         """
         plugins {
           kotlin("jvm")
-          kotlin("kapt")
         }
 
         dependencies {
@@ -164,7 +162,6 @@ class UnusedKaptProcessorTest : RunnerTest() {
     app.buildFile shouldHaveText """
         plugins {
           kotlin("jvm")
-          kotlin("kapt")
         }
 
         dependencies {