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

Add option for OutdatedDocumentation to allow param in constructor pr… #4453

Merged
merged 1 commit into from Jan 7, 2022
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
1 change: 1 addition & 0 deletions detekt-core/src/main/resources/default-detekt-config.yml
Expand Up @@ -66,6 +66,7 @@ comments:
active: false
matchTypeParameters: true
matchDeclarationsOrder: true
allowParamOnConstructorProperties: false
UndocumentedPublicClass:
active: false
excludes: ['**/test/**', '**/androidTest/**', '**/commonTest/**', '**/jvmTest/**', '**/jsTest/**', '**/iosTest/**']
Expand Down
Expand Up @@ -76,6 +76,9 @@ class OutdatedDocumentation(config: Config = Config.empty) : Rule(config) {
@Configuration("if the order of declarations should be preserved")
private val matchDeclarationsOrder: Boolean by config(true)

@Configuration("if we allow constructor parameters to be marked as @param instead of @property")
private val allowParamOnConstructorProperties: Boolean by config(false)

override fun visitClass(klass: KtClass) {
super.visitClass(klass)
reportIfDocumentationIsOutdated(klass) { getClassDeclarations(klass) }
Expand Down Expand Up @@ -119,7 +122,15 @@ class OutdatedDocumentation(config: Config = Config.empty) : Rule(config) {
private fun getDeclarationsForValueParameters(valueParameters: List<KtParameter>): List<Declaration> {
return valueParameters.mapNotNull {
it.name?.let { name ->
val type = if (it.isPropertyParameter()) DeclarationType.PROPERTY else DeclarationType.PARAM
val type = if (it.isPropertyParameter()) {
if (allowParamOnConstructorProperties) {
DeclarationType.ANY
} else {
DeclarationType.PROPERTY
}
} else {
DeclarationType.PARAM
}
Declaration(name, type)
}
}
Expand Down Expand Up @@ -166,11 +177,21 @@ class OutdatedDocumentation(config: Config = Config.empty) : Rule(config) {
}

private fun declarationsMatch(doc: List<Declaration>, element: List<Declaration>): Boolean {
return if (matchDeclarationsOrder) {
doc == element
if (doc.size != element.size) {
return false
}

val zippedElements = if (matchDeclarationsOrder) {
doc.zip(element)
} else {
doc.sortedBy { it.name } == element.sortedBy { it.name }
doc.sortedBy { it.name }.zip(element.sortedBy { it.name })
}

return zippedElements.all { (doc, element) -> declarationMatches(doc, element) }
}

private fun declarationMatches(doc: Declaration, element: Declaration): Boolean {
return element.name == doc.name && (element.type == DeclarationType.ANY || element.type == doc.type)
}

private fun reportCodeSmell(element: KtNamedDeclaration) {
Expand All @@ -193,6 +214,6 @@ class OutdatedDocumentation(config: Config = Config.empty) : Rule(config) {
)

enum class DeclarationType {
PARAM, PROPERTY
PARAM, PROPERTY, ANY
}
}
Expand Up @@ -364,5 +364,67 @@ class OutdatedDocumentationSpec : Spek({
assertThat(configuredSubject.compileAndLint(incorrectDeclarationsOrderWithType)).isEmpty()
}
}
describe("configuration allowParamOnConstructorProperties") {
val configuredSubject by memoized {
OutdatedDocumentation(TestConfig(mapOf("allowParamOnConstructorProperties" to "true")))
}

it("should not report when property is documented as param") {
val propertyAsParam = """
/**
* @param someParam Description of param
* @param someProp Description of property
*/
class MyClass(someParam: String, val someProp: String)
"""
assertThat(configuredSubject.compileAndLint(propertyAsParam)).isEmpty()
}

it("should not report when property is documented as property") {
val propertyAsParam = """
/**
* @param someParam Description of param
* @property someProp Description of property
*/
class MyClass(someParam: String, val someProp: String)
"""
assertThat(configuredSubject.compileAndLint(propertyAsParam)).isEmpty()
}
}

describe("configuration matchDeclarationsOrder and allowParamOnConstructorProperties") {
val configuredSubject by memoized {
OutdatedDocumentation(
TestConfig(
mapOf(
"matchDeclarationsOrder" to "false",
"allowParamOnConstructorProperties" to "true"
)
)
)
}

it("should not report when property is documented as param") {
val propertyAsParam = """
/**
* @param someParam Description of param
* @param someProp Description of property
*/
class MyClass(someParam: String, val someProp: String)
"""
assertThat(configuredSubject.compileAndLint(propertyAsParam)).isEmpty()
}

it("should not report when property is documented as property") {
val propertyAsParam = """
/**
* @param someParam Description of param
* @property someProp Description of property
*/
class MyClass(someParam: String, val someProp: String)
"""
assertThat(configuredSubject.compileAndLint(propertyAsParam)).isEmpty()
}
}
}
})