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..641f62beb3 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$ @@ -112,10 +113,10 @@ public List complete(IDocument document, int offset, ICompilatio context.setActiveTemplateName(template.getName()); content = evaluateGenericTemplate(context, template); } - setTextEdit(item, content, completionItemDefaults); + setTextEdit(item, content, range); if (!getClientPreferences().isResolveAdditionalTextEditsSupport()) { - setAdditionalTextEdit(item, compilationUnit, context, range, template); + setAdditionalTextEdit(item, compilationUnit, context, template); } if (isCompletionItemLabelDetailsSupport()) { @@ -151,19 +152,13 @@ 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); - } + private void setTextEdit(final CompletionItem item, String content, Range range) { + item.setTextEdit(Either.forLeft(new TextEdit(range, content))); } public static void setAdditionalTextEdit(final CompletionItem item, ICompilationUnit compilationUnit, - JavaPostfixContext context, Range range, Template template) { + JavaPostfixContext context, Template template) { List additionalEdits = new ArrayList<>(); - // use additional test 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()) { for (org.eclipse.text.edits.TextEdit edit : jdtTextEdits) { @@ -171,7 +166,9 @@ public static void setAdditionalTextEdit(final CompletionItem item, ICompilation additionalEdits.addAll(converter.convert()); } } - item.setAdditionalTextEdits(additionalEdits); + if (!additionalEdits.isEmpty()) { + item.setAdditionalTextEdits(additionalEdits); + } } private boolean isCompletionItemLabelDetailsSupport() { 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..2e62195d88 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 @@ -150,7 +150,7 @@ public CompletionItem resolve(CompletionItem param, IProgressMonitor monitor) { } if (manager.getClientPreferences().isResolveAdditionalTextEditsSupport()) { - PostfixTemplateEngine.setAdditionalTextEdit(param, unit, postfixContext, range, template); + PostfixTemplateEngine.setAdditionalTextEdit(param, unit, postfixContext, template); } } 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..5145908250 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 @@ -80,6 +80,7 @@ public void tearDown() throws Exception { public void testCastLazyResolve() throws JavaModelException { try { preferences.setCompletionLazyResolveTextEditEnabled(true); + when(preferenceManager.getClientPreferences().isResolveAdditionalTextEditsSupport()).thenReturn(true); //@formatter:off ICompilationUnit unit = getWorkingCopy( "src/org/sample/Test.java", @@ -98,10 +99,12 @@ 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(item.getInsertTextFormat(), InsertTextFormat.Snippet); - Range range = item.getAdditionalTextEdits().get(0).getRange(); - assertEquals(new Range(new Position(3, 2), new Position(3, 8)), range); + Range range = new Range(new Position(3, 2), new Position(3, 8)); + assertPostfixTextEdit(item, "((${1})${inner_expression})${0}", range); + + CompletionItem resolved = server.resolveCompletionItem(item).join(); + assertPostfixTextEdit(resolved, "((${1})a)${0}", range); } finally { preferences.setCompletionLazyResolveTextEditEnabled(false); } @@ -127,10 +130,8 @@ 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(item.getInsertTextFormat(), InsertTextFormat.Snippet); - Range range = item.getAdditionalTextEdits().get(0).getRange(); - assertEquals(new Range(new Position(3, 2), new Position(3, 19)), range); + assertPostfixTextEdit(item, "assert identifier;", new Range(new Position(3, 2), new Position(3, 19))); } @Test @@ -157,11 +158,9 @@ 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(item.getInsertTextFormat(), InsertTextFormat.Snippet); assertEquals(item.getInsertTextMode(), InsertTextMode.AdjustIndentation); - Range range = item.getAdditionalTextEdits().get(0).getRange(); - assertEquals(new Range(new Position(3, 2), new Position(3, 8)), range); + assertPostfixTextEdit(item, "((${1})a)${0}", new Range(new Position(3, 2), new Position(3, 8))); } @Test @@ -185,11 +184,9 @@ 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(item.getInsertTextFormat(), InsertTextFormat.Snippet); assertNull(item.getInsertTextMode()); - Range range = item.getAdditionalTextEdits().get(0).getRange(); - assertEquals(new Range(new Position(3, 2), new Position(3, 6)), range); + assertPostfixTextEdit(item, "if (a) {\n\t${0}\n}", new Range(new Position(3, 2), new Position(3, 6))); } @Test @@ -212,10 +209,8 @@ 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(item.getInsertTextFormat(), InsertTextFormat.Snippet); - Range range = item.getAdditionalTextEdits().get(0).getRange(); - assertEquals(new Range(new Position(3, 2), new Position(3, 8)), range); + assertPostfixTextEdit(item, "if (!a) {\n\t${0}\n}", new Range(new Position(3, 2), new Position(3, 8))); } @Test @@ -238,10 +233,8 @@ 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(item.getInsertTextFormat(), InsertTextFormat.Snippet); - Range range = item.getAdditionalTextEdits().get(0).getRange(); - assertEquals(new Range(new Position(3, 2), new Position(3, 7)), range); + assertPostfixTextEdit(item, "for (String ${1:a2} : a) {\n\t${0}\n}", new Range(new Position(3, 2), new Position(3, 7))); } @Test @@ -264,10 +257,8 @@ 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(item.getInsertTextFormat(), InsertTextFormat.Snippet); - Range range = item.getAdditionalTextEdits().get(0).getRange(); - assertEquals(new Range(new Position(3, 2), new Position(3, 8)), range); + assertPostfixTextEdit(item, "for (int ${1:a2} = 0; ${1:a2} < a.length; ${1:a2}++) {\n\t${0}\n}", new Range(new Position(3, 2), new Position(3, 8))); } @Test @@ -290,10 +281,8 @@ 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(item.getInsertTextFormat(), InsertTextFormat.Snippet); - Range range = item.getAdditionalTextEdits().get(0).getRange(); - assertEquals(new Range(new Position(3, 2), new Position(3, 8)), range); + assertPostfixTextEdit(item, "for (int ${1:a2} = a.length - 1; ${1:a2} >= 0; ${1:a2}--) {\n\t${0}\n}", new Range(new Position(3, 2), new Position(3, 8))); } @Test @@ -316,10 +305,8 @@ 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(item.getInsertTextFormat(), InsertTextFormat.Snippet); - Range range = item.getAdditionalTextEdits().get(0).getRange(); - assertEquals(new Range(new Position(3, 2), new Position(3, 9)), range); + assertPostfixTextEdit(item, "if (a != null) {\n\t${0}\n}", new Range(new Position(3, 2), new Position(3, 9))); } @Test @@ -342,10 +329,8 @@ 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(item.getInsertTextFormat(), InsertTextFormat.Snippet); - Range range = item.getAdditionalTextEdits().get(0).getRange(); - assertEquals(new Range(new Position(3, 2), new Position(3, 8)), range); + assertPostfixTextEdit(item, "if (a == null) {\n\t${0}\n}", new Range(new Position(3, 2), new Position(3, 8))); } @Test @@ -368,10 +353,8 @@ 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(item.getInsertTextFormat(), InsertTextFormat.Snippet); - Range range = item.getAdditionalTextEdits().get(0).getRange(); - assertEquals(new Range(new Position(3, 2), new Position(3, 16)), range); + assertPostfixTextEdit(item, "Optional.ofNullable(identifier)", new Range(new Position(3, 2), new Position(3, 16))); } @Test @@ -394,10 +377,8 @@ 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(item.getInsertTextFormat(), InsertTextFormat.Snippet); - Range range = item.getAdditionalTextEdits().get(0).getRange(); - assertEquals(new Range(new Position(3, 2), new Position(3, 7)), range); + assertPostfixTextEdit(item, "!a", new Range(new Position(3, 2), new Position(3, 7))); } @Test @@ -420,10 +401,8 @@ 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(item.getInsertTextFormat(), InsertTextFormat.Snippet); - Range range = item.getAdditionalTextEdits().get(0).getRange(); - assertEquals(new Range(new Position(3, 2), new Position(3, 10)), range); + assertPostfixTextEdit(item, "System.out.println(a);${0}", new Range(new Position(3, 2), new Position(3, 10))); } @Test @@ -448,9 +427,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()); - //check that the fields covered by itemDefaults are set to null - assertNull(ci.getTextEdit()); + assertPostfixTextEdit(ci, "System.out.println(new Test());${0}", new Range(new Position(4, 2), new Position(4, 17))); + // The full postfix range differs from the shared item default, so this item must override it. + assertNull(ci.getTextEditText()); assertNull(ci.getInsertTextFormat()); assertNull(ci.getInsertTextMode()); assertEquals(CompletionItemKind.Snippet, ci.getKind()); @@ -477,10 +456,8 @@ 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(item.getInsertTextFormat(), InsertTextFormat.Snippet); - Range range = item.getAdditionalTextEdits().get(0).getRange(); - assertEquals(new Range(new Position(4, 2), new Position(4, 12)), range); + assertPostfixTextEdit(item, "System.out.println(foo);${0}", new Range(new Position(4, 2), new Position(4, 12))); } @Test @@ -504,10 +481,8 @@ 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(item.getInsertTextFormat(), InsertTextFormat.Snippet); - Range range = item.getAdditionalTextEdits().get(0).getRange(); - assertEquals(new Range(new Position(4, 2), new Position(4, 13)), range); + assertPostfixTextEdit(item, "System.out.println(\"foo = \" + foo);${0}", new Range(new Position(4, 2), new Position(4, 13))); } @Test @@ -531,10 +506,8 @@ 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(item.getInsertTextFormat(), InsertTextFormat.Snippet); - Range range = item.getAdditionalTextEdits().get(0).getRange(); - assertEquals(new Range(new Position(4, 2), new Position(4, 12)), range); + assertPostfixTextEdit(item, "System.out.printf(\"\", foo);${0}", new Range(new Position(4, 2), new Position(4, 12))); } @Test @@ -558,10 +531,8 @@ 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(item.getInsertTextFormat(), InsertTextFormat.Snippet); - Range range = item.getAdditionalTextEdits().get(0).getRange(); - assertEquals(new Range(new Position(4, 2), new Position(4, 12)), range); + assertPostfixTextEdit(item, "System.err.println(foo);${0}", new Range(new Position(4, 2), new Position(4, 12))); } @Test @@ -583,9 +554,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});"); - Range range = item.getAdditionalTextEdits().get(0).getRange(); - assertEquals(new Range(new Position(3, 2), new Position(3, 10)), range); + assertPostfixTextEdit(item, "String.format(a, ${0});", new Range(new Position(3, 2), new Position(3, 10))); } @Test @@ -609,10 +578,8 @@ 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(item.getInsertTextFormat(), InsertTextFormat.Snippet); - Range range = item.getAdditionalTextEdits().get(0).getRange(); - assertEquals(new Range(new Position(4, 2), new Position(4, 9)), range); + assertPostfixTextEdit(item, "throw e;", new Range(new Position(4, 2), new Position(4, 9))); } @Test @@ -635,10 +602,8 @@ 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(item.getInsertTextFormat(), InsertTextFormat.Snippet); - Range range = item.getAdditionalTextEdits().get(0).getRange(); - assertEquals(new Range(new Position(3, 2), new Position(3, 7)), range); + assertPostfixTextEdit(item, "String ${1:a2} = a;${0}", new Range(new Position(3, 2), new Position(3, 7))); } @Test @@ -662,12 +627,13 @@ 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(item.getInsertTextFormat(), InsertTextFormat.Snippet); + assertPrimaryTextEdit(item, "List ${1:emptyList} = Collections.emptyList();${0}", + new Range(new Position(4, 2), new Position(4, 29))); List additionalTextEdits = item.getAdditionalTextEdits(); - Range range =additionalTextEdits.get(0).getRange(); - assertEquals(new Range(new Position(4, 2), new Position(4, 29)), range); + assertNotNull(additionalTextEdits); assertTrue(additionalTextEdits.stream().anyMatch(e -> e.getNewText().contains("import java.util.List;"))); + assertAdditionalTextEditsDoNotOverlapPrimary(item); } @Test @@ -690,10 +656,8 @@ 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(item.getInsertTextFormat(), InsertTextFormat.Snippet); - Range range = item.getAdditionalTextEdits().get(0).getRange(); - assertEquals(new Range(new Position(3, 2), new Position(3, 7)), range); + assertPostfixTextEdit(item, "(a)", new Range(new Position(3, 2), new Position(3, 7))); } @Test @@ -716,10 +680,8 @@ 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(item.getInsertTextFormat(), InsertTextFormat.Snippet); - Range range = item.getAdditionalTextEdits().get(0).getRange(); - assertEquals(new Range(new Position(3, 2), new Position(3, 9)), range); + assertPostfixTextEdit(item, "while (a) {\n\t${0}\n}", new Range(new Position(3, 2), new Position(3, 9))); } @Test @@ -829,6 +791,36 @@ private String createCompletionRequest(ICompilationUnit unit, int line, int kar) .replace("${char}", String.valueOf(kar)); } + private void assertPostfixTextEdit(CompletionItem item, String newText, Range range) { + assertPrimaryTextEdit(item, newText, range); + assertAdditionalTextEditsDoNotOverlapPrimary(item); + } + + private void assertPrimaryTextEdit(CompletionItem item, String newText, Range range) { + assertNotNull(item.getTextEdit()); + assertTrue(item.getTextEdit().isLeft()); + assertEquals(new TextEdit(range, newText), item.getTextEdit().getLeft()); + } + + private void assertAdditionalTextEditsDoNotOverlapPrimary(CompletionItem item) { + if (item.getAdditionalTextEdits() == null) { + return; + } + Range primaryRange = item.getTextEdit().getLeft().getRange(); + for (TextEdit additionalEdit : item.getAdditionalTextEdits()) { + assertFalse(rangesOverlap(primaryRange, additionalEdit.getRange())); + } + } + + private boolean rangesOverlap(Range left, Range right) { + return compare(left.getStart(), right.getEnd()) < 0 && compare(right.getStart(), left.getEnd()) < 0; + } + + private int compare(Position left, Position right) { + int lineComparison = Integer.compare(left.getLine(), right.getLine()); + return lineComparison != 0 ? lineComparison : Integer.compare(left.getCharacter(), right.getCharacter()); + } + private void mockLSP3Client() { mockLSPClient(true, true, true); }