Implement abstract members, now includes abstract variables

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()
 )
 

server/src/main/kotlin/org/javacs/kt/codeaction/quickfix/ImplementAbstractFunctionsQuickFix.kt → 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<Diagnostic>): List<Either<Command, CodeAction>> {
         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,13 @@ 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 ->
+                when (member) {
+                    is FunctionDescriptor -> createFunctionStub(member)
+                    is PropertyDescriptor -> createVariableStub(member)
+                    else -> null
+                }
             }
         } else {
             null
@@ -132,6 +137,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 +188,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)

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 { }"))
+    }
 }

server/src/test/resources/quickfixes/samefile.kt

@@ -23,3 +23,17 @@ interface NullMethodAndReturn<T> {
 }
 
 class NullClass : NullMethodAndReturn<String> {}
+
+abstract class MyAbstract {
+    val otherValToTestAbstractOverride = 1
+
+    abstract val name: String
+
+    abstract fun myFun()
+}
+
+class MyImplClass : MyAbstract() {}
+
+class My2ndClass : MyAbstract() {
+    override val name = "Nils"
+}

server/src/test/resources/quickfixes/standardlib.kt

@@ -5,3 +5,5 @@ import java.util.Comparator
 class MyThread : Runnable {}
 
 class MyComperable : Comparator<String> {}
+
+class MyList : AbstractList<String>() {}