Ignore unsupported CompletionItemKind during completionItem/resolve. F…

…ixes #675 Signed-off-by: Fred Bricon <fbricon@gmail.com>
eclipse-jdtls · Jun 1, 2018 · e4b3554 · e4b3554
1 parent 206167c
commit e4b3554
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 1 deletion.
diff --git a/....core/src/org/eclipse/jdt/ls/core/internal/contentassist/CompletionProposalRequestor.java b/....core/src/org/eclipse/jdt/ls/core/internal/contentassist/CompletionProposalRequestor.java
@@ -14,6 +14,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IPath;
@@ -34,6 +35,8 @@
 import org.eclipse.lsp4j.CompletionItem;
 import org.eclipse.lsp4j.CompletionItemKind;
 
+import com.google.common.collect.ImmutableSet;
+
 public final class CompletionProposalRequestor extends CompletionRequestor {
 
 	private List<CompletionProposal> proposals = new ArrayList<>();
@@ -42,6 +45,19 @@ public final class CompletionProposalRequestor extends CompletionRequestor {
 	private CompletionResponse response;
 	private boolean fIsTestCodeExcluded;
 
+	// Update SUPPORTED_KINDS when mapKind changes
+	// @formatter:off
+	public static final Set<CompletionItemKind> SUPPORTED_KINDS = ImmutableSet.of(CompletionItemKind.Constructor,
+																				CompletionItemKind.Class,
+																				CompletionItemKind.Module,
+																				CompletionItemKind.Field,
+																				CompletionItemKind.Keyword,
+																				CompletionItemKind.Reference,
+																				CompletionItemKind.Variable,
+																				CompletionItemKind.Function,
+																				CompletionItemKind.Text);
+	// @formatter:on
+
 	public CompletionProposalRequestor(ICompilationUnit aUnit, int offset) {
 		this.unit = aUnit;
 		response = new CompletionResponse();
@@ -120,6 +136,7 @@ public void acceptContext(CompletionContext context) {
 
 
 	private CompletionItemKind mapKind(final int kind) {
+		//When a new CompletionItemKind is added, don't forget to update SUPPORTED_KINDS
 		switch (kind) {
 		case CompletionProposal.ANONYMOUS_CLASS_CONSTRUCTOR_INVOCATION:
 		case CompletionProposal.CONSTRUCTOR_INVOCATION:

diff --git a/...e.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CompletionResolveHandler.java b/...e.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CompletionResolveHandler.java
@@ -31,6 +31,7 @@
 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.preferences.PreferenceManager;
 import org.eclipse.lsp4j.CompletionItem;
@@ -71,7 +72,7 @@ public CompletionItem resolve(CompletionItem param, IProgressMonitor monitor) {
 		Map<String, String> data = JSONUtility.toModel(param.getData(),Map.class);
 		// clean resolve data
 		param.setData(null);
-		if (data == null || !data.containsKey(DATA_FIELD_URI) || !data.containsKey(DATA_FIELD_REQUEST_ID) || !data.containsKey(DATA_FIELD_PROPOSAL_ID)) {
+		if (!CompletionProposalRequestor.SUPPORTED_KINDS.contains(param.getKind()) || data == null || !data.containsKey(DATA_FIELD_URI) || !data.containsKey(DATA_FIELD_REQUEST_ID) || !data.containsKey(DATA_FIELD_PROPOSAL_ID)) {
 			return param;
 		}
 		int proposalId = Integer.parseInt(data.get(DATA_FIELD_PROPOSAL_ID));

diff --git a/...pse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/CompletionHandlerTest.java b/...pse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/CompletionHandlerTest.java
@@ -17,6 +17,7 @@
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
@@ -586,6 +587,9 @@ public void testSnippet_interface() throws JavaModelException {
 		assertEquals("interface", item.getLabel());
 		String te = item.getInsertText();
 		assertEquals("package org.sample;\n\n/**\n * Test\n */\npublic interface Test {\n\n\t${0}\n}", te);
+
+		//check resolution doesn't blow up (https://github.com/eclipse/eclipse.jdt.ls/issues/675)
+		assertSame(item, server.resolveCompletionItem(item).join());
 	}
 
 	@Test