Get detekt to auto-correct missing @jvmoverloads (cashapp#2965)

* Get detekt to auto-correct missing @jvmoverloads * Add test for auto-correct * Fail on autoCorrectable to avoid having to customize CI verus local builds
afkelsall · Oct 9, 2023 · df47b53 · df47b53
1 parent 7d03e8a
commit df47b53
Show file tree

Hide file tree

Showing 7 changed files with 94 additions and 11 deletions.
diff --git a/build.gradle.kts b/build.gradle.kts
@@ -126,6 +126,7 @@ subprojects {
       parallel = true
       buildUponDefaultConfig = false
       ignoreFailures = false
+      autoCorrect = true
       config.setFrom(files("$rootDir/detekt.yaml"))
     }
   } else {

diff --git a/buildSrc/src/main/kotlin/Dependencies.kt b/buildSrc/src/main/kotlin/Dependencies.kt
@@ -22,6 +22,7 @@ object Dependencies {
   val datasourceProxy = "net.ttddyy:datasource-proxy:1.7"
   val dependencyAnalysisPluginVersion = "1.20.0"
   val detektApi = "io.gitlab.arturbosch.detekt:detekt-api:1.23.1"
+  val detektParser = "io.gitlab.arturbosch.detekt:detekt-parser:1.23.1"
   val detektGradlePlugin = "io.gitlab.arturbosch.detekt:detekt-gradle-plugin:1.23.1"
   val detektPsiUtils = "io.gitlab.arturbosch.detekt:detekt-psi-utils:1.23.1"
   val detektTest = "io.gitlab.arturbosch.detekt:detekt-test:1.23.1"

diff --git a/detekt.yaml b/detekt.yaml
@@ -1,5 +1,7 @@
 # See https://github.com/detekt/detekt/blob/master/detekt-core/src/main/resources/default-detekt-config.yml
 # for default settings.
+build:
+  excludeCorrectable: false
 
 output-reports:
   active: true
@@ -43,3 +45,4 @@ detektive:
   active: true
   AnnotatePublicApisWithJvmOverloads:
     active: true
+    autoCorrect: true
diff --git a/detektive/build.gradle.kts b/detektive/build.gradle.kts
@@ -14,6 +14,7 @@ dependencies {
   compileOnly(Dependencies.kotlinCompilerEmbeddable)
 
   testImplementation(Dependencies.assertj)
+  testImplementation(Dependencies.detektParser)
   testImplementation(Dependencies.detektTest)
   testImplementation(Dependencies.detektTestUtils)
   testImplementation(Dependencies.guice)

diff --git a/detektive/src/main/kotlin/cash/detektive/javacompat/AnnotatePublicApisWithJvmOverloads.kt b/detektive/src/main/kotlin/cash/detektive/javacompat/AnnotatePublicApisWithJvmOverloads.kt
@@ -2,12 +2,15 @@ package cash.detektive.javacompat
 
 import io.gitlab.arturbosch.detekt.api.CodeSmell
 import io.gitlab.arturbosch.detekt.api.Config
+import io.gitlab.arturbosch.detekt.api.CorrectableCodeSmell
 import io.gitlab.arturbosch.detekt.api.Debt
 import io.gitlab.arturbosch.detekt.api.Entity
 import io.gitlab.arturbosch.detekt.api.Issue
 import io.gitlab.arturbosch.detekt.api.Rule
 import io.gitlab.arturbosch.detekt.api.Severity
+import io.gitlab.arturbosch.detekt.api.internal.AutoCorrectable
 import io.gitlab.arturbosch.detekt.api.internal.RequiresTypeResolution
+import io.gitlab.arturbosch.detekt.api.internal.isSuppressedBy
 import io.gitlab.arturbosch.detekt.rules.fqNameOrNull
 import org.jetbrains.kotlin.descriptors.CallableMemberDescriptor
 import org.jetbrains.kotlin.descriptors.DeclarationDescriptor
@@ -16,15 +19,18 @@ import org.jetbrains.kotlin.descriptors.effectiveVisibility
 import org.jetbrains.kotlin.psi.KtAnnotated
 import org.jetbrains.kotlin.psi.KtAnnotationEntry
 import org.jetbrains.kotlin.psi.KtClass
+import org.jetbrains.kotlin.psi.KtElement
 import org.jetbrains.kotlin.psi.KtFunction
 import org.jetbrains.kotlin.psi.KtNamedFunction
 import org.jetbrains.kotlin.psi.KtPrimaryConstructor
+import org.jetbrains.kotlin.psi.KtPsiFactory
 import org.jetbrains.kotlin.psi.psiUtil.containingClassOrObject
 import org.jetbrains.kotlin.resolve.BindingContext
 import org.jetbrains.kotlin.resolve.descriptorUtil.isPublishedApi
 import kotlin.reflect.KClass
 
 @RequiresTypeResolution
+@AutoCorrectable(since = "1.0.0")
 class AnnotatePublicApisWithJvmOverloads(config: Config) : Rule(config) {
 
   override val issue = Issue(
@@ -50,14 +56,24 @@ class AnnotatePublicApisWithJvmOverloads(config: Config) : Rule(config) {
     }
     if (isApplicable(element, bindingContext[BindingContext.DECLARATION_TO_DESCRIPTOR, element])) {
       if (!element.annotationEntries.any { it.isOfType(JvmOverloads::class) }) {
-        report(
-          CodeSmell(
-            issue,
-            Entity.atName(element),
-            "Public ${elementType.name.lowercase()} '${element.nameAsSafeName}' " +
-              "with default arguments, but without @JvmOverloads annotation"
-          )
-        )
+        val message = "Public ${elementType.name.lowercase()} '${element.nameAsSafeName}' " +
+          "with default arguments, but without @JvmOverloads annotation"
+
+        if (autoCorrect) {
+          if (!(element as KtElement).isSuppressedBy(ruleId, aliases, ruleSetConfig.parentPath)) {
+            val annotation = element.addAnnotationEntry(KtPsiFactory.contextual(element.parent, markGenerated = true).createAnnotationEntry("@JvmOverloads"))
+            if (elementType == ElementType.CONSTRUCTOR) {
+              annotation.addBefore(KtPsiFactory.contextual(element.parent, markGenerated = true).createWhiteSpace(), null)
+              element.addAfter(KtPsiFactory.contextual(element.parent, markGenerated = true).createWhiteSpace(), null)
+            }
+            else if (elementType == ElementType.FUNCTION) {
+              annotation.addBefore(KtPsiFactory.contextual(element.parent, markGenerated = true).createNewLine(), null)
+            }
+          }
+          report(CorrectableCodeSmell(issue, Entity.atName(element), message, autoCorrectEnabled = true))
+        } else {
+          report(CodeSmell(issue, Entity.atName(element), message))
+        }
       }
     }
   }

diff --git a/...ktive/src/test/kotlin/cash/detektive/javacompat/AnnotatePublicApisWithJvmOverloadsTest.kt b/...ktive/src/test/kotlin/cash/detektive/javacompat/AnnotatePublicApisWithJvmOverloadsTest.kt
@@ -1,21 +1,60 @@
 package cash.detektive.javacompat
 
 import cash.detektive.javacompat.AnnotatePublicApisWithJvmOverloads.ElementType
+import io.github.detekt.parser.DetektPomModel
+import io.github.detekt.test.utils.compileForTest
 import io.gitlab.arturbosch.detekt.api.Config
 import io.gitlab.arturbosch.detekt.api.Severity
+import io.gitlab.arturbosch.detekt.api.internal.CompilerResources
 import io.gitlab.arturbosch.detekt.rules.KotlinCoreEnvironmentTest
+import io.gitlab.arturbosch.detekt.test.TestConfig
 import io.gitlab.arturbosch.detekt.test.compileAndLintWithContext
+import io.gitlab.arturbosch.detekt.test.getContextForPaths
 import org.assertj.core.api.Assertions.assertThat
 import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment
+import org.jetbrains.kotlin.com.intellij.mock.MockProject
+import org.jetbrains.kotlin.com.intellij.openapi.diagnostic.Logger
+import org.jetbrains.kotlin.com.intellij.pom.PomModel
+import org.jetbrains.kotlin.config.CompilerConfigurationKey
+import org.jetbrains.kotlin.config.languageVersionSettings
+import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowValueFactoryImpl
+import org.jetbrains.kotlin.utils.PrintingLogger
+import org.junit.jupiter.api.AfterAll
+import org.junit.jupiter.api.BeforeAll
 import org.junit.jupiter.api.BeforeEach
+import org.junit.jupiter.api.Test
 import org.junit.jupiter.params.ParameterizedTest
 import org.junit.jupiter.params.provider.MethodSource
+import java.io.File
 
 @KotlinCoreEnvironmentTest
 internal class AnnotatePublicApisWithJvmOverloadsTest(private val env: KotlinCoreEnvironment) {
-
   @BeforeEach
-  fun setUp() {}
+  fun setUp() {
+  }
+
+  @Test
+  fun testAutoCorrect() {
+    (env.project as MockProject).registerService(PomModel::class.java, DetektPomModel(env.project))
+    env.configuration.add(CompilerConfigurationKey(Config.AUTO_CORRECT_KEY), true)
+    val languageVersionSettings = env.configuration.languageVersionSettings
+
+    val ktFile = compileForTest(
+      File("src/test/kotlin/cash/detektive/javacompat/TestCode.kt").toPath()
+    )
+
+    AnnotatePublicApisWithJvmOverloads(TestConfig(Config.AUTO_CORRECT_KEY to true)).visitFile(
+      ktFile,
+      env.getContextForPaths(listOf(ktFile)),
+      CompilerResources(languageVersionSettings, DataFlowValueFactoryImpl(languageVersionSettings))
+    )
+
+    val modifiedContents = ktFile.viewProvider.contents
+    assertThat(modifiedContents).contains(
+      "class TestCode @JvmOverloads constructor(val things: List<String> = emptyList())"
+    )
+    assertThat(modifiedContents).contains("@JvmOverloads\n fun doIt")
+  }
 
   @ParameterizedTest
   @MethodSource("errorTestCases")
@@ -46,9 +85,26 @@ internal class AnnotatePublicApisWithJvmOverloadsTest(private val env: KotlinCor
 
   companion object {
     data class ErrorTestCase(
-      val description: String, val elementType: ElementType, val elementName: String, val code: String
+      val description: String,
+      val elementType: ElementType,
+      val elementName: String,
+      val code: String
     )
 
+    private var defaultLoggerFactory = Logger.getFactory()
+
+    @BeforeAll
+    @JvmStatic
+    fun beforeAll() {
+      Logger.setFactory { PrintingLogger(System.out) }
+    }
+
+    @AfterAll
+    @JvmStatic
+    fun afterAll() {
+      Logger.setFactory(defaultLoggerFactory)
+    }
+
     @JvmStatic
     fun errorTestCases() = listOf(
       ErrorTestCase(

diff --git a/detektive/src/test/kotlin/cash/detektive/javacompat/TestCode.kt b/detektive/src/test/kotlin/cash/detektive/javacompat/TestCode.kt
@@ -0,0 +1,5 @@
+package cash.detektive.javacompat
+
+class TestCode(val things: List<String> = emptyList()) {
+  fun doIt(x: String = "", y: Int) {}
+}