Merge pull request #399 from themkat/rename_decl_site

Rename variable from declaration site

Author: fwcd

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

@@ -6,6 +6,7 @@ import org.javacs.kt.compiler.CompilationKind
 import org.javacs.kt.position.changedRegion
 import org.javacs.kt.position.location
 import org.javacs.kt.position.position
+import org.javacs.kt.position.range
 import org.javacs.kt.util.findParent
 import org.javacs.kt.util.nullResult
 import org.javacs.kt.util.toPath
@@ -175,10 +176,15 @@ class CompiledFile(
     }
 
 
+    /**
+     * Find the declaration of the element at the cursor.
+     */
+    fun findDeclaration(cursor: Int): Pair<KtNamedDeclaration, Location>? = findDeclarationReference(cursor) ?: findDeclarationCursorSite(cursor)
+
     /**
      * Find the declaration of the element at the cursor. Only works if the element at the cursor is a reference.
      */
-    fun findDeclaration(cursor: Int): Pair<KtNamedDeclaration, Location>? {
+    private fun findDeclarationReference(cursor: Int): Pair<KtNamedDeclaration, Location>? {
         val (_, target) = referenceAtPoint(cursor) ?: return null
         val psi = target.findPsi()
 
@@ -193,6 +199,22 @@ class CompiledFile(
         }
     }
 
+    /**
+     * Find the declaration of the element at the cursor.
+     * Works even in cases where the element at the cursor might not be a reference, so works for declaration sites.
+     */
+    private fun findDeclarationCursorSite(cursor: Int): Pair<KtNamedDeclaration, Location>? {
+        // current symbol might be a declaration. This function is used as a fallback when
+        // findDeclaration fails
+        val declaration = elementAtPoint(cursor)?.findParent<KtNamedDeclaration>()
+
+        return declaration?.let {
+            Pair(it,
+                 Location(it.containingFile.name,
+                          range(content, it.nameIdentifier?.textRange ?: return null)))
+        }
+    }
+
     /**
      * Find the lexical-scope surrounding `cursor`.
      * This may be out-of-date if the user is typing quickly.

server/src/main/kotlin/org/javacs/kt/highlight/DocumentHighlight.kt

@@ -12,7 +12,6 @@ import org.jetbrains.kotlin.psi.KtNamedDeclaration
 
 fun documentHighlightsAt(file: CompiledFile, cursor: Int): List<DocumentHighlight> {
     val (declaration, declarationLocation) = file.findDeclaration(cursor)
-        ?: file.findDeclarationCursorSite(cursor)
         ?: return emptyList()
     val references = findReferencesToDeclarationInFile(declaration, file)
 
@@ -23,18 +22,4 @@ fun documentHighlightsAt(file: CompiledFile, cursor: Int): List<DocumentHighligh
     } + references.map { DocumentHighlight(it, DocumentHighlightKind.Text) }
 }
 
-private fun CompiledFile.findDeclarationCursorSite(cursor: Int): Pair<KtNamedDeclaration, Location>? {
-    // current symbol might be a declaration. This function is used as a fallback when
-    // findDeclaration fails
-    val declaration = elementAtPoint(cursor)?.findParent<KtNamedDeclaration>()
-
-    return declaration?.let {
-        // in this scenario we know that the declaration will be at the cursor site, so uri is not
-        // important
-        Pair(it,
-             Location("",
-                      range(content, it.nameIdentifier?.textRange ?: return null)))
-    }
-}
-
 private fun KtNamedDeclaration.isInFile(file: KtFile) = this.containingFile == file

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

@@ -43,3 +43,43 @@ class RenameDefinitionTest : SingleFileTestFixture("rename", "SomeOtherClass.kt"
         assertThat(changes[0].left.edits[0].range.end, equalTo(Position(2, 20)))
     }
 }
+
+class RenameDeclarationSiteTest : SingleFileTestFixture("rename", "DeclSite.kt") {
+
+    @Test
+    fun `should rename variable from usage site`() {
+        val usageFile = workspaceRoot.resolve("UsageSite.kt").toString()
+        val edits = languageServer.textDocumentService.rename(renameParams(usageFile, 4, 13, "newvarname")).get()!!
+        val changes = edits.documentChanges
+
+        assertThat(changes.size, equalTo(2))
+
+        val firstChange = changes[0].left
+        assertThat(firstChange.textDocument.uri, containsString("DeclSite.kt"))
+        assertThat(firstChange.edits[0].newText, equalTo("newvarname"))
+        assertThat(firstChange.edits[0].range, equalTo(range(3, 5, 3, 10)))
+
+        val secondChange = changes[1].left
+        assertThat(secondChange.textDocument.uri, containsString("UsageSite.kt"))
+        assertThat(secondChange.edits[0].newText, equalTo("newvarname"))
+        assertThat(secondChange.edits[0].range, equalTo(range(4, 13, 4, 18)))
+    }
+
+    @Test
+    fun `should rename variable from declaration site`() {
+        val edits = languageServer.textDocumentService.rename(renameParams(file, 3, 6, "newvarname")).get()!!
+        val changes = edits.documentChanges
+
+        assertThat(changes.size, equalTo(2))
+
+        val firstChange = changes[0].left
+        assertThat(firstChange.textDocument.uri, containsString("DeclSite.kt"))
+        assertThat(firstChange.edits[0].newText, equalTo("newvarname"))
+        assertThat(firstChange.edits[0].range, equalTo(range(3, 5, 3, 10)))
+
+        val secondChange = changes[1].left
+        assertThat(secondChange.textDocument.uri, containsString("UsageSite.kt"))
+        assertThat(secondChange.edits[0].newText, equalTo("newvarname"))
+        assertThat(secondChange.edits[0].range, equalTo(range(4, 13, 4, 18)))
+    }
+}

server/src/test/resources/rename/DeclSite.kt

@@ -0,0 +1,4 @@
+package declsite
+
+val myvar = 2
+

server/src/test/resources/rename/UsageSite.kt

@@ -0,0 +1,5 @@
+package declsite
+
+fun myfunc() {
+    println(myvar)
+}