Bug 564305 - fixed unsorted filtered proposals on incremental completeY20200711-1200Y20200708-1200Y20200704-1200Y20200703-0300S4_17_0_M1I20200713-2230I20200713-1800I20200712-1800I20200711-2250I20200711-1800I20200710-1800I20200710-0230I20200708-1800I20200708-0600I20200707-1800I20200707-0600I20200706-2300I20200706-1800I20200706-0600I20200705-1800I20200705-0710I20200705-0600I20200704-1800I20200704-0600I20200703-1800I20200702-2150I20200702-1800

On incremental completion with async content assist and insert common
prefix enabled, the proposal order in fFilteredProposals did not match
the displayed order, causing unexpected proposals to be inserted.

The fComputedPropsals list will not be sorted in this scenario, so it
can't be used to overwrite fFilteredProposals.
fFilteredProposals seems to be always initialized by
computeAndPopulateProposals anyway.

Change-Id: I0da117281502b0a11ea462a38e129e3c59c252f1
Signed-off-by: Julian Honnen <julian.honnen@vector.com>

7 files changed, 182 insertions, 32 deletions

diff --git a/org.eclipse.jface.text.tests/META-INF/MANIFEST.MF b/org.eclipse.jface.text.tests/META-INF/MANIFEST.MF
index af38031bc94..861096ba73f 100644
--- a/org.eclipse.jface.text.tests/META-INF/MANIFEST.MF
+++ b/org.eclipse.jface.text.tests/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %Plugin.name
 Bundle-SymbolicName: org.eclipse.jface.text.tests
-Bundle-Version: 3.11.1100.qualifier
+Bundle-Version: 3.11.1200.qualifier
 Bundle-Vendor: %Plugin.providerName
 Bundle-Localization: plugin
 Export-Package: 
diff --git a/org.eclipse.jface.text.tests/pom.xml b/org.eclipse.jface.text.tests/pom.xml
index 74f7dec35b6..4cd808de798 100644
--- a/org.eclipse.jface.text.tests/pom.xml
+++ b/org.eclipse.jface.text.tests/pom.xml
@@ -19,7 +19,7 @@
   </parent>
   <groupId>org.eclipse.jface</groupId>
   <artifactId>org.eclipse.jface.text.tests</artifactId>
-  <version>3.11.1100-SNAPSHOT</version>
+  <version>3.11.1200-SNAPSHOT</version>
   <packaging>eclipse-test-plugin</packaging>
   <properties>
   	<testSuite>${project.artifactId}</testSuite>
diff --git a/org.eclipse.jface.text.tests/src/org/eclipse/jface/text/tests/JFaceTextTestSuite.java b/org.eclipse.jface.text.tests/src/org/eclipse/jface/text/tests/JFaceTextTestSuite.java
index 05f39a73d76..29c36033029 100644
--- a/org.eclipse.jface.text.tests/src/org/eclipse/jface/text/tests/JFaceTextTestSuite.java
+++ b/org.eclipse.jface.text.tests/src/org/eclipse/jface/text/tests/JFaceTextTestSuite.java
@@ -23,6 +23,7 @@ import org.eclipse.jface.text.tests.contentassist.AsyncContentAssistTest;
 import org.eclipse.jface.text.tests.contentassist.ContextInformationPresenterTest;
 import org.eclipse.jface.text.tests.contentassist.ContextInformationTest;
 import org.eclipse.jface.text.tests.contentassist.FilteringAsyncContentAssistTests;
+import org.eclipse.jface.text.tests.contentassist.IncrementalAsyncContentAssistTests;
 import org.eclipse.jface.text.tests.reconciler.AbstractReconcilerTest;
 import org.eclipse.jface.text.tests.rules.DefaultPartitionerTest;
 import org.eclipse.jface.text.tests.rules.DefaultPartitionerZeroLengthTest;
@@ -53,6 +54,7 @@ import org.eclipse.jface.text.tests.templates.persistence.TemplatePersistenceDat
 		DefaultPairMatcherTest2.class,
 		AsyncContentAssistTest.class,
 		FilteringAsyncContentAssistTests.class,
+		IncrementalAsyncContentAssistTests.class,
 		ContextInformationTest.class,
 		ContextInformationPresenterTest.class,
 
diff --git a/org.eclipse.jface.text.tests/src/org/eclipse/jface/text/tests/contentassist/FilteringAsyncContentAssistTests.java b/org.eclipse.jface.text.tests/src/org/eclipse/jface/text/tests/contentassist/FilteringAsyncContentAssistTests.java
index 405b41c7078..4ff60959401 100644
--- a/org.eclipse.jface.text.tests/src/org/eclipse/jface/text/tests/contentassist/FilteringAsyncContentAssistTests.java
+++ b/org.eclipse.jface.text.tests/src/org/eclipse/jface/text/tests/contentassist/FilteringAsyncContentAssistTests.java
@@ -10,10 +10,13 @@
  *******************************************************************************/
 package org.eclipse.jface.text.tests.contentassist;
 
+import static java.util.Collections.singletonList;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
 import java.lang.reflect.Field;
+import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 import java.util.concurrent.CountDownLatch;
 import java.util.function.Predicate;
@@ -35,12 +38,14 @@ import org.eclipse.jface.text.BadLocationException;
 import org.eclipse.jface.text.Document;
 import org.eclipse.jface.text.DocumentEvent;
 import org.eclipse.jface.text.IDocument;
+import org.eclipse.jface.text.IInformationControlCreator;
 import org.eclipse.jface.text.ITextViewer;
 import org.eclipse.jface.text.contentassist.ContentAssistant;
 import org.eclipse.jface.text.contentassist.ContextInformationValidator;
 import org.eclipse.jface.text.contentassist.ICompletionProposal;
 import org.eclipse.jface.text.contentassist.ICompletionProposalExtension;
 import org.eclipse.jface.text.contentassist.ICompletionProposalExtension2;
+import org.eclipse.jface.text.contentassist.ICompletionProposalExtension3;
 import org.eclipse.jface.text.contentassist.IContentAssistProcessor;
 import org.eclipse.jface.text.contentassist.IContextInformation;
 import org.eclipse.jface.text.contentassist.IContextInformationValidator;
@@ -203,7 +208,7 @@ public class FilteringAsyncContentAssistTests {
 		IDocument document = viewer.getDocument();
 
 		ca.addContentAssistProcessor(new ImmediateContentAssistProcessor("xx"), IDocument.DEFAULT_CONTENT_TYPE);
-		ca.addContentAssistProcessor(new DelayedContentAssistProcessor("yy", 3000, false),
+		ca.addContentAssistProcessor(new DelayedContentAssistProcessor(singletonList("yy"), 3000, false),
 				IDocument.DEFAULT_CONTENT_TYPE);
 
 		ca.install(viewer);
@@ -247,7 +252,7 @@ public class FilteringAsyncContentAssistTests {
 	public void testCA_WithFirstDelayedThenImmediateProposals() throws Exception {
 		IDocument document = viewer.getDocument();
 
-		ca.addContentAssistProcessor(new LongInitialContentAssistProcessor("abc", 500, true),
+		ca.addContentAssistProcessor(new LongInitialContentAssistProcessor(singletonList("abc"), 500, true),
 				IDocument.DEFAULT_CONTENT_TYPE);
 
 		ca.install(viewer);
@@ -287,7 +292,7 @@ public class FilteringAsyncContentAssistTests {
 		IDocument document = viewer.getDocument();
 
 		ca.addContentAssistProcessor(new ImmediateContentAssistProcessor("xxxx"), IDocument.DEFAULT_CONTENT_TYPE);
-		ca.addContentAssistProcessor(new DelayedContentAssistProcessor("yyyy", 5000, false),
+		ca.addContentAssistProcessor(new DelayedContentAssistProcessor(singletonList("yyyy"), 5000, false),
 				IDocument.DEFAULT_CONTENT_TYPE);
 
 		ca.install(viewer);
@@ -354,37 +359,38 @@ public class FilteringAsyncContentAssistTests {
 
 	}
 
-	private class ImmediateContentAssistProcessor implements IContentAssistProcessor {
+	static class ImmediateContentAssistProcessor implements IContentAssistProcessor {
 
-		final private String template;
+		final private List<String> templates;
 		final private boolean incomplete;
 
-		ImmediateContentAssistProcessor(String template) {
-			this(template, false);
+		ImmediateContentAssistProcessor(String... templates) {
+			this(Arrays.asList(templates), false);
 		}
 
-		ImmediateContentAssistProcessor(String template, boolean incomplete) {
-			this.template = template;
+		ImmediateContentAssistProcessor(List<String> templates, boolean incomplete) {
+			this.templates= templates;
 			this.incomplete = incomplete;
 		}
 
 		@Override
 		public ICompletionProposal[] computeCompletionProposals(ITextViewer textViewer, int offset) {
+			List<ICompletionProposal> proposals= new ArrayList<>();
 			try {
 				IDocument document= textViewer.getDocument();
-				if (document != null && (document.getLength() == 0 || isSubstringFoundOrderedInString(document.get(0, offset), template))) {
-					if (incomplete) {
-						return new ICompletionProposal[] {
-								new IncompleteCompletionProposal(template, offset, 0, offset, template) };
-					} else {
-						CompletionProposal proposal = new CompletionProposal(template, offset, 0, offset, template);
-						return new ICompletionProposal[] { proposal };
+				for (String template : templates) {
+					if (document != null && (document.getLength() == 0 || isSubstringFoundOrderedInString(document.get(0, offset), template))) {
+						if (incomplete) {
+							proposals.add(new IncompleteCompletionProposal(template, offset, 0, offset, template));
+						} else {
+							proposals.add(new CompletionProposal(template, offset, 0, offset, template));
+						}
 					}
 				}
 			} catch (BadLocationException e) {
 				throw new IllegalStateException("Error computing proposals");
 			}
-			return new ICompletionProposal[0];
+			return proposals.toArray(new ICompletionProposal[0]);
 		}
 
 		@Override
@@ -414,12 +420,12 @@ public class FilteringAsyncContentAssistTests {
 
 	}
 
-	private class DelayedContentAssistProcessor extends ImmediateContentAssistProcessor {
+	static class DelayedContentAssistProcessor extends ImmediateContentAssistProcessor {
 
 		protected long delay;
 
-		DelayedContentAssistProcessor(String template, long delay, boolean incomplete) {
-			super(template, incomplete);
+		DelayedContentAssistProcessor(List<String> templates, long delay, boolean incomplete) {
+			super(templates, incomplete);
 			this.delay = delay;
 		}
 
@@ -432,14 +438,14 @@ public class FilteringAsyncContentAssistTests {
 					throw new IllegalStateException("Cannot generate delayed content assist proposals!");
 				}
 			}
-			return super.computeCompletionProposals(viewer, offset);
+			return super.computeCompletionProposals(textViewer, offset);
 		}
 	}
 
 	private class LongInitialContentAssistProcessor extends DelayedContentAssistProcessor {
 
-		LongInitialContentAssistProcessor(String template, long delay, boolean incomplete) {
-			super(template, delay, incomplete);
+		LongInitialContentAssistProcessor(List<String> templates, long delay, boolean incomplete) {
+			super(templates, delay, incomplete);
 		}
 
 		@Override
@@ -454,8 +460,8 @@ public class FilteringAsyncContentAssistTests {
 
 		final CountDownLatch blocked= new CountDownLatch(1);
 
-		BlockingProcessor(String template) {
-			super(template, false);
+		BlockingProcessor(String... templates) {
+			super(Arrays.asList(templates), false);
 		}
 
 		@Override
@@ -484,7 +490,7 @@ public class FilteringAsyncContentAssistTests {
 	}
 
 	@SuppressWarnings("unchecked")
-	private static List<ICompletionProposal> getFilteredProposals(ContentAssistant ca) throws Exception {
+	static List<ICompletionProposal> getFilteredProposals(ContentAssistant ca) throws Exception {
 		Field f = ContentAssistant.class.getDeclaredField("fProposalPopup");
 		f.setAccessible(true);
 		Object caPopup = f.get(ca);
@@ -566,8 +572,8 @@ public class FilteringAsyncContentAssistTests {
 		}
 	}
 
-	private static class CompletionProposal extends IncompleteCompletionProposal
-		implements ICompletionProposalExtension, ICompletionProposalExtension2 {
+	static class CompletionProposal extends IncompleteCompletionProposal
+			implements ICompletionProposalExtension, ICompletionProposalExtension2, ICompletionProposalExtension3 {
 
 		public CompletionProposal(String replacementString, int replacementOffset, int replacementLength,
 				int cursorPosition, String displayString) {
@@ -624,6 +630,25 @@ public class FilteringAsyncContentAssistTests {
 			return 0;
 		}
 
+		@Override
+		public int getPrefixCompletionStart(IDocument document, int completionOffset) {
+			return 0;
+		}
+
+		@Override
+		public CharSequence getPrefixCompletionText(IDocument document, int completionOffset) {
+			return getDisplayString();
+		}
+
+		@Override
+		public IInformationControlCreator getInformationControlCreator() {
+			return null;
+		}
+
+		@Override
+		public String toString() {
+			return getDisplayString();
+		}
 	}
 
 	@SuppressWarnings("boxing")
diff --git a/org.eclipse.jface.text.tests/src/org/eclipse/jface/text/tests/contentassist/IncrementalAsyncContentAssistTests.java b/org.eclipse.jface.text.tests/src/org/eclipse/jface/text/tests/contentassist/IncrementalAsyncContentAssistTests.java
new file mode 100644
index 00000000000..ef79f05b4e4
--- /dev/null
+++ b/org.eclipse.jface.text.tests/src/org/eclipse/jface/text/tests/contentassist/IncrementalAsyncContentAssistTests.java
@@ -0,0 +1,124 @@
+/*******************************************************************************
+ * Copyright (c) 2020 Julian Honnen
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     Julian Honnen - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.jface.text.tests.contentassist;
+
+import static java.util.stream.Collectors.toList;
+import static org.junit.Assert.assertEquals;
+
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.List;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.widgets.Shell;
+
+import org.eclipse.jface.text.Document;
+import org.eclipse.jface.text.IDocument;
+import org.eclipse.jface.text.contentassist.ContentAssistant;
+import org.eclipse.jface.text.contentassist.ICompletionProposal;
+import org.eclipse.jface.text.source.SourceViewer;
+import org.eclipse.jface.text.tests.util.DisplayHelper;
+
+public class IncrementalAsyncContentAssistTests {
+
+	private Shell shell;
+
+	private SourceViewer viewer;
+
+	private ContentAssistant ca;
+
+	@Before
+	public void setup() {
+		tearDown();
+
+		shell= new Shell();
+		shell.setSize(300, 300);
+		shell.open();
+
+		viewer= new SourceViewer(shell, null, SWT.NONE);
+		Document document= new Document();
+		viewer.setDocument(document);
+		ca= new ContentAssistant(true);
+
+		Comparator<ICompletionProposal> comparator= Comparator.comparing(p -> p.getDisplayString());
+		ca.setSorter(comparator::compare);
+	}
+
+	@After
+	public void tearDown() {
+		if (shell != null) {
+			ca.uninstall();
+			if (!shell.isDisposed()) {
+				shell.dispose();
+			}
+			shell= null;
+		}
+	}
+
+	@Test
+	public void testIncrementalComplete() throws Exception {
+		ca.addContentAssistProcessor(new FilteringAsyncContentAssistTests.ImmediateContentAssistProcessor("testC", "testB", "testA"), IDocument.DEFAULT_CONTENT_TYPE);
+
+		viewer.getDocument().set("t");
+
+		ca.install(viewer);
+		viewer.setSelectedRange(1, 0);
+
+		ca.completePrefix();
+
+		DisplayHelper.sleep(shell.getDisplay(), 300);
+
+		List<String> filteredProposals= FilteringAsyncContentAssistTests.getFilteredProposals(ca).stream() //
+				.map(p -> p.getDisplayString()) //
+				.collect(toList());
+		assertEquals(Arrays.asList("testA", "testB", "testC"), filteredProposals);
+	}
+
+	@Test
+	public void testIncrementalComplete_async() throws Exception {
+		long delay= 200;
+		ca.addContentAssistProcessor(new FilteringAsyncContentAssistTests.DelayedContentAssistProcessor(Arrays.asList("testC", "testB", "testA"), delay, false), IDocument.DEFAULT_CONTENT_TYPE);
+
+		viewer.getDocument().set("t");
+
+		ca.install(viewer);
+		viewer.setSelectedRange(1, 0);
+
+		ca.completePrefix();
+
+		DisplayHelper.sleep(shell.getDisplay(), delay + 100);
+
+		List<String> filteredProposals= FilteringAsyncContentAssistTests.getFilteredProposals(ca).stream() //
+				.map(p -> p.getDisplayString()) //
+				.collect(toList());
+		assertEquals(Arrays.asList("testA", "testB", "testC"), filteredProposals);
+	}
+
+	@Test
+	public void testIncrementalCompleteOfSingleProposal() throws Exception {
+		ca.enableAutoInsert(true);
+		ca.addContentAssistProcessor(new FilteringAsyncContentAssistTests.ImmediateContentAssistProcessor("testA"), IDocument.DEFAULT_CONTENT_TYPE);
+
+		ca.install(viewer);
+		viewer.setSelectedRange(0, 0);
+
+		ca.completePrefix();
+
+		DisplayHelper.sleep(shell.getDisplay(), 300);
+
+		assertEquals("testA", viewer.getDocument().get());
+	}
+
+}
diff --git a/org.eclipse.jface.text/META-INF/MANIFEST.MF b/org.eclipse.jface.text/META-INF/MANIFEST.MF
index 7b622c5bfdf..02dfbb32207 100644
--- a/org.eclipse.jface.text/META-INF/MANIFEST.MF
+++ b/org.eclipse.jface.text/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.jface.text
-Bundle-Version: 3.16.300.qualifier
+Bundle-Version: 3.16.400.qualifier
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin
 Export-Package: 
diff --git a/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/AsyncCompletionProposalPopup.java b/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/AsyncCompletionProposalPopup.java
index 30b06d89c0d..b29c6b2bc06 100644
--- a/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/AsyncCompletionProposalPopup.java
+++ b/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/AsyncCompletionProposalPopup.java
@@ -293,7 +293,6 @@ class AsyncCompletionProposalPopup extends CompletionProposalPopup {
 				displayProposals();
 			}
 		}, true, false, true);
-		fFilteredProposals= new ArrayList<>(fComputedProposals != null ? fComputedProposals : Collections.emptyList());
 		return getErrorMessage();
 	}