Fix AnnotationExcluder

detekt · Jan 28, 2022 · b549bb1 · b549bb1
1 parent d09c6b6
commit b549bb1
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
@@ -28,14 +28,14 @@ class AnnotationExcluderSpec {
             value = [
                 "Component,@Component,true",
                 "Component,@dagger.Component,true",
-                "Component,@Factory,true", // false positive
-                "Component,@Component.Factory,true", // false positive
-                "Component,@dagger.Component.Factory,true", // false positive
+                "Component,@Factory,false",
+                "Component,@Component.Factory,false",
+                "Component,@dagger.Component.Factory,false",
                 "dagger.Component,@Component,true",
                 "dagger.Component,@dagger.Component,true",
-                "dagger.Component,@Factory,true", // false positive
+                "dagger.Component,@Factory,false",
                 "dagger.Component,@Component.Factory,false",
-                "dagger.Component,@dagger.Component.Factory,true", // false positive
+                "dagger.Component,@dagger.Component.Factory,false",
                 "Component.Factory,@Component,false",
                 "Component.Factory,@dagger.Component,false",
                 "Component.Factory,@Factory,true",
@@ -44,7 +44,7 @@ class AnnotationExcluderSpec {
                 "dagger.Component.Factory,@Component,false",
                 "dagger.Component.Factory,@dagger.Component,false",
                 "dagger.Component.Factory,@Factory,true",
-                "dagger.Component.Factory,@Component.Factory,false", // false negative
+                "dagger.Component.Factory,@Component.Factory,true",
                 "dagger.Component.Factory,@dagger.Component.Factory,true",
                 "Factory,@Component,false",
                 "Factory,@dagger.Component,false",
@@ -54,17 +54,17 @@ class AnnotationExcluderSpec {
                 "dagger.*,@Component,true",
                 "dagger.*,@dagger.Component,true",
                 "dagger.*,@Factory,true",
-                "dagger.*,@Component.Factory,false", // false positive
+                "dagger.*,@Component.Factory,true",
                 "dagger.*,@dagger.Component.Factory,true",
                 "*.Component.Factory,@Component,false",
                 "*.Component.Factory,@dagger.Component,false",
                 "*.Component.Factory,@Factory,true",
-                "*.Component.Factory,@Component.Factory,false", // false positive
+                "*.Component.Factory,@Component.Factory,true",
                 "*.Component.Factory,@dagger.Component.Factory,true",
                 "*.Component.*,@Component,false",
                 "*.Component.*,@dagger.Component,false",
                 "*.Component.*,@Factory,true",
-                "*.Component.*,@Component.Factory,false", // false positive
+                "*.Component.*,@Component.Factory,true",
                 "*.Component.*,@dagger.Component.Factory,true",
                 "foo.Component,@Component,false",
                 "foo.Component,@dagger.Component,false",