Only append data on completion item selected

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/contentassist/CompletionRanking.java

@@ -105,7 +105,10 @@ public Map<String, String> getData() {
 	/**
 	 * A map data structure that will be appended to completion item's data field. When a completion
 	 * item is selected, the selected item will be passed to the provider. Providers can use the stored
-	 * data to do post process on demand.
+	 * data to do post process on demand. Please note that the data will only be appended when item
+	 * selected event happens. The data will not exist during textDocument/completion and completionItem/resolve
+	 * phases.
+	 *
 	 * <p>
 	 * The key <code>"COMPLETION_EXECUTION_TIME"</code> is preserved to store the time calculating all the
 	 * completion items at the server side in millisecond.

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/contentassist/CompletionProposalRequestor.java

@@ -16,6 +16,7 @@
 import java.util.Arrays;
 import java.util.Comparator;
 import java.util.HashMap;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -244,6 +245,8 @@ public List<CompletionItem> getCompletionItems(IProgressMonitor monitor) {
 		if (!proposals.isEmpty()){
 			initializeCompletionListItemDefaults(proposals.get(0));
 		}
+
+		List<Map<String, String>> contributedData = new LinkedList<>();
 		//Let's compute replacement texts for the most relevant results only
 		for (int i = 0; i < limit; i++) {
 			CompletionProposal proposal = proposals.get(i);
@@ -260,11 +263,8 @@ public List<CompletionItem> getCompletionItems(IProgressMonitor monitor) {
 							item.setFilterText(item.getInsertText());
 						}
 					}
-					Map<String, String> itemData = (Map<String, String>) item.getData();
 					Map<String, String> rankingData = rankingResult.getData();
-					for (String key : rankingData.keySet()) {
-						itemData.put(key, rankingData.get(key));
-					}
+					contributedData.add(rankingData);
 				}
 				completionItems.add(item);
 			} catch (Exception e) {
@@ -281,6 +281,7 @@ public List<CompletionItem> getCompletionItems(IProgressMonitor monitor) {
 		}
 		response.setItems(completionItems);
 		response.setCommonData(CompletionResolveHandler.DATA_FIELD_URI, uri);
+		response.setCompletionItemData(contributedData);
 		CompletionResponses.store(response);
 
 		return completionItems;

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CompletionHandler.java

@@ -178,6 +178,11 @@ public void onDidCompletionItemSelect(String requestId, String proposalId) throw
 			((Map<String, String>)item.getData()).put(CompletionRanking.COMPLETION_EXECUTION_TIME, executionTime);
 		}
 
+		Map<String, String> contributedData = completionResponse.getCompletionItemData(pId);
+		if (contributedData != null) {
+			((Map<String, String>)item.getData()).putAll(contributedData);
+		}
+
 		List<ICompletionRankingProvider> providers =
 				((CompletionContributionService) JavaLanguageServerPlugin.getCompletionContributionService()).getRankingProviders();
 		for (ICompletionRankingProvider provider : providers) {

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CompletionResolveHandler.java

@@ -93,8 +93,6 @@ public CompletionResolveHandler(PreferenceManager manager) {
 	public static final String DATA_FIELD_NAME = "name";
 	public static final String DATA_FIELD_REQUEST_ID = "rid";
 	public static final String DATA_FIELD_PROPOSAL_ID = "pid";
-	public static final String DATA_FIELD_CONSTANT_VALUE = "constant_value";
-	public static final String DATA_METHOD_DEFAULT_VALUE = "default_value";
 
 	public CompletionItem resolve(CompletionItem param, IProgressMonitor monitor) {
 

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CompletionResponse.java

@@ -12,6 +12,7 @@
  *******************************************************************************/
 package org.eclipse.jdt.ls.core.internal.handlers;
 
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -38,6 +39,11 @@ public class CompletionResponse {
 	private Map<String, String> commonData = new HashMap<>();
 	private List<CompletionProposal> proposals;
 	private List<CompletionItem> items;
+	/**
+	 * Stores the data that are specific to each completion item.
+	 * Those data are contributed by the ranking providers.
+	 */
+	private List<Map<String, String>> completionItemData;
 
 	public CompletionResponse() {
 		id = idSeed.getAndIncrement();
@@ -109,4 +115,15 @@ public List<CompletionItem> getItems() {
 	public void setItems(List<CompletionItem> items) {
 		this.items = items;
 	}
+
+	public Map<String, String> getCompletionItemData(int index) {
+		if (completionItemData == null || index >= completionItemData.size()) {
+			return Collections.emptyMap();
+		}
+		return completionItemData.get(index);
+	}
+
+	public void setCompletionItemData(List<Map<String, String>> completionItemData) {
+		this.completionItemData = completionItemData;
+	}
 }

org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/CompletionRankingProviderTest.java

@@ -16,8 +16,13 @@
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.reset;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
-import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -36,7 +41,12 @@
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
 
+@RunWith(MockitoJUnitRunner.class)
 public class CompletionRankingProviderTest extends AbstractCompilationUnitBasedTest {
 
 	private static String COMPLETION_TEMPLATE =
@@ -55,18 +65,20 @@ public class CompletionRankingProviderTest extends AbstractCompilationUnitBasedT
 					"    \"jsonrpc\": \"2.0\"\n" +
 					"}";
 
+	@Mock
 	private TestRankingProvider provider;
 
 	@Before
 	public void setUp() {
-		provider = new TestRankingProvider();
+		when(provider.rank(any(), any(), any(), any())).thenCallRealMethod();
+		doNothing().when(provider).onDidCompletionItemSelect(any());
 		JavaLanguageServerPlugin.getCompletionContributionService().registerRankingProvider(provider);
 	}
 
 	@After
 	public void tearDown() {
 		JavaLanguageServerPlugin.getCompletionContributionService().unregisterRankingProvider(provider);
-		provider = null;
+		reset(provider);
 	}
 
 	@Test
@@ -85,27 +97,32 @@ public void testRank() throws Exception {
 
 		CompletionItem recommended = list.getItems().get(0);
 		assertTrue(recommended.getLabel().startsWith("★"));
-		assertTrue(((Map)recommended.getData()).containsKey("foo"));
 		assertEquals(recommended.getFilterText(), recommended.getInsertText());
 	}
 
 	@Test
-	public void testOnDidCompletionItemSelect() throws Exception {
+	public void testOnDidCompletionItemSelect() throws Exception {ICompilationUnit unit = getWorkingCopy(
+				"src/java/Foo.java",
+				"public class Foo {\n"+
+						"	void foo() {\n"+
+						" 		Integer.\n" +
+						"	}\n"+
+				"}\n");
+
+		requestCompletions(unit, "Integer.");
 		CompletionHandler handler = new CompletionHandler(JavaLanguageServerPlugin.getPreferencesManager());
-		CompletionResponse response = new CompletionResponse();
-		CompletionItem completionItem = new CompletionItem();
-		completionItem.setData(new HashMap<>());
-		response.setItems(Arrays.asList(completionItem));
-		CompletionResponses.store(response);
-		handler.onDidCompletionItemSelect(String.valueOf(response.getId()), "0");
-
-		assertTrue(provider.onDidCompletionItemSelectInvoked);
+
+		ArgumentCaptor<CompletionItem> argument = ArgumentCaptor.forClass(CompletionItem.class);
+		handler.onDidCompletionItemSelect(String.valueOf((new CompletionResponse()).getId() - 1), "0");
+
+		verify(provider, times(1)).onDidCompletionItemSelect(argument.capture());
+		Map<String, String> data = (Map<String, String>) argument.getValue().getData();
+		assertEquals("bar", data.get("foo"));
+		assertTrue(data.containsKey(CompletionRanking.COMPLETION_EXECUTION_TIME));
 	}
 
 	class TestRankingProvider implements ICompletionRankingProvider {
 
-		boolean onDidCompletionItemSelectInvoked = false;
-
 		@Override
 		public CompletionRanking[] rank(List<CompletionProposal> proposals, org.eclipse.jdt.core.CompletionContext context, ICompilationUnit unit, IProgressMonitor monitor) {
 			CompletionRanking[] rankings = new CompletionRanking[proposals.size()];
@@ -120,7 +137,6 @@ public CompletionRanking[] rank(List<CompletionProposal> proposals, org.eclipse.
 
 		@Override
 		public void onDidCompletionItemSelect(CompletionItem item) {
-			onDidCompletionItemSelectInvoked = true;
 		}
 	}