Bug 546793 - [code mining] parameter names shifting offset

The code minings can be redrawn with data based on a state that doesn't
match the actual viewer content because of a change since code mining
update was triggered.
This cancels the code mining update in such case, to avoid erroneous
redraw, and only rely on usual and generic annotation shifting in text
viewer (that works well).

Change-Id: I4f370f837ccb9df2749f4fcac46fa17ce12673d3
Signed-off-by: Mickael Istria <mistria@redhat.com>
diff --git a/org.eclipse.jdt.text.tests/src/org/eclipse/jdt/text/tests/codemining/ParameterNamesCodeMiningTest.java b/org.eclipse.jdt.text.tests/src/org/eclipse/jdt/text/tests/codemining/ParameterNamesCodeMiningTest.java
index a2c6573..c8b6de5 100644
--- a/org.eclipse.jdt.text.tests/src/org/eclipse/jdt/text/tests/codemining/ParameterNamesCodeMiningTest.java
+++ b/org.eclipse.jdt.text.tests/src/org/eclipse/jdt/text/tests/codemining/ParameterNamesCodeMiningTest.java
@@ -34,7 +34,6 @@
 import org.eclipse.jface.preference.IPreferenceStore;
 
 import org.eclipse.jface.text.codemining.ICodeMiningProvider;
-import org.eclipse.jface.text.source.ISourceViewer;
 import org.eclipse.jface.text.source.projection.ProjectionViewer;
 import org.eclipse.jface.text.tests.util.DisplayHelper;
 
@@ -55,7 +54,9 @@
 import org.eclipse.jdt.ui.PreferenceConstants;
 
 import org.eclipse.jdt.internal.ui.JavaPlugin;
+import org.eclipse.jdt.internal.ui.javaeditor.CompilationUnitEditor;
 import org.eclipse.jdt.internal.ui.javaeditor.EditorUtility;
+import org.eclipse.jdt.internal.ui.javaeditor.JavaCodeMiningReconciler;
 import org.eclipse.jdt.internal.ui.javaeditor.JavaEditor;
 import org.eclipse.jdt.internal.ui.javaeditor.JavaSourceViewer;
 import org.eclipse.jdt.internal.ui.javaeditor.codemining.JavaMethodParameterCodeMiningProvider;
@@ -69,6 +70,7 @@
 	private IJavaProject fProject;
 	private IPackageFragment fPackage;
 	private JavaMethodParameterCodeMiningProvider fParameterNameCodeMiningProvider;
+	private boolean wasCodeMiningEnabled;
 
 	public static Test suite() {
 		return new TestSuite(ParameterNamesCodeMiningTest.class);
@@ -86,58 +88,75 @@
 		IPackageFragmentRoot root= JavaProjectHelper.addSourceContainer(fProject, "src");
 		fPackage= root.getPackageFragment("");
 		fParameterNameCodeMiningProvider= new JavaMethodParameterCodeMiningProvider();
+		wasCodeMiningEnabled= PreferenceConstants.getPreferenceStore().getBoolean(PreferenceConstants.EDITOR_CODEMINING_ENABLED);
+		PreferenceConstants.getPreferenceStore().setValue(PreferenceConstants.EDITOR_CODEMINING_ENABLED, true);
 	}
 
 	@Override
 	@After
 	protected void tearDown() throws Exception {
 		super.tearDown();
+		PreferenceConstants.getPreferenceStore().setValue(PreferenceConstants.EDITOR_CODEMINING_ENABLED, wasCodeMiningEnabled);
 		IWorkbenchPage workbenchPage= PlatformUI.getWorkbench().getActiveWorkbenchWindow().getActivePage();
 		for (IEditorReference ref : workbenchPage.getEditorReferences()) {
 			workbenchPage.closeEditor(ref.getEditor(false), false);
 		}
 	}
 
+	private void waitReconciled(JavaSourceViewer viewer) {
+		assertTrue("Editor not reconciled", new DisplayHelper() {
+			@Override
+			protected boolean condition() {
+				return JavaCodeMiningReconciler.isReconciled(viewer);
+			}
+		}.waitForCondition(viewer.getTextWidget().getDisplay(), 2000));
+	}
+
 	public void testParameterNamesOK() throws Exception {
 		String contents= "public class Foo {\n" +
-				"	int n = Math.max(1, 2);\n" +
+				"	int n= Math.max(1, 2);\n" +
 				"}\n";
 		ICompilationUnit compilationUnit= fPackage.createCompilationUnit("Foo.java", contents, true, new NullProgressMonitor());
 		JavaEditor editor= (JavaEditor) EditorUtility.openInEditor(compilationUnit);
 		fParameterNameCodeMiningProvider.setContext(editor);
-		ISourceViewer viewer = editor.getViewer();
+		JavaSourceViewer viewer= (JavaSourceViewer)editor.getViewer();
+		waitReconciled(viewer);
+
 		assertEquals(2, fParameterNameCodeMiningProvider.provideCodeMinings(viewer, new NullProgressMonitor()).get().size());
 	}
 
 	public void testVarargs() throws Exception {
 		String contents= "public class Foo {\n" +
-				"	String s = String.format(\"%d %d\", 1, 2);\n" +
+				"	String s= String.format(\"%d %d\", 1, 2);\n" +
 				"}\n";
 		ICompilationUnit compilationUnit= fPackage.createCompilationUnit("Foo.java", contents, true, new NullProgressMonitor());
 		JavaEditor editor= (JavaEditor) EditorUtility.openInEditor(compilationUnit);
 		fParameterNameCodeMiningProvider.setContext(editor);
-		ISourceViewer viewer = editor.getViewer();
+		JavaSourceViewer viewer= (JavaSourceViewer)editor.getViewer();
+		waitReconciled(viewer);
 		assertEquals(2, fParameterNameCodeMiningProvider.provideCodeMinings(viewer, new NullProgressMonitor()).get().size());
 	}
 
 	public void testMultiLines() throws Exception {
 		String contents =
 				"class Foo {\n" +
-				"	long n = Math.max(System.currentTimeMillis(\n" +
+				"	long n= Math.max(System.currentTimeMillis(\n" +
 				"					), 0);\n" +
 				"}";
 		ICompilationUnit compilationUnit= fPackage.createCompilationUnit("Foo.java", contents, true, new NullProgressMonitor());
-		JavaEditor editor= (JavaEditor) EditorUtility.openInEditor(compilationUnit);
+		CompilationUnitEditor editor= (CompilationUnitEditor) EditorUtility.openInEditor(compilationUnit);
 		fParameterNameCodeMiningProvider.setContext(editor);
-		JavaSourceViewer viewer = (JavaSourceViewer)editor.getViewer();
+		JavaSourceViewer viewer= (JavaSourceViewer)editor.getViewer();
 		viewer.setCodeMiningProviders(new ICodeMiningProvider[] {
 				fParameterNameCodeMiningProvider
 		});
+		waitReconciled(viewer);
+		StyledText widget= viewer.getTextWidget();
+		//
 		assertEquals(2, fParameterNameCodeMiningProvider.provideCodeMinings(viewer, new NullProgressMonitor()).get().size());
 		//
-		StyledText widget = viewer.getTextWidget();
-		int charWidth = widget.getTextBounds(0, 1).width;
-		LongSupplier drawnCodeMiningsCount = () -> Arrays.stream(widget.getStyleRanges()).filter(style ->
+		int charWidth= widget.getTextBounds(0, 1).width;
+		LongSupplier drawnCodeMiningsCount= () -> Arrays.stream(widget.getStyleRanges()).filter(style ->
 			style.metrics != null && style.metrics.width > charWidth).count();
 		new DisplayHelper() {
 			@Override
@@ -151,14 +170,15 @@
 	public void testUnresolvedMethodBinding() throws Exception {
 		String contents= "public class Foo {\n" +
 		"	public void mehod() {\n" +
-		"		List<String> list = Arrays.asList(\"foo\", \"bar\");\n" +
+		"		List<String> list= Arrays.asList(\"foo\", \"bar\");\n" +
 		"		System.out.printf(\"%s %s\", list.get(0), list.get(1));\n" +
 		"	}\n" +
 		"}";
 		ICompilationUnit compilationUnit= fPackage.createCompilationUnit("Foo.java", contents, true, new NullProgressMonitor());
 		JavaEditor editor= (JavaEditor) EditorUtility.openInEditor(compilationUnit);
 		fParameterNameCodeMiningProvider.setContext(editor);
-		ISourceViewer viewer = editor.getViewer();
+		JavaSourceViewer viewer= (JavaSourceViewer)editor.getViewer();
+		waitReconciled(viewer);
 		// Only code mining on "printf" parameters
 		assertEquals(2, fParameterNameCodeMiningProvider.provideCodeMinings(viewer, new NullProgressMonitor()).get().size());
 	}
@@ -175,19 +195,20 @@
 				" * aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" +
 				" */" +
 				"public class Foo {\n" +
-				"	int n = Math.max(1, 2);\n" +
+				"	int n= Math.max(1, 2);\n" +
 				"}";
 		ICompilationUnit compilationUnit= fPackage.createCompilationUnit("Foo.java", contents, true, new NullProgressMonitor());
 		JavaEditor editor= (JavaEditor) EditorUtility.openInEditor(compilationUnit);
 		fParameterNameCodeMiningProvider.setContext(editor);
-		JavaSourceViewer viewer = (JavaSourceViewer)editor.getViewer();
+		JavaSourceViewer viewer= (JavaSourceViewer)editor.getViewer();
 		viewer.doOperation(ProjectionViewer.COLLAPSE_ALL);
 		viewer.setCodeMiningProviders(new ICodeMiningProvider[] {
 			fParameterNameCodeMiningProvider
 		});
+		waitReconciled(viewer);
 		//
 		ILog log= WorkbenchPlugin.getDefault().getLog();
-		AtomicReference<IStatus> errorInLog = new AtomicReference<>();
+		AtomicReference<IStatus> errorInLog= new AtomicReference<>();
 		ILogListener logListener= (status, plugin) -> {
 			if (status.getSeverity() == IStatus.ERROR) {
 				errorInLog.set(status);
@@ -207,18 +228,19 @@
 				" *\n" +
 				" */" +
 				"public class Foo {\n" +
-				"	int n = Math.max(1, 2);\n" +
+				"	int n= Math.max(1, 2);\n" +
 				"}";
 		ICompilationUnit compilationUnit= fPackage.createCompilationUnit("Foo.java", contents, true, new NullProgressMonitor());
 		JavaEditor editor= (JavaEditor) EditorUtility.openInEditor(compilationUnit);
 		fParameterNameCodeMiningProvider.setContext(editor);
-		JavaSourceViewer viewer = (JavaSourceViewer)editor.getViewer();
+		JavaSourceViewer viewer= (JavaSourceViewer)editor.getViewer();
 		viewer.setCodeMiningProviders(new ICodeMiningProvider[] {
 			fParameterNameCodeMiningProvider
 		});
+		waitReconciled(viewer);
 		//
-		StyledText widget = viewer.getTextWidget();
-		int charWidth = widget.getTextBounds(0, 1).width;
+		StyledText widget= viewer.getTextWidget();
+		int charWidth= widget.getTextBounds(0, 1).width;
 		assertTrue("Code mining not available on expected chars", new DisplayHelper() {
 			@Override
 			protected boolean condition() {
@@ -238,13 +260,13 @@
 		//
 		viewer.setSelectedRange(viewer.getDocument().get().indexOf("max") + 1, 0);
 		IPreferenceStore preferenceStore= JavaPlugin.getDefault().getPreferenceStore();
-		boolean initial = preferenceStore.getBoolean(PreferenceConstants.EDITOR_MARK_OCCURRENCES);
+		boolean initial= preferenceStore.getBoolean(PreferenceConstants.EDITOR_MARK_OCCURRENCES);
 		try {
 			preferenceStore.setValue(PreferenceConstants.EDITOR_MARK_OCCURRENCES, true);
 			assertTrue("Occurence annotation not added", new org.eclipse.jdt.text.tests.performance.DisplayHelper() {
 				@Override
 				protected boolean condition() {
-					AtomicInteger annotationCount = new AtomicInteger();
+					AtomicInteger annotationCount= new AtomicInteger();
 					viewer.getAnnotationModel().getAnnotationIterator().forEachRemaining(annotation -> {
 						if (annotation.getType().contains("occurrence")) {
 							annotationCount.incrementAndGet();
@@ -259,7 +281,7 @@
 					return Arrays.stream(widget.getStyleRanges())
 							.filter(range -> range.metrics != null && range.metrics.width > charWidth)
 							.allMatch(style -> {
-								char c = widget.getText().charAt(style.start + 1);
+								char c= widget.getText().charAt(style.start + 1);
 								return c == '1' || c == '2';
 							});
 				}
@@ -271,12 +293,12 @@
 
 	private static boolean welcomeClosed;
 	private static void closeIntro(final IWorkbench wb) {
-		IWorkbenchWindow window = wb.getActiveWorkbenchWindow();
+		IWorkbenchWindow window= wb.getActiveWorkbenchWindow();
 		if (window != null) {
-			IIntroManager im = wb.getIntroManager();
-			IIntroPart intro = im.getIntro();
+			IIntroManager im= wb.getIntroManager();
+			IIntroPart intro= im.getIntro();
 			if (intro != null) {
-				welcomeClosed = im.closeIntro(intro);
+				welcomeClosed= im.closeIntro(intro);
 			}
 		}
 	}
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/javaeditor/JavaCodeMiningReconciler.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/javaeditor/JavaCodeMiningReconciler.java
index 12c520b..a2cc8a6 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/javaeditor/JavaCodeMiningReconciler.java
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/javaeditor/JavaCodeMiningReconciler.java
@@ -13,6 +13,9 @@
  *******************************************************************************/
 package org.eclipse.jdt.internal.ui.javaeditor;
 
+import java.util.HashSet;
+import java.util.Set;
+
 import org.eclipse.core.runtime.IProgressMonitor;
 
 import org.eclipse.jface.text.source.ISourceViewer;
@@ -20,7 +23,6 @@
 
 import org.eclipse.jdt.core.dom.CompilationUnit;
 
-import org.eclipse.jdt.internal.ui.javaeditor.codemining.JavaElementCodeMiningProvider;
 import org.eclipse.jdt.internal.ui.text.java.IJavaReconcilingListener;
 
 /**
@@ -28,21 +30,32 @@
  */
 public class JavaCodeMiningReconciler implements IJavaReconcilingListener {
 
+	/**
+	 * Stores the set of viewers for which source is reconciled and requests
+	 * for references can be performed.
+	 */
+	private static final Set<ISourceViewerExtension5> reconciledViewers= new HashSet<>();
+
 	/** The Java editor this Java code mining reconciler is installed on */
 	private JavaEditor fEditor;
 
 	/** The source viewer this Java code mining reconciler is installed on */
 	private ISourceViewerExtension5 fSourceViewer;
 
+
 	@Override
 	public void reconciled(CompilationUnit ast, boolean forced, IProgressMonitor progressMonitor) {
-		JavaElementCodeMiningProvider.markReconciledViewer(fSourceViewer);
-		fSourceViewer.updateCodeMinings();
+		final ISourceViewerExtension5 sourceViewer= fSourceViewer; // take a copy as this can be null-ed in the meantime
+		if (sourceViewer != null) {
+			reconciledViewers.add(sourceViewer);
+			sourceViewer.updateCodeMinings();
+		}
 	}
 
 	@Override
 	public void aboutToBeReconciled() {
-		// Do nothing
+		// interrupt code minings if modification occurs
+		reconciledViewers.remove(fSourceViewer);
 	}
 
 	/**
@@ -70,7 +83,7 @@
 	 * Uninstall this reconciler from the editor.
 	 */
 	public void uninstall() {
-		JavaElementCodeMiningProvider.discardViewer(fSourceViewer);
+		reconciledViewers.remove(fSourceViewer);
 		if (fEditor instanceof CompilationUnitEditor) {
 			((CompilationUnitEditor) fEditor).removeReconcileListener(this);
 		}
@@ -78,4 +91,8 @@
 		fSourceViewer= null;
 	}
 
+	public static boolean isReconciled(ISourceViewerExtension5 viewer) {
+		return reconciledViewers.contains(viewer);
+	}
+
 }
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/javaeditor/codemining/JavaElementCodeMiningProvider.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/javaeditor/codemining/JavaElementCodeMiningProvider.java
index 15c092d..3741904 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/javaeditor/codemining/JavaElementCodeMiningProvider.java
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/javaeditor/codemining/JavaElementCodeMiningProvider.java
@@ -15,9 +15,7 @@
 
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashSet;
 import java.util.List;
-import java.util.Set;
 import java.util.concurrent.CompletableFuture;
 
 import org.eclipse.core.runtime.IProgressMonitor;
@@ -39,6 +37,7 @@
 import org.eclipse.jdt.ui.PreferenceConstants;
 
 import org.eclipse.jdt.internal.ui.javaeditor.EditorUtility;
+import org.eclipse.jdt.internal.ui.javaeditor.JavaCodeMiningReconciler;
 import org.eclipse.jdt.internal.ui.javaeditor.JavaEditor;
 import org.eclipse.jdt.internal.ui.preferences.JavaPreferencesPropertyTester;
 
@@ -54,12 +53,6 @@
  */
 public class JavaElementCodeMiningProvider extends AbstractCodeMiningProvider {
 
-	/**
-	 * Stores the set of viewers for which source is reconciled and requests
-	 * for references can be performed.
-	 */
-	private static final Set<ISourceViewerExtension5> reconciledViewers = new HashSet<>();
-
 	private final boolean showAtLeastOne;
 
 	private final boolean showReferences;
@@ -92,8 +85,8 @@
 		}
 		if (viewer instanceof ISourceViewerExtension5) {
 			ISourceViewerExtension5 codeMiningViewer = (ISourceViewerExtension5)viewer;
-			if (!reconciledViewers.contains(codeMiningViewer)) {
-				// the provider isn't able to return code minings for non-reconciled viewerss
+			if (!JavaCodeMiningReconciler.isReconciled(codeMiningViewer)) {
+				// the provider isn't able to return code minings for non-reconciled viewers
 				return CompletableFuture.completedFuture(Collections.emptyList());
 			}
 		}
@@ -108,6 +101,13 @@
 				IJavaElement[] elements= unit.getChildren();
 				List<ICodeMining> minings= new ArrayList<>(elements.length);
 				collectMinings(unit, textEditor, unit.getChildren(), minings, viewer, monitor);
+				// interrupt if editor was marked to be reconciled in the meantime
+				if (viewer instanceof ISourceViewerExtension5) {
+					ISourceViewerExtension5 codeMiningViewer= (ISourceViewerExtension5)viewer;
+					if (!JavaCodeMiningReconciler.isReconciled(codeMiningViewer)) {
+						monitor.setCanceled(true);
+					}
+				}
 				monitor.isCanceled();
 				return minings;
 			} catch (JavaModelException e) {
@@ -186,23 +186,4 @@
 		}
 	}
 
-	/**
-	 * Marks the content of the viewer is reconciled with AST. This is
-	 * required for the code minings to be computed.
-	 * If code minings are requested on a viewer which wasn't marked as
-	 * reconciled via this method, the provider skips the queries and
-	 * returns an empty list.
-	 * @param viewer a viewer with reconciled input.
-	 */
-	public static void markReconciledViewer(ISourceViewerExtension5 viewer) {
-		reconciledViewers.add(viewer);
-	}
-
-	/**
-	 * Removes a viewer from the list of viewers the provider will process.
-	 * @param viewer the viewer to stop providing minings for.
-	 */
-	public static void discardViewer(ISourceViewerExtension5 viewer) {
-		reconciledViewers.remove(viewer);
-	}
 }
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/javaeditor/codemining/JavaMethodParameterCodeMiningProvider.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/javaeditor/codemining/JavaMethodParameterCodeMiningProvider.java
index 8478cc4..fcb3010 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/javaeditor/codemining/JavaMethodParameterCodeMiningProvider.java
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/javaeditor/codemining/JavaMethodParameterCodeMiningProvider.java
@@ -14,6 +14,7 @@
 package org.eclipse.jdt.internal.ui.javaeditor.codemining;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.CompletableFuture;
 
@@ -22,6 +23,7 @@
 import org.eclipse.jface.text.ITextViewer;
 import org.eclipse.jface.text.codemining.AbstractCodeMiningProvider;
 import org.eclipse.jface.text.codemining.ICodeMining;
+import org.eclipse.jface.text.source.ISourceViewerExtension5;
 
 import org.eclipse.ui.texteditor.ITextEditor;
 
@@ -35,6 +37,7 @@
 
 import org.eclipse.jdt.internal.ui.JavaPlugin;
 import org.eclipse.jdt.internal.ui.javaeditor.EditorUtility;
+import org.eclipse.jdt.internal.ui.javaeditor.JavaCodeMiningReconciler;
 
 /**
  * Java code mining provider to show method parameters code minings.
@@ -46,6 +49,13 @@
 
 	@Override
 	public CompletableFuture<List<? extends ICodeMining>> provideCodeMinings(ITextViewer viewer, IProgressMonitor monitor) {
+		if (viewer instanceof ISourceViewerExtension5) {
+			ISourceViewerExtension5 codeMiningViewer= (ISourceViewerExtension5)viewer;
+			if (!JavaCodeMiningReconciler.isReconciled(codeMiningViewer)) {
+				// the provider isn't able to return code minings for non-reconciled viewers
+				return CompletableFuture.completedFuture(Collections.emptyList());
+			}
+		}
 		return CompletableFuture.supplyAsync(() -> {
 			monitor.isCanceled();
 			ITextEditor textEditor= super.getAdapter(ITextEditor.class);
@@ -57,6 +67,13 @@
 				IJavaElement[] elements= unit.getChildren();
 				List<ICodeMining> minings= new ArrayList<>(elements.length);
 				collectLineContentCodeMinings(unit, minings);
+				if (viewer instanceof ISourceViewerExtension5) {
+					ISourceViewerExtension5 codeMiningViewer= (ISourceViewerExtension5)viewer;
+					if (!JavaCodeMiningReconciler.isReconciled(codeMiningViewer)) {
+						// the provider isn't able to return code minings for non-reconciled viewers
+						monitor.setCanceled(true);
+					}
+				}
 				monitor.isCanceled();
 				return minings;
 			} catch (JavaModelException e) {