From 501473b05b6e83d84fe1977b1ee39011e1fbebf1 Mon Sep 17 00:00:00 2001 From: "Vitaly V. Pinchuk" Date: Mon, 25 Apr 2022 19:15:47 +0300 Subject: [PATCH 1/5] 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. --- .../main/resources/default-detekt-config.yml | 3 + .../printer/defaultconfig/Exclusion.kt | 1 + .../documentation/CommentSmellProvider.kt | 3 +- .../ReferencedEncapsulatedProperty.kt | 77 +++++++++++++++ .../ReferencedEncapsulatedPropertySpec.kt | 98 +++++++++++++++++++ 5 files changed, 181 insertions(+), 1 deletion(-) create mode 100644 detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/ReferencedEncapsulatedProperty.kt create mode 100644 detekt-rules-documentation/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/ReferencedEncapsulatedPropertySpec.kt diff --git a/detekt-core/src/main/resources/default-detekt-config.yml b/detekt-core/src/main/resources/default-detekt-config.yml index 9879910ba86..38c7d4b3a13 100644 --- a/detekt-core/src/main/resources/default-detekt-config.yml +++ b/detekt-core/src/main/resources/default-detekt-config.yml @@ -67,6 +67,9 @@ comments: matchTypeParameters: true matchDeclarationsOrder: true allowParamOnConstructorProperties: false + ReferencedEncapsulatedProperty: + active: false + excludes: ['**/test/**', '**/androidTest/**', '**/commonTest/**', '**/jvmTest/**', '**/jsTest/**', '**/iosTest/**'] UndocumentedPublicClass: active: false excludes: ['**/test/**', '**/androidTest/**', '**/commonTest/**', '**/jvmTest/**', '**/jsTest/**', '**/iosTest/**'] diff --git a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/defaultconfig/Exclusion.kt b/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/defaultconfig/Exclusion.kt index 7eca3361cb2..31082623265 100644 --- a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/defaultconfig/Exclusion.kt +++ b/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", + "ReferencedEncapsulatedProperty", ) } diff --git a/detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/CommentSmellProvider.kt b/detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/CommentSmellProvider.kt index 8d93bd8e9a1..7d580a5b4eb 100644 --- a/detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/CommentSmellProvider.kt +++ b/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), + ReferencedEncapsulatedProperty(config) ) ) } diff --git a/detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/ReferencedEncapsulatedProperty.kt b/detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/ReferencedEncapsulatedProperty.kt new file mode 100644 index 00000000000..2cd084cbddf --- /dev/null +++ b/detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/ReferencedEncapsulatedProperty.kt @@ -0,0 +1,77 @@ +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.isPublic + +/** + * This rule will report any KDoc comments that refer to encapsulated properties of a class. + * Clients do not need to know the implementation details. + * See [Encapsulation](https://en.wikipedia.org/wiki/Encapsulation_(computer_programming)) + */ +class ReferencedEncapsulatedProperty(config: Config = Config.empty) : Rule(config) { + + override val issue = Issue( + javaClass.simpleName, + Severity.Maintainability, + "KDoc comments should not refer to encapsulated properties.", + Debt.FIVE_MINS + ) + + override fun visitProperty(property: KtProperty) { + val enclosingClass = property.getTopmostParentOfType() + val comment = enclosingClass?.docComment?.text ?: return + + if (property.isEncapsulatedInherited() && property.isReferencedInherited(comment)) { + report(property) + } + + super.visitProperty(property) + } + + private fun KtProperty.isEncapsulatedInherited(): Boolean { + if (!isPublic) { + return true + } + var classOrObject = containingClassOrObject + while (classOrObject != null && 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 != null && 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 encapsulated and should not be referenced from KDoc comments." + ) + ) + } +} diff --git a/detekt-rules-documentation/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/ReferencedEncapsulatedPropertySpec.kt b/detekt-rules-documentation/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/ReferencedEncapsulatedPropertySpec.kt new file mode 100644 index 00000000000..9eeadd7059c --- /dev/null +++ b/detekt-rules-documentation/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/ReferencedEncapsulatedPropertySpec.kt @@ -0,0 +1,98 @@ +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.Nested +import org.junit.jupiter.api.Test + +class ReferencedEncapsulatedPropertySpec { + val subject = ReferencedEncapsulatedProperty() + + @Nested + inner class `ReferencedEncapsulatedProperty rule` { + + @Test + fun `reports referenced encapsulated properties`() { + val code = """ + /** + * Comment + * [prop1] - encapsulated property + * [prop2] - public property + */ + class Test { + private val prop1 = 0 // report + val prop2 = 0 // do not report + } + """.trimIndent() + assertThat(subject.compileAndLint(code)).hasSize(1) + } + + @Test + fun `reports referenced encapsulated properties in private class`() { + val code = """ + /** + * Comment + * [prop1] - encapsulated property + * [prop2] - public property + */ + private class Test { + private val prop1 = 0 // report + val prop2 = 0 // do not report + } + """.trimIndent() + assertThat(subject.compileAndLint(code)).hasSize(1) + } + + @Test + fun `reports referenced encapsulated properties in nested objects`() { + val code = """ + /** + * Comment + * [prop1] - encapsulated property + * [A.prop2] - encapsulated property + * [A.B.prop3] - encapsulated property + * [A.C.prop4] - encapsulated property + */ + class Test { + private val prop1 = 0 + + object A { + 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 referenced public properties in nested objects`() { + val code = """ + /** + * Comment + * [prop1] - public property + * [A.B.prop2] - public property + * [C.prop3] - public property + */ + class Test { + val prop1 = 0 + object A { + object B { + val prop2 = 0 + } + } + object C { + val prop3 = 0 + } + } + """.trimIndent() + assertThat(subject.compileAndLint(code)).isEmpty() + } + } +} From b8700b694432006575aa71b6f97af0a75c6182ce Mon Sep 17 00:00:00 2001 From: "Vitaly V. Pinchuk" Date: Tue, 26 Apr 2022 22:18:27 +0300 Subject: [PATCH 2/5] Rename rule to 'KDocReferencesNonPublicProperty' and fix naming --- .../main/resources/default-detekt-config.yml | 6 +-- .../printer/defaultconfig/Exclusion.kt | 2 +- .../documentation/CommentSmellProvider.kt | 2 +- ....kt => KDocReferencesNonPublicProperty.kt} | 41 +++++++++++++++---- ...=> KDocReferencesNonPublicPropertySpec.kt} | 34 ++++++++------- 5 files changed, 54 insertions(+), 31 deletions(-) rename detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/{ReferencedEncapsulatedProperty.kt => KDocReferencesNonPublicProperty.kt} (73%) rename detekt-rules-documentation/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/{ReferencedEncapsulatedPropertySpec.kt => KDocReferencesNonPublicPropertySpec.kt} (69%) diff --git a/detekt-core/src/main/resources/default-detekt-config.yml b/detekt-core/src/main/resources/default-detekt-config.yml index 38c7d4b3a13..ca229b4b698 100644 --- a/detekt-core/src/main/resources/default-detekt-config.yml +++ b/detekt-core/src/main/resources/default-detekt-config.yml @@ -62,14 +62,14 @@ comments: EndOfSentenceFormat: active: false endOfSentenceFormat: '([.?!][ \t\n\r\f<])|([.?!:]$)' + KDocReferencesNonPublicProperty: + active: false + excludes: ['**/test/**', '**/androidTest/**', '**/commonTest/**', '**/jvmTest/**', '**/jsTest/**', '**/iosTest/**'] OutdatedDocumentation: active: false matchTypeParameters: true matchDeclarationsOrder: true allowParamOnConstructorProperties: false - ReferencedEncapsulatedProperty: - active: false - excludes: ['**/test/**', '**/androidTest/**', '**/commonTest/**', '**/jvmTest/**', '**/jsTest/**', '**/iosTest/**'] UndocumentedPublicClass: active: false excludes: ['**/test/**', '**/androidTest/**', '**/commonTest/**', '**/jvmTest/**', '**/jsTest/**', '**/iosTest/**'] diff --git a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/defaultconfig/Exclusion.kt b/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/defaultconfig/Exclusion.kt index 31082623265..380dfa87258 100644 --- a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/defaultconfig/Exclusion.kt +++ b/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/defaultconfig/Exclusion.kt @@ -41,7 +41,7 @@ private object TestExclusions : Exclusions() { "UndocumentedPublicFunction", "UndocumentedPublicProperty", "UnsafeCallOnNullableType", - "ReferencedEncapsulatedProperty", + "KDocReferencesNonPublicProperty", ) } diff --git a/detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/CommentSmellProvider.kt b/detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/CommentSmellProvider.kt index 7d580a5b4eb..860996b59ca 100644 --- a/detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/CommentSmellProvider.kt +++ b/detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/CommentSmellProvider.kt @@ -25,7 +25,7 @@ class CommentSmellProvider : DefaultRuleSetProvider { UndocumentedPublicFunction(config), UndocumentedPublicProperty(config), AbsentOrWrongFileLicense(config), - ReferencedEncapsulatedProperty(config) + KDocReferencesNonPublicProperty(config) ) ) } diff --git a/detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/ReferencedEncapsulatedProperty.kt b/detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicProperty.kt similarity index 73% rename from detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/ReferencedEncapsulatedProperty.kt rename to detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicProperty.kt index 2cd084cbddf..7e2571187b7 100644 --- a/detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/ReferencedEncapsulatedProperty.kt +++ b/detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicProperty.kt @@ -16,31 +16,56 @@ import org.jetbrains.kotlin.psi.psiUtil.getTopmostParentOfType import org.jetbrains.kotlin.psi.psiUtil.isPublic /** - * This rule will report any KDoc comments that refer to encapsulated properties of a class. + * 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. + * * See [Encapsulation](https://en.wikipedia.org/wiki/Encapsulation_(computer_programming)) + * + * + * /** + * * Comment + * * [prop1] - non-public property + * * [prop2] - public property + * */ + * class Test { + * private val prop1 = 0 + * val prop2 = 0 + * } + * + * + * + * /** + * * Comment + * * [prop2] - public property + * */ + * class Test { + * private val prop1 = 0 + * val prop2 = 0 + * } + * + * */ -class ReferencedEncapsulatedProperty(config: Config = Config.empty) : Rule(config) { +class KDocReferencesNonPublicProperty(config: Config = Config.empty) : Rule(config) { override val issue = Issue( javaClass.simpleName, Severity.Maintainability, - "KDoc comments should not refer to encapsulated properties.", + "KDoc comments should not refer to non-public properties.", Debt.FIVE_MINS ) override fun visitProperty(property: KtProperty) { + super.visitProperty(property) + val enclosingClass = property.getTopmostParentOfType() val comment = enclosingClass?.docComment?.text ?: return - if (property.isEncapsulatedInherited() && property.isReferencedInherited(comment)) { + if (property.isNonPublicInherited() && property.isReferencedInherited(comment)) { report(property) } - - super.visitProperty(property) } - private fun KtProperty.isEncapsulatedInherited(): Boolean { + private fun KtProperty.isNonPublicInherited(): Boolean { if (!isPublic) { return true } @@ -70,7 +95,7 @@ class ReferencedEncapsulatedProperty(config: Config = Config.empty) : Rule(confi issue, Entity.atName(property), "The property ${property.nameAsSafeName} " + - "is encapsulated and should not be referenced from KDoc comments." + "is non-public and should not be referenced from KDoc comments." ) ) } diff --git a/detekt-rules-documentation/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/ReferencedEncapsulatedPropertySpec.kt b/detekt-rules-documentation/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicPropertySpec.kt similarity index 69% rename from detekt-rules-documentation/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/ReferencedEncapsulatedPropertySpec.kt rename to detekt-rules-documentation/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicPropertySpec.kt index 9eeadd7059c..9d340dc9ec5 100644 --- a/detekt-rules-documentation/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/ReferencedEncapsulatedPropertySpec.kt +++ b/detekt-rules-documentation/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicPropertySpec.kt @@ -2,56 +2,54 @@ 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.Nested import org.junit.jupiter.api.Test -class ReferencedEncapsulatedPropertySpec { - val subject = ReferencedEncapsulatedProperty() +class KDocReferencesNonPublicPropertySpec { + val subject = KDocReferencesNonPublicProperty() - @Nested - inner class `ReferencedEncapsulatedProperty rule` { + inner class `KDocReferencesNonPublicProperty rule` { @Test - fun `reports referenced encapsulated properties`() { + fun `reports referenced non-public properties`() { val code = """ /** * Comment - * [prop1] - encapsulated property + * [prop1] - non-public property * [prop2] - public property */ class Test { - private val prop1 = 0 // report - val prop2 = 0 // do not report + private val prop1 = 0 + val prop2 = 0 } """.trimIndent() assertThat(subject.compileAndLint(code)).hasSize(1) } @Test - fun `reports referenced encapsulated properties in private class`() { + fun `reports referenced non-public properties in private class`() { val code = """ /** * Comment - * [prop1] - encapsulated property + * [prop1] - non-public property * [prop2] - public property */ private class Test { - private val prop1 = 0 // report - val prop2 = 0 // do not report + private val prop1 = 0 + val prop2 = 0 } """.trimIndent() assertThat(subject.compileAndLint(code)).hasSize(1) } @Test - fun `reports referenced encapsulated properties in nested objects`() { + fun `reports referenced non-public properties in nested objects`() { val code = """ /** * Comment - * [prop1] - encapsulated property - * [A.prop2] - encapsulated property - * [A.B.prop3] - encapsulated property - * [A.C.prop4] - encapsulated property + * [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 From b86ae6f92185d2284e668c614fb9c11b6200e3e3 Mon Sep 17 00:00:00 2001 From: "Vitaly V. Pinchuk" Date: Thu, 28 Apr 2022 13:34:01 +0300 Subject: [PATCH 3/5] Set 'protected' modifier as allowed --- .../rules/documentation/KDocReferencesNonPublicProperty.kt | 5 +++-- .../documentation/KDocReferencesNonPublicPropertySpec.kt | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicProperty.kt b/detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicProperty.kt index 7e2571187b7..81a3a16ea88 100644 --- a/detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicProperty.kt +++ b/detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicProperty.kt @@ -13,6 +13,7 @@ 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 /** @@ -66,12 +67,12 @@ class KDocReferencesNonPublicProperty(config: Config = Config.empty) : Rule(conf } private fun KtProperty.isNonPublicInherited(): Boolean { - if (!isPublic) { + if (!isPublic && !isProtected()) { return true } var classOrObject = containingClassOrObject while (classOrObject != null && classOrObject is KtObjectDeclaration) { - if (!classOrObject.isPublic) { + if (!classOrObject.isPublic && !isProtected()) { return true } classOrObject = classOrObject.containingClassOrObject diff --git a/detekt-rules-documentation/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicPropertySpec.kt b/detekt-rules-documentation/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicPropertySpec.kt index 9d340dc9ec5..54616c035f8 100644 --- a/detekt-rules-documentation/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicPropertySpec.kt +++ b/detekt-rules-documentation/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicPropertySpec.kt @@ -78,8 +78,8 @@ class KDocReferencesNonPublicPropertySpec { * [A.B.prop2] - public property * [C.prop3] - public property */ - class Test { - val prop1 = 0 + open class Test { + protected val prop1 = 0 object A { object B { val prop2 = 0 From eed733ce98902d133dc5ddd973d04f92a2d0a04b Mon Sep 17 00:00:00 2001 From: "Vitaly V. Pinchuk" Date: Thu, 5 May 2022 06:22:19 +0300 Subject: [PATCH 4/5] Improve tests --- .../KDocReferencesNonPublicProperty.kt | 6 +- .../KDocReferencesNonPublicPropertySpec.kt | 184 +++++++++++------- 2 files changed, 114 insertions(+), 76 deletions(-) diff --git a/detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicProperty.kt b/detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicProperty.kt index 81a3a16ea88..f424afc5cd7 100644 --- a/detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicProperty.kt +++ b/detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicProperty.kt @@ -71,8 +71,8 @@ class KDocReferencesNonPublicProperty(config: Config = Config.empty) : Rule(conf return true } var classOrObject = containingClassOrObject - while (classOrObject != null && classOrObject is KtObjectDeclaration) { - if (!classOrObject.isPublic && !isProtected()) { + while (classOrObject is KtObjectDeclaration) { + if (!classOrObject.isPublic) { return true } classOrObject = classOrObject.containingClassOrObject @@ -83,7 +83,7 @@ class KDocReferencesNonPublicProperty(config: Config = Config.empty) : Rule(conf private fun KtProperty.isReferencedInherited(comment: String): Boolean { var qualifiedName = nameAsSafeName.asString() var classOrObject = containingClassOrObject - while (classOrObject != null && classOrObject is KtObjectDeclaration) { + while (classOrObject is KtObjectDeclaration) { qualifiedName = "${classOrObject.nameAsSafeName.asString()}.$qualifiedName" classOrObject = classOrObject.containingClassOrObject } diff --git a/detekt-rules-documentation/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicPropertySpec.kt b/detekt-rules-documentation/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicPropertySpec.kt index 54616c035f8..f877196a198 100644 --- a/detekt-rules-documentation/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicPropertySpec.kt +++ b/detekt-rules-documentation/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicPropertySpec.kt @@ -7,90 +7,128 @@ import org.junit.jupiter.api.Test class KDocReferencesNonPublicPropertySpec { val subject = KDocReferencesNonPublicProperty() - inner class `KDocReferencesNonPublicProperty rule` { - - @Test - fun `reports referenced non-public properties`() { - val code = """ - /** - * Comment - * [prop1] - non-public property - * [prop2] - public property - */ - class Test { - private val prop1 = 0 - val prop2 = 0 - } - """.trimIndent() - assertThat(subject.compileAndLint(code)).hasSize(1) + @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 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 + @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 - object A { - private val prop2 = 0 - - private object B { - val prop3 = 0 - } - object C { - private val prop4 = 0 - } + private object B { + val prop3 = 0 + } + object C { + private val prop4 = 0 } } - """.trimIndent() - assertThat(subject.compileAndLint(code)).hasSize(4) } + """.trimIndent() + assertThat(subject.compileAndLint(code)).hasSize(4) + } - @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 prop2 = 0 - } - } - object C { - val prop3 = 0 + @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 } } - """.trimIndent() - assertThat(subject.compileAndLint(code)).isEmpty() + object C { + val prop3 = 0 + } } + """.trimIndent() + assertThat(subject.compileAndLint(code)).isEmpty() } } From 4679ae2fbd9947c3b1fbe66b6f49b47eb910d2e0 Mon Sep 17 00:00:00 2001 From: "Vitaly V. Pinchuk" Date: Fri, 6 May 2022 16:50:15 +0300 Subject: [PATCH 5/5] Fix rule documentation --- .../rules/documentation/KDocReferencesNonPublicProperty.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicProperty.kt b/detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicProperty.kt index f424afc5cd7..fe2f8312193 100644 --- a/detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicProperty.kt +++ b/detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/KDocReferencesNonPublicProperty.kt @@ -20,8 +20,6 @@ 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. * - * See [Encapsulation](https://en.wikipedia.org/wiki/Encapsulation_(computer_programming)) - * * * /** * * Comment