From 5d49b80fe3007e335e89a1c00a569af3886da45a Mon Sep 17 00:00:00 2001 From: Marie Katrine Ekeberg Date: Tue, 2 Aug 2022 20:10:40 +0200 Subject: [PATCH 1/9] Skeleton of new general override members implementation. #359 --- .../kt/KotlinProtocolExtensionService.kt | 10 +++ .../org/javacs/kt/KotlinProtocolExtensions.kt | 5 ++ .../kt/overridemembers/OverrideMembers.kt | 11 +++ .../org/javacs/kt/OverrideMemberTest.kt | 77 +++++++++++++++++++ .../overridemember/OverrideMembers.kt | 21 +++++ 5 files changed, 124 insertions(+) create mode 100644 server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt create mode 100644 server/src/test/kotlin/org/javacs/kt/OverrideMemberTest.kt create mode 100644 server/src/test/resources/overridemember/OverrideMembers.kt diff --git a/server/src/main/kotlin/org/javacs/kt/KotlinProtocolExtensionService.kt b/server/src/main/kotlin/org/javacs/kt/KotlinProtocolExtensionService.kt index 4fa37acf7..131a39375 100644 --- a/server/src/main/kotlin/org/javacs/kt/KotlinProtocolExtensionService.kt +++ b/server/src/main/kotlin/org/javacs/kt/KotlinProtocolExtensionService.kt @@ -4,6 +4,8 @@ import org.eclipse.lsp4j.* import org.javacs.kt.util.AsyncExecutor import org.javacs.kt.util.parseURI import org.javacs.kt.resolve.resolveMain +import org.javacs.kt.position.offset +import org.javacs.kt.listOverridableMembers import java.util.concurrent.CompletableFuture import java.nio.file.Paths @@ -39,4 +41,12 @@ class KotlinProtocolExtensionService( "projectRoot" to workspacePath ) } + + override fun overrideMember(position: TextDocumentPositionParams): CompletableFuture> = async.compute { + val fileUri = parseURI(position.textDocument.uri) + val compiledFile = sp.currentVersion(fileUri) + val cursorOffset = offset(compiledFile.content, position.position) + + listOverridableMembers(compiledFile, cursorOffset) + } } diff --git a/server/src/main/kotlin/org/javacs/kt/KotlinProtocolExtensions.kt b/server/src/main/kotlin/org/javacs/kt/KotlinProtocolExtensions.kt index 808ba0dce..a599f11f4 100644 --- a/server/src/main/kotlin/org/javacs/kt/KotlinProtocolExtensions.kt +++ b/server/src/main/kotlin/org/javacs/kt/KotlinProtocolExtensions.kt @@ -15,4 +15,9 @@ interface KotlinProtocolExtensions { @JsonRequest fun mainClass(textDocument: TextDocumentIdentifier): CompletableFuture> + + // TODO: what is the best return value in this case? CodeAction? + // TODO: should the naming be something like listOverrideableMembers? or something similar instead? + @JsonRequest + fun overrideMember(position: TextDocumentPositionParams): CompletableFuture> } diff --git a/server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt b/server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt new file mode 100644 index 000000000..59afa5a6d --- /dev/null +++ b/server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt @@ -0,0 +1,11 @@ +package org.javacs.kt + +import org.eclipse.lsp4j.Range +import org.eclipse.lsp4j.CodeAction + + +fun listOverridableMembers(file: CompiledFile, cursor: Int): List { + // TODO: implement + + return listOf() +} diff --git a/server/src/test/kotlin/org/javacs/kt/OverrideMemberTest.kt b/server/src/test/kotlin/org/javacs/kt/OverrideMemberTest.kt new file mode 100644 index 000000000..658bab353 --- /dev/null +++ b/server/src/test/kotlin/org/javacs/kt/OverrideMemberTest.kt @@ -0,0 +1,77 @@ +package org.javacs.kt + +import com.google.gson.Gson +import org.eclipse.lsp4j.ExecuteCommandParams +import org.eclipse.lsp4j.Position +import org.eclipse.lsp4j.Range +import org.eclipse.lsp4j.TextDocumentIdentifier +import org.eclipse.lsp4j.TextDocumentPositionParams +import org.junit.Test +import org.hamcrest.Matchers.equalTo +import org.hamcrest.Matchers.hasSize +import org.junit.Assert.assertThat + +// TODO: what should the title be? just the signature? or the name of the member? should we separate between methods and variables? + +class OverrideMemberTest : SingleFileTestFixture("overridemember", "OverrideMembers.kt") { + + val root = testResourcesRoot().resolve(workspaceRoot) + val fileUri = root.resolve(file).toUri().toString() + + @Test + fun `should show all overrides for class`() { + val result = languageServer.getProtocolExtensionService().overrideMember(TextDocumentPositionParams(TextDocumentIdentifier(fileUri), position(9, 8))).get() + + assertThat(result, hasSize(2)) + + val firstCodeAction = result[0] + assertThat(firstCodeAction.title, equalTo("text")) + + val firstTextEdit = firstCodeAction.edit.changes + assertThat(firstTextEdit.containsKey(fileUri), equalTo(true)) + assertThat(firstTextEdit[fileUri], hasSize(1)) + + val memberToImplementEdit = firstTextEdit[fileUri]?.get(0) + assertThat(memberToImplementEdit?.range, equalTo(range(9, 32, 9, 32))) + assertThat(memberToImplementEdit?.newText, equalTo(System.lineSeparator() + System.lineSeparator() + " override val text: String = ???")) + + + val secondCodeAction = result[1] + assertThat(secondCodeAction.title, equalTo("print")) + + val secondTextEdit = secondCodeAction.edit.changes + assertThat(secondTextEdit.containsKey(fileUri), equalTo(true)) + assertThat(secondTextEdit[fileUri], hasSize(1)) + + val functionToImplementEdit = secondTextEdit[fileUri]?.get(0) + assertThat(functionToImplementEdit?.range, equalTo(range(9, 32, 9, 32))) + assertThat(functionToImplementEdit?.newText, equalTo(System.lineSeparator() + System.lineSeparator() + " override fun print() {}")) + } + + @Test + fun `should show one override for class where other alternatives are already implemented`() { + val result = languageServer.getProtocolExtensionService().overrideMember(TextDocumentPositionParams(TextDocumentIdentifier(fileUri), position(11, 8))).get() + + assertThat(result, hasSize(1)) + + val codeAction = result[0] + assertThat(codeAction.title, equalTo("print")) + + val textEdit = codeAction.edit.changes + assertThat(textEdit.containsKey(fileUri), equalTo(true)) + assertThat(textEdit[fileUri], hasSize(1)) + + val functionToImplementEdit = textEdit[fileUri]?.get(0) + assertThat(functionToImplementEdit?.range, equalTo(range(12, 57, 12, 57))) + assertThat(functionToImplementEdit?.newText, equalTo(System.lineSeparator() + System.lineSeparator() + " override fun print() {}")) + } + + @Test + fun `should show NO overrides for class where all other alternatives are already implemented`() { + val result = languageServer.getProtocolExtensionService().overrideMember(TextDocumentPositionParams(TextDocumentIdentifier(fileUri), position(15, 8))).get() + + assertThat(result, hasSize(0)) + } + + // TODO: test for kotlin sdk and jdk classes to verify that it works +} diff --git a/server/src/test/resources/overridemember/OverrideMembers.kt b/server/src/test/resources/overridemember/OverrideMembers.kt new file mode 100644 index 000000000..2772b6338 --- /dev/null +++ b/server/src/test/resources/overridemember/OverrideMembers.kt @@ -0,0 +1,21 @@ +interface Printable { + val text: String + + fun print() { + println("not implemented yet yo") + } +} + +class MyPrintable: Printable {} + +class OtherPrintable: Printable { + override val text: String = "you had me at lasagna" +} + +class CompletePrintable: Printable { + override val text: String = "something something something darkside" + + override fun print() { + println("not implemented yet yo") + } +} From 5207d575ce6710e98e75997fccf9537002ae38db Mon Sep 17 00:00:00 2001 From: Marie Katrine Ekeberg Date: Wed, 3 Aug 2022 19:14:25 +0200 Subject: [PATCH 2/9] Implement most of the general override functionality. Stubborn equals method --- .../kt/KotlinProtocolExtensionService.kt | 2 +- .../ImplementAbstractMembersQuickFix.kt | 132 +------- .../kt/overridemembers/OverrideMembers.kt | 298 +++++++++++++++++- .../org/javacs/kt/OverrideMemberTest.kt | 95 ++++-- .../overridemember/OverrideMembers.kt | 16 + 5 files changed, 380 insertions(+), 163 deletions(-) diff --git a/server/src/main/kotlin/org/javacs/kt/KotlinProtocolExtensionService.kt b/server/src/main/kotlin/org/javacs/kt/KotlinProtocolExtensionService.kt index 131a39375..76428a510 100644 --- a/server/src/main/kotlin/org/javacs/kt/KotlinProtocolExtensionService.kt +++ b/server/src/main/kotlin/org/javacs/kt/KotlinProtocolExtensionService.kt @@ -5,7 +5,7 @@ import org.javacs.kt.util.AsyncExecutor import org.javacs.kt.util.parseURI import org.javacs.kt.resolve.resolveMain import org.javacs.kt.position.offset -import org.javacs.kt.listOverridableMembers +import org.javacs.kt.overridemembers.listOverridableMembers import java.util.concurrent.CompletableFuture import java.nio.file.Paths 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 7603e6f43..131208aa2 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 @@ -7,6 +7,14 @@ import org.javacs.kt.index.SymbolIndex import org.javacs.kt.position.offset import org.javacs.kt.position.position import org.javacs.kt.util.toPath +import org.javacs.kt.overridemembers.createFunctionStub +import org.javacs.kt.overridemembers.createVariableStub +import org.javacs.kt.overridemembers.getClassDescriptor +import org.javacs.kt.overridemembers.getDeclarationPadding +import org.javacs.kt.overridemembers.getNewMembersStartPosition +import org.javacs.kt.overridemembers.getSuperClassTypeProjections +import org.javacs.kt.overridemembers.hasNoBody +import org.javacs.kt.overridemembers.overridesDeclaration import org.jetbrains.kotlin.descriptors.ClassDescriptor import org.jetbrains.kotlin.descriptors.ClassConstructorDescriptor import org.jetbrains.kotlin.descriptors.DeclarationDescriptor @@ -108,127 +116,3 @@ private fun getAbstractMembersStubs(file: CompiledFile, kotlinClass: KtClass) = null } }.flatten() - -// interfaces are ClassDescriptors by default. When calling AbstractClass super methods, we get a ClassConstructorDescriptor -private fun getClassDescriptor(descriptor: DeclarationDescriptor?): ClassDescriptor? = if (descriptor is ClassDescriptor) { - descriptor -} else if (descriptor is ClassConstructorDescriptor) { - descriptor.containingDeclaration -} else { - null -} - -private fun getSuperClassTypeProjections(file: CompiledFile, superType: KtSuperTypeListEntry): List = superType.typeReference?.typeElement?.children?.filter { - it is KtTypeArgumentList -}?.flatMap { - (it as KtTypeArgumentList).arguments -}?.mapNotNull { - (file.referenceExpressionAtPoint(it?.startOffset ?: 0)?.second as? ClassDescriptor)?.defaultType?.asTypeProjection() -} ?: emptyList() - -// Checks if the class overrides the given declaration -private fun overridesDeclaration(kotlinClass: KtClass, descriptor: FunctionDescriptor): Boolean = - kotlinClass.declarations.any { - if (it.name == descriptor.name.asString() && it.hasModifier(KtTokens.OVERRIDE_KEYWORD)) { - if (it is KtNamedFunction) { - parametersMatch(it, descriptor) - } else { - true - } - } else { - false - } - } - -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) { - for (index in 0 until function.valueParameters.size) { - if (function.valueParameters[index].name != functionDescriptor.valueParameters[index].name.asString()) { - return false - } else if (function.valueParameters[index].typeReference?.typeName() != functionDescriptor.valueParameters[index].type.unwrappedType().toString()) { - // Note: Since we treat Java overrides as non nullable by default, the above test will fail when the user has made the type nullable. - // TODO: look into this - return false - } - } - - if (function.typeParameters.size == functionDescriptor.typeParameters.size) { - for (index in 0 until function.typeParameters.size) { - if (function.typeParameters[index].variance != functionDescriptor.typeParameters[index].variance) { - return false - } - } - } - - return true - } - - return false -} - -private fun KtTypeReference.typeName(): String? = this.name ?: this.typeElement?.children?.filter { - it is KtSimpleNameExpression -}?.map { - (it as KtSimpleNameExpression).getReferencedName() -}?.firstOrNull() - -private fun createFunctionStub(function: FunctionDescriptor): String { - val name = function.name - val arguments = function.valueParameters.map { argument -> - val argumentName = argument.name - val argumentType = argument.type.unwrappedType() - - "$argumentName: $argumentType" - }.joinToString(", ") - val returnType = function.returnType?.unwrappedType()?.toString()?.takeIf { "Unit" != it } - - 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) - -private fun getDeclarationPadding(file: CompiledFile, kotlinClass: KtClass): String { - // If the class is not empty, the amount of padding is the same as the one in the last declaration of the class - val paddingSize = if (kotlinClass.declarations.isNotEmpty()) { - val lastFunctionStartOffset = kotlinClass.declarations.last().startOffset - position(file.content, lastFunctionStartOffset).character - } else { - // Otherwise, we just use a default tab size in addition to any existing padding - // on the class itself (note that the class could be inside another class, for example) - position(file.content, kotlinClass.startOffset).character + DEFAULT_TAB_SIZE - } - - return " ".repeat(paddingSize) -} - -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 member is put at the beginning of the class - val body = kotlinClass.body - if (body != null) { - position(file.content, body.startOffset + 1) - } else { - // 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/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt b/server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt index 59afa5a6d..dd3734b37 100644 --- a/server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt +++ b/server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt @@ -1,11 +1,301 @@ -package org.javacs.kt +package org.javacs.kt.overridemembers -import org.eclipse.lsp4j.Range import org.eclipse.lsp4j.CodeAction +import org.eclipse.lsp4j.Position +import org.eclipse.lsp4j.Range +import org.eclipse.lsp4j.TextEdit +import org.eclipse.lsp4j.WorkspaceEdit +import org.javacs.kt.CompiledFile +import org.javacs.kt.util.toPath +import org.javacs.kt.LOG +import org.javacs.kt.position.position +import org.jetbrains.kotlin.psi.KtClass +import org.jetbrains.kotlin.psi.KtTypeArgumentList +import org.jetbrains.kotlin.psi.KtSuperTypeListEntry +import org.jetbrains.kotlin.psi.KtNamedFunction +import org.jetbrains.kotlin.psi.KtTypeReference +import org.jetbrains.kotlin.psi.KtSimpleNameExpression +import org.jetbrains.kotlin.descriptors.Modality +import org.jetbrains.kotlin.descriptors.ClassDescriptor +import org.jetbrains.kotlin.descriptors.DeclarationDescriptor +import org.jetbrains.kotlin.descriptors.ClassConstructorDescriptor +import org.jetbrains.kotlin.descriptors.FunctionDescriptor +import org.jetbrains.kotlin.descriptors.isInterface +import org.jetbrains.kotlin.descriptors.PropertyDescriptor +import org.jetbrains.kotlin.js.resolve.diagnostics.findPsi +import org.jetbrains.kotlin.psi.psiUtil.endOffset +import org.jetbrains.kotlin.psi.psiUtil.isAbstract +import org.jetbrains.kotlin.psi.psiUtil.startOffset +import org.jetbrains.kotlin.types.TypeProjection +import org.jetbrains.kotlin.types.KotlinType +import org.jetbrains.kotlin.lexer.KtTokens +import org.jetbrains.kotlin.types.typeUtil.asTypeProjection +// TODO: see where this should ideally be placed +private const val DEFAULT_TAB_SIZE = 4 fun listOverridableMembers(file: CompiledFile, cursor: Int): List { - // TODO: implement + val kotlinClass = file.parseAtPoint(cursor) + + if (kotlinClass is KtClass) { + return createOverrideAlternatives(file, kotlinClass) + } - return listOf() + return emptyList() } + +private fun createOverrideAlternatives(file: CompiledFile, kotlinClass: KtClass): List { + // Get the functions that need to be implemented + val membersToImplement = getUnimplementedMembersStubs(file, kotlinClass) + + val uri = file.parse.toPath().toUri().toString() + + // 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 newMembersStartPosition = getNewMembersStartPosition(file, kotlinClass) + // TODO: if both used here and in the abstract member stuff.... should we put it in a method both can use? + // TODO: how should this be implemented? + // val bodyAppendBeginning = + // listOf(TextEdit(Range(newMembersStartPosition, newMembersStartPosition), "{")).takeIf { + // kotlinClass.hasNoBody() + // } + // ?: emptyList() + // val bodyAppendEnd = + // listOf( + // TextEdit( + // Range(newMembersStartPosition, newMembersStartPosition), + // System.lineSeparator() + "}") + // ) + // .takeIf { kotlinClass.hasNoBody() } + // ?: emptyList() + + LOG.info("Members: {}", membersToImplement) + + // loop through the memberstoimplement and create code actions + return membersToImplement.map { member -> + val newText = System.lineSeparator() + System.lineSeparator() + padding + member + val textEdit = TextEdit(Range(newMembersStartPosition, newMembersStartPosition), newText) + + // TODO: how should we get the name of the property? if needed? + val codeAction = CodeAction() + codeAction.edit = WorkspaceEdit(mapOf(uri to listOf(textEdit))) + codeAction.title = member + + codeAction + } +} + +// TODO: any way can repeat less code between this and the getAbstractMembersStubs in the ImplementAbstractMembersQuickfix? +private fun getUnimplementedMembersStubs(file: CompiledFile, kotlinClass: KtClass): List = + // For each of the super types used by this class + kotlinClass + .superTypeListEntries + .mapNotNull { + // Find the definition of this super type + val referenceAtPoint = file.referenceExpressionAtPoint(it.startOffset) + val descriptor = referenceAtPoint?.second + val classDescriptor = getClassDescriptor(descriptor) + + // If the super class is abstract, interface or just plain open + if (null != classDescriptor && classDescriptor.canBeExtended() + ) { + val superClassTypeArguments = getSuperClassTypeProjections(file, it) + classDescriptor + .getMemberScope(superClassTypeArguments) + .getContributedDescriptors() + .filter { classMember -> + (classMember is FunctionDescriptor && + classMember.canBeOverriden() && + !overridesDeclaration(kotlinClass, classMember)) || + (classMember is PropertyDescriptor && + classMember.canBeOverriden() && + !overridesDeclaration(kotlinClass, classMember)) + } + .mapNotNull { member -> + when (member) { + is FunctionDescriptor -> createFunctionStub(member) + is PropertyDescriptor -> createVariableStub(member) + else -> null + } + } + } else { + null + } + } + .flatten() + +private fun ClassDescriptor.canBeExtended() = this.kind.isInterface || + this.modality == Modality.ABSTRACT || + this.modality == Modality.OPEN + +private fun FunctionDescriptor.canBeOverriden() = Modality.ABSTRACT == this.modality || Modality.OPEN == this.modality + +private fun PropertyDescriptor.canBeOverriden() = Modality.ABSTRACT == this.modality || Modality.OPEN == this.modality + +// interfaces are ClassDescriptors by default. When calling AbstractClass super methods, we get a ClassConstructorDescriptor +fun getClassDescriptor(descriptor: DeclarationDescriptor?): ClassDescriptor? = + if (descriptor is ClassDescriptor) { + descriptor + } else if (descriptor is ClassConstructorDescriptor) { + descriptor.containingDeclaration + } else { + null + } + +fun getSuperClassTypeProjections( + file: CompiledFile, + superType: KtSuperTypeListEntry +): List = + superType + .typeReference + ?.typeElement + ?.children + ?.filter { it is KtTypeArgumentList } + ?.flatMap { (it as KtTypeArgumentList).arguments } + ?.mapNotNull { + (file.referenceExpressionAtPoint(it?.startOffset ?: 0)?.second as? + ClassDescriptor) + ?.defaultType?.asTypeProjection() + } + ?: emptyList() + +// Checks if the class overrides the given declaration +fun overridesDeclaration(kotlinClass: KtClass, descriptor: FunctionDescriptor): Boolean = + kotlinClass.declarations.any { + if (it.name == descriptor.name.asString() && it.hasModifier(KtTokens.OVERRIDE_KEYWORD) + ) { + if (it is KtNamedFunction) { + parametersMatch(it, descriptor) + } else { + true + } + } else { + false + } + } + +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) { + for (index in 0 until function.valueParameters.size) { + LOG.info("Method: {} {} - {} {}", function.valueParameters[index].name, functionDescriptor.valueParameters[index].name.asString(), function.valueParameters[index].typeReference?.typeName(), functionDescriptor.valueParameters[index] + .type + .unwrappedType() + .toString()) + if (function.valueParameters[index].name != + functionDescriptor.valueParameters[index].name.asString() + ) { + return false + } else if (function.valueParameters[index].typeReference?.typeName() != + functionDescriptor.valueParameters[index] + .type + .unwrappedType() + .toString() + ) { + // Note: Since we treat Java overrides as non nullable by default, the above test + // will fail when the user has made the type nullable. + // TODO: look into this + // TODO: look into the weird issue with equals... + return false + } + } + + if (function.typeParameters.size == functionDescriptor.typeParameters.size) { + for (index in 0 until function.typeParameters.size) { + if (function.typeParameters[index].variance != + functionDescriptor.typeParameters[index].variance + ) { + return false + } + } + } + + return true + } + + return false +} + +private fun KtTypeReference.typeName(): String? = + this.name + ?: this.typeElement + ?.children + ?.filter { it is KtSimpleNameExpression } + ?.map { (it as KtSimpleNameExpression).getReferencedName() } + ?.firstOrNull() + +fun createFunctionStub(function: FunctionDescriptor): String { + val name = function.name + val arguments = + function.valueParameters + .map { argument -> + val argumentName = argument.name + val argumentType = argument.type.unwrappedType() + + "$argumentName: $argumentType" + } + .joinToString(", ") + val returnType = function.returnType?.unwrappedType()?.toString()?.takeIf { "Unit" != it } + + return "override fun $name($arguments)${returnType?.let { ": $it" } ?: ""} { }" +} + +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) + +fun getDeclarationPadding(file: CompiledFile, kotlinClass: KtClass): String { + // If the class is not empty, the amount of padding is the same as the one in the last + // declaration of the class + val paddingSize = + if (kotlinClass.declarations.isNotEmpty()) { + val lastFunctionStartOffset = kotlinClass.declarations.last().startOffset + position(file.content, lastFunctionStartOffset).character + } else { + // Otherwise, we just use a default tab size in addition to any existing padding + // on the class itself (note that the class could be inside another class, for + // example) + position(file.content, kotlinClass.startOffset).character + DEFAULT_TAB_SIZE + } + + return " ".repeat(paddingSize) +} + +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 member is put at the beginning of the class + val body = kotlinClass.body + if (body != null) { + position(file.content, body.startOffset + 1) + } else { + // 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 + } + } + +fun KtClass.hasNoBody() = null == this.body diff --git a/server/src/test/kotlin/org/javacs/kt/OverrideMemberTest.kt b/server/src/test/kotlin/org/javacs/kt/OverrideMemberTest.kt index 658bab353..358755aab 100644 --- a/server/src/test/kotlin/org/javacs/kt/OverrideMemberTest.kt +++ b/server/src/test/kotlin/org/javacs/kt/OverrideMemberTest.kt @@ -7,11 +7,14 @@ import org.eclipse.lsp4j.Range import org.eclipse.lsp4j.TextDocumentIdentifier import org.eclipse.lsp4j.TextDocumentPositionParams import org.junit.Test +import org.hamcrest.core.Every.everyItem +import org.hamcrest.Matchers.containsInAnyOrder import org.hamcrest.Matchers.equalTo import org.hamcrest.Matchers.hasSize import org.junit.Assert.assertThat // TODO: what should the title be? just the signature? or the name of the member? should we separate between methods and variables? +// easiest is probably just to show the signatures? class OverrideMemberTest : SingleFileTestFixture("overridemember", "OverrideMembers.kt") { @@ -21,49 +24,50 @@ class OverrideMemberTest : SingleFileTestFixture("overridemember", "OverrideMemb @Test fun `should show all overrides for class`() { val result = languageServer.getProtocolExtensionService().overrideMember(TextDocumentPositionParams(TextDocumentIdentifier(fileUri), position(9, 8))).get() - - assertThat(result, hasSize(2)) - - val firstCodeAction = result[0] - assertThat(firstCodeAction.title, equalTo("text")) - - val firstTextEdit = firstCodeAction.edit.changes - assertThat(firstTextEdit.containsKey(fileUri), equalTo(true)) - assertThat(firstTextEdit[fileUri], hasSize(1)) - - val memberToImplementEdit = firstTextEdit[fileUri]?.get(0) - assertThat(memberToImplementEdit?.range, equalTo(range(9, 32, 9, 32))) - assertThat(memberToImplementEdit?.newText, equalTo(System.lineSeparator() + System.lineSeparator() + " override val text: String = ???")) + val titles = result.map { it.title } + val edits = result.flatMap { it.edit.changes[fileUri]!! } + val newTexts = edits.map { it.newText } + val ranges = edits.map { it.range } - val secondCodeAction = result[1] - assertThat(secondCodeAction.title, equalTo("print")) + assertThat(titles, containsInAnyOrder("override val text: String = TODO(\"SET VALUE\")", + "override fun print() { }", + "override fun equals(other: Any?): Boolean { }", + "override fun hashCode(): Int { }", + "override fun toString(): String { }")) - val secondTextEdit = secondCodeAction.edit.changes - assertThat(secondTextEdit.containsKey(fileUri), equalTo(true)) - assertThat(secondTextEdit[fileUri], hasSize(1)) + val padding = System.lineSeparator() + System.lineSeparator() + " " + assertThat(newTexts, containsInAnyOrder(padding + "override val text: String = TODO(\"SET VALUE\")", + padding + "override fun print() { }", + padding + "override fun equals(other: Any?): Boolean { }", + padding + "override fun hashCode(): Int { }", + padding + "override fun toString(): String { }")) - val functionToImplementEdit = secondTextEdit[fileUri]?.get(0) - assertThat(functionToImplementEdit?.range, equalTo(range(9, 32, 9, 32))) - assertThat(functionToImplementEdit?.newText, equalTo(System.lineSeparator() + System.lineSeparator() + " override fun print() {}")) + + assertThat(ranges, everyItem(equalTo(range(9, 31, 9, 31)))) } @Test - fun `should show one override for class where other alternatives are already implemented`() { + fun `should show one less override for class where one member is already implemented`() { val result = languageServer.getProtocolExtensionService().overrideMember(TextDocumentPositionParams(TextDocumentIdentifier(fileUri), position(11, 8))).get() - - assertThat(result, hasSize(1)) - - val codeAction = result[0] - assertThat(codeAction.title, equalTo("print")) - - val textEdit = codeAction.edit.changes - assertThat(textEdit.containsKey(fileUri), equalTo(true)) - assertThat(textEdit[fileUri], hasSize(1)) + + val titles = result.map { it.title } + val edits = result.flatMap { it.edit.changes[fileUri]!! } + val newTexts = edits.map { it.newText } + val ranges = edits.map { it.range } + + assertThat(titles, containsInAnyOrder("override fun print() { }", + "override fun equals(other: Any?): Boolean { }", + "override fun hashCode(): Int { }", + "override fun toString(): String { }")) + + val padding = System.lineSeparator() + System.lineSeparator() + " " + assertThat(newTexts, containsInAnyOrder(padding + "override fun print() { }", + padding + "override fun equals(other: Any?): Boolean { }", + padding + "override fun hashCode(): Int { }", + padding + "override fun toString(): String { }")) - val functionToImplementEdit = textEdit[fileUri]?.get(0) - assertThat(functionToImplementEdit?.range, equalTo(range(12, 57, 12, 57))) - assertThat(functionToImplementEdit?.newText, equalTo(System.lineSeparator() + System.lineSeparator() + " override fun print() {}")) + assertThat(ranges, everyItem(equalTo(range(12, 56, 12, 56)))) } @Test @@ -73,5 +77,28 @@ class OverrideMemberTest : SingleFileTestFixture("overridemember", "OverrideMemb assertThat(result, hasSize(0)) } + @Test + fun `should find method in open class`() { + val result = languageServer.getProtocolExtensionService().overrideMember(TextDocumentPositionParams(TextDocumentIdentifier(fileUri), position(37, 8))).get() + + val titles = result.map { it.title } + val edits = result.flatMap { it.edit.changes[fileUri]!! } + val newTexts = edits.map { it.newText } + val ranges = edits.map { it.range } + + assertThat(titles, containsInAnyOrder("override fun numOpenDoorsWithName(input: String): Int { }", + "override fun equals(other: Any?): Boolean { }", + "override fun hashCode(): Int { }", + "override fun toString(): String { }")) + + val padding = System.lineSeparator() + System.lineSeparator() + " " + assertThat(newTexts, containsInAnyOrder(padding + "override fun numOpenDoorsWithName(input: String): Int { }", + padding + "override fun equals(other: Any?): Boolean { }", + padding + "override fun hashCode(): Int { }", + padding + "override fun toString(): String { }")) + + assertThat(ranges, everyItem(equalTo(range(37, 25, 37, 25)))) + } + // TODO: test for kotlin sdk and jdk classes to verify that it works } diff --git a/server/src/test/resources/overridemember/OverrideMembers.kt b/server/src/test/resources/overridemember/OverrideMembers.kt index 2772b6338..1d3434321 100644 --- a/server/src/test/resources/overridemember/OverrideMembers.kt +++ b/server/src/test/resources/overridemember/OverrideMembers.kt @@ -15,7 +15,23 @@ class OtherPrintable: Printable { class CompletePrintable: Printable { override val text: String = "something something something darkside" + override fun equals(other: Any?): Boolean { return true } + + override fun hashCode(): Int { return 1 } + + override fun toString(): String { + return "something something complete" + } + override fun print() { println("not implemented yet yo") } } + +open class MyOpen { + open fun numOpenDoorsWithName(input: String): Int { + return 2 + } +} + +class Closed: MyOpen() {} From 52f80e927720ec05d4f383be72ad702df1643615 Mon Sep 17 00:00:00 2001 From: Marie Katrine Ekeberg Date: Wed, 3 Aug 2022 19:33:23 +0200 Subject: [PATCH 3/9] Fix weird issue with overriding equals --- .../javacs/kt/overridemembers/OverrideMembers.kt | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt b/server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt index dd3734b37..6dfef58c6 100644 --- a/server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt +++ b/server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt @@ -26,6 +26,7 @@ import org.jetbrains.kotlin.js.resolve.diagnostics.findPsi import org.jetbrains.kotlin.psi.psiUtil.endOffset import org.jetbrains.kotlin.psi.psiUtil.isAbstract import org.jetbrains.kotlin.psi.psiUtil.startOffset +import org.jetbrains.kotlin.psi.psiUtil.unwrapNullability import org.jetbrains.kotlin.types.TypeProjection import org.jetbrains.kotlin.types.KotlinType import org.jetbrains.kotlin.lexer.KtTokens @@ -188,24 +189,26 @@ private fun parametersMatch( ): Boolean { if (function.valueParameters.size == functionDescriptor.valueParameters.size) { for (index in 0 until function.valueParameters.size) { - LOG.info("Method: {} {} - {} {}", function.valueParameters[index].name, functionDescriptor.valueParameters[index].name.asString(), function.valueParameters[index].typeReference?.typeName(), functionDescriptor.valueParameters[index] + LOG.info("Method: {} {} - {} {}", function.valueParameters[index].name, functionDescriptor.valueParameters[index].name.asString(), function.valueParameters[index].typeReference?.typeElement?.unwrapNullability()?.name, functionDescriptor.valueParameters[index] .type .unwrappedType() .toString()) if (function.valueParameters[index].name != - functionDescriptor.valueParameters[index].name.asString() + functionDescriptor.valueParameters[index].name.asString() ) { return false } else if (function.valueParameters[index].typeReference?.typeName() != functionDescriptor.valueParameters[index] .type .unwrappedType() - .toString() + .toString() && function.valueParameters[index].typeReference?.typeName() != null ) { + // Any and Any? seems to be null for Kt* psi objects for some reason? At least for equals + // TODO: look further into this + // Note: Since we treat Java overrides as non nullable by default, the above test // will fail when the user has made the type nullable. // TODO: look into this - // TODO: look into the weird issue with equals... return false } } @@ -213,7 +216,7 @@ private fun parametersMatch( if (function.typeParameters.size == functionDescriptor.typeParameters.size) { for (index in 0 until function.typeParameters.size) { if (function.typeParameters[index].variance != - functionDescriptor.typeParameters[index].variance + functionDescriptor.typeParameters[index].variance ) { return false } From 095ddaf3091d585d4bffaa97d44128b2cd82fee6 Mon Sep 17 00:00:00 2001 From: Marie Katrine Ekeberg Date: Wed, 3 Aug 2022 20:07:26 +0200 Subject: [PATCH 4/9] Add testcase for jdk superclass and fix minor issues --- .../org/javacs/kt/KotlinProtocolExtensions.kt | 2 - .../kt/overridemembers/OverrideMembers.kt | 27 ++--------- .../org/javacs/kt/OverrideMemberTest.kt | 47 ++++++++++++++++++- .../overridemember/OverrideMembers.kt | 2 + 4 files changed, 51 insertions(+), 27 deletions(-) diff --git a/server/src/main/kotlin/org/javacs/kt/KotlinProtocolExtensions.kt b/server/src/main/kotlin/org/javacs/kt/KotlinProtocolExtensions.kt index a599f11f4..b29c4b247 100644 --- a/server/src/main/kotlin/org/javacs/kt/KotlinProtocolExtensions.kt +++ b/server/src/main/kotlin/org/javacs/kt/KotlinProtocolExtensions.kt @@ -16,8 +16,6 @@ interface KotlinProtocolExtensions { @JsonRequest fun mainClass(textDocument: TextDocumentIdentifier): CompletableFuture> - // TODO: what is the best return value in this case? CodeAction? - // TODO: should the naming be something like listOverrideableMembers? or something similar instead? @JsonRequest fun overrideMember(position: TextDocumentPositionParams): CompletableFuture> } diff --git a/server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt b/server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt index 6dfef58c6..f0f74053b 100644 --- a/server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt +++ b/server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt @@ -18,6 +18,7 @@ import org.jetbrains.kotlin.psi.KtSimpleNameExpression import org.jetbrains.kotlin.descriptors.Modality import org.jetbrains.kotlin.descriptors.ClassDescriptor import org.jetbrains.kotlin.descriptors.DeclarationDescriptor +import org.jetbrains.kotlin.descriptors.DescriptorVisibilities import org.jetbrains.kotlin.descriptors.ClassConstructorDescriptor import org.jetbrains.kotlin.descriptors.FunctionDescriptor import org.jetbrains.kotlin.descriptors.isInterface @@ -56,30 +57,12 @@ private fun createOverrideAlternatives(file: CompiledFile, kotlinClass: KtClass) // Get the location where the new code will be placed val newMembersStartPosition = getNewMembersStartPosition(file, kotlinClass) - // TODO: if both used here and in the abstract member stuff.... should we put it in a method both can use? - // TODO: how should this be implemented? - // val bodyAppendBeginning = - // listOf(TextEdit(Range(newMembersStartPosition, newMembersStartPosition), "{")).takeIf { - // kotlinClass.hasNoBody() - // } - // ?: emptyList() - // val bodyAppendEnd = - // listOf( - // TextEdit( - // Range(newMembersStartPosition, newMembersStartPosition), - // System.lineSeparator() + "}") - // ) - // .takeIf { kotlinClass.hasNoBody() } - // ?: emptyList() - - LOG.info("Members: {}", membersToImplement) // loop through the memberstoimplement and create code actions return membersToImplement.map { member -> val newText = System.lineSeparator() + System.lineSeparator() + padding + member val textEdit = TextEdit(Range(newMembersStartPosition, newMembersStartPosition), newText) - // TODO: how should we get the name of the property? if needed? val codeAction = CodeAction() codeAction.edit = WorkspaceEdit(mapOf(uri to listOf(textEdit))) codeAction.title = member @@ -131,9 +114,9 @@ private fun ClassDescriptor.canBeExtended() = this.kind.isInterface || this.modality == Modality.ABSTRACT || this.modality == Modality.OPEN -private fun FunctionDescriptor.canBeOverriden() = Modality.ABSTRACT == this.modality || Modality.OPEN == this.modality +private fun FunctionDescriptor.canBeOverriden() = (Modality.ABSTRACT == this.modality || Modality.OPEN == this.modality) && Modality.FINAL != this.modality && this.visibility != DescriptorVisibilities.PRIVATE && this.visibility != DescriptorVisibilities.PROTECTED -private fun PropertyDescriptor.canBeOverriden() = Modality.ABSTRACT == this.modality || Modality.OPEN == this.modality +private fun PropertyDescriptor.canBeOverriden() = (Modality.ABSTRACT == this.modality || Modality.OPEN == this.modality) && Modality.FINAL != this.modality && this.visibility != DescriptorVisibilities.PRIVATE && this.visibility != DescriptorVisibilities.PROTECTED // interfaces are ClassDescriptors by default. When calling AbstractClass super methods, we get a ClassConstructorDescriptor fun getClassDescriptor(descriptor: DeclarationDescriptor?): ClassDescriptor? = @@ -189,10 +172,6 @@ private fun parametersMatch( ): Boolean { if (function.valueParameters.size == functionDescriptor.valueParameters.size) { for (index in 0 until function.valueParameters.size) { - LOG.info("Method: {} {} - {} {}", function.valueParameters[index].name, functionDescriptor.valueParameters[index].name.asString(), function.valueParameters[index].typeReference?.typeElement?.unwrapNullability()?.name, functionDescriptor.valueParameters[index] - .type - .unwrappedType() - .toString()) if (function.valueParameters[index].name != functionDescriptor.valueParameters[index].name.asString() ) { diff --git a/server/src/test/kotlin/org/javacs/kt/OverrideMemberTest.kt b/server/src/test/kotlin/org/javacs/kt/OverrideMemberTest.kt index 358755aab..4813bc694 100644 --- a/server/src/test/kotlin/org/javacs/kt/OverrideMemberTest.kt +++ b/server/src/test/kotlin/org/javacs/kt/OverrideMemberTest.kt @@ -100,5 +100,50 @@ class OverrideMemberTest : SingleFileTestFixture("overridemember", "OverrideMemb assertThat(ranges, everyItem(equalTo(range(37, 25, 37, 25)))) } - // TODO: test for kotlin sdk and jdk classes to verify that it works + @Test + fun `should find members in jdk object`() { + val result = languageServer.getProtocolExtensionService().overrideMember(TextDocumentPositionParams(TextDocumentIdentifier(fileUri), position(39, 9))).get() + + val titles = result.map { it.title } + val edits = result.flatMap { it.edit.changes[fileUri]!! } + val newTexts = edits.map { it.newText } + val ranges = edits.map { it.range } + + assertThat(titles, containsInAnyOrder("override fun equals(other: Any?): Boolean { }", + "override fun hashCode(): Int { }", + "override fun toString(): String { }", + "override fun run() { }", + "override fun clone(): Any { }", + "override fun start() { }", + "override fun interrupt() { }", + "override fun isInterrupted(): Boolean { }", + "override fun countStackFrames(): Int { }", + "override fun getContextClassLoader(): ClassLoader { }", + "override fun setContextClassLoader(cl: ClassLoader) { }", + "override fun getStackTrace(): (Array<(StackTraceElement..StackTraceElement?)>..Array) { }", + "override fun getId(): Long { }", + "override fun getState(): State { }", + "override fun getUncaughtExceptionHandler(): UncaughtExceptionHandler { }", + "override fun setUncaughtExceptionHandler(eh: UncaughtExceptionHandler) { }")) + + val padding = System.lineSeparator() + System.lineSeparator() + " " + assertThat(newTexts, containsInAnyOrder(padding + "override fun equals(other: Any?): Boolean { }", + padding + "override fun hashCode(): Int { }", + padding + "override fun toString(): String { }", + padding + "override fun run() { }", + padding + "override fun clone(): Any { }", + padding + "override fun start() { }", + padding + "override fun interrupt() { }", + padding + "override fun isInterrupted(): Boolean { }", + padding + "override fun countStackFrames(): Int { }", + padding + "override fun getContextClassLoader(): ClassLoader { }", + padding + "override fun setContextClassLoader(cl: ClassLoader) { }", + padding + "override fun getStackTrace(): (Array<(StackTraceElement..StackTraceElement?)>..Array) { }", + padding + "override fun getId(): Long { }", + padding + "override fun getState(): State { }", + padding + "override fun getUncaughtExceptionHandler(): UncaughtExceptionHandler { }", + padding + "override fun setUncaughtExceptionHandler(eh: UncaughtExceptionHandler) { }")) + + assertThat(ranges, everyItem(equalTo(range(39, 25, 39, 25)))) + } } diff --git a/server/src/test/resources/overridemember/OverrideMembers.kt b/server/src/test/resources/overridemember/OverrideMembers.kt index 1d3434321..4e6b14465 100644 --- a/server/src/test/resources/overridemember/OverrideMembers.kt +++ b/server/src/test/resources/overridemember/OverrideMembers.kt @@ -35,3 +35,5 @@ open class MyOpen { } class Closed: MyOpen() {} + +class MyThread: Thread {} From 965f844e6dffa5ecdae7edb597fcf953cebe63a0 Mon Sep 17 00:00:00 2001 From: Marie Katrine Ekeberg Date: Thu, 4 Aug 2022 17:05:25 +0200 Subject: [PATCH 5/9] Minor tweaks and indentation fix --- .../kt/overridemembers/OverrideMembers.kt | 71 +++++++++---------- 1 file changed, 35 insertions(+), 36 deletions(-) diff --git a/server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt b/server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt index f0f74053b..152baf602 100644 --- a/server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt +++ b/server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt @@ -7,7 +7,6 @@ import org.eclipse.lsp4j.TextEdit import org.eclipse.lsp4j.WorkspaceEdit import org.javacs.kt.CompiledFile import org.javacs.kt.util.toPath -import org.javacs.kt.LOG import org.javacs.kt.position.position import org.jetbrains.kotlin.psi.KtClass import org.jetbrains.kotlin.psi.KtTypeArgumentList @@ -73,42 +72,42 @@ private fun createOverrideAlternatives(file: CompiledFile, kotlinClass: KtClass) // TODO: any way can repeat less code between this and the getAbstractMembersStubs in the ImplementAbstractMembersQuickfix? private fun getUnimplementedMembersStubs(file: CompiledFile, kotlinClass: KtClass): List = - // For each of the super types used by this class - kotlinClass - .superTypeListEntries - .mapNotNull { - // Find the definition of this super type - val referenceAtPoint = file.referenceExpressionAtPoint(it.startOffset) - val descriptor = referenceAtPoint?.second - val classDescriptor = getClassDescriptor(descriptor) - - // If the super class is abstract, interface or just plain open - if (null != classDescriptor && classDescriptor.canBeExtended() - ) { - val superClassTypeArguments = getSuperClassTypeProjections(file, it) - classDescriptor - .getMemberScope(superClassTypeArguments) - .getContributedDescriptors() - .filter { classMember -> - (classMember is FunctionDescriptor && - classMember.canBeOverriden() && - !overridesDeclaration(kotlinClass, classMember)) || - (classMember is PropertyDescriptor && - classMember.canBeOverriden() && - !overridesDeclaration(kotlinClass, classMember)) - } - .mapNotNull { member -> - when (member) { - is FunctionDescriptor -> createFunctionStub(member) - is PropertyDescriptor -> createVariableStub(member) - else -> null - } - } - } else { - null + // For each of the super types used by this class + // TODO: does not seem to handle the implicit Any and Object super types that well. Need to find out if that is easily solvable. Finds the methods from them if any super class or interface is present + kotlinClass + .superTypeListEntries + .mapNotNull { + // Find the definition of this super type + val referenceAtPoint = file.referenceExpressionAtPoint(it.startOffset) + val descriptor = referenceAtPoint?.second + val classDescriptor = getClassDescriptor(descriptor) + + // If the super class is abstract, interface or just plain open + if (null != classDescriptor && classDescriptor.canBeExtended()) { + val superClassTypeArguments = getSuperClassTypeProjections(file, it) + classDescriptor + .getMemberScope(superClassTypeArguments) + .getContributedDescriptors() + .filter { classMember -> + (classMember is FunctionDescriptor && + classMember.canBeOverriden() && + !overridesDeclaration(kotlinClass, classMember)) || + (classMember is PropertyDescriptor && + classMember.canBeOverriden() && + !overridesDeclaration(kotlinClass, classMember)) } - } - .flatten() + .mapNotNull { member -> + when (member) { + is FunctionDescriptor -> createFunctionStub(member) + is PropertyDescriptor -> createVariableStub(member) + else -> null + } + } + } else { + null + } + } + .flatten() private fun ClassDescriptor.canBeExtended() = this.kind.isInterface || this.modality == Modality.ABSTRACT || From 74136b442e38574b3f7eecd4b9d318f1a8bbdce6 Mon Sep 17 00:00:00 2001 From: Marie Katrine Ekeberg Date: Thu, 4 Aug 2022 17:43:17 +0200 Subject: [PATCH 6/9] Removed todo comment --- server/src/test/kotlin/org/javacs/kt/OverrideMemberTest.kt | 3 --- 1 file changed, 3 deletions(-) diff --git a/server/src/test/kotlin/org/javacs/kt/OverrideMemberTest.kt b/server/src/test/kotlin/org/javacs/kt/OverrideMemberTest.kt index 4813bc694..e2f6c1f18 100644 --- a/server/src/test/kotlin/org/javacs/kt/OverrideMemberTest.kt +++ b/server/src/test/kotlin/org/javacs/kt/OverrideMemberTest.kt @@ -13,9 +13,6 @@ import org.hamcrest.Matchers.equalTo import org.hamcrest.Matchers.hasSize import org.junit.Assert.assertThat -// TODO: what should the title be? just the signature? or the name of the member? should we separate between methods and variables? -// easiest is probably just to show the signatures? - class OverrideMemberTest : SingleFileTestFixture("overridemember", "OverrideMembers.kt") { val root = testResourcesRoot().resolve(workspaceRoot) From c5c4f519481b2e20a0750a785b418cc5ade51043 Mon Sep 17 00:00:00 2001 From: Marie Katrine Ekeberg Date: Fri, 12 Aug 2022 21:42:15 +0200 Subject: [PATCH 7/9] Remove unused variable --- .../kt/codeaction/quickfix/ImplementAbstractMembersQuickFix.kt | 2 -- 1 file changed, 2 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 131208aa2..e4b0a6b11 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 @@ -40,8 +40,6 @@ import org.jetbrains.kotlin.types.KotlinType import org.jetbrains.kotlin.types.TypeProjection import org.jetbrains.kotlin.types.typeUtil.asTypeProjection -private const val DEFAULT_TAB_SIZE = 4 - class ImplementAbstractMembersQuickFix : QuickFix { override fun compute(file: CompiledFile, index: SymbolIndex, range: Range, diagnostics: List): List> { val diagnostic = findDiagnosticMatch(diagnostics, range) From ed1d4adb58968172424b3c17d6b77155e420dbea Mon Sep 17 00:00:00 2001 From: Marie Katrine Ekeberg Date: Fri, 12 Aug 2022 21:45:29 +0200 Subject: [PATCH 8/9] Simplify code based upon PR comments --- .../kt/overridemembers/OverrideMembers.kt | 31 +++++++------------ 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt b/server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt index 152baf602..cd3e66ead 100644 --- a/server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt +++ b/server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt @@ -22,6 +22,7 @@ import org.jetbrains.kotlin.descriptors.ClassConstructorDescriptor import org.jetbrains.kotlin.descriptors.FunctionDescriptor import org.jetbrains.kotlin.descriptors.isInterface import org.jetbrains.kotlin.descriptors.PropertyDescriptor +import org.jetbrains.kotlin.descriptors.MemberDescriptor import org.jetbrains.kotlin.js.resolve.diagnostics.findPsi import org.jetbrains.kotlin.psi.psiUtil.endOffset import org.jetbrains.kotlin.psi.psiUtil.isAbstract @@ -112,11 +113,9 @@ private fun getUnimplementedMembersStubs(file: CompiledFile, kotlinClass: KtClas private fun ClassDescriptor.canBeExtended() = this.kind.isInterface || this.modality == Modality.ABSTRACT || this.modality == Modality.OPEN - -private fun FunctionDescriptor.canBeOverriden() = (Modality.ABSTRACT == this.modality || Modality.OPEN == this.modality) && Modality.FINAL != this.modality && this.visibility != DescriptorVisibilities.PRIVATE && this.visibility != DescriptorVisibilities.PROTECTED - -private fun PropertyDescriptor.canBeOverriden() = (Modality.ABSTRACT == this.modality || Modality.OPEN == this.modality) && Modality.FINAL != this.modality && this.visibility != DescriptorVisibilities.PRIVATE && this.visibility != DescriptorVisibilities.PROTECTED +private fun MemberDescriptor.canBeOverriden() = (Modality.ABSTRACT == this.modality || Modality.OPEN == this.modality) && Modality.FINAL != this.modality && this.visibility != DescriptorVisibilities.PRIVATE && this.visibility != DescriptorVisibilities.PROTECTED + // interfaces are ClassDescriptors by default. When calling AbstractClass super methods, we get a ClassConstructorDescriptor fun getClassDescriptor(descriptor: DeclarationDescriptor?): ClassDescriptor? = if (descriptor is ClassDescriptor) { @@ -145,24 +144,18 @@ fun getSuperClassTypeProjections( ?: emptyList() // Checks if the class overrides the given declaration -fun overridesDeclaration(kotlinClass: KtClass, descriptor: FunctionDescriptor): Boolean = - kotlinClass.declarations.any { - if (it.name == descriptor.name.asString() && it.hasModifier(KtTokens.OVERRIDE_KEYWORD) - ) { - if (it is KtNamedFunction) { - parametersMatch(it, descriptor) - } else { - true - } - } else { - false - } +fun overridesDeclaration(kotlinClass: KtClass, descriptor: MemberDescriptor): Boolean = + when (descriptor) { + is FunctionDescriptor -> kotlinClass.declarations.any { + it.name == descriptor.name.asString() + && it.hasModifier(KtTokens.OVERRIDE_KEYWORD) + && ((it as? KtNamedFunction)?.let { parametersMatch(it, descriptor) } ?: true) } - -fun overridesDeclaration(kotlinClass: KtClass, descriptor: PropertyDescriptor): Boolean = - kotlinClass.declarations.any { + is PropertyDescriptor -> kotlinClass.declarations.any { it.name == descriptor.name.asString() && it.hasModifier(KtTokens.OVERRIDE_KEYWORD) } + else -> false + } // Checks if two functions have matching parameters private fun parametersMatch( From 596c6a3268e2fa6d9cbeb13daa551e03af464e54 Mon Sep 17 00:00:00 2001 From: Marie Katrine Ekeberg Date: Sat, 13 Aug 2022 09:56:14 +0200 Subject: [PATCH 9/9] Simplify expression based upon earlier refactoring using MemberDescriptor --- .../org/javacs/kt/overridemembers/OverrideMembers.kt | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt b/server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt index cd3e66ead..e404d94a9 100644 --- a/server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt +++ b/server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt @@ -90,12 +90,9 @@ private fun getUnimplementedMembersStubs(file: CompiledFile, kotlinClass: KtClas .getMemberScope(superClassTypeArguments) .getContributedDescriptors() .filter { classMember -> - (classMember is FunctionDescriptor && + classMember is MemberDescriptor && classMember.canBeOverriden() && - !overridesDeclaration(kotlinClass, classMember)) || - (classMember is PropertyDescriptor && - classMember.canBeOverriden() && - !overridesDeclaration(kotlinClass, classMember)) + !overridesDeclaration(kotlinClass, classMember) } .mapNotNull { member -> when (member) {