From cff82d9d5906f0aa90e7c598c74ae34358e8a93f Mon Sep 17 00:00:00 2001 From: MEHER SRUJANA MATCHA Date: Fri, 17 Jul 2026 17:41:09 +0100 Subject: [PATCH] Fix #2687: correct postfix completion text when selected by mouse click Postfix templates (sysout, syserr, etc.) computed their real text and replacement range lazily, only during completionItem/resolve. Clients that skip resolve on mouse-click (e.g. VS Code) inserted the unresolved placeholder text instead. Postfix completion now always computes the final text eagerly and anchors its edit the same way other completion items do, so behavior no longer depends on resolve being called. Co-Authored-By: Claude Sonnet 5 --- .../template/java/PostfixTemplateEngine.java | 37 ++++++-------- .../handlers/CompletionResolveHandler.java | 13 ----- .../handlers/PostfixCompletionTest.java | 50 +++++++++---------- 3 files changed, 39 insertions(+), 61 deletions(-) diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corext/template/java/PostfixTemplateEngine.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corext/template/java/PostfixTemplateEngine.java index c0f572ffe2..a8c331ea21 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corext/template/java/PostfixTemplateEngine.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corext/template/java/PostfixTemplateEngine.java @@ -46,6 +46,7 @@ import org.eclipse.lsp4j.CompletionItemLabelDetails; import org.eclipse.lsp4j.Range; import org.eclipse.lsp4j.TextEdit; +import org.eclipse.lsp4j.jsonrpc.messages.Either; public class PostfixTemplateEngine { private static String Switch_Name = "switch"; //$NON-NLS-1$ @@ -105,19 +106,21 @@ public List complete(IDocument document, int offset, ICompilatio CompletionUtils.setInsertTextFormat(item, completionItemDefaults); CompletionUtils.setInsertTextMode(item, completionItemDefaults); - String content = ""; - if (isCompletionLazyResolveTextEditEnabled()) { - content = SnippetUtils.templateToSnippet(template.getPattern()); - } else { - context.setActiveTemplateName(template.getName()); - content = evaluateGenericTemplate(context, template); + context.setActiveTemplateName(template.getName()); + String content = evaluateGenericTemplate(context, template); + if (content == null) { + continue; } - setTextEdit(item, content, completionItemDefaults); - - if (!getClientPreferences().isResolveAdditionalTextEditsSupport()) { - setAdditionalTextEdit(item, compilationUnit, context, range, template); + try { + Range insertionPoint = JDTUtils.toRange(compilationUnit, completionCtx.getOffset(), 0); + item.setTextEdit(Either.forLeft(new TextEdit(insertionPoint, content))); + } catch (JavaModelException e) { + JavaLanguageServerPlugin.logException(e.getMessage(), e); + continue; } + setAdditionalTextEdit(item, compilationUnit, context, range, template); + if (isCompletionItemLabelDetailsSupport()) { CompletionItemLabelDetails itemLabelDetails = new CompletionItemLabelDetails(); itemLabelDetails.setDescription(template.getDescription()); @@ -151,18 +154,10 @@ public List complete(IDocument document, int offset, ICompilatio return res; } - private void setTextEdit(final CompletionItem item, String content, CompletionItemDefaults completionItemDefaults) { - if (getClientPreferences().isCompletionListItemDefaultsSupport() && completionItemDefaults.getEditRange() != null) { - item.setTextEditText(content); - } else { - item.setInsertText(content); - } - } - public static void setAdditionalTextEdit(final CompletionItem item, ICompilationUnit compilationUnit, JavaPostfixContext context, Range range, Template template) { List additionalEdits = new ArrayList<>(); - // use additional test edit to remove the code that needs to be replaced + // use additional text edit to remove the code that needs to be replaced additionalEdits.add(new TextEdit(range, "")); List jdtTextEdits = context.getAdditionalTextEdits(template.getName()); if (jdtTextEdits != null && !jdtTextEdits.isEmpty()) { @@ -247,8 +242,4 @@ public static String evaluateGenericTemplate(JavaPostfixContext postfixContext, private ClientPreferences getClientPreferences() { return JavaLanguageServerPlugin.getPreferencesManager().getClientPreferences(); } - - private boolean isCompletionLazyResolveTextEditEnabled() { - return JavaLanguageServerPlugin.getPreferencesManager().getPreferences().isCompletionLazyResolveTextEditEnabled(); - } } diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CompletionResolveHandler.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CompletionResolveHandler.java index 41e12909d6..11e7db3496 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CompletionResolveHandler.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CompletionResolveHandler.java @@ -58,9 +58,6 @@ import org.eclipse.lsp4j.CompletionItemKind; import org.eclipse.lsp4j.MarkupContent; import org.eclipse.lsp4j.MarkupKind; -import org.eclipse.lsp4j.Range; -import org.eclipse.lsp4j.TextEdit; -import org.eclipse.lsp4j.jsonrpc.messages.Either; import org.eclipse.osgi.util.NLS; import com.google.common.util.concurrent.SimpleTimeLimiter; @@ -134,12 +131,6 @@ public CompletionItem resolve(CompletionItem param, IProgressMonitor monitor) { Template template = ((PostfixCompletionProposal) proposal).getTemplate(); postfixContext.setActiveTemplateName(template.getName()); String content = PostfixTemplateEngine.evaluateGenericTemplate(postfixContext, template); - int length = postfixContext.getEnd() - postfixContext.getStart(); - Range range = JDTUtils.toRange(unit, postfixContext.getStart(), length); - if (manager.getPreferences().isCompletionLazyResolveTextEditEnabled()) { - TextEdit textEdit = new TextEdit(range, content); - param.setTextEdit(Either.forLeft(textEdit)); - } if (manager.getClientPreferences().isCompletionResolveDocumentSupport()) { param.setDocumentation(SnippetUtils.beautifyDocument(content)); @@ -148,10 +139,6 @@ public CompletionItem resolve(CompletionItem param, IProgressMonitor monitor) { if (manager.getClientPreferences().isCompletionResolveDocumentSupport()) { param.setDetail(template.getDescription()); } - - if (manager.getClientPreferences().isResolveAdditionalTextEditsSupport()) { - PostfixTemplateEngine.setAdditionalTextEdit(param, unit, postfixContext, range, template); - } } param.setData(null); diff --git a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/PostfixCompletionTest.java b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/PostfixCompletionTest.java index 9204349b68..4c5ab4df69 100644 --- a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/PostfixCompletionTest.java +++ b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/PostfixCompletionTest.java @@ -98,7 +98,7 @@ public void testCastLazyResolve() throws JavaModelException { List items = new ArrayList<>(list.getItems()); CompletionItem item = items.get(0); assertEquals("cast", item.getLabel()); - assertEquals(item.getInsertText(), "((${1})${inner_expression})${0}"); + assertEquals("((${1})a)${0}", item.getTextEdit().getLeft().getNewText()); assertEquals(item.getInsertTextFormat(), InsertTextFormat.Snippet); Range range = item.getAdditionalTextEdits().get(0).getRange(); assertEquals(new Range(new Position(3, 2), new Position(3, 8)), range); @@ -127,7 +127,7 @@ public void test_assert() throws JavaModelException { List items = new ArrayList<>(list.getItems()); CompletionItem item = items.get(0); assertEquals("assert", item.getLabel()); - assertEquals(item.getInsertText(), "assert identifier;"); + assertEquals("assert identifier;", item.getTextEdit().getLeft().getNewText()); assertEquals(item.getInsertTextFormat(), InsertTextFormat.Snippet); Range range = item.getAdditionalTextEdits().get(0).getRange(); assertEquals(new Range(new Position(3, 2), new Position(3, 19)), range); @@ -157,7 +157,7 @@ public void test_cast() throws JavaModelException { assertEquals("cast", item.getLabel()); assertNull(item.getLabelDetails().getDetail()); assertEquals("Casts the expression to a new type", item.getLabelDetails().getDescription()); - assertEquals(item.getInsertText(), "((${1})a)${0}"); + assertEquals("((${1})a)${0}", item.getTextEdit().getLeft().getNewText()); assertEquals(item.getInsertTextFormat(), InsertTextFormat.Snippet); assertEquals(item.getInsertTextMode(), InsertTextMode.AdjustIndentation); Range range = item.getAdditionalTextEdits().get(0).getRange(); @@ -185,7 +185,7 @@ public void test_if() throws JavaModelException { List items = new ArrayList<>(list.getItems()); CompletionItem item = items.get(0); assertEquals("if", item.getLabel()); - assertEquals(item.getInsertText(), "if (a) {\n\t${0}\n}"); + assertEquals("if (a) {\n\t${0}\n}", item.getTextEdit().getLeft().getNewText()); assertEquals(item.getInsertTextFormat(), InsertTextFormat.Snippet); assertNull(item.getInsertTextMode()); Range range = item.getAdditionalTextEdits().get(0).getRange(); @@ -212,7 +212,7 @@ public void test_else() throws JavaModelException { List items = new ArrayList<>(list.getItems()); CompletionItem item = items.get(0); assertEquals("else", item.getLabel()); - assertEquals(item.getInsertText(), "if (!a) {\n\t${0}\n}"); + assertEquals("if (!a) {\n\t${0}\n}", item.getTextEdit().getLeft().getNewText()); assertEquals(item.getInsertTextFormat(), InsertTextFormat.Snippet); Range range = item.getAdditionalTextEdits().get(0).getRange(); assertEquals(new Range(new Position(3, 2), new Position(3, 8)), range); @@ -238,7 +238,7 @@ public void test_for() throws JavaModelException { List items = new ArrayList<>(list.getItems()); CompletionItem item = items.get(0); assertEquals("for", item.getLabel()); - assertEquals(item.getInsertText(), "for (String ${1:a2} : a) {\n\t${0}\n}"); + assertEquals("for (String ${1:a2} : a) {\n\t${0}\n}", item.getTextEdit().getLeft().getNewText()); assertEquals(item.getInsertTextFormat(), InsertTextFormat.Snippet); Range range = item.getAdditionalTextEdits().get(0).getRange(); assertEquals(new Range(new Position(3, 2), new Position(3, 7)), range); @@ -264,7 +264,7 @@ public void test_fori() throws JavaModelException { List items = new ArrayList<>(list.getItems()); CompletionItem item = items.get(0); assertEquals("fori", item.getLabel()); - assertEquals(item.getInsertText(), "for (int ${1:a2} = 0; ${1:a2} < a.length; ${1:a2}++) {\n\t${0}\n}"); + assertEquals("for (int ${1:a2} = 0; ${1:a2} < a.length; ${1:a2}++) {\n\t${0}\n}", item.getTextEdit().getLeft().getNewText()); assertEquals(item.getInsertTextFormat(), InsertTextFormat.Snippet); Range range = item.getAdditionalTextEdits().get(0).getRange(); assertEquals(new Range(new Position(3, 2), new Position(3, 8)), range); @@ -290,7 +290,7 @@ public void test_forr() throws JavaModelException { List items = new ArrayList<>(list.getItems()); CompletionItem item = items.get(0); assertEquals("forr", item.getLabel()); - assertEquals(item.getInsertText(), "for (int ${1:a2} = a.length - 1; ${1:a2} >= 0; ${1:a2}--) {\n\t${0}\n}"); + assertEquals("for (int ${1:a2} = a.length - 1; ${1:a2} >= 0; ${1:a2}--) {\n\t${0}\n}", item.getTextEdit().getLeft().getNewText()); assertEquals(item.getInsertTextFormat(), InsertTextFormat.Snippet); Range range = item.getAdditionalTextEdits().get(0).getRange(); assertEquals(new Range(new Position(3, 2), new Position(3, 8)), range); @@ -316,7 +316,7 @@ public void test_nnull() throws JavaModelException { List items = new ArrayList<>(list.getItems()); CompletionItem item = items.get(0); assertEquals("nnull", item.getLabel()); - assertEquals(item.getInsertText(), "if (a != null) {\n\t${0}\n}"); + assertEquals("if (a != null) {\n\t${0}\n}", item.getTextEdit().getLeft().getNewText()); assertEquals(item.getInsertTextFormat(), InsertTextFormat.Snippet); Range range = item.getAdditionalTextEdits().get(0).getRange(); assertEquals(new Range(new Position(3, 2), new Position(3, 9)), range); @@ -342,7 +342,7 @@ public void test_null() throws JavaModelException { List items = new ArrayList<>(list.getItems()); CompletionItem item = items.get(0); assertEquals("null", item.getLabel()); - assertEquals(item.getInsertText(), "if (a == null) {\n\t${0}\n}"); + assertEquals("if (a == null) {\n\t${0}\n}", item.getTextEdit().getLeft().getNewText()); assertEquals(item.getInsertTextFormat(), InsertTextFormat.Snippet); Range range = item.getAdditionalTextEdits().get(0).getRange(); assertEquals(new Range(new Position(3, 2), new Position(3, 8)), range); @@ -368,7 +368,7 @@ public void test_opt() throws JavaModelException { List items = new ArrayList<>(list.getItems()); CompletionItem item = items.get(0); assertEquals("opt", item.getLabel()); - assertEquals(item.getInsertText(), "Optional.ofNullable(identifier)"); + assertEquals("Optional.ofNullable(identifier)", item.getTextEdit().getLeft().getNewText()); assertEquals(item.getInsertTextFormat(), InsertTextFormat.Snippet); Range range = item.getAdditionalTextEdits().get(0).getRange(); assertEquals(new Range(new Position(3, 2), new Position(3, 16)), range); @@ -394,7 +394,7 @@ public void test_not() throws JavaModelException { List items = new ArrayList<>(list.getItems()); CompletionItem item = items.get(0); assertEquals("not", item.getLabel()); - assertEquals(item.getInsertText(), "!a"); + assertEquals("!a", item.getTextEdit().getLeft().getNewText()); assertEquals(item.getInsertTextFormat(), InsertTextFormat.Snippet); Range range = item.getAdditionalTextEdits().get(0).getRange(); assertEquals(new Range(new Position(3, 2), new Position(3, 7)), range); @@ -420,7 +420,7 @@ public void test_sysout() throws JavaModelException { List items = new ArrayList<>(list.getItems()); CompletionItem item = items.get(0); assertEquals("sysout", item.getLabel()); - assertEquals(item.getInsertText(), "System.out.println(a);${0}"); + assertEquals("System.out.println(a);${0}", item.getTextEdit().getLeft().getNewText()); assertEquals(item.getInsertTextFormat(), InsertTextFormat.Snippet); Range range = item.getAdditionalTextEdits().get(0).getRange(); assertEquals(new Range(new Position(3, 2), new Position(3, 10)), range); @@ -448,9 +448,9 @@ public void test_sysout_itemDefaults_enabled() throws Exception { CompletionItem ci = list.getItems().stream().filter(item -> item.getLabel().startsWith("sysout")).findFirst().orElse(null); assertNotNull(ci); - assertEquals("System.out.println(new Test());${0}", ci.getTextEditText()); + assertEquals("System.out.println(new Test());${0}", ci.getTextEdit().getLeft().getNewText()); //check that the fields covered by itemDefaults are set to null - assertNull(ci.getTextEdit()); + assertNull(ci.getTextEditText()); assertNull(ci.getInsertTextFormat()); assertNull(ci.getInsertTextMode()); assertEquals(CompletionItemKind.Snippet, ci.getKind()); @@ -477,7 +477,7 @@ public void test_sysout_object() throws JavaModelException { List items = new ArrayList<>(list.getItems()); CompletionItem item = items.get(0); assertEquals("sysout", item.getLabel()); - assertEquals(item.getInsertText(), "System.out.println(foo);${0}"); + assertEquals("System.out.println(foo);${0}", item.getTextEdit().getLeft().getNewText()); assertEquals(item.getInsertTextFormat(), InsertTextFormat.Snippet); Range range = item.getAdditionalTextEdits().get(0).getRange(); assertEquals(new Range(new Position(4, 2), new Position(4, 12)), range); @@ -504,7 +504,7 @@ public void test_sysoutv_object() throws JavaModelException { List items = new ArrayList<>(list.getItems()); CompletionItem item = items.get(0); assertEquals("sysoutv", item.getLabel()); - assertEquals(item.getInsertText(), "System.out.println(\"foo = \" + foo);${0}"); + assertEquals("System.out.println(\"foo = \" + foo);${0}", item.getTextEdit().getLeft().getNewText()); assertEquals(item.getInsertTextFormat(), InsertTextFormat.Snippet); Range range = item.getAdditionalTextEdits().get(0).getRange(); assertEquals(new Range(new Position(4, 2), new Position(4, 13)), range); @@ -531,7 +531,7 @@ public void test_sysouf_object() throws JavaModelException { List items = new ArrayList<>(list.getItems()); CompletionItem item = items.get(0); assertEquals("sysouf", item.getLabel()); - assertEquals(item.getInsertText(), "System.out.printf(\"\", foo);${0}"); + assertEquals("System.out.printf(\"\", foo);${0}", item.getTextEdit().getLeft().getNewText()); assertEquals(item.getInsertTextFormat(), InsertTextFormat.Snippet); Range range = item.getAdditionalTextEdits().get(0).getRange(); assertEquals(new Range(new Position(4, 2), new Position(4, 12)), range); @@ -558,7 +558,7 @@ public void test_syserr_object() throws JavaModelException { List items = new ArrayList<>(list.getItems()); CompletionItem item = items.get(0); assertEquals("syserr", item.getLabel()); - assertEquals(item.getInsertText(), "System.err.println(foo);${0}"); + assertEquals("System.err.println(foo);${0}", item.getTextEdit().getLeft().getNewText()); assertEquals(item.getInsertTextFormat(), InsertTextFormat.Snippet); Range range = item.getAdditionalTextEdits().get(0).getRange(); assertEquals(new Range(new Position(4, 2), new Position(4, 12)), range); @@ -583,7 +583,7 @@ public void test_format() throws JavaModelException { CompletionItem item = list.getItems().stream().filter(i -> i.getKind() == CompletionItemKind.Snippet).findFirst().orElse(null); assertEquals("format", item.getLabel()); - assertEquals(item.getTextEditText(), "String.format(a, ${0});"); + assertEquals("String.format(a, ${0});", item.getTextEdit().getLeft().getNewText()); Range range = item.getAdditionalTextEdits().get(0).getRange(); assertEquals(new Range(new Position(3, 2), new Position(3, 10)), range); } @@ -609,7 +609,7 @@ public void test_throw() throws JavaModelException { List items = new ArrayList<>(list.getItems()); CompletionItem item = items.get(0); assertEquals("throw", item.getLabel()); - assertEquals(item.getInsertText(), "throw e;"); + assertEquals("throw e;", item.getTextEdit().getLeft().getNewText()); assertEquals(item.getInsertTextFormat(), InsertTextFormat.Snippet); Range range = item.getAdditionalTextEdits().get(0).getRange(); assertEquals(new Range(new Position(4, 2), new Position(4, 9)), range); @@ -635,7 +635,7 @@ public void test_var() throws JavaModelException { List items = new ArrayList<>(list.getItems()); CompletionItem item = items.get(0); assertEquals("var", item.getLabel()); - assertEquals(item.getInsertText(), "String ${1:a2} = a;${0}"); + assertEquals("String ${1:a2} = a;${0}", item.getTextEdit().getLeft().getNewText()); assertEquals(item.getInsertTextFormat(), InsertTextFormat.Snippet); Range range = item.getAdditionalTextEdits().get(0).getRange(); assertEquals(new Range(new Position(3, 2), new Position(3, 7)), range); @@ -662,7 +662,7 @@ public void test_var2() throws JavaModelException { List items = new ArrayList<>(list.getItems()); CompletionItem item = items.get(0); assertEquals("var", item.getLabel()); - assertEquals(item.getInsertText(), "List ${1:emptyList} = Collections.emptyList();${0}"); + assertEquals("List ${1:emptyList} = Collections.emptyList();${0}", item.getTextEdit().getLeft().getNewText()); assertEquals(item.getInsertTextFormat(), InsertTextFormat.Snippet); List additionalTextEdits = item.getAdditionalTextEdits(); Range range =additionalTextEdits.get(0).getRange(); @@ -690,7 +690,7 @@ public void test_par() throws JavaModelException { List items = new ArrayList<>(list.getItems()); CompletionItem item = items.get(0); assertEquals("par", item.getLabel()); - assertEquals(item.getInsertText(), "(a)"); + assertEquals("(a)", item.getTextEdit().getLeft().getNewText()); assertEquals(item.getInsertTextFormat(), InsertTextFormat.Snippet); Range range = item.getAdditionalTextEdits().get(0).getRange(); assertEquals(new Range(new Position(3, 2), new Position(3, 7)), range); @@ -716,7 +716,7 @@ public void test_while() throws JavaModelException { List items = new ArrayList<>(list.getItems()); CompletionItem item = items.get(0); assertEquals("while", item.getLabel()); - assertEquals(item.getInsertText(), "while (a) {\n\t${0}\n}"); + assertEquals("while (a) {\n\t${0}\n}", item.getTextEdit().getLeft().getNewText()); assertEquals(item.getInsertTextFormat(), InsertTextFormat.Snippet); Range range = item.getAdditionalTextEdits().get(0).getRange(); assertEquals(new Range(new Position(3, 2), new Position(3, 9)), range);