refactor(amazonq): implement VirtualFile -> URI util for messaging params (#5462)

samgst-amazon · web-flow · commit c0f84b1737a9 · 2025-03-13T09:05:17.000-07:00
* integrate virtualFile -&gt; URI util function for consistent handling

* fix test

* fix windows tests

* remove breaking function

* more windows fix

* windows

* didRename fix

* didRename fix

* add one more util call to oldUri in didRename
diff --git a/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/textdocument/TextDocumentServiceHandler.kt b/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/textdocument/TextDocumentServiceHandler.kt
@@ -24,6 +24,7 @@ import org.eclipse.lsp4j.TextDocumentIdentifier
 import org.eclipse.lsp4j.TextDocumentItem
 import org.eclipse.lsp4j.VersionedTextDocumentIdentifier
 import software.aws.toolkits.jetbrains.services.amazonq.lsp.AmazonQLspService
+import software.aws.toolkits.jetbrains.services.amazonq.lsp.util.FileUriUtil.toUriString
 import software.aws.toolkits.jetbrains.utils.pluginAwareExecuteOnPooledThread
 
 class TextDocumentServiceHandler(
@@ -56,7 +57,7 @@ class TextDocumentServiceHandler(
     override fun beforeDocumentSaving(document: Document) {
         AmazonQLspService.executeIfRunning(project) { languageServer ->
             val file = FileDocumentManager.getInstance().getFile(document) ?: return@executeIfRunning
-            file.toNioPath().toUri().toString().takeIf { it.isNotEmpty() }?.let { uri ->
+            toUriString(file)?.let { uri ->
                 languageServer.textDocumentService.didSave(
                     DidSaveTextDocumentParams().apply {
                         textDocument = TextDocumentIdentifier().apply {
@@ -74,7 +75,7 @@ class TextDocumentServiceHandler(
             pluginAwareExecuteOnPooledThread {
                 events.filterIsInstance<VFileContentChangeEvent>().forEach { event ->
                     val document = FileDocumentManager.getInstance().getCachedDocument(event.file) ?: return@forEach
-                    event.file.toNioPath().toUri().toString().takeIf { it.isNotEmpty() }?.let { uri ->
+                    toUriString(event.file)?.let { uri ->
                         languageServer.textDocumentService.didChange(
                             DidChangeTextDocumentParams().apply {
                                 textDocument = VersionedTextDocumentIdentifier().apply {
@@ -99,7 +100,7 @@ class TextDocumentServiceHandler(
         file: VirtualFile,
     ) {
         AmazonQLspService.executeIfRunning(project) { languageServer ->
-            file.toNioPath().toUri().toString().takeIf { it.isNotEmpty() }?.let { uri ->
+            toUriString(file)?.let { uri ->
                 languageServer.textDocumentService.didOpen(
                     DidOpenTextDocumentParams().apply {
                         textDocument = TextDocumentItem().apply {
@@ -117,7 +118,7 @@ class TextDocumentServiceHandler(
         file: VirtualFile,
     ) {
         AmazonQLspService.executeIfRunning(project) { languageServer ->
-            file.toNioPath().toUri().toString().takeIf { it.isNotEmpty() }?.let { uri ->
+            toUriString(file)?.let { uri ->
                 languageServer.textDocumentService.didClose(
                     DidCloseTextDocumentParams().apply {
                         textDocument = TextDocumentIdentifier().apply {
diff --git a/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/util/WorkspaceFolderUtil.kt b/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/util/WorkspaceFolderUtil.kt
@@ -6,6 +6,7 @@ package software.aws.toolkits.jetbrains.services.amazonq.lsp.util
 import com.intellij.openapi.project.Project
 import com.intellij.openapi.roots.ProjectRootManager
 import org.eclipse.lsp4j.WorkspaceFolder
+import software.aws.toolkits.jetbrains.services.amazonq.lsp.util.FileUriUtil.toUriString
 
 object WorkspaceFolderUtil {
     fun createWorkspaceFolders(project: Project): List<WorkspaceFolder> =
@@ -15,7 +16,7 @@ object WorkspaceFolderUtil {
             ProjectRootManager.getInstance(project).contentRoots.map { contentRoot ->
                 WorkspaceFolder().apply {
                     name = contentRoot.name
-                    this.uri = contentRoot.url
+                    this.uri = toUriString(contentRoot)
                 }
             }
         }
diff --git a/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/workspace/WorkspaceServiceHandler.kt b/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/workspace/WorkspaceServiceHandler.kt
@@ -27,6 +27,7 @@ import org.eclipse.lsp4j.RenameFilesParams
 import org.eclipse.lsp4j.WorkspaceFolder
 import org.eclipse.lsp4j.WorkspaceFoldersChangeEvent
 import software.aws.toolkits.jetbrains.services.amazonq.lsp.AmazonQLspService
+import software.aws.toolkits.jetbrains.services.amazonq.lsp.util.FileUriUtil.toUriString
 import software.aws.toolkits.jetbrains.services.amazonq.lsp.util.WorkspaceFolderUtil.createWorkspaceFolders
 import software.aws.toolkits.jetbrains.utils.pluginAwareExecuteOnPooledThread
 import java.nio.file.FileSystems
@@ -59,7 +60,7 @@ class WorkspaceServiceHandler(
         AmazonQLspService.executeIfRunning(project) { languageServer ->
             val validFiles = events.mapNotNull { event ->
                 val file = event.file?.takeIf { shouldHandleFile(it) } ?: return@mapNotNull null
-                file.toNioPath().toUri().toString().takeIf { it.isNotEmpty() }?.let { uri ->
+                toUriString(file)?.let { uri ->
                     FileCreate().apply {
                         this.uri = uri
                     }
@@ -80,7 +81,7 @@ class WorkspaceServiceHandler(
         AmazonQLspService.executeIfRunning(project) { languageServer ->
             val validFiles = events.mapNotNull { event ->
                 val file = event.file?.takeIf { shouldHandleFile(it) } ?: return@mapNotNull null
-                file.toNioPath().toUri().toString().takeIf { it.isNotEmpty() }?.let { uri ->
+                toUriString(file)?.let { uri ->
                     FileDelete().apply {
                         this.uri = uri
                     }
@@ -103,13 +104,12 @@ class WorkspaceServiceHandler(
                 .filter { it.propertyName == VirtualFile.PROP_NAME }
                 .mapNotNull { event ->
                     val file = event.file.takeIf { shouldHandleFile(it) } ?: return@mapNotNull null
-                    val oldName = event.oldValue as? String ?: return@mapNotNull null
                     if (event.newValue !is String) return@mapNotNull null
 
                     // Construct old and new URIs
-                    val parentPath = file.parent?.toNioPath() ?: return@mapNotNull null
-                    val oldUri = parentPath.resolve(oldName).toUri().toString()
-                    val newUri = file.toNioPath().toUri().toString()
+                    val parentFile = file.parent ?: return@mapNotNull null
+                    val oldUri = toUriString(parentFile)
+                    val newUri = toUriString(file)
 
                     FileRename().apply {
                         this.oldUri = oldUri
@@ -130,7 +130,7 @@ class WorkspaceServiceHandler(
     private fun didChangeWatchedFiles(events: List<VFileEvent>) {
         AmazonQLspService.executeIfRunning(project) { languageServer ->
             val validChanges = events.mapNotNull { event ->
-                event.file?.toNioPath()?.toUri()?.toString()?.takeIf { it.isNotEmpty() }?.let { uri ->
+                event.file?.let { toUriString(it) }?.let { uri ->
                     FileEvent().apply {
                         this.uri = uri
                         type = when (event) {
diff --git a/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/textdocument/TextDocumentServiceHandlerTest.kt b/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/textdocument/TextDocumentServiceHandlerTest.kt
@@ -18,6 +18,7 @@ import com.intellij.util.messages.MessageBusConnection
 import io.mockk.every
 import io.mockk.just
 import io.mockk.mockk
+import io.mockk.mockkObject
 import io.mockk.mockkStatic
 import io.mockk.runs
 import io.mockk.slot
@@ -34,6 +35,7 @@ import org.junit.Before
 import org.junit.Test
 import software.aws.toolkits.jetbrains.services.amazonq.lsp.AmazonQLanguageServer
 import software.aws.toolkits.jetbrains.services.amazonq.lsp.AmazonQLspService
+import software.aws.toolkits.jetbrains.services.amazonq.lsp.util.FileUriUtil
 import java.net.URI
 import java.nio.file.Path
 import java.util.concurrent.Callable
@@ -99,14 +101,7 @@ class TextDocumentServiceHandlerTest {
             every { text } returns "test content"
         }
 
-        val path = mockk<Path> {
-            every { toUri() } returns uri
-        }
-
-        val file = mockk<VirtualFile> {
-            every { this@mockk.path } returns uri.path
-            every { toNioPath() } returns path
-        }
+        val file = createMockVirtualFile(uri)
 
         // Mock FileDocumentManager
         val fileDocumentManager = mockk<FileDocumentManager> {
@@ -125,7 +120,7 @@ class TextDocumentServiceHandlerTest {
             verify { mockTextDocumentService.didSave(capture(paramsSlot)) }
 
             with(paramsSlot.captured) {
-                assertEquals(uri.toString(), textDocument.uri)
+                assertEquals(normalizeFileUri(uri.toString()), textDocument.uri)
                 assertEquals("test content", text)
             }
         }
@@ -136,17 +131,8 @@ class TextDocumentServiceHandlerTest {
         // Create test file
         val uri = URI.create("file:///test/path/file.txt")
         val content = "test content"
-        val inputStream = content.byteInputStream()
 
-        val path = mockk<Path> {
-            every { toUri() } returns uri
-        }
-
-        val file = mockk<VirtualFile> {
-            every { this@mockk.path } returns uri.path
-            every { toNioPath() } returns path
-            every { this@mockk.inputStream } returns inputStream
-        }
+        val file = createMockVirtualFile(uri, content)
 
         // Call the handler method
         sut.fileOpened(mockk(), file)
@@ -156,28 +142,22 @@ class TextDocumentServiceHandlerTest {
         verify { mockTextDocumentService.didOpen(capture(paramsSlot)) }
 
         with(paramsSlot.captured.textDocument) {
-            assertEquals(uri.toString(), this.uri)
+            assertEquals(normalizeFileUri(uri.toString()), this.uri)
             assertEquals(content, text)
         }
     }
 
     @Test
     fun `didClose runs on fileClosed`() = runTest {
         val uri = URI.create("file:///test/path/file.txt")
-        val path = mockk<Path> {
-            every { toUri() } returns uri
-        }
-        val file = mockk<VirtualFile> {
-            every { this@mockk.path } returns uri.path
-            every { toNioPath() } returns path
-        }
+        val file = createMockVirtualFile(uri)
 
         sut.fileClosed(mockk(), file)
 
         val paramsSlot = slot<DidCloseTextDocumentParams>()
         verify { mockTextDocumentService.didClose(capture(paramsSlot)) }
 
-        assertEquals(uri.toString(), paramsSlot.captured.textDocument.uri)
+        assertEquals(normalizeFileUri(uri.toString()), paramsSlot.captured.textDocument.uri)
     }
 
     @Test
@@ -188,14 +168,7 @@ class TextDocumentServiceHandlerTest {
             every { modificationStamp } returns 123L
         }
 
-        val path = mockk<Path> {
-            every { toUri() } returns uri
-        }
-
-        val file = mockk<VirtualFile> {
-            every { this@mockk.path } returns uri.path
-            every { toNioPath() } returns path
-        }
+        val file = createMockVirtualFile(uri)
 
         val changeEvent = mockk<VFileContentChangeEvent> {
             every { this@mockk.file } returns file
@@ -218,7 +191,7 @@ class TextDocumentServiceHandlerTest {
         verify { mockTextDocumentService.didChange(capture(paramsSlot)) }
 
         with(paramsSlot.captured) {
-            assertEquals(uri.toString(), textDocument.uri)
+            assertEquals(normalizeFileUri(uri.toString()), textDocument.uri)
             assertEquals(123, textDocument.version)
             assertEquals("changed content", contentChanges[0].text)
         }
@@ -227,23 +200,22 @@ class TextDocumentServiceHandlerTest {
     @Test
     fun `didSave does not run when URI is empty`() = runTest {
         val document = mockk<Document>()
-        val path = mockk<Path> {
-            every { toUri() } returns URI.create("")
-        }
-        val file = mockk<VirtualFile> {
-            every { toNioPath() } returns path
-        }
+        val file = createMockVirtualFile(URI.create(""))
 
-        val fileDocumentManager = mockk<FileDocumentManager> {
-            every { getFile(document) } returns file
-        }
+        mockkObject(FileUriUtil) {
+            every { FileUriUtil.toUriString(file) } returns null
 
-        mockkStatic(FileDocumentManager::class) {
-            every { FileDocumentManager.getInstance() } returns fileDocumentManager
+            val fileDocumentManager = mockk<FileDocumentManager> {
+                every { getFile(document) } returns file
+            }
 
-            sut.beforeDocumentSaving(document)
+            mockkStatic(FileDocumentManager::class) {
+                every { FileDocumentManager.getInstance() } returns fileDocumentManager
 
-            verify(exactly = 0) { mockTextDocumentService.didSave(any()) }
+                sut.beforeDocumentSaving(document)
+
+                verify(exactly = 0) { mockTextDocumentService.didSave(any()) }
+            }
         }
     }
 
@@ -298,4 +270,33 @@ class TextDocumentServiceHandlerTest {
             verify(exactly = 0) { mockTextDocumentService.didChange(any()) }
         }
     }
+
+    private fun createMockVirtualFile(uri: URI, content: String = ""): VirtualFile {
+        val path = mockk<Path> {
+            every { toUri() } returns uri
+        }
+        val inputStream = content.byteInputStream()
+        return mockk<VirtualFile> {
+            every { url } returns uri.path
+            every { toNioPath() } returns path
+            every { isDirectory } returns false
+            every { fileSystem } returns mockk {
+                every { protocol } returns "file"
+            }
+            every { this@mockk.inputStream } returns inputStream
+        }
+    }
+
+    private fun normalizeFileUri(uri: String): String {
+        if (!System.getProperty("os.name").lowercase().contains("windows")) {
+            return uri
+        }
+
+        if (!uri.startsWith("file:///")) {
+            return uri
+        }
+
+        val path = uri.substringAfter("file:///")
+        return "file:///C:/$path"
+    }
 }
diff --git a/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/util/WorkspaceFolderUtilTest.kt b/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/util/WorkspaceFolderUtilTest.kt
@@ -10,6 +10,8 @@ import io.mockk.every
 import io.mockk.mockk
 import org.junit.jupiter.api.Assertions.assertEquals
 import org.junit.jupiter.api.Test
+import java.net.URI
+import java.nio.file.Path
 
 class WorkspaceFolderUtilTest {
 
@@ -27,29 +29,30 @@ class WorkspaceFolderUtilTest {
     fun `createWorkspaceFolders returns workspace folders for non-default project`() {
         val mockProject = mockk<Project>()
         val mockProjectRootManager = mockk<ProjectRootManager>()
-        val mockContentRoot1 = mockk<VirtualFile>()
-        val mockContentRoot2 = mockk<VirtualFile>()
+        val mockContentRoot1 = createMockVirtualFile(
+            URI("file:///path/to/root1"),
+            name = "root1"
+        )
+        val mockContentRoot2 = createMockVirtualFile(
+            URI("file:///path/to/root2"),
+            name = "root2"
+        )
 
         every { mockProject.isDefault } returns false
         every { ProjectRootManager.getInstance(mockProject) } returns mockProjectRootManager
         every { mockProjectRootManager.contentRoots } returns arrayOf(mockContentRoot1, mockContentRoot2)
 
-        every { mockContentRoot1.name } returns "root1"
-        every { mockContentRoot1.url } returns "file:///path/to/root1"
-        every { mockContentRoot2.name } returns "root2"
-        every { mockContentRoot2.url } returns "file:///path/to/root2"
-
         val result = WorkspaceFolderUtil.createWorkspaceFolders(mockProject)
 
         assertEquals(2, result.size)
-        assertEquals("file:///path/to/root1", result[0].uri)
-        assertEquals("file:///path/to/root2", result[1].uri)
+        assertEquals(normalizeFileUri("file:///path/to/root1"), result[0].uri)
+        assertEquals(normalizeFileUri("file:///path/to/root2"), result[1].uri)
         assertEquals("root1", result[0].name)
         assertEquals("root2", result[1].name)
     }
 
     @Test
-    fun `reateWorkspaceFolders returns empty list when project has no content roots`() {
+    fun `createWorkspaceFolders returns empty list when project has no content roots`() {
         val mockProject = mockk<Project>()
         val mockProjectRootManager = mockk<ProjectRootManager>()
 
@@ -61,4 +64,33 @@ class WorkspaceFolderUtilTest {
 
         assertEquals(emptyList<VirtualFile>(), result)
     }
+
+    private fun createMockVirtualFile(uri: URI, name: String): VirtualFile {
+        val path = mockk<Path> {
+            every { toUri() } returns uri
+        }
+        return mockk<VirtualFile> {
+            every { url } returns uri.toString()
+            every { getName() } returns name
+            every { toNioPath() } returns path
+            every { isDirectory } returns false
+            every { fileSystem } returns mockk {
+                every { protocol } returns "file"
+            }
+        }
+    }
+
+    // for windows unit tests
+    private fun normalizeFileUri(uri: String): String {
+        if (!System.getProperty("os.name").lowercase().contains("windows")) {
+            return uri
+        }
+
+        if (!uri.startsWith("file:///")) {
+            return uri
+        }
+
+        val path = uri.substringAfter("file:///")
+        return "file:///C:/$path"
+    }
 }
diff --git a/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/workspace/WorkspaceServiceHandlerTest.kt b/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/workspace/WorkspaceServiceHandlerTest.kt

Original file line number	Diff line number	Diff line change
`@@ -6,6 +6,7 @@ package software.aws.toolkits.jetbrains.services.amazonq.lsp.util`
`6`	`6`	`import com.intellij.openapi.project.Project`
`7`	`7`	`import com.intellij.openapi.roots.ProjectRootManager`
`8`	`8`	`import org.eclipse.lsp4j.WorkspaceFolder`
	`9`	`+import software.aws.toolkits.jetbrains.services.amazonq.lsp.util.FileUriUtil.toUriString`
`9`	`10`
`10`	`11`	`object WorkspaceFolderUtil {`
`11`	`12`	`fun createWorkspaceFolders(project: Project): List<WorkspaceFolder> =`
`@@ -15,7 +16,7 @@ object WorkspaceFolderUtil {`
`15`	`16`	`ProjectRootManager.getInstance(project).contentRoots.map { contentRoot ->`
`16`	`17`	`WorkspaceFolder().apply {`
`17`	`18`	`name = contentRoot.name`
`18`		`- this.uri = contentRoot.url`
	`19`	`+ this.uri = toUriString(contentRoot)`
`19`	`20`	`}`
`20`	`21`	`}`
`21`	`22`	`}`