Bug 512251 - Fix IllegalArgumentException in ContextInformationPopupI20171011-2000I20171011-0305I20171010-2000I20171009-2000I20171008-2000I20171007-1500I20171006-2000I20171005-2000I20171004-2000I20171004-0315I20171003-2000I20171002-2000

Change-Id: I7c6eec0871e7f2b3a2a71c401b19e52a687f126e
Signed-off-by: Stephan Wahlbrink <sw@wahlbrink.eu>

6 files changed, 229 insertions, 15 deletions

diff --git a/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/ContentAssistSubjectControlAdapter.java b/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/ContentAssistSubjectControlAdapter.java
index 8d648d91b84..af6d754196e 100644
--- a/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/ContentAssistSubjectControlAdapter.java
+++ b/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/ContentAssistSubjectControlAdapter.java
@@ -8,6 +8,7 @@
  * Contributors:
  *     IBM Corporation - initial API and implementation
  *     Mickael Istria (Red Hat Inc.) - [251156] Allow multiple contentAssitProviders internally & inheritance
+ *     Stephan Wahlbrink <sw@wahlbrink.eu> - Bug 512251 - Fix IllegalArgumentException in ContextInformationPopup
  *******************************************************************************/
 package org.eclipse.jface.text.contentassist;
 
@@ -120,6 +121,14 @@ class ContentAssistSubjectControlAdapter implements IContentAssistSubjectControl
 		return fViewer.getTextWidget().getLineDelimiter();
 	}
 
+	public boolean isValidWidgetOffset(int widgetOffset) {
+		if (fContentAssistSubjectControl != null) {
+			IDocument document= fContentAssistSubjectControl.getDocument();
+			return (widgetOffset >= 0 && widgetOffset <= document.getLength());
+		}
+		return (widgetOffset >= 0 && widgetOffset <= fViewer.getTextWidget().getCharCount());
+	}
+
 	@Override
 	public void addKeyListener(KeyListener keyListener) {
 		if (fContentAssistSubjectControl != null)
diff --git a/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/ContextInformationPopup.java b/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/ContextInformationPopup.java
index f733d6dad4c..8315265a97f 100644
--- a/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/ContextInformationPopup.java
+++ b/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/ContextInformationPopup.java
@@ -7,6 +7,7 @@
  *
  * Contributors:
  *     IBM Corporation - initial API and implementation
+ *     Stephan Wahlbrink <sw@wahlbrink.eu> - Bug 512251 - Fix IllegalArgumentException in ContextInformationPopup
  *******************************************************************************/
 package org.eclipse.jface.text.contentassist;
 
@@ -274,7 +275,7 @@ class ContextInformationPopup implements IContentAssistListener {
 	 * @since 3.0
 	 */
 	private void internalShowContextInfo(ContextFrame frame) {
-		if (frame != null) {
+		if (frame != null && canShowFrame(frame)) {
 			fContextFrameStack.push(frame);
 			if (fContextFrameStack.size() == 1)
 				fLastContext= null;
@@ -336,6 +337,19 @@ class ContextInformationPopup implements IContentAssistListener {
 	}
 
 	/**
+	 * Pre-checks if the given context frame can be (re)shown.
+	 *
+	 * The function checks if the frame has valid position data. It does not call the context
+	 * information validator.
+	 *
+	 * @param frame the frame to check
+	 * @return <code>true</code> if the context frame OK to use, otherwise <code>false</code>
+	 */
+	private boolean canShowFrame(ContextFrame frame) {
+		return fContentAssistSubjectControlAdapter.isValidWidgetOffset(frame.fVisibleOffset);
+	}
+
+	/**
 	 * Shows the given context frame.
 	 *
 	 * @param frame the frame to display
@@ -457,16 +471,23 @@ class ContextInformationPopup implements IContentAssistListener {
 		if (Helper.okToUse(fContextInfoPopup)) {
 
 			int size= fContextFrameStack.size();
-			if (size > 0) {
+			while (size > 0) {
 				fLastContext= fContextFrameStack.pop();
 				-- size;
-			}
-
-			if (size > 0) {
-				ContextFrame current= fContextFrameStack.peek();
-				internalShowContextFrame(current, false);
-			} else {
 
+				if (size > 0) {
+					ContextFrame current= fContextFrameStack.peek();
+					if (canShowFrame(current)) {
+						internalShowContextFrame(current, false);
+						return;
+					}
+					// continue - try next
+				}
+				else {
+					break;
+				}
+			}
+			{
 				fContentAssistant.removeContentAssistListener(this, ContentAssistant.CONTEXT_INFO_POPUP);
 
 				if (fContentAssistSubjectControlAdapter.getControl() != null)
diff --git a/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/AbstratGenericEditorTest.java b/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/AbstratGenericEditorTest.java
index 2d31f0ad00e..ef1e3b0c4e5 100644
--- a/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/AbstratGenericEditorTest.java
+++ b/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/AbstratGenericEditorTest.java
@@ -23,11 +23,17 @@ import org.eclipse.core.resources.IFile;
 import org.eclipse.core.resources.IProject;
 import org.eclipse.core.resources.ResourcesPlugin;
 
+import org.eclipse.text.tests.Accessor;
+
+import org.eclipse.jface.text.source.SourceViewer;
+
 import org.eclipse.ui.PlatformUI;
 import org.eclipse.ui.internal.genericeditor.ExtensionBasedTextEditor;
 import org.eclipse.ui.intro.IIntroPart;
 import org.eclipse.ui.part.FileEditorInput;
 
+import org.eclipse.ui.texteditor.AbstractTextEditor;
+
 /**
  * Closes intro, create {@link #project}, create {@link #file} and open {@link #editor}; and clean up.
  * Also contains additional utility methods
@@ -81,7 +87,12 @@ public class AbstratGenericEditorTest {
 			file = null;
 		}
 	}
-
+	
+	protected SourceViewer getSourceViewer() {
+		SourceViewer sourceViewer= (SourceViewer) new Accessor(editor, AbstractTextEditor.class).invoke("getSourceViewer", new Object[0]);
+		return sourceViewer;
+	}
+	
 	@After
 	public void tearDown() throws Exception {
 		cleanFileAndEditor();
diff --git a/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/ContextInfoTest.java b/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/ContextInfoTest.java
new file mode 100644
index 00000000000..e92016c05fe
--- /dev/null
+++ b/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/ContextInfoTest.java
@@ -0,0 +1,126 @@
+/*******************************************************************************
+ * Copyright (c) 2017 Stephan Wahlbrink and others.
+ * 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
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     Stephan Wahlbrink <sw@wahlbrink.eu>
+ *******************************************************************************/
+package org.eclipse.ui.genericeditor.tests;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.util.Arrays;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.junit.After;
+import org.junit.Test;
+
+import org.eclipse.swt.custom.StyledText;
+import org.eclipse.swt.widgets.Control;
+import org.eclipse.swt.widgets.Shell;
+import org.eclipse.swt.widgets.Text;
+
+import org.eclipse.text.tests.Accessor;
+
+import org.eclipse.jface.text.contentassist.ContentAssistant;
+import org.eclipse.jface.text.source.SourceViewer;
+
+import org.eclipse.ui.genericeditor.tests.contributions.BarContentAssistProcessor;
+
+import org.eclipse.ui.texteditor.ITextEditorActionConstants;
+import org.eclipse.ui.texteditor.TextOperationAction;
+
+
+public class ContextInfoTest extends AbstratGenericEditorTest {
+
+
+	private Shell completionShell;
+
+
+	@Test
+	public void testContextInfo() throws Exception {
+		cleanFileAndEditor();
+		createAndOpenFile("bar.txt", BarContentAssistProcessor.PROPOSAL);
+
+		final Set<Shell> beforeShells = Arrays.stream(editor.getSite().getShell().getDisplay().getShells()).filter(Shell::isVisible).collect(Collectors.toSet());
+		TextOperationAction action = (TextOperationAction) editor.getAction(ITextEditorActionConstants.CONTENT_ASSIST_CONTEXT_INFORMATION);
+
+		editor.selectAndReveal(4, 0);
+		action.update();
+		action.run();
+		waitAndDispatch(100);
+		this.completionShell= findNewShell(beforeShells);
+		assertEquals("idx= 0", getInfoText(this.completionShell));
+
+		editor.selectAndReveal(8, 0);
+		action.update();
+		action.run();
+		waitAndDispatch(100);
+		this.completionShell= findNewShell(beforeShells);
+		assertEquals("idx= 1", getInfoText(this.completionShell));
+	}
+
+	@Test
+	public void testContextInfo_hide_Bug512251() throws Exception {
+		cleanFileAndEditor();
+		createAndOpenFile("bar.txt", BarContentAssistProcessor.PROPOSAL);
+
+		final Set<Shell> beforeShells = Arrays.stream(editor.getSite().getShell().getDisplay().getShells()).filter(Shell::isVisible).collect(Collectors.toSet());
+		TextOperationAction action = (TextOperationAction) editor.getAction(ITextEditorActionConstants.CONTENT_ASSIST_CONTEXT_INFORMATION);
+
+		editor.selectAndReveal(4, 0);
+		action.update();
+		action.run();
+		waitAndDispatch(100);
+		this.completionShell= findNewShell(beforeShells);
+
+		editor.selectAndReveal(8, 0);
+		action.update();
+		action.run();
+		waitAndDispatch(100);
+		this.completionShell= findNewShell(beforeShells);
+
+		editor.getAction(ITextEditorActionConstants.DELETE_LINE).run();
+
+		SourceViewer sourceViewer= getSourceViewer();
+		ContentAssistant assist= (ContentAssistant) new Accessor(sourceViewer, SourceViewer.class).get("fContentAssistant");
+		new Accessor(assist, ContentAssistant.class).invoke("hide", new Object[0]);
+	}
+
+
+	private Shell findNewShell(Set<Shell> beforeShells) {
+		Shell[] afterShells = Arrays.stream(editor.getSite().getShell().getDisplay().getShells())
+				.filter(Shell::isVisible)
+				.filter(shell -> !beforeShells.contains(shell))
+				.toArray(Shell[]::new);
+		assertEquals("No new shell found", 1, afterShells.length);
+		return afterShells[0];
+	}
+
+	private String getInfoText(final Shell shell) {
+		assertTrue(shell.isVisible());
+		Control[] children= shell.getChildren();
+		for (Control child : children) {
+			if (child instanceof Text) {
+				return ((Text) child).getText();
+			}
+			if (child instanceof StyledText) {
+				return ((StyledText) child).getText();
+			}
+		}
+		return null;
+	}
+
+	@After
+	public void closeShell() {
+		if (this.completionShell != null && !completionShell.isDisposed()) {
+			completionShell.close();
+		}
+	}
+
+}
diff --git a/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/GenericEditorTestSuite.java b/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/GenericEditorTestSuite.java
index bf987dcca97..76c9c1c1a19 100644
--- a/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/GenericEditorTestSuite.java
+++ b/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/GenericEditorTestSuite.java
@@ -17,6 +17,7 @@ import org.junit.runners.Suite.SuiteClasses;
 @RunWith(Suite.class)
 @SuiteClasses({
 		CompletionTest.class,
+		ContextInfoTest.class,
 		StylingTest.class,
 		HoverTest.class,
 		EditorTest.class,
diff --git a/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/contributions/BarContentAssistProcessor.java b/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/contributions/BarContentAssistProcessor.java
index f53da979686..9c22f88cd32 100644
--- a/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/contributions/BarContentAssistProcessor.java
+++ b/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/contributions/BarContentAssistProcessor.java
@@ -7,12 +7,18 @@
  *
  * Contributors:
  * - Mickael Istria (Red Hat Inc.)
+ *     Stephan Wahlbrink <sw@wahlbrink.eu> - Bug 512251 - Fix IllegalArgumentException in ContextInformationPopup
  *******************************************************************************/
 package org.eclipse.ui.genericeditor.tests.contributions;
 
+import java.util.Arrays;
+
 import org.eclipse.jface.text.BadLocationException;
+import org.eclipse.jface.text.IDocument;
+import org.eclipse.jface.text.IRegion;
 import org.eclipse.jface.text.ITextViewer;
 import org.eclipse.jface.text.contentassist.CompletionProposal;
+import org.eclipse.jface.text.contentassist.ContextInformation;
 import org.eclipse.jface.text.contentassist.ICompletionProposal;
 import org.eclipse.jface.text.contentassist.IContentAssistProcessor;
 import org.eclipse.jface.text.contentassist.IContextInformation;
@@ -49,11 +55,6 @@ public class BarContentAssistProcessor implements IContentAssistProcessor {
 	}
 
 	@Override
-	public IContextInformation[] computeContextInformation(ITextViewer viewer, int offset) {
-		return null;
-	}
-
-	@Override
 	public char[] getCompletionProposalAutoActivationCharacters() {
 		return null;
 	}
@@ -63,13 +64,58 @@ public class BarContentAssistProcessor implements IContentAssistProcessor {
 		return null;
 	}
 
+	/**
+	 * Creates context info "idx= <word index in #PROPOSAL>" at the end of a word.
+	 **/
 	@Override
-	public String getErrorMessage() {
+	public IContextInformation[] computeContextInformation(ITextViewer viewer, int offset) {
+		try {
+			IDocument document= viewer.getDocument();
+			int begin= offset;
+			while (begin > 0 && Character.isLetterOrDigit(document.getChar(begin - 1))) {
+				begin--;
+			}
+			if (begin < offset) {
+				String word= document.get(begin, offset - begin);
+				int idx= Arrays.asList(completeString.split("\\W")).indexOf(word);
+				if (idx >= 0) {
+					return new IContextInformation[] {
+							new ContextInformation(word, "idx= " + idx)
+					};
+				}
+			}
+		} catch (BadLocationException e) {
+			e.printStackTrace();
+		}
 		return null;
 	}
 
 	@Override
 	public IContextInformationValidator getContextInformationValidator() {
+		return new IContextInformationValidator() {
+			ITextViewer viewer;
+			int offset;
+			@Override
+			public void install(IContextInformation info, ITextViewer viewer, int offset) {
+				this.viewer= viewer;
+				this.offset= offset;
+			}
+			@Override
+			public boolean isContextInformationValid(int offset) {
+				try {
+					IDocument document= viewer.getDocument();
+					IRegion line= document.getLineInformationOfOffset(this.offset);
+					int end= line.getOffset() + line.getLength();
+					return (offset >= this.offset && offset < end);
+				} catch (BadLocationException e) {
+					return false;
+				}
+			}
+		};
+	}
+
+	@Override
+	public String getErrorMessage() {
 		return null;
 	}