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

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.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 @@
     BulkFileListener,
     DocumentListener,
     Disposable {
+
     init {
         // didOpen & didClose events
         project.messageBus.connect(this).subscribe(
@@ -68,9 +69,8 @@
         // 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 @@
             }
             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 @@
                             textDocument = TextDocumentIdentifier().apply {
                                 this.uri = uri
                             }
+                            // TODO: should respect `textDocumentSync.save.includeText` server capability config
                             text = document.text
                         }
                     )
@@ -125,10 +136,12 @@
     }
 
     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 @@
         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 @@
     }
 
     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()) }
         }
     }