Detect undocumented protected classes, properties, and functions (det…

…ekt#5083) * feat: UndocumentedPublicFunction supports protected functions refs detekt#4633 * feat: UndocumentedPublicProperty supports protected properties refs detekt#4633 Signed-off-by: Kengo TODA <skypencil@gmail.com> * feat: UndocumentedPublicClass supports protected classes refs detekt#4633 Signed-off-by: Kengo TODA <skypencil@gmail.com> * chore: apply detekt rules Signed-off-by: Kengo TODA <skypencil@gmail.com> * docs: update the document Signed-off-by: Kengo TODA <skypencil@gmail.com> * fix: update the API file to pass pre-merge checks Signed-off-by: Kengo TODA <skypencil@gmail.com> * fix: rename a config based on the review comment detekt#5083 (comment) Signed-off-by: Kengo TODA <skypencil@gmail.com> * test: make test snippets compilable Signed-off-by: Kengo TODA <skypencil@gmail.com> * fix: broken binary compatibility Signed-off-by: Kengo TODA <skypencil@gmail.com> * docs: Update detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/UndocumentedPublicClass.kt Co-authored-by: schalkms <30376729+schalkms@users.noreply.github.com> * docs: Update detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/UndocumentedPublicFunction.kt Co-authored-by: schalkms <30376729+schalkms@users.noreply.github.com> * docs: Update detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/UndocumentedPublicProperty.kt Co-authored-by: schalkms <30376729+schalkms@users.noreply.github.com> Co-authored-by: schalkms <30376729+schalkms@users.noreply.github.com>
VitalyVPinchuk · Jul 25, 2022 · a5db5ee · a5db5ee
1 parent a3dfadc
commit a5db5ee
Show file tree

Hide file tree

Showing 10 changed files with 107 additions and 7 deletions.
diff --git a/detekt-core/src/main/resources/default-detekt-config.yml b/detekt-core/src/main/resources/default-detekt-config.yml
@@ -78,12 +78,15 @@ comments:
     searchInInnerClass: true
     searchInInnerObject: true
     searchInInnerInterface: true
+    searchInProtectedClass: false
   UndocumentedPublicFunction:
     active: false
     excludes: ['**/test/**', '**/androidTest/**', '**/commonTest/**', '**/jvmTest/**', '**/jsTest/**', '**/iosTest/**']
+    searchProtectedFunction: false
   UndocumentedPublicProperty:
     active: false
     excludes: ['**/test/**', '**/androidTest/**', '**/commonTest/**', '**/jvmTest/**', '**/jsTest/**', '**/iosTest/**']
+    searchProtectedProperty: false
 
 complexity:
   active: true

diff --git a/detekt-psi-utils/api/detekt-psi-utils.api b/detekt-psi-utils/api/detekt-psi-utils.api
@@ -107,6 +107,7 @@ public final class io/gitlab/arturbosch/detekt/rules/KtModifierListKt {
 	public static final fun isOverride (Lorg/jetbrains/kotlin/psi/KtModifierListOwner;)Z
 	public static final fun isProtected (Lorg/jetbrains/kotlin/psi/KtModifierListOwner;)Z
 	public static final fun isPublicNotOverridden (Lorg/jetbrains/kotlin/psi/KtModifierListOwner;)Z
+	public static final fun isPublicNotOverridden (Lorg/jetbrains/kotlin/psi/KtModifierListOwner;Z)Z
 }
 
 public final class io/gitlab/arturbosch/detekt/rules/KtValueArgumentKt {
@@ -135,6 +136,7 @@ public final class io/gitlab/arturbosch/detekt/rules/ThrowExtensionsKt {
 
 public final class io/gitlab/arturbosch/detekt/rules/TraversingKt {
 	public static final fun isPublicInherited (Lorg/jetbrains/kotlin/psi/KtNamedDeclaration;)Z
+	public static final fun isPublicInherited (Lorg/jetbrains/kotlin/psi/KtNamedDeclaration;Z)Z
 }
 
 public final class io/gitlab/arturbosch/detekt/rules/TypeUtilsKt {

diff --git a/detekt-psi-utils/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/KtModifierList.kt b/detekt-psi-utils/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/KtModifierList.kt
@@ -7,7 +7,14 @@ import org.jetbrains.kotlin.psi.KtModifierListOwner
 import org.jetbrains.kotlin.psi.psiUtil.isPublic
 
 fun KtModifierListOwner.isPublicNotOverridden() =
-    isPublic && !isOverride()
+    isPublicNotOverridden(false)
+
+fun KtModifierListOwner.isPublicNotOverridden(considerProtectedAsPublic: Boolean) =
+    if (considerProtectedAsPublic) {
+        isPublic || isProtected()
+    } else {
+        isPublic
+    } && !isOverride()
 
 fun KtModifierListOwner.isAbstract() = hasModifier(KtTokens.ABSTRACT_KEYWORD)
 

diff --git a/detekt-psi-utils/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/Traversing.kt b/detekt-psi-utils/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/Traversing.kt
@@ -20,10 +20,12 @@ inline fun <reified T : KtElement, reified S : KtElement> KtElement.parentsOfTyp
         }
     }
 
-fun KtNamedDeclaration.isPublicInherited(): Boolean {
+fun KtNamedDeclaration.isPublicInherited(): Boolean = isPublicInherited(false)
+
+fun KtNamedDeclaration.isPublicInherited(considerProtectedAsPublic: Boolean): Boolean {
     var classOrObject = containingClassOrObject
     while (classOrObject != null) {
-        if (!classOrObject.isPublic) {
+        if (!classOrObject.isPublic && !(considerProtectedAsPublic && classOrObject.isProtected())) {
             return false
         }
         classOrObject = classOrObject.containingClassOrObject

diff --git a/...rc/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/UndocumentedPublicClass.kt b/...rc/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/UndocumentedPublicClass.kt
@@ -44,6 +44,9 @@ class UndocumentedPublicClass(config: Config = Config.empty) : Rule(config) {
     @Configuration("if inner interfaces should be searched")
     private val searchInInnerInterface: Boolean by config(true)
 
+    @Configuration("if protected classes should be searched")
+    private val searchInProtectedClass: Boolean by config(false)
+
     override fun visitClass(klass: KtClass) {
         if (requiresDocumentation(klass)) {
             reportIfUndocumented(klass)
@@ -81,7 +84,7 @@ class UndocumentedPublicClass(config: Config = Config.empty) : Rule(config) {
     }
 
     private fun isPublicAndPublicInherited(element: KtClassOrObject) =
-        element.isPublicInherited() && element.isPublicNotOverridden()
+        element.isPublicInherited(searchInProtectedClass) && element.isPublicNotOverridden(searchInProtectedClass)
 
     private fun KtObjectDeclaration.isCompanionWithoutName() =
         isCompanion() && nameAsSafeName.asString() == "Companion"

diff --git a/...main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/UndocumentedPublicFunction.kt b/...main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/UndocumentedPublicFunction.kt
@@ -7,6 +7,9 @@ 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.config
+import io.gitlab.arturbosch.detekt.api.internal.Configuration
+import io.gitlab.arturbosch.detekt.rules.isProtected
 import io.gitlab.arturbosch.detekt.rules.isPublicNotOverridden
 import org.jetbrains.kotlin.psi.KtClassOrObject
 import org.jetbrains.kotlin.psi.KtNamedFunction
@@ -27,6 +30,9 @@ class UndocumentedPublicFunction(config: Config = Config.empty) : Rule(config) {
         Debt.TWENTY_MINS
     )
 
+    @Configuration("if protected functions should be searched")
+    private val searchProtectedFunction: Boolean by config(false)
+
     override fun visitNamedFunction(function: KtNamedFunction) {
         if (function.funKeyword == null && function.isLocal) return
 
@@ -42,5 +48,9 @@ class UndocumentedPublicFunction(config: Config = Config.empty) : Rule(config) {
     }
 
     private fun KtNamedFunction.shouldBeDocumented() =
-        parents.filterIsInstance<KtClassOrObject>().all { it.isPublic } && isPublicNotOverridden()
+        if (searchProtectedFunction) {
+            parents.filterIsInstance<KtClassOrObject>().all { it.isPublic || it.isProtected() }
+        } else {
+            parents.filterIsInstance<KtClassOrObject>().all { it.isPublic }
+        } && isPublicNotOverridden(searchProtectedFunction)
 }
diff --git a/...main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/UndocumentedPublicProperty.kt b/...main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/UndocumentedPublicProperty.kt
@@ -7,6 +7,8 @@ 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.config
+import io.gitlab.arturbosch.detekt.api.internal.Configuration
 import io.gitlab.arturbosch.detekt.rules.isPublicInherited
 import io.gitlab.arturbosch.detekt.rules.isPublicNotOverridden
 import org.jetbrains.kotlin.psi.KtNamedDeclaration
@@ -31,6 +33,9 @@ class UndocumentedPublicProperty(config: Config = Config.empty) : Rule(config) {
         Debt.TWENTY_MINS
     )
 
+    @Configuration("if protected functions should be searched")
+    private val searchProtectedProperty: Boolean by config(false)
+
     override fun visitPrimaryConstructor(constructor: KtPrimaryConstructor) {
         if (constructor.isPublicInherited()) {
             val comment = constructor.containingClassOrObject?.docComment?.text
@@ -43,7 +48,7 @@ class UndocumentedPublicProperty(config: Config = Config.empty) : Rule(config) {
     }
 
     override fun visitProperty(property: KtProperty) {
-        if (property.isPublicInherited() && !property.isLocal && property.shouldBeDocumented()) {
+        if (property.isPublicInherited(searchProtectedProperty) && !property.isLocal && property.shouldBeDocumented()) {
             report(property)
         }
         super.visitProperty(property)
@@ -58,7 +63,7 @@ class UndocumentedPublicProperty(config: Config = Config.empty) : Rule(config) {
     }
 
     private fun KtProperty.shouldBeDocumented() =
-        docComment == null && isTopLevelOrInPublicClass() && isPublicNotOverridden()
+        docComment == null && isTopLevelOrInPublicClass() && isPublicNotOverridden(searchProtectedProperty)
 
     private fun KtProperty.isTopLevelOrInPublicClass() = isTopLevel || containingClassOrObject?.isPublic == true
 

diff --git a/...est/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/UndocumentedPublicClassSpec.kt b/...est/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/UndocumentedPublicClassSpec.kt
@@ -9,6 +9,7 @@ private const val SEARCH_IN_NESTED_CLASS = "searchInNestedClass"
 private const val SEARCH_IN_INNER_CLASS = "searchInInnerClass"
 private const val SEARCH_IN_INNER_OBJECT = "searchInInnerObject"
 private const val SEARCH_IN_INNER_INTERFACE = "searchInInnerInterface"
+private const val SEARCH_IN_PROTECTED_CLASS = "searchInProtectedClass"
 
 class UndocumentedPublicClassSpec {
     val subject = UndocumentedPublicClass()
@@ -233,4 +234,23 @@ class UndocumentedPublicClassSpec {
         """
         assertThat(subject.compileAndLint(code)).isEmpty()
     }
+
+    @Test
+    fun `does not report protected class by default`() {
+        val code = """
+        protected class Test {
+        }
+        """
+        assertThat(subject.compileAndLint(code)).isEmpty()
+    }
+
+    @Test
+    fun `reports protected class if configured`() {
+        val code = """
+        protected class Test {
+        }
+        """
+        val subject = UndocumentedPublicClass(TestConfig(mapOf(SEARCH_IN_PROTECTED_CLASS to "true")))
+        assertThat(subject.compileAndLint(code)).hasSize(1)
+    }
 }
diff --git a/.../kotlin/io/gitlab/arturbosch/detekt/rules/documentation/UndocumentedPublicFunctionSpec.kt b/.../kotlin/io/gitlab/arturbosch/detekt/rules/documentation/UndocumentedPublicFunctionSpec.kt
@@ -1,10 +1,13 @@
 package io.gitlab.arturbosch.detekt.rules.documentation
 
+import io.gitlab.arturbosch.detekt.test.TestConfig
 import io.gitlab.arturbosch.detekt.test.compileAndLint
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.jupiter.api.Nested
 import org.junit.jupiter.api.Test
 
+private const val SEARCH_PROTECTED_FUN = "searchProtectedFunction"
+
 class UndocumentedPublicFunctionSpec {
     val subject = UndocumentedPublicFunction()
 
@@ -141,6 +144,27 @@ class UndocumentedPublicFunctionSpec {
         assertThat(subject.compileAndLint(code)).isEmpty()
     }
 
+    @Test
+    fun `does not report protected functions by default`() {
+        val code = """
+            open class Test {
+                protected fun noComment1() {}
+            }
+        """
+        assertThat(subject.compileAndLint(code)).isEmpty()
+    }
+
+    @Test
+    fun `reports protected functions if configured`() {
+        val code = """
+            open class Test {
+                protected fun noComment1() {}
+            }
+        """
+        val subject = UndocumentedPublicFunction(TestConfig(mapOf(SEARCH_PROTECTED_FUN to "true")))
+        assertThat(subject.compileAndLint(code)).hasSize(1)
+    }
+
     @Nested
     inner class `nested class` {
         @Test

diff --git a/.../kotlin/io/gitlab/arturbosch/detekt/rules/documentation/UndocumentedPublicPropertySpec.kt b/.../kotlin/io/gitlab/arturbosch/detekt/rules/documentation/UndocumentedPublicPropertySpec.kt
@@ -1,10 +1,13 @@
 package io.gitlab.arturbosch.detekt.rules.documentation
 
+import io.gitlab.arturbosch.detekt.test.TestConfig
 import io.gitlab.arturbosch.detekt.test.compileAndLint
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.jupiter.api.Nested
 import org.junit.jupiter.api.Test
 
+private const val SEARCH_PROTECTED_PROPERTY = "searchProtectedProperty"
+
 class UndocumentedPublicPropertySpec {
     val subject = UndocumentedPublicProperty()
 
@@ -213,6 +216,27 @@ class UndocumentedPublicPropertySpec {
         assertThat(subject.compileAndLint(code)).isEmpty()
     }
 
+    @Test
+    fun `does not report undocumented protected properties by default`() {
+        val code = """
+            open class Test {
+                protected val a = 1
+            }
+        """
+        assertThat(subject.compileAndLint(code)).isEmpty()
+    }
+
+    @Test
+    fun `reports undocumented protected properties if configured`() {
+        val code = """
+            open class Test {
+                protected val a = 1
+            }
+        """
+        val subject = UndocumentedPublicProperty(TestConfig(mapOf(SEARCH_PROTECTED_PROPERTY to "true")))
+        assertThat(subject.compileAndLint(code)).hasSize(1)
+    }
+
     @Nested
     inner class `public properties in nested classes` {