From 68ed1a4a57e97407655f9c974c63d480284126cf Mon Sep 17 00:00:00 2001 From: "Vitaly V. Pinchuk" Date: Wed, 8 Jun 2022 14:56:17 +0300 Subject: [PATCH 1/3] Fix Signatures.kt:buildFunctionSignature 'buildFunctionSignature' fails when function body has KDoc. --- .../detekt/api/internal/Signatures.kt | 22 ++- .../detekt/api/internal/SignaturesSpec.kt | 128 ++++++++++++++++++ 2 files changed, 136 insertions(+), 14 deletions(-) create mode 100644 detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/internal/SignaturesSpec.kt diff --git a/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/internal/Signatures.kt b/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/internal/Signatures.kt index 117100b1985..bbcc177c2e7 100644 --- a/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/internal/Signatures.kt +++ b/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/internal/Signatures.kt @@ -92,21 +92,15 @@ private fun buildClassSignature(classOrObject: KtClassOrObject): String { } private fun buildFunctionSignature(element: KtNamedFunction): String { - var methodStart = 0 - element.docComment?.let { - methodStart = it.endOffset - it.startOffset - } - var methodEnd = element.endOffset - element.startOffset - val typeReference = element.typeReference - if (typeReference != null) { - methodEnd = typeReference.endOffset - element.startOffset + val startOffset = element.startOffset + val endOffset = if (element.typeReference != null) { + element.typeReference?.endOffset ?: 0 } else { - element.valueParameterList?.let { - methodEnd = it.endOffset - element.startOffset - } + element.valueParameterList?.endOffset ?: 0 } - require(methodStart < methodEnd) { - "Error building function signature with range $methodStart - $methodEnd for element: ${element.text}" + + require(startOffset < endOffset) { + "Error building function signature with range $startOffset - $endOffset for element: ${element.text}" } - return element.text.substring(methodStart, methodEnd) + return element.text.substring(0, endOffset - startOffset) } diff --git a/detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/internal/SignaturesSpec.kt b/detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/internal/SignaturesSpec.kt new file mode 100644 index 00000000000..d201277fcaa --- /dev/null +++ b/detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/internal/SignaturesSpec.kt @@ -0,0 +1,128 @@ +package io.gitlab.arturbosch.detekt.api.internal + +import io.gitlab.arturbosch.detekt.api.Debt +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.test.compileAndLint +import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.assertThatThrownBy +import org.jetbrains.kotlin.psi.KtNamedFunction +import org.junit.jupiter.api.Test + +class SignaturesSpec { + private var result = "" + private val subject = TestRule { result = it } + + @Test + fun `function with type reference`() { + subject.compileAndLint(functionWithTypeReference()) + assertThat(result).isEqualTo(signatureWithTypeReference()) + } + + @Test + fun `function without type reference`() { + subject.compileAndLint(functionWithoutTypeReference()) + assertThat(result).isEqualTo(signatureWithoutTypeReference()) + } + + @Test + fun `function throws exception`() { + assertThatThrownBy { + subject.compileAndLint(functionWontCompile()) + } + .isInstanceOf(IllegalArgumentException::class.java) + .hasMessageContaining("Error building function signature") + } +} + +private class TestRule(val block: (String) -> Unit) : Rule() { + override val issue = Issue(javaClass.simpleName, Severity.CodeSmell, "", Debt.TWENTY_MINS) + + override fun visitNamedFunction(function: KtNamedFunction) { + block(function.buildFullSignature()) + } +} + +private fun signatureWithTypeReference() = """ + Test.kt${'$'}TestClass.Companion${'$'}@JvmStatic @Parameterized.Parameters(name = "parameterName") /** * Function KDoc */ fun data(): List> +""".trimIndent() + +private fun signatureWithoutTypeReference() = """ + Test.kt${'$'}TestClass.Companion${'$'}@JvmStatic @Parameterized.Parameters(name = "parameterName") /** * Function KDoc */ fun data() +""".trimIndent() + +private fun functionWithTypeReference() = """ + @RunWith(Parameterized::class) + class TestClass { + companion object { + @JvmStatic + @Parameterized.Parameters(name = "parameterName") + /** + * Function KDoc + */ + fun data(): List> = + /** + * more description + * more description + * more description + * more description + * more description + */ + listOf( + arrayOf(1, 2, 3, 4), + arrayOf(11, 22, 33, 44) + ) + } + } +""".trimIndent() + +private fun functionWithoutTypeReference() = """ + @RunWith(Parameterized::class) + class TestClass { + companion object { + @JvmStatic + @Parameterized.Parameters(name = "parameterName") + /** + * Function KDoc + */ + fun data() = + /** + * more description + * more description + * more description + * more description + * more description + */ + listOf( + arrayOf(1, 2, 3, 4), + arrayOf(11, 22, 33, 44) + ) + } + } +""".trimIndent() + +private fun functionWontCompile() = """ + @RunWith(Parameterized.class) + class TestClass { + companion object { + @JvmStatic + @Parameterized.Parameters(name = "parameterName") + /** + * Function KDoc + */ + fun data() = + /** + * more description + * more description + * more description + * more description + * more description + */ + listOf( + arrayOf(1, 2, 3, 4), + arrayOf(11, 22, 33, 44) + ) + } + } +""".trimIndent() From 0fea783bac1322ba2d938fee622743058931165b Mon Sep 17 00:00:00 2001 From: "Vitaly V. Pinchuk" Date: Fri, 17 Jun 2022 11:49:32 +0300 Subject: [PATCH 2/3] Simplify tests --- .../detekt/api/internal/SignaturesSpec.kt | 118 +++--------------- 1 file changed, 14 insertions(+), 104 deletions(-) diff --git a/detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/internal/SignaturesSpec.kt b/detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/internal/SignaturesSpec.kt index d201277fcaa..10cf9e1b552 100644 --- a/detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/internal/SignaturesSpec.kt +++ b/detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/internal/SignaturesSpec.kt @@ -1,128 +1,38 @@ package io.gitlab.arturbosch.detekt.api.internal -import io.gitlab.arturbosch.detekt.api.Debt -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.test.compileAndLint +import io.github.detekt.test.utils.compileContentForTest import org.assertj.core.api.Assertions.assertThat import org.assertj.core.api.Assertions.assertThatThrownBy import org.jetbrains.kotlin.psi.KtNamedFunction import org.junit.jupiter.api.Test class SignaturesSpec { - private var result = "" - private val subject = TestRule { result = it } - @Test fun `function with type reference`() { - subject.compileAndLint(functionWithTypeReference()) - assertThat(result).isEqualTo(signatureWithTypeReference()) + val result = compileContentForTest("fun data(): Int = 0") + .findChildByClass(KtNamedFunction::class.java)!! + .buildFullSignature() + + assertThat(result).isEqualTo("Test.kt\$fun data(): Int") } @Test fun `function without type reference`() { - subject.compileAndLint(functionWithoutTypeReference()) - assertThat(result).isEqualTo(signatureWithoutTypeReference()) + val result = compileContentForTest("fun data() = 0") + .findChildByClass(KtNamedFunction::class.java)!! + .buildFullSignature() + + assertThat(result).isEqualTo("Test.kt\$fun data()") } @Test fun `function throws exception`() { assertThatThrownBy { - subject.compileAndLint(functionWontCompile()) + compileContentForTest("{ fun data() = 0 }") + .findChildByClass(KtNamedFunction::class.java)!! + .buildFullSignature() } .isInstanceOf(IllegalArgumentException::class.java) .hasMessageContaining("Error building function signature") } } - -private class TestRule(val block: (String) -> Unit) : Rule() { - override val issue = Issue(javaClass.simpleName, Severity.CodeSmell, "", Debt.TWENTY_MINS) - - override fun visitNamedFunction(function: KtNamedFunction) { - block(function.buildFullSignature()) - } -} - -private fun signatureWithTypeReference() = """ - Test.kt${'$'}TestClass.Companion${'$'}@JvmStatic @Parameterized.Parameters(name = "parameterName") /** * Function KDoc */ fun data(): List> -""".trimIndent() - -private fun signatureWithoutTypeReference() = """ - Test.kt${'$'}TestClass.Companion${'$'}@JvmStatic @Parameterized.Parameters(name = "parameterName") /** * Function KDoc */ fun data() -""".trimIndent() - -private fun functionWithTypeReference() = """ - @RunWith(Parameterized::class) - class TestClass { - companion object { - @JvmStatic - @Parameterized.Parameters(name = "parameterName") - /** - * Function KDoc - */ - fun data(): List> = - /** - * more description - * more description - * more description - * more description - * more description - */ - listOf( - arrayOf(1, 2, 3, 4), - arrayOf(11, 22, 33, 44) - ) - } - } -""".trimIndent() - -private fun functionWithoutTypeReference() = """ - @RunWith(Parameterized::class) - class TestClass { - companion object { - @JvmStatic - @Parameterized.Parameters(name = "parameterName") - /** - * Function KDoc - */ - fun data() = - /** - * more description - * more description - * more description - * more description - * more description - */ - listOf( - arrayOf(1, 2, 3, 4), - arrayOf(11, 22, 33, 44) - ) - } - } -""".trimIndent() - -private fun functionWontCompile() = """ - @RunWith(Parameterized.class) - class TestClass { - companion object { - @JvmStatic - @Parameterized.Parameters(name = "parameterName") - /** - * Function KDoc - */ - fun data() = - /** - * more description - * more description - * more description - * more description - * more description - */ - listOf( - arrayOf(1, 2, 3, 4), - arrayOf(11, 22, 33, 44) - ) - } - } -""".trimIndent() From 427b4cfdd9b6eae85ec077e79d7c2a2f014b715b Mon Sep 17 00:00:00 2001 From: "Vitaly V. Pinchuk" Date: Tue, 21 Jun 2022 15:36:23 +0300 Subject: [PATCH 3/3] Skip comments --- .../gitlab/arturbosch/detekt/api/internal/Signatures.kt | 7 ++++--- .../arturbosch/detekt/api/internal/SignaturesSpec.kt | 9 +++++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/internal/Signatures.kt b/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/internal/Signatures.kt index bbcc177c2e7..4cac91362d4 100644 --- a/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/internal/Signatures.kt +++ b/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/internal/Signatures.kt @@ -10,6 +10,7 @@ import org.jetbrains.kotlin.psi.psiUtil.endOffset import org.jetbrains.kotlin.psi.psiUtil.getNonStrictParentOfType import org.jetbrains.kotlin.psi.psiUtil.parents import org.jetbrains.kotlin.psi.psiUtil.startOffset +import org.jetbrains.kotlin.psi.psiUtil.startOffsetSkippingComments import java.io.File private val multipleWhitespaces = Regex("\\s{2,}") @@ -92,15 +93,15 @@ private fun buildClassSignature(classOrObject: KtClassOrObject): String { } private fun buildFunctionSignature(element: KtNamedFunction): String { - val startOffset = element.startOffset + val startOffset = element.startOffsetSkippingComments - element.startOffset val endOffset = if (element.typeReference != null) { element.typeReference?.endOffset ?: 0 } else { element.valueParameterList?.endOffset ?: 0 - } + } - element.startOffset require(startOffset < endOffset) { "Error building function signature with range $startOffset - $endOffset for element: ${element.text}" } - return element.text.substring(0, endOffset - startOffset) + return element.text.substring(startOffset, endOffset) } diff --git a/detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/internal/SignaturesSpec.kt b/detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/internal/SignaturesSpec.kt index 10cf9e1b552..acfcf1bf5f9 100644 --- a/detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/internal/SignaturesSpec.kt +++ b/detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/internal/SignaturesSpec.kt @@ -25,6 +25,15 @@ class SignaturesSpec { assertThat(result).isEqualTo("Test.kt\$fun data()") } + @Test + fun `function with comments`() { + val result = compileContentForTest("/* comments */ fun data() = 0") + .findChildByClass(KtNamedFunction::class.java)!! + .buildFullSignature() + + assertThat(result).isEqualTo("Test.kt\$fun data()") + } + @Test fun `function throws exception`() { assertThatThrownBy {