From 092e44e47b642be58fb47825815e762bf0b2ac7a Mon Sep 17 00:00:00 2001 From: Marie Katrine Ekeberg Date: Wed, 8 Jun 2022 19:00:39 +0200 Subject: [PATCH 1/2] Implement abstract members, now includes abstract variables --- .../org/javacs/kt/codeaction/CodeAction.kt | 4 +- ...kt => ImplementAbstractMembersQuickFix.kt} | 27 +++++- .../kotlin/org/javacs/kt/QuickFixesTest.kt | 92 +++++++++++++++++-- .../src/test/resources/quickfixes/samefile.kt | 14 +++ .../test/resources/quickfixes/standardlib.kt | 2 + 5 files changed, 123 insertions(+), 16 deletions(-) rename server/src/main/kotlin/org/javacs/kt/codeaction/quickfix/{ImplementAbstractFunctionsQuickFix.kt => ImplementAbstractMembersQuickFix.kt} (88%) diff --git a/server/src/main/kotlin/org/javacs/kt/codeaction/CodeAction.kt b/server/src/main/kotlin/org/javacs/kt/codeaction/CodeAction.kt index 1adc4f991..7f9bdbdcc 100644 --- a/server/src/main/kotlin/org/javacs/kt/codeaction/CodeAction.kt +++ b/server/src/main/kotlin/org/javacs/kt/codeaction/CodeAction.kt @@ -3,14 +3,14 @@ package org.javacs.kt.codeaction import org.eclipse.lsp4j.* import org.eclipse.lsp4j.jsonrpc.messages.Either import org.javacs.kt.CompiledFile -import org.javacs.kt.codeaction.quickfix.ImplementAbstractFunctionsQuickFix +import org.javacs.kt.codeaction.quickfix.ImplementAbstractMembersQuickFix import org.javacs.kt.codeaction.quickfix.AddMissingImportsQuickFix import org.javacs.kt.command.JAVA_TO_KOTLIN_COMMAND import org.javacs.kt.util.toPath import org.javacs.kt.index.SymbolIndex val QUICK_FIXES = listOf( - ImplementAbstractFunctionsQuickFix(), + ImplementAbstractMembersQuickFix(), AddMissingImportsQuickFix() ) diff --git a/server/src/main/kotlin/org/javacs/kt/codeaction/quickfix/ImplementAbstractFunctionsQuickFix.kt b/server/src/main/kotlin/org/javacs/kt/codeaction/quickfix/ImplementAbstractMembersQuickFix.kt similarity index 88% rename from server/src/main/kotlin/org/javacs/kt/codeaction/quickfix/ImplementAbstractFunctionsQuickFix.kt rename to server/src/main/kotlin/org/javacs/kt/codeaction/quickfix/ImplementAbstractMembersQuickFix.kt index 12910c87d..c1f9be1d2 100644 --- a/server/src/main/kotlin/org/javacs/kt/codeaction/quickfix/ImplementAbstractFunctionsQuickFix.kt +++ b/server/src/main/kotlin/org/javacs/kt/codeaction/quickfix/ImplementAbstractMembersQuickFix.kt @@ -11,6 +11,7 @@ import org.jetbrains.kotlin.descriptors.ClassDescriptor import org.jetbrains.kotlin.descriptors.ClassConstructorDescriptor import org.jetbrains.kotlin.descriptors.DeclarationDescriptor import org.jetbrains.kotlin.descriptors.FunctionDescriptor +import org.jetbrains.kotlin.descriptors.PropertyDescriptor import org.jetbrains.kotlin.descriptors.isInterface import org.jetbrains.kotlin.descriptors.Modality import org.jetbrains.kotlin.js.resolve.diagnostics.findPsi @@ -33,7 +34,7 @@ import org.jetbrains.kotlin.types.typeUtil.asTypeProjection private const val DEFAULT_TAB_SIZE = 4 -class ImplementAbstractFunctionsQuickFix : QuickFix { +class ImplementAbstractMembersQuickFix : QuickFix { override fun compute(file: CompiledFile, index: SymbolIndex, range: Range, diagnostics: List): List> { val diagnostic = findDiagnosticMatch(diagnostics, range) @@ -64,7 +65,7 @@ class ImplementAbstractFunctionsQuickFix : QuickFix { val codeAction = CodeAction() codeAction.edit = WorkspaceEdit(mapOf(uri to textEdits)) codeAction.kind = CodeActionKind.QuickFix - codeAction.title = "Implement abstract functions" + codeAction.title = "Implement abstract members" codeAction.diagnostics = listOf(diagnostic) return listOf(Either.forRight(codeAction)) } @@ -92,9 +93,15 @@ private fun getAbstractFunctionStubs(file: CompiledFile, kotlinClass: KtClass) = if (null != classDescriptor && (classDescriptor.kind.isInterface || classDescriptor.modality == Modality.ABSTRACT)) { val superClassTypeArguments = getSuperClassTypeProjections(file, it) classDescriptor.getMemberScope(superClassTypeArguments).getContributedDescriptors().filter { classMember -> - classMember is FunctionDescriptor && classMember.modality == Modality.ABSTRACT && !overridesDeclaration(kotlinClass, classMember) - }.map { function -> - createFunctionStub(function as FunctionDescriptor) + (classMember is FunctionDescriptor && classMember.modality == Modality.ABSTRACT && !overridesDeclaration(kotlinClass, classMember)) || (classMember is PropertyDescriptor && classMember.modality == Modality.ABSTRACT && !overridesDeclaration(kotlinClass, classMember)) + }.mapNotNull { member -> + if(member is FunctionDescriptor) { + createFunctionStub(member) + } else if(member is PropertyDescriptor) { + createVariableStub(member) + } else { + null + } } } else { null @@ -132,6 +139,11 @@ private fun overridesDeclaration(kotlinClass: KtClass, descriptor: FunctionDescr } } +private fun overridesDeclaration(kotlinClass: KtClass, descriptor: PropertyDescriptor): Boolean = + kotlinClass.declarations.any { + it.name == descriptor.name.asString() && it.hasModifier(KtTokens.OVERRIDE_KEYWORD) + } + // Checks if two functions have matching parameters private fun parametersMatch(function: KtNamedFunction, functionDescriptor: FunctionDescriptor): Boolean { if (function.valueParameters.size == functionDescriptor.valueParameters.size) { @@ -178,6 +190,11 @@ private fun createFunctionStub(function: FunctionDescriptor): String { return "override fun $name($arguments)${returnType?.let { ": $it" } ?: ""} { }" } +private fun createVariableStub(variable: PropertyDescriptor): String { + val variableType = variable.returnType?.unwrappedType()?.toString()?.takeIf { "Unit" != it } + return "override val ${variable.name}${variableType?.let { ": $it" } ?: ""} = TODO(\"SET VALUE\")" +} + // about types: regular Kotlin types are marked T or T?, but types from Java are (T..T?) because nullability cannot be decided. // Therefore we have to unpack in case we have the Java type. Fortunately, the Java types are not marked nullable, so we default to non nullable types. Let the user decide if they want nullable types instead. With this implementation Kotlin types also keeps their nullability private fun KotlinType.unwrappedType(): KotlinType = this.unwrap().makeNullableAsSpecified(this.isMarkedNullable) diff --git a/server/src/test/kotlin/org/javacs/kt/QuickFixesTest.kt b/server/src/test/kotlin/org/javacs/kt/QuickFixesTest.kt index 4a514b89d..f763df54a 100644 --- a/server/src/test/kotlin/org/javacs/kt/QuickFixesTest.kt +++ b/server/src/test/kotlin/org/javacs/kt/QuickFixesTest.kt @@ -8,7 +8,7 @@ import org.hamcrest.Matchers.hasSize import org.junit.Assert.assertThat import org.junit.Test -class ImplementAbstractFunctionsQuickFixTest : SingleFileTestFixture("quickfixes", "SomeSubclass.kt") { +class ImplementAbstractMembersQuickFixTest : SingleFileTestFixture("quickfixes", "SomeSubclass.kt") { @Test fun `gets workspace edit for all abstract methods when none are implemented`() { val diagnostic = Diagnostic(range(3, 1, 3, 19), "") @@ -90,7 +90,7 @@ class ImplementAbstractFunctionsQuickFixTest : SingleFileTestFixture("quickfixes } } -class ImplementAbstractFunctionsQuickFixSameFileTest : SingleFileTestFixture("quickfixes", "samefile.kt") { +class ImplementAbstractMembersQuickFixSameFileTest : SingleFileTestFixture("quickfixes", "samefile.kt") { @Test fun `should find no code actions`() { val only = listOf(CodeActionKind.QuickFix) @@ -111,7 +111,7 @@ class ImplementAbstractFunctionsQuickFixSameFileTest : SingleFileTestFixture("qu assertThat(codeActionResult, hasSize(1)) val codeAction = codeActionResult[0].right assertThat(codeAction.kind, equalTo(CodeActionKind.QuickFix)) - assertThat(codeAction.title, equalTo("Implement abstract functions")) + assertThat(codeAction.title, equalTo("Implement abstract members")) assertThat(codeAction.diagnostics, equalTo(listOf(diagnostics[0]))) val textEdit = codeAction.edit.changes @@ -134,7 +134,7 @@ class ImplementAbstractFunctionsQuickFixSameFileTest : SingleFileTestFixture("qu assertThat(codeActionResult, hasSize(1)) val codeAction = codeActionResult[0].right assertThat(codeAction.kind, equalTo(CodeActionKind.QuickFix)) - assertThat(codeAction.title, equalTo("Implement abstract functions")) + assertThat(codeAction.title, equalTo("Implement abstract members")) val textEdit = codeAction.edit.changes val key = workspaceRoot.resolve(file).toUri().toString() @@ -160,7 +160,7 @@ class ImplementAbstractFunctionsQuickFixSameFileTest : SingleFileTestFixture("qu assertThat(codeActionResult, hasSize(1)) val codeAction = codeActionResult[0].right assertThat(codeAction.kind, equalTo(CodeActionKind.QuickFix)) - assertThat(codeAction.title, equalTo("Implement abstract functions")) + assertThat(codeAction.title, equalTo("Implement abstract members")) val textEdit = codeAction.edit.changes val key = workspaceRoot.resolve(file).toUri().toString() @@ -182,7 +182,7 @@ class ImplementAbstractFunctionsQuickFixSameFileTest : SingleFileTestFixture("qu assertThat(codeActionResult, hasSize(1)) val codeAction = codeActionResult[0].right assertThat(codeAction.kind, equalTo(CodeActionKind.QuickFix)) - assertThat(codeAction.title, equalTo("Implement abstract functions")) + assertThat(codeAction.title, equalTo("Implement abstract members")) val textEdit = codeAction.edit.changes val key = workspaceRoot.resolve(file).toUri().toString() @@ -193,9 +193,57 @@ class ImplementAbstractFunctionsQuickFixSameFileTest : SingleFileTestFixture("qu assertThat(functionToImplementEdit?.range, equalTo(range(25, 48, 25, 48))) assertThat(functionToImplementEdit?.newText, equalTo(System.lineSeparator() + System.lineSeparator() + " override fun myMethod(myStr: String?): String? { }")) } + + @Test + fun `should find abstract variable and function`() { + val only = listOf(CodeActionKind.QuickFix) + val codeActionParams = codeActionParams(file, 35, 1, 35, 18, 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(2)) + + val firstMemberToImplementEdit = textEdit[key]?.get(0) + assertThat(firstMemberToImplementEdit?.range, equalTo(range(35, 35, 35, 35))) + assertThat(firstMemberToImplementEdit?.newText, equalTo(System.lineSeparator() + System.lineSeparator() + " override val name: String = TODO(\"SET VALUE\")")) + + val secondMemberToImplementEdit = textEdit[key]?.get(1) + assertThat(secondMemberToImplementEdit?.range, equalTo(range(35, 35, 35, 35))) + assertThat(secondMemberToImplementEdit?.newText, equalTo(System.lineSeparator() + System.lineSeparator() + " override fun myFun() { }")) + } + + @Test + fun `should find abstract function when variable is already implemented`() { + val only = listOf(CodeActionKind.QuickFix) + val codeActionParams = codeActionParams(file, 37, 1, 37, 17, 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(1)) + + val memberToImplementEdit = textEdit[key]?.get(0) + assertThat(memberToImplementEdit?.range, equalTo(range(38, 31, 38, 31))) + assertThat(memberToImplementEdit?.newText, equalTo(System.lineSeparator() + System.lineSeparator() + " override fun myFun() { }")) + } } -class ImplementAbstractFunctionsQuickFixExternalLibraryTest : SingleFileTestFixture("quickfixes", "standardlib.kt") { +class ImplementAbstractMembersQuickFixExternalLibraryTest : SingleFileTestFixture("quickfixes", "standardlib.kt") { @Test fun `should find one abstract method from Runnable to implement`() { val only = listOf(CodeActionKind.QuickFix) @@ -206,7 +254,7 @@ class ImplementAbstractFunctionsQuickFixExternalLibraryTest : SingleFileTestFixt assertThat(codeActionResult, hasSize(1)) val codeAction = codeActionResult[0].right assertThat(codeAction.kind, equalTo(CodeActionKind.QuickFix)) - assertThat(codeAction.title, equalTo("Implement abstract functions")) + assertThat(codeAction.title, equalTo("Implement abstract members")) val textEdit = codeAction.edit.changes val key = workspaceRoot.resolve(file).toUri().toString() @@ -228,7 +276,7 @@ class ImplementAbstractFunctionsQuickFixExternalLibraryTest : SingleFileTestFixt assertThat(codeActionResult, hasSize(1)) val codeAction = codeActionResult[0].right assertThat(codeAction.kind, equalTo(CodeActionKind.QuickFix)) - assertThat(codeAction.title, equalTo("Implement abstract functions")) + assertThat(codeAction.title, equalTo("Implement abstract members")) val textEdit = codeAction.edit.changes val key = workspaceRoot.resolve(file).toUri().toString() @@ -239,4 +287,30 @@ class ImplementAbstractFunctionsQuickFixExternalLibraryTest : SingleFileTestFixt assertThat(functionToImplementEdit?.range, equalTo(range(7, 42, 7, 42))) assertThat(functionToImplementEdit?.newText, equalTo(System.lineSeparator() + System.lineSeparator() + " override fun compare(p0: String, p1: String): Int { }")) } + + @Test + fun `should find abstract members for AbstractList`() { + val only = listOf(CodeActionKind.QuickFix) + val codeActionParams = codeActionParams(file, 9, 1, 9, 13, 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(2)) + + val firstMemberToImplementEdit = textEdit[key]?.get(0) + assertThat(firstMemberToImplementEdit?.range, equalTo(range(9, 40, 9, 40))) + assertThat(firstMemberToImplementEdit?.newText, equalTo(System.lineSeparator() + System.lineSeparator() + " override val size: Int = TODO(\"SET VALUE\")")) + + val secondMemberToImplementEdit = textEdit[key]?.get(1) + assertThat(secondMemberToImplementEdit?.range, equalTo(range(9, 40, 9, 40))) + assertThat(secondMemberToImplementEdit?.newText, equalTo(System.lineSeparator() + System.lineSeparator() + " override fun get(index: Int): String { }")) + } } diff --git a/server/src/test/resources/quickfixes/samefile.kt b/server/src/test/resources/quickfixes/samefile.kt index 6e322092d..dfbdfa90a 100644 --- a/server/src/test/resources/quickfixes/samefile.kt +++ b/server/src/test/resources/quickfixes/samefile.kt @@ -23,3 +23,17 @@ interface NullMethodAndReturn { } class NullClass : NullMethodAndReturn {} + +abstract class MyAbstract { + val otherValToTestAbstractOverride = 1 + + abstract val name: String + + abstract fun myFun() +} + +class MyImplClass : MyAbstract() {} + +class My2ndClass : MyAbstract() { + override val name = "Nils" +} diff --git a/server/src/test/resources/quickfixes/standardlib.kt b/server/src/test/resources/quickfixes/standardlib.kt index 665eab2d2..bb5acb08d 100644 --- a/server/src/test/resources/quickfixes/standardlib.kt +++ b/server/src/test/resources/quickfixes/standardlib.kt @@ -5,3 +5,5 @@ import java.util.Comparator class MyThread : Runnable {} class MyComperable : Comparator {} + +class MyList : AbstractList() {} From ec0c9c5946acbff3be8aa9766c77e8f6e6a37ece Mon Sep 17 00:00:00 2001 From: Marie Katrine Ekeberg Date: Thu, 9 Jun 2022 17:05:49 +0200 Subject: [PATCH 2/2] Prettify code based upon PR comment --- .../quickfix/ImplementAbstractMembersQuickFix.kt | 10 ++++------ 1 file changed, 4 insertions(+), 6 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 c1f9be1d2..0c2d33b34 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 @@ -95,12 +95,10 @@ private fun getAbstractFunctionStubs(file: CompiledFile, kotlinClass: KtClass) = classDescriptor.getMemberScope(superClassTypeArguments).getContributedDescriptors().filter { classMember -> (classMember is FunctionDescriptor && classMember.modality == Modality.ABSTRACT && !overridesDeclaration(kotlinClass, classMember)) || (classMember is PropertyDescriptor && classMember.modality == Modality.ABSTRACT && !overridesDeclaration(kotlinClass, classMember)) }.mapNotNull { member -> - if(member is FunctionDescriptor) { - createFunctionStub(member) - } else if(member is PropertyDescriptor) { - createVariableStub(member) - } else { - null + when (member) { + is FunctionDescriptor -> createFunctionStub(member) + is PropertyDescriptor -> createVariableStub(member) + else -> null } } } else {