Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions detekt-core/src/main/resources/default-detekt-config.yml
Expand Up @@ -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
Expand Down
Expand Up @@ -41,6 +41,7 @@ private object TestExclusions : Exclusions() {
"UndocumentedPublicFunction",
"UndocumentedPublicProperty",
"UnsafeCallOnNullableType",
"KDocReferencesNonPublicProperty",
)
}

Expand Down
Expand Up @@ -24,7 +24,8 @@ class CommentSmellProvider : DefaultRuleSetProvider {
UndocumentedPublicClass(config),
UndocumentedPublicFunction(config),
UndocumentedPublicProperty(config),
AbsentOrWrongFileLicense(config)
AbsentOrWrongFileLicense(config),
KDocReferencesNonPublicProperty(config)
)
)
}
@@ -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."
)
)
}
}
@@ -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()
}
}