From d8c7bab010f674749d12b771f407375137bbf148 Mon Sep 17 00:00:00 2001 From: Marie Katrine Ekeberg Date: Mon, 27 Jun 2022 18:24:31 +0200 Subject: [PATCH 1/2] Fix issue where nothing happens on empty body for implement abstract members quick fix --- .../ImplementAbstractMembersQuickFix.kt | 14 +++++++-- .../kotlin/org/javacs/kt/QuickFixesTest.kt | 30 +++++++++++++++++++ .../src/test/resources/quickfixes/samefile.kt | 8 +++++ 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/server/src/main/kotlin/org/javacs/kt/codeaction/quickfix/ImplementAbstractMembersQuickFix.kt b/server/src/main/kotlin/org/javacs/kt/codeaction/quickfix/ImplementAbstractMembersQuickFix.kt index 0c2d33b34..769cd14b1 100644 --- a/server/src/main/kotlin/org/javacs/kt/codeaction/quickfix/ImplementAbstractMembersQuickFix.kt +++ b/server/src/main/kotlin/org/javacs/kt/codeaction/quickfix/ImplementAbstractMembersQuickFix.kt @@ -53,14 +53,17 @@ class ImplementAbstractMembersQuickFix : QuickFix { val uri = file.parse.toPath().toUri().toString() // Get the padding to be introduced before the function declarations val padding = getDeclarationPadding(file, kotlinClass) + // Get the location where the new code will be placed val newFunctionStartPosition = getNewFunctionStartPosition(file, kotlinClass) + val bodyAppendBeginning = listOf(TextEdit(Range(newFunctionStartPosition, newFunctionStartPosition), "{")).takeIf { kotlinClass.hasNoBody() } ?: emptyList() + val bodyAppendEnd = listOf(TextEdit(Range(newFunctionStartPosition, newFunctionStartPosition), System.lineSeparator() + "}")).takeIf { kotlinClass.hasNoBody() } ?: emptyList() - val textEdits = functionsToImplement.map { + val textEdits = bodyAppendBeginning + functionsToImplement.map { // We leave two new lines before the function is inserted val newText = System.lineSeparator() + System.lineSeparator() + padding + it TextEdit(Range(newFunctionStartPosition, newFunctionStartPosition), newText) - } + } + bodyAppendEnd val codeAction = CodeAction() codeAction.edit = WorkspaceEdit(mapOf(uri to textEdits)) @@ -221,6 +224,11 @@ private fun getNewFunctionStartPosition(file: CompiledFile, kotlinClass: KtClass if (body != null) { position(file.content, body.startOffset + 1) } else { - null + // function has no body. We have to create one. New position is right after entire kotlin class text (with space) + val newPosCorrectLine = position(file.content, kotlinClass.startOffset + 1) + newPosCorrectLine.character = (kotlinClass.text.length + 2) + newPosCorrectLine } } + +private fun KtClass.hasNoBody() = null == this.body diff --git a/server/src/test/kotlin/org/javacs/kt/QuickFixesTest.kt b/server/src/test/kotlin/org/javacs/kt/QuickFixesTest.kt index f763df54a..4a73190a3 100644 --- a/server/src/test/kotlin/org/javacs/kt/QuickFixesTest.kt +++ b/server/src/test/kotlin/org/javacs/kt/QuickFixesTest.kt @@ -241,6 +241,36 @@ class ImplementAbstractMembersQuickFixSameFileTest : SingleFileTestFixture("quic assertThat(memberToImplementEdit?.range, equalTo(range(38, 31, 38, 31))) assertThat(memberToImplementEdit?.newText, equalTo(System.lineSeparator() + System.lineSeparator() + " override fun myFun() { }")) } + + @Test + fun `should find abstract members when class has no body (square brackets)`() { + val only = listOf(CodeActionKind.QuickFix) + val codeActionParams = codeActionParams(file, 47, 1, 47, 12, diagnostics, only) + + val codeActionResult = languageServer.textDocumentService.codeAction(codeActionParams).get() + + assertThat(codeActionResult, hasSize(1)) + val codeAction = codeActionResult[0].right + assertThat(codeAction.kind, equalTo(CodeActionKind.QuickFix)) + assertThat(codeAction.title, equalTo("Implement abstract members")) + + val textEdit = codeAction.edit.changes + val key = workspaceRoot.resolve(file).toUri().toString() + assertThat(textEdit.containsKey(key), equalTo(true)) + assertThat(textEdit[key], hasSize(3)) + + val firstMemberToImplementEdit = textEdit[key]?.get(0) + assertThat(firstMemberToImplementEdit?.range, equalTo(range(47, 23, 47, 23))) + assertThat(firstMemberToImplementEdit?.newText, equalTo("{")) + + val secondMemberToImplementEdit = textEdit[key]?.get(1) + assertThat(secondMemberToImplementEdit?.range, equalTo(range(47, 23, 47, 23))) + assertThat(secondMemberToImplementEdit?.newText, equalTo(System.lineSeparator() + System.lineSeparator() + " override fun behaviour() { }")) + + val thirdMemberToImplementEdit = textEdit[key]?.get(2) + assertThat(thirdMemberToImplementEdit?.range, equalTo(range(47, 23, 47, 23))) + assertThat(thirdMemberToImplementEdit?.newText, equalTo(System.lineSeparator() + "}")) + } } class ImplementAbstractMembersQuickFixExternalLibraryTest : SingleFileTestFixture("quickfixes", "standardlib.kt") { diff --git a/server/src/test/resources/quickfixes/samefile.kt b/server/src/test/resources/quickfixes/samefile.kt index dfbdfa90a..3d17c623e 100644 --- a/server/src/test/resources/quickfixes/samefile.kt +++ b/server/src/test/resources/quickfixes/samefile.kt @@ -37,3 +37,11 @@ class MyImplClass : MyAbstract() {} class My2ndClass : MyAbstract() { override val name = "Nils" } + + +// defect GH-366, part of the solution +interface IThing { + fun behaviour +} + +class Thing : IThing From 8ee1628b3191e8ed0c687a32b44d96c8790c4464 Mon Sep 17 00:00:00 2001 From: Marie Katrine Ekeberg Date: Mon, 27 Jun 2022 18:28:37 +0200 Subject: [PATCH 2/2] Consistency change based upon what the class is currently doing --- .../ImplementAbstractMembersQuickFix.kt | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/server/src/main/kotlin/org/javacs/kt/codeaction/quickfix/ImplementAbstractMembersQuickFix.kt b/server/src/main/kotlin/org/javacs/kt/codeaction/quickfix/ImplementAbstractMembersQuickFix.kt index 769cd14b1..7603e6f43 100644 --- a/server/src/main/kotlin/org/javacs/kt/codeaction/quickfix/ImplementAbstractMembersQuickFix.kt +++ b/server/src/main/kotlin/org/javacs/kt/codeaction/quickfix/ImplementAbstractMembersQuickFix.kt @@ -44,25 +44,25 @@ class ImplementAbstractMembersQuickFix : QuickFix { // If the client side and the server side diagnostics contain a valid diagnostic for this range. if (diagnostic != null && anyDiagnosticMatch(kotlinDiagnostics, startCursor, endCursor)) { - // Get the class with the missing functions + // Get the class with the missing members val kotlinClass = file.parseAtPoint(startCursor) if (kotlinClass is KtClass) { // Get the functions that need to be implemented - val functionsToImplement = getAbstractFunctionStubs(file, kotlinClass) + val membersToImplement = getAbstractMembersStubs(file, kotlinClass) val uri = file.parse.toPath().toUri().toString() - // Get the padding to be introduced before the function declarations + // Get the padding to be introduced before the member declarations val padding = getDeclarationPadding(file, kotlinClass) // Get the location where the new code will be placed - val newFunctionStartPosition = getNewFunctionStartPosition(file, kotlinClass) - val bodyAppendBeginning = listOf(TextEdit(Range(newFunctionStartPosition, newFunctionStartPosition), "{")).takeIf { kotlinClass.hasNoBody() } ?: emptyList() - val bodyAppendEnd = listOf(TextEdit(Range(newFunctionStartPosition, newFunctionStartPosition), System.lineSeparator() + "}")).takeIf { kotlinClass.hasNoBody() } ?: emptyList() + val newMembersStartPosition = getNewMembersStartPosition(file, kotlinClass) + val bodyAppendBeginning = listOf(TextEdit(Range(newMembersStartPosition, newMembersStartPosition), "{")).takeIf { kotlinClass.hasNoBody() } ?: emptyList() + val bodyAppendEnd = listOf(TextEdit(Range(newMembersStartPosition, newMembersStartPosition), System.lineSeparator() + "}")).takeIf { kotlinClass.hasNoBody() } ?: emptyList() - val textEdits = bodyAppendBeginning + functionsToImplement.map { - // We leave two new lines before the function is inserted + val textEdits = bodyAppendBeginning + membersToImplement.map { + // We leave two new lines before the member is inserted val newText = System.lineSeparator() + System.lineSeparator() + padding + it - TextEdit(Range(newFunctionStartPosition, newFunctionStartPosition), newText) + TextEdit(Range(newMembersStartPosition, newMembersStartPosition), newText) } + bodyAppendEnd val codeAction = CodeAction() @@ -83,7 +83,7 @@ fun findDiagnosticMatch(diagnostics: List, range: Range) = private fun anyDiagnosticMatch(diagnostics: Diagnostics, startCursor: Int, endCursor: Int) = diagnostics.any { diagnosticMatch(it, startCursor, endCursor, hashSetOf("ABSTRACT_MEMBER_NOT_IMPLEMENTED", "ABSTRACT_CLASS_MEMBER_NOT_IMPLEMENTED")) } -private fun getAbstractFunctionStubs(file: CompiledFile, kotlinClass: KtClass) = +private fun getAbstractMembersStubs(file: CompiledFile, kotlinClass: KtClass) = // For each of the super types used by this class kotlinClass.superTypeListEntries.mapNotNull { // Find the definition of this super type @@ -214,12 +214,12 @@ private fun getDeclarationPadding(file: CompiledFile, kotlinClass: KtClass): Str return " ".repeat(paddingSize) } -private fun getNewFunctionStartPosition(file: CompiledFile, kotlinClass: KtClass): Position? = - // If the class is not empty, the new function will be put right after the last declaration +private fun getNewMembersStartPosition(file: CompiledFile, kotlinClass: KtClass): Position? = + // If the class is not empty, the new member will be put right after the last declaration if (kotlinClass.declarations.isNotEmpty()) { val lastFunctionEndOffset = kotlinClass.declarations.last().endOffset position(file.content, lastFunctionEndOffset) - } else { // Otherwise, the function is put at the beginning of the class + } else { // Otherwise, the member is put at the beginning of the class val body = kotlinClass.body if (body != null) { position(file.content, body.startOffset + 1)