Position of code mining annotation need to be removed with delay

[tobiasmelcher]

to fix issue #3121

[github-actions]

Test Results

 2 778 files  ±0   2 778 suites  ±0   1h 38m 46s ⏱️ - 14m 31s
 7 932 tests ±0   7 704 ✅ ±0  228 💤 ±0  0 ❌ ±0 
23 349 runs  ±0  22 603 ✅ ±0  746 💤 ±0  0 ❌ ±0 

Results for commit 9091d01. ± Comparison against base commit f1bbc96.

[BeckerWdf]

should we merge this so that it's part of M3 (promotion is tomorrow) ?

[merks]

Yes, please try to get this merged if folks are comfortable with the change. Testing for M3 can only be a good thing.

[tobiasmelcher]

Let me try to motivate why I did the change.

Following steps are executed In the video of the issue #3121.

1. copilot is showing a code prediction via a code mining after user has typed in "/*" in the editor
2. user now presses the enter key to insert the full javadoc block comment. The code mining is removed (because user did not accept the code prediction and triggered another action) and the document is modified by inserting the javadoc comment block.

I see on my machine (Windows) following call sequence after the user presses the enter key to insert the javadoc block comment:

First a CodeMiningManager.renderCodeMinings is called to update the code minings. The stack shows that the CodeMiningLineHeaderAnnotation is removed from the AnnotationModel.

ForkJoinPool.commonPool-worker-16:removeAnnotation org.eclipse.jface.internal.text.codemining.CodeMiningLineHeaderAnnotation@f28d969
java.lang.Exception
	at org.eclipse.jface.text.source.AnnotationModel.removeAnnotation(AnnotationModel.java:845)
	at org.eclipse.ui.texteditor.AbstractMarkerAnnotationModel.removeAnnotation(AbstractMarkerAnnotationModel.java:719)
	at org.eclipse.jdt.internal.ui.javaeditor.CompilationUnitDocumentProvider$CompilationUnitAnnotationModel.removeAnnotation(CompilationUnitDocumentProvider.java:863)
	at org.eclipse.jface.text.source.AnnotationModel.replaceAnnotations(AnnotationModel.java:395)
	at org.eclipse.jface.text.source.AnnotationModel.replaceAnnotations(AnnotationModel.java:374)
	at org.eclipse.jface.text.source.inlined.InlinedAnnotationSupport.updateAnnotations(InlinedAnnotationSupport.java:464)
	at org.eclipse.jface.internal.text.codemining.CodeMiningManager.renderCodeMinings(CodeMiningManager.java:295)
	at org.eclipse.jface.internal.text.codemining.CodeMiningManager.lambda$1(CodeMiningManager.java:151)
	at java.base/java.util.concurrent.CompletableFuture$UniAccept.tryFire(CompletableFuture.java:718)
	at java.base/java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:510)
	at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1773)
	at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.exec(CompletableFuture.java:1760)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:387)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1312)
	at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1843)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1808)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:188)

Afterwards several document change events are triggered and the DefaultPositionUpdater is called which has the task to update the positions of the annotations. The position of the org.eclipse.jface.internal.text.codemining.CodeMiningLineHeaderAnnotation@f28d969 is not updated because it is already removed via the previous call.

	at org.eclipse.jface.text.DefaultPositionUpdater.update(DefaultPositionUpdater.java:207)
	at org.eclipse.jface.text.AbstractDocument.updatePositions(AbstractDocument.java:1145)
	at org.eclipse.jface.text.AbstractDocument.updateDocumentStructures(AbstractDocument.java:678)
	at org.eclipse.jface.text.AbstractDocument.fireDocumentChanged(AbstractDocument.java:764)
	at org.eclipse.jface.text.AbstractDocument.replace(AbstractDocument.java:1092)
	at org.eclipse.jface.text.AbstractDocument.replace(AbstractDocument.java:1110)
	at org.eclipse.text.edits.ReplaceEdit.performDocumentUpdating(ReplaceEdit.java:79)
	at org.eclipse.text.edits.TextEdit.traverseDocumentUpdating(TextEdit.java:920)
	at org.eclipse.text.edits.TextEdit.traverseDocumentUpdating(TextEdit.java:913)
	at org.eclipse.text.edits.TextEditProcessor.executeDo(TextEditProcessor.java:196)
	at org.eclipse.text.edits.TextEdit.dispatchPerformEdits(TextEdit.java:742)
	at org.eclipse.text.edits.TextEditProcessor.performEdits(TextEditProcessor.java:158)
	at org.eclipse.text.edits.TextEdit.apply(TextEdit.java:714)
	at org.eclipse.jface.text.templates.TemplateContextType.resolve(TemplateContextType.java:257)
	at org.eclipse.jdt.internal.core.manipulation.CodeTemplateContext.evaluate(CodeTemplateContext.java:81)
	at org.eclipse.jdt.internal.core.manipulation.StubUtility.getMethodComment(StubUtility.java:458)

In the end we see an AnnotationPainter.updatePainting call which catches up the annotation model and draws also all code minings (take a look at the asyncCall at AnnotationPainter.java:1083). The call InlinedAnnotationDrawingStrategy.draw also handles the deleted code minings by calling textWidget.setLineVerticalIndent(0) to remove the additional drawn text editor line heights of the LineHeaderAnnotation. This code is executed but doesn't work in the given scenario because the offsets of the LineHeaderAnnotation were not updated -> Reason: removeAnnotation is called too early. This pull request now ensures that the Positions of the LineHeaderAnnotation are updated in the given scenario so that textWidget.setLineVerticalIndent(0) is called properly and code minings disappear as requested.

private static void draw(LineHeaderAnnotation annotation, GC gc, StyledText textWidget, int offset, int length,
			Color color) {
		if (isInlinedAnnotationDeleted(textWidget, offset, annotation)) {
			return;
		}
                ...
private static boolean isInlinedAnnotationDeleted(StyledText textWidget, int offset, AbstractInlinedAnnotation annotation) {
		int line= textWidget.getLineAtOffset(offset);
		int charCount= textWidget.getCharCount();
		if (isDeleted(annotation, charCount)) {
			// When annotation is deleted, update metrics to null to remove extra spaces of the line header annotation.
			if (textWidget.getLineVerticalIndent(line) > 0) {
				textWidget.setLineVerticalIndent(line, 0);
			}
			return true;
		}
		return false;
	}

	at org.eclipse.jface.text.source.inlined.InlinedAnnotationDrawingStrategy.draw(InlinedAnnotationDrawingStrategy.java:162)
	at org.eclipse.jface.text.source.inlined.InlinedAnnotationDrawingStrategy.draw(InlinedAnnotationDrawingStrategy.java:140)
	at org.eclipse.jface.text.source.inlined.InlinedAnnotationDrawingStrategy.draw(InlinedAnnotationDrawingStrategy.java:87)
	at org.eclipse.jface.text.source.AnnotationPainter.drawDecoration(AnnotationPainter.java:1435)
	at org.eclipse.jface.text.source.AnnotationPainter.catchupWithModel(AnnotationPainter.java:614)
	at org.eclipse.jface.text.source.AnnotationPainter.updatePainting(AnnotationPainter.java:955)
	at org.eclipse.jface.text.source.AnnotationPainter.lambda$0(AnnotationPainter.java:1083)
	at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:40)
	at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:132)
	at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:4133)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3749)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.run(PartRenderingEngine.java:1151)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:339)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1042)
	at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:153)
	at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:678)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:339)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:583)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:173)
	at com.sap.adt.product.branding.internal.application.AdtApplication.start(AdtApplication.java:69)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:219)

[BeckerWdf]

would be good to hear @mickaelistria opinion on this change

[BeckerWdf]

#3121 (comment) say that this fixes the issue.
Let's merge for M3 so that we get a good testing from a broad user group. If we find regression we can easily revert this change.