Fix AnnotationExcluder

detekt · Jan 24, 2022 · 0900eea · 0900eea
1 parent 8406c5c
commit 0900eea
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 22 deletions.
diff --git a/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/AnnotationExcluder.kt b/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/AnnotationExcluder.kt
@@ -1,5 +1,6 @@
 package io.gitlab.arturbosch.detekt.api
 
+import io.github.detekt.psi.FullQualifiedNameGuesser
 import org.jetbrains.kotlin.psi.KtAnnotationEntry
 import org.jetbrains.kotlin.psi.KtFile
 
@@ -12,18 +13,11 @@ class AnnotationExcluder(
     root: KtFile,
     excludes: List<String>,
 ) {
-    private val excludes = excludes.map {
-        it.removePrefix("*").removeSuffix("*")
+    private val excludes: List<Regex> = excludes.map {
+        it.replace(".", "\\.").replace("*", ".*").toRegex()
     }
 
-    private val resolvedAnnotations = root.importList?.run {
-        imports
-            .asSequence()
-            .filterNot { it.isAllUnder }
-            .mapNotNull { it.importedFqName?.asString() }
-            .map { it.substringAfterLast('.') to it }
-            .toMap()
-    }.orEmpty()
+    private val fullQualifiedNameGuesser = FullQualifiedNameGuesser(root)
 
     @Deprecated("Use AnnotationExcluder(KtFile, List<String>) instead")
     constructor(root: KtFile, excludes: SplitPattern) : this(root, excludes.mapAll { it })
@@ -35,8 +29,28 @@ class AnnotationExcluder(
     fun shouldExclude(annotations: List<KtAnnotationEntry>): Boolean = annotations.any(::isExcluded)
 
     private fun isExcluded(annotation: KtAnnotationEntry): Boolean {
-        val annotationText = annotation.typeReference?.text ?: return false
-        val value = resolvedAnnotations[annotationText] ?: annotationText
-        return excludes.any { value.contains(it, ignoreCase = true) }
+        val annotationText = annotation.typeReference?.text?.ifEmpty { null } ?: return false
+        /*
+         We can't know if the annotationText is a full-qualified name or not. We can have these cases:
+         @Component
+         @Component.Factory
+         @dagger.Component.Factory
+         For that reason we use a heuristic here: If the first character is lower case we assume it's a package name
+         */
+        val possibleNames = if (!annotationText.first().isLowerCase()) {
+            fullQualifiedNameGuesser.getFullQualifiedName(annotationText)
+        } else {
+            listOf(annotationText)
+        }.flatMap { fqName ->
+            fqName
+                .split(".")
+                .dropWhile { it.first().isLowerCase() }
+                .reversed()
+                .scan("") { acc, name ->
+                    if (acc.isEmpty()) name else "$name.$acc"
+                }
+                .drop(1) + fqName
+        }
+        return excludes.any { exclude -> possibleNames.any { exclude.matches(it) } }
     }
 }
diff --git a/detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/AnnotationExcluderSpec.kt b/detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/AnnotationExcluderSpec.kt
@@ -29,14 +29,14 @@ class AnnotationExcluderSpec : Spek({
             mapOf(
                 Case("Component", "@Component") to true,
                 Case("Component", "@dagger.Component") to true,
-                Case("Component", "@Factory") to true, // false positive
-                Case("Component", "@Component.Factory") to true, // false positive
-                Case("Component", "@dagger.Component.Factory") to true, // false positive
+                Case("Component", "@Factory") to false,
+                Case("Component", "@Component.Factory") to false,
+                Case("Component", "@dagger.Component.Factory") to false,
                 Case("dagger.Component", "@Component") to true,
                 Case("dagger.Component", "@dagger.Component") to true,
-                Case("dagger.Component", "@Factory") to true, // false positive
+                Case("dagger.Component", "@Factory") to false,
                 Case("dagger.Component", "@Component.Factory") to false,
-                Case("dagger.Component", "@dagger.Component.Factory") to true, // false positive
+                Case("dagger.Component", "@dagger.Component.Factory") to false,
                 Case("Component.Factory", "@Component") to false,
                 Case("Component.Factory", "@dagger.Component") to false,
                 Case("Component.Factory", "@Factory") to true,
@@ -45,7 +45,7 @@ class AnnotationExcluderSpec : Spek({
                 Case("dagger.Component.Factory", "@Component") to false,
                 Case("dagger.Component.Factory", "@dagger.Component") to false,
                 Case("dagger.Component.Factory", "@Factory") to true,
-                Case("dagger.Component.Factory", "@Component.Factory") to false, // false negative
+                Case("dagger.Component.Factory", "@Component.Factory") to true,
                 Case("dagger.Component.Factory", "@dagger.Component.Factory") to true,
                 Case("Factory", "@Component") to false,
                 Case("Factory", "@dagger.Component") to false,
@@ -55,17 +55,17 @@ class AnnotationExcluderSpec : Spek({
                 Case("dagger.*", "@Component") to true,
                 Case("dagger.*", "@dagger.Component") to true,
                 Case("dagger.*", "@Factory") to true,
-                Case("dagger.*", "@Component.Factory") to false, // false positive
+                Case("dagger.*", "@Component.Factory") to true,
                 Case("dagger.*", "@dagger.Component.Factory") to true,
                 Case("*.Component.Factory", "@Component") to false,
                 Case("*.Component.Factory", "@dagger.Component") to false,
                 Case("*.Component.Factory", "@Factory") to true,
-                Case("*.Component.Factory", "@Component.Factory") to false, // false positive
+                Case("*.Component.Factory", "@Component.Factory") to true,
                 Case("*.Component.Factory", "@dagger.Component.Factory") to true,
                 Case("*.Component.*", "@Component") to false,
                 Case("*.Component.*", "@dagger.Component") to false,
                 Case("*.Component.*", "@Factory") to true,
-                Case("*.Component.*", "@Component.Factory") to false, // false positive
+                Case("*.Component.*", "@Component.Factory") to true,
                 Case("*.Component.*", "@dagger.Component.Factory") to true,
                 Case("foo.Component", "@Component") to false,
                 Case("foo.Component", "@dagger.Component") to false,