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

Fix java getter / setter name generation #2356

Merged
merged 6 commits into from Feb 18, 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
Expand Up @@ -606,9 +606,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() {
CharlesG-Branch marked this conversation as resolved.
Show resolved Hide resolved

@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)
}
}
}
}