fix(amazonq): only send textDocument/didOpen for explicitly focused editors (#5872)

* fix(amazonq): only send `textDocument/didOpen` for explicitly focused editors

delay does not appear sufficent to satisfy LSP sensitivity to message floods on start (perhaps some weird stdio buffer overflow issue on windows)

instead, match observed behavior of VSC/Eclipse and only send editors that have seen focus at least once

* build

* shadow

Author: rli

plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/textdocument/TextDocumentServiceHandler.kt

@@ -16,14 +16,14 @@ import com.intellij.openapi.fileEditor.FileEditorManagerEvent
 import com.intellij.openapi.fileEditor.FileEditorManagerListener
 import com.intellij.openapi.fileEditor.TextEditor
 import com.intellij.openapi.project.Project
+import com.intellij.openapi.util.Disposer
 import com.intellij.openapi.util.Key
 import com.intellij.openapi.vfs.VirtualFile
 import com.intellij.openapi.vfs.VirtualFileManager
 import com.intellij.openapi.vfs.newvfs.BulkFileListener
 import com.intellij.openapi.vfs.newvfs.events.VFileContentChangeEvent
 import com.intellij.openapi.vfs.newvfs.events.VFileEvent
 import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.delay
 import kotlinx.coroutines.launch
 import org.eclipse.lsp4j.DidChangeTextDocumentParams
 import org.eclipse.lsp4j.DidCloseTextDocumentParams
@@ -46,6 +46,7 @@ class TextDocumentServiceHandler(
     BulkFileListener,
     DocumentListener,
     Disposable {
+
     init {
         // didOpen & didClose events
         project.messageBus.connect(this).subscribe(
@@ -68,9 +69,8 @@ class TextDocumentServiceHandler(
         // open files on startup
         cs.launch {
             val fileEditorManager = FileEditorManager.getInstance(project)
-            fileEditorManager.openFiles.forEach { file ->
+            fileEditorManager.selectedFiles.forEach { file ->
                 handleFileOpened(file)
-                delay(100)
             }
         }
     }
@@ -84,23 +84,33 @@ class TextDocumentServiceHandler(
             }
             ApplicationManager.getApplication().runReadAction {
                 FileDocumentManager.getInstance().getDocument(file)?.addDocumentListener(listener)
-                file.putUserData(KEY_REAL_TIME_EDIT_LISTENER, listener)
             }
-        }
+            file.putUserData(KEY_REAL_TIME_EDIT_LISTENER, listener)
+
+            Disposer.register(this) {
+                ApplicationManager.getApplication().runReadAction {
+                    val existingListener = file.getUserData(KEY_REAL_TIME_EDIT_LISTENER)
+                    if (existingListener != null) {
+                        FileDocumentManager.getInstance().getDocument(file)?.removeDocumentListener(existingListener)
+                        file.putUserData(KEY_REAL_TIME_EDIT_LISTENER, null)
+                    }
+                }
+            }
 
-        cs.launch {
-            AmazonQLspService.executeAsyncIfRunning(project) { languageServer ->
-                toUriString(file)?.let { uri ->
-                    languageServer.textDocumentService.didOpen(
-                        DidOpenTextDocumentParams().apply {
-                            textDocument = TextDocumentItem().apply {
-                                this.uri = uri
-                                text = file.inputStream.readAllBytes().decodeToString()
-                                languageId = file.fileType.name.lowercase()
-                                version = file.modificationStamp.toInt()
+            cs.launch {
+                AmazonQLspService.executeAsyncIfRunning(project) { languageServer ->
+                    toUriString(file)?.let { uri ->
+                        languageServer.textDocumentService.didOpen(
+                            DidOpenTextDocumentParams().apply {
+                                textDocument = TextDocumentItem().apply {
+                                    this.uri = uri
+                                    text = file.inputStream.readAllBytes().decodeToString()
+                                    languageId = file.fileType.name.lowercase()
+                                    version = file.modificationStamp.toInt()
+                                }
                             }
-                        }
-                    )
+                        )
+                    }
                 }
             }
         }
@@ -116,6 +126,7 @@ class TextDocumentServiceHandler(
                             textDocument = TextDocumentIdentifier().apply {
                                 this.uri = uri
                             }
+                            // TODO: should respect `textDocumentSync.save.includeText` server capability config
                             text = document.text
                         }
                     )
@@ -125,10 +136,12 @@ class TextDocumentServiceHandler(
     }
 
     override fun after(events: MutableList<out VFileEvent>) {
-        cs.launch {
-            AmazonQLspService.executeAsyncIfRunning(project) { languageServer ->
-                events.filterIsInstance<VFileContentChangeEvent>().forEach { event ->
-                    val document = FileDocumentManager.getInstance().getCachedDocument(event.file) ?: return@forEach
+        events.filterIsInstance<VFileContentChangeEvent>().forEach { event ->
+            val document = FileDocumentManager.getInstance().getCachedDocument(event.file) ?: return@forEach
+
+            handleFileOpened(event.file)
+            cs.launch {
+                AmazonQLspService.executeAsyncIfRunning(project) { languageServer ->
                     toUriString(event.file)?.let { uri ->
                         languageServer.textDocumentService.didChange(
                             DidChangeTextDocumentParams().apply {
@@ -164,17 +177,18 @@ class TextDocumentServiceHandler(
         if (listener != null) {
             FileDocumentManager.getInstance().getDocument(file)?.removeDocumentListener(listener)
             file.putUserData(KEY_REAL_TIME_EDIT_LISTENER, null)
-        }
-        cs.launch {
-            AmazonQLspService.executeAsyncIfRunning(project) { languageServer ->
-                toUriString(file)?.let { uri ->
-                    languageServer.textDocumentService.didClose(
-                        DidCloseTextDocumentParams().apply {
-                            textDocument = TextDocumentIdentifier().apply {
-                                this.uri = uri
+
+            cs.launch {
+                AmazonQLspService.executeAsyncIfRunning(project) { languageServer ->
+                    toUriString(file)?.let { uri ->
+                        languageServer.textDocumentService.didClose(
+                            DidCloseTextDocumentParams().apply {
+                                textDocument = TextDocumentIdentifier().apply {
+                                    this.uri = uri
+                                }
                             }
-                        }
-                    )
+                        )
+                    }
                 }
             }
         }
@@ -185,10 +199,12 @@ class TextDocumentServiceHandler(
     }
 
     private fun handleActiveEditorChange(fileEditor: FileEditor?) {
+        val editor = (fileEditor as? TextEditor)?.editor ?: return
+        editor.virtualFile?.let { handleFileOpened(it) }
+
         // Extract text editor if it's a TextEditor, otherwise null
-        val editor = (fileEditor as? TextEditor)?.editor
-        val textDocumentIdentifier = editor?.let { TextDocumentIdentifier(toUriString(it.virtualFile)) }
-        val cursorState = editor?.let { getCursorState(it) }
+        val textDocumentIdentifier = TextDocumentIdentifier(toUriString(editor.virtualFile))
+        val cursorState = getCursorState(editor)
 
         val params = mapOf(
             "textDocument" to textDocumentIdentifier,

plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/textdocument/TextDocumentServiceHandlerTest.kt

@@ -6,7 +6,9 @@ package software.aws.toolkits.jetbrains.services.amazonq.lsp.textdocument
 import com.intellij.openapi.application.writeAction
 import com.intellij.openapi.editor.Document
 import com.intellij.openapi.fileEditor.FileDocumentManager
+import com.intellij.openapi.fileEditor.ex.FileEditorManagerEx
 import com.intellij.openapi.fileTypes.FileType
+import com.intellij.openapi.util.Disposer
 import com.intellij.openapi.vfs.VirtualFile
 import com.intellij.openapi.vfs.newvfs.events.VFileContentChangeEvent
 import com.intellij.openapi.vfs.newvfs.events.VFileEvent
@@ -24,7 +26,6 @@ import io.mockk.mockkStatic
 import io.mockk.slot
 import io.mockk.spyk
 import io.mockk.verify
-import kotlinx.coroutines.test.TestScope
 import kotlinx.coroutines.test.advanceUntilIdle
 import kotlinx.coroutines.test.runTest
 import kotlinx.coroutines.withContext
@@ -35,9 +36,11 @@ import org.eclipse.lsp4j.DidOpenTextDocumentParams
 import org.eclipse.lsp4j.DidSaveTextDocumentParams
 import org.eclipse.lsp4j.jsonrpc.messages.ResponseMessage
 import org.eclipse.lsp4j.services.TextDocumentService
+import org.junit.After
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
+import org.junit.rules.TestName
 import software.aws.toolkits.jetbrains.core.coroutines.EDT
 import software.aws.toolkits.jetbrains.services.amazonq.lsp.AmazonQLanguageServer
 import software.aws.toolkits.jetbrains.services.amazonq.lsp.AmazonQLspService
@@ -54,9 +57,6 @@ class TextDocumentServiceHandlerTest {
     private lateinit var mockTextDocumentService: TextDocumentService
     private lateinit var sut: TextDocumentServiceHandler
 
-    // not ideal
-    private lateinit var testScope: TestScope
-
     @get:Rule
     val projectRule = object : CodeInsightTestFixtureRule() {
         override fun createTestFixture(): CodeInsightTestFixture {
@@ -74,6 +74,9 @@ class TextDocumentServiceHandlerTest {
     @get:Rule
     val disposableRule = DisposableRule()
 
+    @get:Rule
+    val testName = TestName()
+
     @Before
     fun setup() {
         mockTextDocumentService = mockk<TextDocumentService>()
@@ -99,13 +102,20 @@ class TextDocumentServiceHandlerTest {
         every { mockTextDocumentService.didSave(any()) } returns Unit
         every { mockTextDocumentService.didOpen(any()) } returns Unit
         every { mockTextDocumentService.didClose(any()) } returns Unit
+    }
 
-        testScope = TestScope()
-        sut = TextDocumentServiceHandler(projectRule.project, testScope)
+    @After
+    fun tearDown() {
+        try {
+            Disposer.dispose(sut)
+        } catch (_: Exception) {
+        }
     }
 
     @Test
-    fun `didSave runs on beforeDocumentSaving`() {
+    fun `didSave runs on beforeDocumentSaving`() = runTest {
+        sut = TextDocumentServiceHandler(projectRule.project, this)
+
         // Create test document and file
         val uri = URI.create("file:///test/path/file.txt")
         val document = mockk<Document> {
@@ -127,7 +137,7 @@ class TextDocumentServiceHandlerTest {
             sut.beforeDocumentSaving(document)
 
             // Verify the correct LSP method was called with matching parameters
-            testScope.advanceUntilIdle()
+            advanceUntilIdle()
             val paramsSlot = slot<DidSaveTextDocumentParams>()
             verify { mockTextDocumentService.didSave(capture(paramsSlot)) }
 
@@ -142,12 +152,12 @@ class TextDocumentServiceHandlerTest {
     fun `didOpen runs on service init`() = runTest {
         val content = "test content"
         val file = withContext(EDT) {
-            projectRule.fixture.createFile("name", content).also { projectRule.fixture.openFileInEditor(it) }
+            projectRule.fixture.createFile(testName.methodName, content).also { projectRule.fixture.openFileInEditor(it) }
         }
-
+        advanceUntilIdle()
         sut = TextDocumentServiceHandler(projectRule.project, this)
-
         advanceUntilIdle()
+
         val paramsSlot = mutableListOf<DidOpenTextDocumentParams>()
         verify { mockTextDocumentService.didOpen(capture(paramsSlot)) }
 
@@ -160,15 +170,14 @@ class TextDocumentServiceHandlerTest {
 
     @Test
     fun `didOpen runs on fileOpened`() = runTest {
+        sut = TextDocumentServiceHandler(projectRule.project, this)
+        advanceUntilIdle()
         val content = "test content"
         val file = withContext(EDT) {
-            projectRule.fixture.createFile("name", content).also { projectRule.fixture.openFileInEditor(it) }
+            projectRule.fixture.createFile(testName.methodName, content).also { projectRule.fixture.openFileInEditor(it) }
         }
-
-        sut.fileOpened(mockk(), file)
-
         advanceUntilIdle()
-        testScope.advanceUntilIdle()
+
         val paramsSlot = mutableListOf<DidOpenTextDocumentParams>()
         verify { mockTextDocumentService.didOpen(capture(paramsSlot)) }
 
@@ -181,23 +190,26 @@ class TextDocumentServiceHandlerTest {
 
     @Test
     fun `didClose runs on fileClosed`() = runTest {
-        val uri = URI.create("file:///test/path/file.txt")
-        val file = createMockVirtualFile(uri)
-
-        sut.fileClosed(mockk(), file)
+        sut = TextDocumentServiceHandler(projectRule.project, this)
+        val file = withContext(EDT) {
+            projectRule.fixture.createFile(testName.methodName, "").also {
+                projectRule.fixture.openFileInEditor(it)
+                FileEditorManagerEx.getInstanceEx(projectRule.project).closeAllFiles()
+            }
+        }
 
         advanceUntilIdle()
-        testScope.advanceUntilIdle()
         val paramsSlot = slot<DidCloseTextDocumentParams>()
         verify { mockTextDocumentService.didClose(capture(paramsSlot)) }
 
-        assertThat(paramsSlot.captured.textDocument.uri).isEqualTo(normalizeFileUri(uri.toString()))
+        assertThat(paramsSlot.captured.textDocument.uri).isEqualTo(file.toNioPath().toUri().toString())
     }
 
     @Test
     fun `didChange runs on content change events`() = runTest {
+        sut = TextDocumentServiceHandler(projectRule.project, this)
         val file = withContext(EDT) {
-            projectRule.fixture.createFile("name", "").also {
+            projectRule.fixture.createFile(testName.methodName, "").also {
                 projectRule.fixture.openFileInEditor(it)
 
                 writeAction {
@@ -208,7 +220,6 @@ class TextDocumentServiceHandlerTest {
 
         // Verify the correct LSP method was called with matching parameters
         advanceUntilIdle()
-        testScope.advanceUntilIdle()
         val paramsSlot = mutableListOf<DidChangeTextDocumentParams>()
         verify { mockTextDocumentService.didChange(capture(paramsSlot)) }
 
@@ -220,6 +231,7 @@ class TextDocumentServiceHandlerTest {
 
     @Test
     fun `didSave does not run when URI is empty`() = runTest {
+        sut = TextDocumentServiceHandler(projectRule.project, this)
         val document = mockk<Document>()
         val file = createMockVirtualFile(URI.create(""))
 
@@ -235,14 +247,15 @@ class TextDocumentServiceHandlerTest {
 
                 sut.beforeDocumentSaving(document)
 
-                testScope.advanceUntilIdle()
+                advanceUntilIdle()
                 verify(exactly = 0) { mockTextDocumentService.didSave(any()) }
             }
         }
     }
 
     @Test
     fun `didSave does not run when file is null`() = runTest {
+        sut = TextDocumentServiceHandler(projectRule.project, this)
         val document = mockk<Document>()
 
         val fileDocumentManager = mockk<FileDocumentManager> {
@@ -254,23 +267,25 @@ class TextDocumentServiceHandlerTest {
 
             sut.beforeDocumentSaving(document)
 
-            testScope.advanceUntilIdle()
+            advanceUntilIdle()
             verify(exactly = 0) { mockTextDocumentService.didSave(any()) }
         }
     }
 
     @Test
     fun `didChange ignores non-content change events`() = runTest {
+        sut = TextDocumentServiceHandler(projectRule.project, this)
         val nonContentEvent = mockk<VFileEvent>() // Some other type of VFileEvent
 
         sut.after(mutableListOf(nonContentEvent))
 
-        testScope.advanceUntilIdle()
+        advanceUntilIdle()
         verify(exactly = 0) { mockTextDocumentService.didChange(any()) }
     }
 
     @Test
     fun `didChange skips files without cached documents`() = runTest {
+        sut = TextDocumentServiceHandler(projectRule.project, this)
         val uri = URI.create("file:///test/path/file.txt")
         val path = mockk<Path> {
             every { toUri() } returns uri
@@ -291,7 +306,7 @@ class TextDocumentServiceHandlerTest {
 
             sut.after(mutableListOf(changeEvent))
 
-            testScope.advanceUntilIdle()
+            advanceUntilIdle()
             verify(exactly = 0) { mockTextDocumentService.didChange(any()) }
         }
     }