From 261e9f204f5a38fb53a8ed0df1225628f6a62d1d Mon Sep 17 00:00:00 2001 From: Richard Li Date: Tue, 1 Jul 2025 21:08:54 -0700 Subject: [PATCH 1/3] 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 --- .../TextDocumentServiceHandler.kt | 82 +++++++++++-------- 1 file changed, 49 insertions(+), 33 deletions(-) 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 index 3f0d3854cba..464876bb419 100644 --- 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 @@ -16,6 +16,7 @@ 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 @@ -23,7 +24,6 @@ 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) } } } @@ -86,21 +86,31 @@ class TextDocumentServiceHandler( FileDocumentManager.getInstance().getDocument(file)?.addDocumentListener(listener) file.putUserData(KEY_REAL_TIME_EDIT_LISTENER, listener) } - } - 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() + Disposer.register(this) { + ApplicationManager.getApplication().runReadAction { + val listener = file.getUserData(KEY_REAL_TIME_EDIT_LISTENER) + 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.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) { - cs.launch { - AmazonQLspService.executeAsyncIfRunning(project) { languageServer -> - events.filterIsInstance().forEach { event -> - val document = FileDocumentManager.getInstance().getCachedDocument(event.file) ?: return@forEach + events.filterIsInstance().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, From a73e93368988feb57d1cf8754bc42aa9e08603c7 Mon Sep 17 00:00:00 2001 From: Richard Li Date: Wed, 2 Jul 2025 15:53:20 -0700 Subject: [PATCH 2/3] build --- .../TextDocumentServiceHandler.kt | 2 +- .../TextDocumentServiceHandlerTest.kt | 71 +++++++++++-------- 2 files changed, 44 insertions(+), 29 deletions(-) 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 index 464876bb419..bf747a93bb1 100644 --- 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 @@ -84,8 +84,8 @@ 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 { 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 index 616cf72812a..a351db5937c 100644 --- 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 @@ -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() @@ -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 { @@ -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() 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() 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() 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() 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() 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() val file = createMockVirtualFile(URI.create("")) @@ -235,7 +247,7 @@ class TextDocumentServiceHandlerTest { sut.beforeDocumentSaving(document) - testScope.advanceUntilIdle() + advanceUntilIdle() verify(exactly = 0) { mockTextDocumentService.didSave(any()) } } } @@ -243,6 +255,7 @@ class TextDocumentServiceHandlerTest { @Test fun `didSave does not run when file is null`() = runTest { + sut = TextDocumentServiceHandler(projectRule.project, this) val document = mockk() val fileDocumentManager = mockk { @@ -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() // 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 { every { toUri() } returns uri @@ -291,7 +306,7 @@ class TextDocumentServiceHandlerTest { sut.after(mutableListOf(changeEvent)) - testScope.advanceUntilIdle() + advanceUntilIdle() verify(exactly = 0) { mockTextDocumentService.didChange(any()) } } } From 19341912ee5a36a802a6b949e71236bac1e52283 Mon Sep 17 00:00:00 2001 From: Richard Li Date: Wed, 2 Jul 2025 16:49:55 -0700 Subject: [PATCH 3/3] shadow --- .../amazonq/lsp/textdocument/TextDocumentServiceHandler.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 index bf747a93bb1..10e597915bb 100644 --- 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 @@ -89,9 +89,9 @@ class TextDocumentServiceHandler( Disposer.register(this) { ApplicationManager.getApplication().runReadAction { - val listener = file.getUserData(KEY_REAL_TIME_EDIT_LISTENER) - if (listener != null) { - FileDocumentManager.getInstance().getDocument(file)?.removeDocumentListener(listener) + 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) } }