Report KDoc comments that refer to non-public properties of a class (#…

…4768)

* Report KDoc comments that refer to encapsulated properties of a class

New rule to address the issue of breaking encapsulation principle when KDoc comments have references to encapsulated (private) properties.
Clients do not need to know the implementation details.

* Rename rule to 'KDocReferencesNonPublicProperty' and fix naming

* Set 'protected' modifier as allowed

* Improve tests

* Fix rule documentation

detekt-core/src/main/resources/default-detekt-config.yml

@@ -62,6 +62,9 @@ comments:
   EndOfSentenceFormat:
     active: false
     endOfSentenceFormat: '([.?!][ \t\n\r\f<])|([.?!:]$)'
+  KDocReferencesNonPublicProperty:
+    active: false
+    excludes: ['**/test/**', '**/androidTest/**', '**/commonTest/**', '**/jvmTest/**', '**/jsTest/**', '**/iosTest/**']
   OutdatedDocumentation:
     active: false
     matchTypeParameters: true

detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/defaultconfig/Exclusion.kt

@@ -41,6 +41,7 @@ private object TestExclusions : Exclusions() {
         "UndocumentedPublicFunction",
         "UndocumentedPublicProperty",
         "UnsafeCallOnNullableType",
+        "KDocReferencesNonPublicProperty",
     )
 }
 

detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/CommentSmellProvider.kt

@@ -24,7 +24,8 @@ class CommentSmellProvider : DefaultRuleSetProvider {
             UndocumentedPublicClass(config),
             UndocumentedPublicFunction(config),
             UndocumentedPublicProperty(config),
-            AbsentOrWrongFileLicense(config)
+            AbsentOrWrongFileLicense(config),
+            KDocReferencesNonPublicProperty(config)
         )
     )
 }

detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicProperty.kt

@@ -0,0 +1,101 @@
+package io.gitlab.arturbosch.detekt.rules.documentation
+
+import io.gitlab.arturbosch.detekt.api.CodeSmell
+import io.gitlab.arturbosch.detekt.api.Config
+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 org.jetbrains.kotlin.psi.KtClass
+import org.jetbrains.kotlin.psi.KtNamedDeclaration
+import org.jetbrains.kotlin.psi.KtObjectDeclaration
+import org.jetbrains.kotlin.psi.KtProperty
+import org.jetbrains.kotlin.psi.psiUtil.containingClassOrObject
+import org.jetbrains.kotlin.psi.psiUtil.getTopmostParentOfType
+import org.jetbrains.kotlin.psi.psiUtil.isProtected
+import org.jetbrains.kotlin.psi.psiUtil.isPublic
+
+/**
+ * This rule will report any KDoc comments that refer to non-public properties of a class.
+ * Clients do not need to know the implementation details.
+ *
+ * <noncompliant>
+ * /**
+ *  * Comment
+ *  * [prop1] - non-public property
+ *  * [prop2] - public property
+ *  */
+ * class Test {
+ *     private val prop1 = 0
+ *     val prop2 = 0
+ * }
+ * </noncompliant>
+ *
+ * <compliant>
+ * /**
+ *  * Comment
+ *  * [prop2] - public property
+ *  */
+ * class Test {
+ *     private val prop1 = 0
+ *     val prop2 = 0
+ * }
+ * </compliant>
+ *
+ */
+class KDocReferencesNonPublicProperty(config: Config = Config.empty) : Rule(config) {
+
+    override val issue = Issue(
+        javaClass.simpleName,
+        Severity.Maintainability,
+        "KDoc comments should not refer to non-public properties.",
+        Debt.FIVE_MINS
+    )
+
+    override fun visitProperty(property: KtProperty) {
+        super.visitProperty(property)
+
+        val enclosingClass = property.getTopmostParentOfType<KtClass>()
+        val comment = enclosingClass?.docComment?.text ?: return
+
+        if (property.isNonPublicInherited() && property.isReferencedInherited(comment)) {
+            report(property)
+        }
+    }
+
+    private fun KtProperty.isNonPublicInherited(): Boolean {
+        if (!isPublic && !isProtected()) {
+            return true
+        }
+        var classOrObject = containingClassOrObject
+        while (classOrObject is KtObjectDeclaration) {
+            if (!classOrObject.isPublic) {
+                return true
+            }
+            classOrObject = classOrObject.containingClassOrObject
+        }
+        return false
+    }
+
+    private fun KtProperty.isReferencedInherited(comment: String): Boolean {
+        var qualifiedName = nameAsSafeName.asString()
+        var classOrObject = containingClassOrObject
+        while (classOrObject is KtObjectDeclaration) {
+            qualifiedName = "${classOrObject.nameAsSafeName.asString()}.$qualifiedName"
+            classOrObject = classOrObject.containingClassOrObject
+        }
+        return comment.contains("[$qualifiedName]")
+    }
+
+    private fun report(property: KtNamedDeclaration) {
+        report(
+            CodeSmell(
+                issue,
+                Entity.atName(property),
+                "The property ${property.nameAsSafeName} " +
+                    "is non-public and should not be referenced from KDoc comments."
+            )
+        )
+    }
+}

detekt-rules-documentation/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicPropertySpec.kt

@@ -0,0 +1,134 @@
+package io.gitlab.arturbosch.detekt.rules.documentation
+
+import io.gitlab.arturbosch.detekt.test.compileAndLint
+import org.assertj.core.api.Assertions.assertThat
+import org.junit.jupiter.api.Test
+
+class KDocReferencesNonPublicPropertySpec {
+    val subject = KDocReferencesNonPublicProperty()
+
+    @Test
+    fun `reports referenced non-public properties`() {
+        val code = """ 
+        /**
+         * Comment
+         * [prop1] - non-public property
+         * [prop2] - public property
+         */
+        class Test {
+            private val nonReferencedProp = 0
+            private val prop1 = 0
+            val prop2 = 0
+        }
+        """.trimIndent()
+        assertThat(subject.compileAndLint(code)).hasSize(1)
+    }
+
+    @Test
+    fun `reports referenced non-public properties in private class`() {
+        val code = """ 
+        /**
+         * Comment
+         * [prop1] - non-public property
+         * [prop2] - public property
+         */
+        private class Test {
+            private val nonReferencedProp = 0
+            private val prop1 = 0
+            val prop2 = 0
+        }
+        """.trimIndent()
+        assertThat(subject.compileAndLint(code)).hasSize(1)
+    }
+
+    @Test
+    fun `reports referenced non-public properties in nested objects`() {
+        val code = """ 
+        /**
+         * Comment
+         * [prop1] - non-public property
+         * [A.prop2] - non-public property
+         * [A.B.prop3] - non-public property
+         * [A.C.prop4] - non-public property
+         */
+        class Test {
+            private val prop1 = 0
+            
+            object A {
+                private val nonReferencedProp = 0
+                private val prop2 = 0
+                
+                private object B {
+                    val prop3 = 0
+                }
+                object C {
+                    private val prop4 = 0
+                }
+            }
+        }
+        """.trimIndent()
+        assertThat(subject.compileAndLint(code)).hasSize(4)
+    }
+
+    @Test
+    fun `does not report properties with no KDoc`() {
+        val code = """ 
+        class Test {
+            private val prop1 = 0
+            val prop2 = 0
+        }
+        """.trimIndent()
+        assertThat(subject.compileAndLint(code)).isEmpty()
+    }
+
+    @Test
+    fun `does not report properties with empty comments`() {
+        val code = """
+        /**
+        */
+        class Test {
+            private val prop1 = 0
+            val prop2 = 0
+        }
+        """.trimIndent()
+        assertThat(subject.compileAndLint(code)).isEmpty()
+    }
+
+    @Test
+    fun `does not report properties not enclosed in a class`() {
+        val code = """
+        /**
+        * [prop1]
+        * [prop2]
+        */
+        private val prop1 = 0
+        val prop2 = 0
+        """.trimIndent()
+        assertThat(subject.compileAndLint(code)).isEmpty()
+    }
+
+    @Test
+    fun `does not report referenced public properties in nested objects`() {
+        val code = """ 
+        /**
+         * Comment
+         * [prop1] - public property
+         * [A.B.prop2] - public property
+         * [C.prop3] - public property
+         */
+        open class Test {
+            protected val prop1 = 0
+            object A {
+                object B {
+                    val nonReferencedProp = 0
+                    val prop2 = 0
+                }
+            }
+            object C {
+                val prop3 = 0
+            }
+        }
+        """.trimIndent()
+        assertThat(subject.compileAndLint(code)).isEmpty()
+    }
+}