Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fill additionalTextEdits during completionItem/resolve #1487

Merged
merged 3 commits into from
Jun 30, 2020
Merged

fill additionalTextEdits during completionItem/resolve #1487

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d1b10d6
support resolve additionalTextEdits
Eskibear Jun 24, 2020
8a901a9
don't fill unimplemented methods in new type constructor
Eskibear Jun 26, 2020
157ca61
use placeholder only when client supports snippet
Eskibear Jun 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions ...e/src/org/eclipse/jdt/ls/core/internal/contentassist/AnonymousTypeCompletionProposal.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,10 @@ public AnonymousTypeCompletionProposal(ICompilationUnit cu, int replacementOffse
* @see JavaTypeCompletionProposal#updateReplacementString(IDocument,char,int,ImportRewrite)
*/
public String updateReplacementString(IDocument document, int offset, ImportRewrite impRewrite) throws CoreException, BadLocationException {
String newBody = createNewBody(impRewrite);
if (newBody == null) {
return null;
}
// Construct empty body for performance concern
// See https://github.com/microsoft/language-server-protocol/issues/1032#issuecomment-648748013
String newBody = "{\n\t${0}\n}";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only use ${0} if snippets are supported by the client


StringBuffer buf = new StringBuffer("new A()"); //$NON-NLS-1$
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StringBuilder

buf.append(newBody);
// use the code formatter
Expand Down
53 changes: 36 additions & 17 deletions ...org/eclipse/jdt/ls/core/internal/contentassist/CompletionProposalReplacementProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ public class CompletionProposalReplacementProvider {
private static final char RPAREN = ')';
private static final char SEMICOLON = ';';
private static final char COMMA = ',';
private static final char LESS= '<';
private static final char GREATER= '>';
private static final char LESS = '<';
private static final char GREATER = '>';
private final ICompilationUnit compilationUnit;
private final int offset;
private final CompletionContext context;
Expand All @@ -96,13 +96,28 @@ public CompletionProposalReplacementProvider(ICompilationUnit compilationUnit, C
}

/**
* Updates the replacement and any additional replacement for the given item.
* Update the replacement together with additionalTextEdits for the given item.
* It's originally designed to defer expensive calculation of the imports into completion/resolve stage.
* @param proposal
* @param item
* @param trigger
*/
public void updateAdditionalTextEdits(CompletionProposal proposal, CompletionItem item, char trigger) {
updateReplacement(proposal, item, trigger, true);
}

/**
* Updates the replacement but NO additional replacement for the given item.
*
* @param proposal
* @param item
* @param trigger
*/
public void updateReplacement(CompletionProposal proposal, CompletionItem item, char trigger) {
updateReplacement(proposal, item, trigger, false);
}

private void updateReplacement(CompletionProposal proposal, CompletionItem item, char trigger, boolean isResolving) {
// reset importRewrite
this.importRewrite = TypeProposalUtils.createImportRewrite(compilationUnit);

Expand All @@ -111,21 +126,22 @@ public void updateReplacement(CompletionProposal proposal, CompletionItem item,
StringBuilder completionBuffer = new StringBuilder();
Range range = null;
if (isSupportingRequiredProposals(proposal)) {
CompletionProposal[] requiredProposals= proposal.getRequiredProposals();
CompletionProposal[] requiredProposals = proposal.getRequiredProposals();
if (requiredProposals != null) {
for (CompletionProposal requiredProposal : requiredProposals) {
switch(requiredProposal.getKind()) {
switch (requiredProposal.getKind()) {
case CompletionProposal.TYPE_IMPORT:
case CompletionProposal.METHOD_IMPORT:
case CompletionProposal.FIELD_IMPORT:
appendImportProposal(completionBuffer, requiredProposal, proposal.getKind());
break;
case CompletionProposal.TYPE_REF:
org.eclipse.lsp4j.TextEdit edit = toRequiredTypeEdit(requiredProposal, trigger, proposal.canUseDiamond(context));
if (proposal.getKind() == CompletionProposal.CONSTRUCTOR_INVOCATION || proposal.getKind() == CompletionProposal.ANONYMOUS_CLASS_CONSTRUCTOR_INVOCATION
|| proposal.getKind() == CompletionProposal.ANONYMOUS_CLASS_DECLARATION) {
completionBuffer.append(edit.getNewText());
range = edit.getRange();
if (proposal.getKind() == CompletionProposal.CONSTRUCTOR_INVOCATION
|| proposal.getKind() == CompletionProposal.ANONYMOUS_CLASS_CONSTRUCTOR_INVOCATION
|| proposal.getKind() == CompletionProposal.ANONYMOUS_CLASS_DECLARATION) {
completionBuffer.append(edit.getNewText());
range = edit.getRange();
} else {
additionalTextEdits.add(edit);
}
Expand All @@ -152,7 +168,7 @@ public void updateReplacement(CompletionProposal proposal, CompletionItem item,
}
range = toReplacementRange(proposal);
}
if(proposal.getKind() == CompletionProposal.METHOD_DECLARATION){
if (proposal.getKind() == CompletionProposal.METHOD_DECLARATION) {
appendMethodOverrideReplacement(completionBuffer, proposal);
} else if (proposal.getKind() == CompletionProposal.POTENTIAL_METHOD_DECLARATION && proposal instanceof GetterSetterCompletionProposal) {
appendMethodPotentialReplacement(completionBuffer, (GetterSetterCompletionProposal) proposal);
Expand All @@ -162,21 +178,24 @@ public void updateReplacement(CompletionProposal proposal, CompletionItem item,
appendReplacementString(completionBuffer, proposal);
}
//select insertTextFormat.
if( client.isCompletionSnippetsSupported()){
if (client.isCompletionSnippetsSupported()) {
item.setInsertTextFormat(InsertTextFormat.Snippet);
}else{
} else {
item.setInsertTextFormat(InsertTextFormat.PlainText);
}
String text = completionBuffer.toString();
if(range != null){
if (range != null) {
item.setTextEdit(new org.eclipse.lsp4j.TextEdit(range, text));
}else{
} else {
// fallback
item.setInsertText(text);
}
addImports(additionalTextEdits);
if(!additionalTextEdits.isEmpty()){
item.setAdditionalTextEdits(additionalTextEdits);

if (!client.isResolveAdditionalTextEditsSupport() || isResolving) {
addImports(additionalTextEdits);
if(!additionalTextEdits.isEmpty()){
item.setAdditionalTextEdits(additionalTextEdits);
}
}
}

Expand Down
5 changes: 5 additions & 0 deletions ...e.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CompletionResolveHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.eclipse.jdt.ls.core.internal.JDTUtils;
import org.eclipse.jdt.ls.core.internal.JSONUtility;
import org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin;
import org.eclipse.jdt.ls.core.internal.contentassist.CompletionProposalReplacementProvider;
import org.eclipse.jdt.ls.core.internal.contentassist.CompletionProposalRequestor;
import org.eclipse.jdt.ls.core.internal.javadoc.JavadocContentAccess;
import org.eclipse.jdt.ls.core.internal.javadoc.JavadocContentAccess2;
Expand Down Expand Up @@ -103,6 +104,10 @@ public CompletionItem resolve(CompletionItem param, IProgressMonitor monitor) {
param.setData(null);
return param;
}
if (manager.getClientPreferences().isResolveAdditionalTextEditsSupport()) {
CompletionProposalReplacementProvider proposalProvider = new CompletionProposalReplacementProvider(unit, completionResponse.getContext(), completionResponse.getOffset(), manager.getPreferences(), manager.getClientPreferences());
proposalProvider.updateAdditionalTextEdits(completionResponse.getProposals().get(proposalId), param, '\0');
}
if (data.containsKey(DATA_FIELD_DECLARATION_SIGNATURE)) {
String typeName = stripSignatureToFQN(String.valueOf(data.get(DATA_FIELD_DECLARATION_SIGNATURE)));
try {
Expand Down
4 changes: 4 additions & 0 deletions ...lipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/ClientPreferences.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,10 @@ public boolean isGradleChecksumWrapperPromptSupport() {
return Boolean.parseBoolean(extendedClientCapabilities.getOrDefault("gradleChecksumWrapperPromptSupport", "false").toString());
}

public boolean isResolveAdditionalTextEditsSupport() {
return Boolean.parseBoolean(extendedClientCapabilities.getOrDefault("resolveAdditionalTextEditsSupport", "false").toString());
}

public boolean isSupportsCompletionDocumentationMarkdown() {
//@formatter:off
return v3supported && capabilities.getTextDocument().getCompletion() != null
Expand Down
78 changes: 37 additions & 41 deletions ...pse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/CompletionHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1811,7 +1811,7 @@ public void testCompletion_getter() throws Exception {
" return strField;\n" +
"}", ci.getTextEdit());
}

@Test
public void testCompletion_getterNoJavadoc() throws Exception {
preferences.setCodeGenerationTemplateGenerateComments(false);
Expand All @@ -1836,7 +1836,7 @@ public void testCompletion_getterNoJavadoc() throws Exception {
assertEquals(CompletionItemKind.Method, ci.getKind());
assertEquals("999999979", ci.getSortText());
assertNotNull(ci.getTextEdit());
assertTextEdit(2, 4, 7,
assertTextEdit(2, 4, 7,
"public String getStrField() {\n" +
" return strField;\n" +
"}", ci.getTextEdit());
Expand Down Expand Up @@ -1932,11 +1932,7 @@ public void testCompletion_AnonymousType() throws Exception {
assertEquals("999998684", ci.getSortText());
assertNotNull(ci.getTextEdit());
assertTextEdit(2, 23, 23, "IFoo(){\n" +
"\n" +
" @Override\n" +
" public String getName() {\n" +
" ${0:// TODO Auto-generated method stub\n\t\t\treturn null;}\n" +
" }\n" +
" ${0}\n" +
"};", ci.getTextEdit());
}

Expand Down Expand Up @@ -1967,15 +1963,7 @@ public void testCompletion_AnonymousTypeMoreMethods() throws Exception {
assertEquals("999998684", ci.getSortText());
assertNotNull(ci.getTextEdit());
assertTextEdit(2, 23, 23, "IFoo(){\n" +
"\n @Override\n" +
" public void setName(String name) {\n" +
" ${0:// TODO Auto-generated method stub\n\t\t\t}\n" +
" }\n" +
"\n @Override\n" +
" public String getName() {\n" +
" // TODO Auto-generated method stub\n" +
" return null;\n" +
" }\n" +
" ${0}\n" +
"};", ci.getTextEdit());
}

Expand All @@ -2002,11 +1990,7 @@ public void testCompletion_AnonymousDeclarationType() throws Exception {
assertEquals("999999372", ci.getSortText());
assertNotNull(ci.getTextEdit());
assertTextEdit(2, 20, 22, "(){\n" +
"\n" +
" @Override\n" +
" public void run() {\n" +
" ${0:// TODO Auto-generated method stub\n\t\t}\n" +
" }\n" +
" ${0}\n" +
"}", ci.getTextEdit());
}

Expand All @@ -2033,11 +2017,7 @@ public void testCompletion_AnonymousDeclarationType2() throws Exception {
assertEquals("999999372", ci.getSortText());
assertNotNull(ci.getTextEdit());
assertTextEdit(2, 20, 24, "(){\n" +
"\n" +
" @Override\n" +
" public void run() {\n" +
" ${0:// TODO Auto-generated method stub\n\t\t}\n" +
" }\n" +
" ${0}\n" +
"}", ci.getTextEdit());
}

Expand Down Expand Up @@ -2066,11 +2046,7 @@ public void testCompletion_AnonymousDeclarationType3() throws Exception {
assertEquals("999999372", ci.getSortText());
assertNotNull(ci.getTextEdit());
assertTextEdit(2, 32, 33, "(){\n" +
"\n" +
" @Override\n" +
" public void run() {\n" +
" ${0:// TODO Auto-generated method stub\n\t\t}\n" +
" }\n" +
" ${0}\n" +
"}", ci.getTextEdit());
}

Expand Down Expand Up @@ -2100,11 +2076,7 @@ public void testCompletion_AnonymousDeclarationType4() throws Exception {
assertEquals("999999372", ci.getSortText());
assertNotNull(ci.getTextEdit());
assertTextEdit(2, 32, 33, "(){\n" +
"\n" +
" @Override\n" +
" public void run() {\n" +
" ${0:// TODO Auto-generated method stub\n\t\t}\n" +
" }\n" +
" ${0}\n" +
"}", ci.getTextEdit());
}

Expand All @@ -2129,11 +2101,7 @@ public void testCompletion_AnonymousDeclarationType5() throws Exception {
assertEquals("999999372", ci.getSortText());
assertNotNull(ci.getTextEdit());
assertTextEdit(2, 33, 33, "(){\n" +
"\n" +
" @Override\n" +
" public void run() {\n" +
" ${0:// TODO Auto-generated method stub\n\t\t}\n" +
" }\n" +
" ${0}\n" +
"}", ci.getTextEdit());
}

Expand Down Expand Up @@ -2764,6 +2732,34 @@ public void testCompletion_additionalTextEdit() throws Exception {
assertNull(resolvedItem.getAdditionalTextEdits());
}

@Test
public void testCompletion_resolveAdditionalTextEdits() throws Exception {
ClientPreferences mockCapabilies = mock(ClientPreferences.class);
when(mockCapabilies.isResolveAdditionalTextEditsSupport()).thenReturn(true);
when(preferenceManager.getClientPreferences()).thenReturn(mockCapabilies);
//@formatter:off
ICompilationUnit unit = getWorkingCopy(
"src/java/Foo.java",
"public class Foo {\n"+
" void foo() {\n"+
" HashMa\n"+
" }\n"+
"}\n");
//@formatter:on
int[] loc = findCompletionLocation(unit, "HashMa");
CompletionList list = server.completion(JsonMessageHelper.getParams(createCompletionRequest(unit, loc[0], loc[1]))).join().getRight();
assertNotNull(list);
assertFalse("No proposals were found",list.getItems().isEmpty());
CompletionItem ci = list.getItems().get(0);
assertNull(ci.getAdditionalTextEdits());
assertEquals("HashMap - java.util", ci.getLabel());
CompletionItem resolvedItem = server.resolveCompletionItem(ci).join();
List<TextEdit> additionalEdits = resolvedItem.getAdditionalTextEdits();
assertNotNull(additionalEdits);
assertEquals(1, additionalEdits.size());
assertEquals("import java.util.HashMap;\n\n", additionalEdits.get(0).getNewText());
}

@Test
public void testCompletion_Enum() throws JavaModelException {
ICompilationUnit unit = getWorkingCopy("src/org/sample/Test.java",
Expand Down