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

Detect undocumented protected classes, properties, and functions #5083

Merged
merged 12 commits into from Jul 23, 2022
Merged
3 changes: 3 additions & 0 deletions detekt-core/src/main/resources/default-detekt-config.yml
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions detekt-psi-utils/api/detekt-psi-utils.api
Expand Up @@ -106,6 +106,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 {
Expand Down Expand Up @@ -134,6 +135,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 {
Expand Down
Expand Up @@ -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)

Expand Down
Expand Up @@ -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
Expand Down
Expand Up @@ -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 class should be searched")
KengoTODA marked this conversation as resolved.
Show resolved Hide resolved
private val searchInProtectedClass: Boolean by config(false)

override fun visitClass(klass: KtClass) {
if (requiresDocumentation(klass)) {
reportIfUndocumented(klass)
Expand Down Expand Up @@ -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"
Expand Down
Expand Up @@ -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
Expand All @@ -27,6 +30,9 @@ class UndocumentedPublicFunction(config: Config = Config.empty) : Rule(config) {
Debt.TWENTY_MINS
)

@Configuration("if protected function should be searched")
KengoTODA marked this conversation as resolved.
Show resolved Hide resolved
private val searchProtectedFunction: Boolean by config(false)

override fun visitNamedFunction(function: KtNamedFunction) {
if (function.funKeyword == null && function.isLocal) return

Expand All @@ -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)
}
Expand Up @@ -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
Expand All @@ -31,6 +33,9 @@ class UndocumentedPublicProperty(config: Config = Config.empty) : Rule(config) {
Debt.TWENTY_MINS
)

@Configuration("if protected function should be searched")
KengoTODA marked this conversation as resolved.
Show resolved Hide resolved
private val searchProtectedProperty: Boolean by config(false)

override fun visitPrimaryConstructor(constructor: KtPrimaryConstructor) {
if (constructor.isPublicInherited()) {
val comment = constructor.containingClassOrObject?.docComment?.text
Expand All @@ -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)
Expand All @@ -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

Expand Down
Expand Up @@ -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()
Expand Down Expand Up @@ -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)
}
}
@@ -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()

Expand Down Expand Up @@ -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
Expand Down
@@ -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()

Expand Down Expand Up @@ -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` {

Expand Down