Skip to content

Commit

Permalink
Fix java getter / setter name generation (#2356)
Browse files Browse the repository at this point in the history
Kotlin has special rules for conversion around properties that start with "is" 
For more info see:
https://kotlinlang.org/docs/java-interop.html#getters-and-setters
https://kotlinlang.org/docs/java-to-kotlin-interop.html#properties
  • Loading branch information
CharlesG-Branch authored and vmishenev committed Apr 1, 2022
1 parent ec83639 commit 51d043b
Show file tree
Hide file tree
Showing 4 changed files with 219 additions and 2 deletions.
Expand Up @@ -616,9 +616,25 @@ private class DokkaDescriptorVisitor(
)

val name = run {
val modifier = if (isGetter) "get" else "set"
val rawName = propertyDescriptor.name.asString()
"$modifier${rawName.capitalize()}"
/*
* Kotlin has special rules for conversion around properties that
* start with "is" For more info see:
* https://kotlinlang.org/docs/java-interop.html#getters-and-setters
* https://kotlinlang.org/docs/java-to-kotlin-interop.html#properties
*
* Based on our testing, this rule only applies when the letter after
* the "is" is *not* lowercase. This means that words like "issue" won't
* have the rule applied but "is_foobar" and "is1of" will have the rule applied.
*/
val specialCaseIs = rawName.startsWith("is")
&& rawName.getOrNull(2)?.isLowerCase() == false

if (specialCaseIs) {
if (isGetter) rawName else rawName.replaceFirst("is", "set")
} else {
if (isGetter) "get${rawName.capitalize()}" else "set${rawName.capitalize()}"
}
}

val parameters =
Expand Down
118 changes: 118 additions & 0 deletions plugins/base/src/test/kotlin/translators/AccessorMethodNamingTest.kt
@@ -0,0 +1,118 @@
package translators

import org.junit.jupiter.api.Assertions.*
import org.jetbrains.dokka.base.testApi.testRunner.BaseAbstractTest
import org.jetbrains.dokka.model.DProperty
import org.junit.jupiter.api.Test

/**
* https://kotlinlang.org/docs/java-to-kotlin-interop.html#properties
* https://kotlinlang.org/docs/java-interop.html#getters-and-setters
*/
class AccessorMethodNamingTest : BaseAbstractTest() {

@Test
fun `standard property`() {
testAccessors("data class TestCase(var standardString: String, var standardBoolean: Boolean)") {
doTest("standardString", "getStandardString", "setStandardString")
doTest("standardBoolean", "getStandardBoolean", "setStandardBoolean")
}
}

@Test
fun `properties that start with the word 'is' use the special is rules`() {
testAccessors("data class TestCase(var isFoo: String, var isBar: Boolean)") {
doTest("isFoo", "isFoo", "setFoo")
doTest("isBar", "isBar", "setBar")
}
}

@Test
fun `properties that start with a word that starts with 'is' use get and set`() {
testAccessors("data class TestCase(var issuesFetched: Int, var issuesWereDisplayed: Boolean)") {
doTest("issuesFetched", "getIssuesFetched", "setIssuesFetched")
doTest("issuesWereDisplayed", "getIssuesWereDisplayed", "setIssuesWereDisplayed")
}
}

@Test
fun `properties that start with the word 'is' followed by underscore use the special is rules`() {
testAccessors("data class TestCase(var is_foo: String, var is_bar: Boolean)") {
doTest("is_foo", "is_foo", "set_foo")
doTest("is_bar", "is_bar", "set_bar")
}
}

@Test
fun `properties that start with the word 'is' followed by a number use the special is rules`() {
testAccessors("data class TestCase(var is1of: String, var is2of: Boolean)") {
doTest("is1of", "is1of", "set1of")
doTest("is2of", "is2of", "set2of")
}
}

@Test
fun `sanity check short names`() {
testAccessors(
"""
data class TestCase(
var i: Boolean,
var `is`: Boolean,
var isz: Boolean,
var isA: Int,
var isB: Boolean,
)
""".trimIndent()
) {
doTest("i", "getI", "setI")
doTest("is", "getIs", "setIs")
doTest("isz", "getIsz", "setIsz")
doTest("isA", "isA", "setA")
doTest("isB", "isB", "setB")
}
}

private fun testAccessors(code: String, block: PropertyTestCase.() -> Unit) {
val configuration = dokkaConfiguration {
suppressObviousFunctions = false
sourceSets {
sourceSet {
sourceRoots = listOf("src/main/kotlin")
}
}
}

testInline("""
/src/main/kotlin/sample/TestCase.kt
package sample
$code
""".trimIndent(),
configuration) {
documentablesMergingStage = { module ->
val properties = module.packages.single().classlikes.first().properties
PropertyTestCase(properties).apply {
block()
finish()
}
}
}
}

private class PropertyTestCase(private val properties: List<DProperty>) {
private var testsDone: Int = 0

fun doTest(kotlinName: String, getter: String? = null, setter: String? = null) {
properties.first { it.name == kotlinName }.let {
assertEquals(getter, it.getter?.name)
assertEquals(setter, it.setter?.name)
}
testsDone += 1
}

fun finish() {
assertTrue(testsDone > 0, "No tests in TestCase")
assertEquals(testsDone, properties.size)
}
}
}
3 changes: 3 additions & 0 deletions plugins/javadoc/build.gradle.kts
Expand Up @@ -11,6 +11,9 @@ dependencies {

val coroutines_version: String by project
implementation("org.jetbrains.kotlinx:kotlinx-coroutines-core:$coroutines_version")

val jsoup_version: String by project
testImplementation("org.jsoup:jsoup:$jsoup_version")
}

registerDokkaArtifactPublication("javadocPlugin") {
Expand Down
@@ -0,0 +1,80 @@
package org.jetbrains.dokka.javadoc

import org.jsoup.Jsoup
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.Assertions.assertEquals
import utils.*

internal class JavadocAccessorNamingTest : AbstractJavadocTemplateMapTest() {

val configuration = dokkaConfiguration {
suppressObviousFunctions = true
sourceSets {
sourceSet {
sourceRoots = listOf("src/main/kotlin")
}
}
}

/**
* This is a quick sanity check for the AccessorMethodNamingTest
*/
@Test
fun verifySpecialIsRuleIsApplied() {
val writerPlugin = TestOutputWriterPlugin()

testInline(
"""
/src/main/kotlin/sample/TestCase.kt
package sample
/**
* Test links:
* - [TestCase.issuesFetched]
* - [TestCase.isFoo]
*/
data class TestCase(
var issuesFetched: Int,
var isFoo: String,
)
""".trimIndent(),
configuration,
cleanupOutput = false,
pluginOverrides = listOf(writerPlugin, JavadocPlugin())
) {
renderingStage = { _, _ ->
val html = writerPlugin.writer.contents["sample/TestCase.html"].let { Jsoup.parse(it) }
val props = html
.select("#memberSummary_tabpanel")
.select("th[scope=row].colSecond")
.select("code")
.map { it.text() }
.toSet()

assertEquals(setOf(
"getIssuesFetched()",
"setIssuesFetched(Integer issuesFetched)",
"isFoo()",
"setFoo(String isFoo)",
), props)

val descriptionLinks = html
.select("div.description")
.select("p")
.select("a")
.eachAttr("href")
.map { a -> a.takeLastWhile { it != '#' } }

assertEquals(setOf(
"issuesFetched",
"isFoo()",
), descriptionLinks.toSet())

// Make sure that the ids from above actually exist
assertEquals(1, html.select("[id = isFoo()]").size)
// Bug! Nothing in the doc has the right id
assertEquals(0, html.select("[id = issuesFetched]").size)
}
}
}
}

0 comments on commit 51d043b

Please sign in to comment.