Skip to content

Commit 095ddaf

Browse files
committed
Add testcase for jdk superclass and fix minor issues
1 parent 52f80e9 commit 095ddaf

File tree

4 files changed

+51
-27
lines changed

4 files changed

+51
-27
lines changed

server/src/main/kotlin/org/javacs/kt/KotlinProtocolExtensions.kt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@ interface KotlinProtocolExtensions {
1616
@JsonRequest
1717
fun mainClass(textDocument: TextDocumentIdentifier): CompletableFuture<Map<String, Any?>>
1818

19-
// TODO: what is the best return value in this case? CodeAction?
20-
// TODO: should the naming be something like listOverrideableMembers? or something similar instead?
2119
@JsonRequest
2220
fun overrideMember(position: TextDocumentPositionParams): CompletableFuture<List<CodeAction>>
2321
}

server/src/main/kotlin/org/javacs/kt/overridemembers/OverrideMembers.kt

Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import org.jetbrains.kotlin.psi.KtSimpleNameExpression
1818
import org.jetbrains.kotlin.descriptors.Modality
1919
import org.jetbrains.kotlin.descriptors.ClassDescriptor
2020
import org.jetbrains.kotlin.descriptors.DeclarationDescriptor
21+
import org.jetbrains.kotlin.descriptors.DescriptorVisibilities
2122
import org.jetbrains.kotlin.descriptors.ClassConstructorDescriptor
2223
import org.jetbrains.kotlin.descriptors.FunctionDescriptor
2324
import org.jetbrains.kotlin.descriptors.isInterface
@@ -56,30 +57,12 @@ private fun createOverrideAlternatives(file: CompiledFile, kotlinClass: KtClass)
5657

5758
// Get the location where the new code will be placed
5859
val newMembersStartPosition = getNewMembersStartPosition(file, kotlinClass)
59-
// TODO: if both used here and in the abstract member stuff.... should we put it in a method both can use?
60-
// TODO: how should this be implemented?
61-
// val bodyAppendBeginning =
62-
// listOf(TextEdit(Range(newMembersStartPosition, newMembersStartPosition), "{")).takeIf {
63-
// kotlinClass.hasNoBody()
64-
// }
65-
// ?: emptyList()
66-
// val bodyAppendEnd =
67-
// listOf(
68-
// TextEdit(
69-
// Range(newMembersStartPosition, newMembersStartPosition),
70-
// System.lineSeparator() + "}")
71-
// )
72-
// .takeIf { kotlinClass.hasNoBody() }
73-
// ?: emptyList()
74-
75-
LOG.info("Members: {}", membersToImplement)
7660

7761
// loop through the memberstoimplement and create code actions
7862
return membersToImplement.map { member ->
7963
val newText = System.lineSeparator() + System.lineSeparator() + padding + member
8064
val textEdit = TextEdit(Range(newMembersStartPosition, newMembersStartPosition), newText)
8165

82-
// TODO: how should we get the name of the property? if needed?
8366
val codeAction = CodeAction()
8467
codeAction.edit = WorkspaceEdit(mapOf(uri to listOf(textEdit)))
8568
codeAction.title = member
@@ -131,9 +114,9 @@ private fun ClassDescriptor.canBeExtended() = this.kind.isInterface ||
131114
this.modality == Modality.ABSTRACT ||
132115
this.modality == Modality.OPEN
133116

134-
private fun FunctionDescriptor.canBeOverriden() = Modality.ABSTRACT == this.modality || Modality.OPEN == this.modality
117+
private fun FunctionDescriptor.canBeOverriden() = (Modality.ABSTRACT == this.modality || Modality.OPEN == this.modality) && Modality.FINAL != this.modality && this.visibility != DescriptorVisibilities.PRIVATE && this.visibility != DescriptorVisibilities.PROTECTED
135118

136-
private fun PropertyDescriptor.canBeOverriden() = Modality.ABSTRACT == this.modality || Modality.OPEN == this.modality
119+
private fun PropertyDescriptor.canBeOverriden() = (Modality.ABSTRACT == this.modality || Modality.OPEN == this.modality) && Modality.FINAL != this.modality && this.visibility != DescriptorVisibilities.PRIVATE && this.visibility != DescriptorVisibilities.PROTECTED
137120

138121
// interfaces are ClassDescriptors by default. When calling AbstractClass super methods, we get a ClassConstructorDescriptor
139122
fun getClassDescriptor(descriptor: DeclarationDescriptor?): ClassDescriptor? =
@@ -189,10 +172,6 @@ private fun parametersMatch(
189172
): Boolean {
190173
if (function.valueParameters.size == functionDescriptor.valueParameters.size) {
191174
for (index in 0 until function.valueParameters.size) {
192-
LOG.info("Method: {} {} - {} {}", function.valueParameters[index].name, functionDescriptor.valueParameters[index].name.asString(), function.valueParameters[index].typeReference?.typeElement?.unwrapNullability()?.name, functionDescriptor.valueParameters[index]
193-
.type
194-
.unwrappedType()
195-
.toString())
196175
if (function.valueParameters[index].name !=
197176
functionDescriptor.valueParameters[index].name.asString()
198177
) {

server/src/test/kotlin/org/javacs/kt/OverrideMemberTest.kt

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,5 +100,50 @@ class OverrideMemberTest : SingleFileTestFixture("overridemember", "OverrideMemb
100100
assertThat(ranges, everyItem(equalTo(range(37, 25, 37, 25))))
101101
}
102102

103-
// TODO: test for kotlin sdk and jdk classes to verify that it works
103+
@Test
104+
fun `should find members in jdk object`() {
105+
val result = languageServer.getProtocolExtensionService().overrideMember(TextDocumentPositionParams(TextDocumentIdentifier(fileUri), position(39, 9))).get()
106+
107+
val titles = result.map { it.title }
108+
val edits = result.flatMap { it.edit.changes[fileUri]!! }
109+
val newTexts = edits.map { it.newText }
110+
val ranges = edits.map { it.range }
111+
112+
assertThat(titles, containsInAnyOrder("override fun equals(other: Any?): Boolean { }",
113+
"override fun hashCode(): Int { }",
114+
"override fun toString(): String { }",
115+
"override fun run() { }",
116+
"override fun clone(): Any { }",
117+
"override fun start() { }",
118+
"override fun interrupt() { }",
119+
"override fun isInterrupted(): Boolean { }",
120+
"override fun countStackFrames(): Int { }",
121+
"override fun getContextClassLoader(): ClassLoader { }",
122+
"override fun setContextClassLoader(cl: ClassLoader) { }",
123+
"override fun getStackTrace(): (Array<(StackTraceElement..StackTraceElement?)>..Array<out (StackTraceElement..StackTraceElement?)>) { }",
124+
"override fun getId(): Long { }",
125+
"override fun getState(): State { }",
126+
"override fun getUncaughtExceptionHandler(): UncaughtExceptionHandler { }",
127+
"override fun setUncaughtExceptionHandler(eh: UncaughtExceptionHandler) { }"))
128+
129+
val padding = System.lineSeparator() + System.lineSeparator() + " "
130+
assertThat(newTexts, containsInAnyOrder(padding + "override fun equals(other: Any?): Boolean { }",
131+
padding + "override fun hashCode(): Int { }",
132+
padding + "override fun toString(): String { }",
133+
padding + "override fun run() { }",
134+
padding + "override fun clone(): Any { }",
135+
padding + "override fun start() { }",
136+
padding + "override fun interrupt() { }",
137+
padding + "override fun isInterrupted(): Boolean { }",
138+
padding + "override fun countStackFrames(): Int { }",
139+
padding + "override fun getContextClassLoader(): ClassLoader { }",
140+
padding + "override fun setContextClassLoader(cl: ClassLoader) { }",
141+
padding + "override fun getStackTrace(): (Array<(StackTraceElement..StackTraceElement?)>..Array<out (StackTraceElement..StackTraceElement?)>) { }",
142+
padding + "override fun getId(): Long { }",
143+
padding + "override fun getState(): State { }",
144+
padding + "override fun getUncaughtExceptionHandler(): UncaughtExceptionHandler { }",
145+
padding + "override fun setUncaughtExceptionHandler(eh: UncaughtExceptionHandler) { }"))
146+
147+
assertThat(ranges, everyItem(equalTo(range(39, 25, 39, 25))))
148+
}
104149
}

server/src/test/resources/overridemember/OverrideMembers.kt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,5 @@ open class MyOpen {
3535
}
3636

3737
class Closed: MyOpen() {}
38+
39+
class MyThread: Thread {}

0 commit comments

Comments
 (0)