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..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 @@ -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) } } } @@ -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) { - 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, 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()) } } }