From 90546e2380acc85a344f561d2878afa9f483e17e Mon Sep 17 00:00:00 2001 From: azerr Date: Wed, 27 Mar 2024 15:43:56 +0100 Subject: [PATCH] fix: Wrong generated content after an LSP completion apply without TextEdit Fixes #181 Signed-off-by: azerr --- .../redhat/devtools/lsp4ij/LSPIJUtils.java | 2 +- .../completion/LSPCompletionProposal.java | 20 +++- .../completion/ClojureCompletionTest.java | 99 +++++++++++++++++++ .../LSPCompletionFixtureTestCase.java | 3 + 4 files changed, 121 insertions(+), 3 deletions(-) create mode 100644 src/test/java/com/redhat/devtools/lsp4ij/features/completion/ClojureCompletionTest.java diff --git a/src/main/java/com/redhat/devtools/lsp4ij/LSPIJUtils.java b/src/main/java/com/redhat/devtools/lsp4ij/LSPIJUtils.java index fc4ccefe3..7e5cfbd02 100644 --- a/src/main/java/com/redhat/devtools/lsp4ij/LSPIJUtils.java +++ b/src/main/java/com/redhat/devtools/lsp4ij/LSPIJUtils.java @@ -283,7 +283,7 @@ public static int toOffset(@NotNull Position position, @NotNull Document documen line = 0; } else { // If a line number is greater than the number of lines in a document, it defaults back to the number of lines in the document. - line = Math.min(line, document.getLineCount() - 1); + line = Math.min(line, document.getLineCount() > 0 ? document.getLineCount() -1 : 0); } int lineStartOffset = document.getLineStartOffset(line); int lineEndOffset = document.getLineEndOffset(line); diff --git a/src/main/java/com/redhat/devtools/lsp4ij/features/completion/LSPCompletionProposal.java b/src/main/java/com/redhat/devtools/lsp4ij/features/completion/LSPCompletionProposal.java index 82cfa0e32..dd52373bf 100644 --- a/src/main/java/com/redhat/devtools/lsp4ij/features/completion/LSPCompletionProposal.java +++ b/src/main/java/com/redhat/devtools/lsp4ij/features/completion/LSPCompletionProposal.java @@ -261,9 +261,25 @@ protected void apply(Document document, char trigger, int stateMask, int offset) } try { if (textEdit == null) { + // ex: + // { + // "label": "let", + // "kind": 15, + // "detail": "Insert let", + // "insertText": "(let [${1:binding} ${2:value}])", + // "insertTextFormat": 2 + insertText = getInsertText(); - Position start = LSPIJUtils.toPosition(this.bestOffset, document); - Position end = LSPIJUtils.toPosition(offset, document); // need 2 distinct objects + int startOffset = this.bestOffset; + int endOffset = offset; + // Try to get the text range to replace it. + // foo.b|ar --> foo.[bar] + TextRange textRange = LSPIJUtils.getTokenRange(document, this.initialOffset); + if (textRange != null) { + startOffset = textRange.getStartOffset(); + } + Position start = LSPIJUtils.toPosition(startOffset, document); + Position end = LSPIJUtils.toPosition(endOffset, document); // need 2 distinct objects textEdit = new TextEdit(new Range(start, end), insertText); } else if (offset > this.initialOffset) { // characters were added after completion was activated diff --git a/src/test/java/com/redhat/devtools/lsp4ij/features/completion/ClojureCompletionTest.java b/src/test/java/com/redhat/devtools/lsp4ij/features/completion/ClojureCompletionTest.java new file mode 100644 index 000000000..09efb6147 --- /dev/null +++ b/src/test/java/com/redhat/devtools/lsp4ij/features/completion/ClojureCompletionTest.java @@ -0,0 +1,99 @@ +/******************************************************************************* + * Copyright (c) 2024 Red Hat, Inc. + * Distributed under license by Red Hat, Inc. All rights reserved. + * This program is made available under the terms of the + * Eclipse Public License v2.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v20.html + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + ******************************************************************************/ +package com.redhat.devtools.lsp4ij.features.completion; + +import com.redhat.devtools.lsp4ij.fixtures.LSPCompletionFixtureTestCase; + +/** + * Completion tests by emulating LSP 'textDocument/completion' responses + * from the clojure-lsp language server. + */ +public class ClojureCompletionTest extends LSPCompletionFixtureTestCase { + + public void testCompletionWithoutTextEditAndEmptyContent() { + // clojure-lsp returns completion items without text edit. + // 1. Test completion items result + assertCompletion("test.ts", + "", """ + [ + { + "label": "let", + "kind": 15, + "detail": "Insert let", + "insertText": "(let [${1:binding} ${2:value}])", + "insertTextFormat": 2 + }, + { + "label": "letfn", + "kind": 15, + "detail": "Insert letfn", + "insertText": "(letfn [(${1:name} [${2:args}]\\n $0)])", + "insertTextFormat": 2 + } + ]""" + , "let", "letfn"); + // 2. Test new editor content after applying the first completion item + assertApplyCompletionItem(0, "(let [binding value])"); + } + + public void testCompletionWithoutTextEdit() { + // clojure-lsp returns completion items without text edit. + // 1. Test completion items result + assertCompletion("test.ts", + "let", """ + [ + { + "label": "let", + "kind": 15, + "detail": "Insert let", + "insertText": "(let [${1:binding} ${2:value}])", + "insertTextFormat": 2 + }, + { + "label": "letfn", + "kind": 15, + "detail": "Insert letfn", + "insertText": "(letfn [(${1:name} [${2:args}]\\n $0)])", + "insertTextFormat": 2 + } + ]""" + , "let", "letfn"); + // 2. Test new editor content after applying the first completion item + assertApplyCompletionItem(0, "(let [binding value])"); + } + + public void testCompletionWithoutTextEditAndCaretInsideToken() { + // clojure-lsp returns completion items without text edit. + // 1. Test completion items result + assertCompletion("test.ts", + "let", """ + [ + { + "label": "let", + "kind": 15, + "detail": "Insert let", + "insertText": "(let [${1:binding} ${2:value}])", + "insertTextFormat": 2 + }, + { + "label": "letfn", + "kind": 15, + "detail": "Insert letfn", + "insertText": "(letfn [(${1:name} [${2:args}]\\n $0)])", + "insertTextFormat": 2 + } + ]""" + , "let", "letfn"); + // 2. Test new editor content after applying the first completion item + assertApplyCompletionItem(0, "(let [binding value])t"); + } + +} diff --git a/src/test/java/com/redhat/devtools/lsp4ij/fixtures/LSPCompletionFixtureTestCase.java b/src/test/java/com/redhat/devtools/lsp4ij/fixtures/LSPCompletionFixtureTestCase.java index 80bd1a75a..23d92520b 100644 --- a/src/test/java/com/redhat/devtools/lsp4ij/fixtures/LSPCompletionFixtureTestCase.java +++ b/src/test/java/com/redhat/devtools/lsp4ij/fixtures/LSPCompletionFixtureTestCase.java @@ -35,6 +35,9 @@ protected void assertCompletion(@NotNull String fileName, @NotNull String editorContentText, @NotNull String jsonCompletionList, @NotNull String... expectedItems) { + if (jsonCompletionList.trim().startsWith("[")) { + jsonCompletionList = "{\"items\":" + jsonCompletionList + "}"; + } assertCompletion(fileName, editorContentText, JSONUtils.getLsp4jGson().fromJson(jsonCompletionList, CompletionList.class), expectedItems); }