LSPTextHover reuses completed hover result for unchanged region and does not issue a new textDocument/hover request

[betamaxbandit]

Summary

LSPTextHover appears to reuse a previously completed hover request when the hover region and viewer are unchanged, instead of issuing a fresh textDocument/hover request.

This causes stale hover content when hover data depends on external state and not only on the current document text.

Environment

• Eclipse with LSP4E
• Language server / proxy setup where hover content may change based on external project metadata
• The document text and hover region remain unchanged

Problem

If I hover symbol A, a hover request is sent and the result is shown.

If some external state changes after that, hovering the same symbol A again at the same location does not send a new textDocument/hover request.

However, if I hover a different symbol first, and then return to A, a new hover request is sent and the updated value is shown.

So the stale behavior seems tied to reusing the previously completed hover request for the same region.

Expected behavior

A repeated hover on the same region should be able to trigger a fresh textDocument/hover request after the previous request has completed.

Hover content can depend on external state, not only document text, so reusing the previous completed request indefinitely can produce stale results.

Actual behavior

For the same region:

• first hover sends a request
• completed result is reused
• subsequent hover on the same region sends no new request
• moving to another region and back causes a fresh request

Suspected cause

In LSPTextHover, the hover request is only refreshed when:

• request == null
• viewer changes
• region changes

A completed request for the same region is reused.

The same pattern also affects getHoverRegion(...).

To reproduce

The problem can be demonstrated using the following code.

test.c

#include "test.h"

void test(void) {
    int testA = _A_;
    int testB = _B_;
}

test.h

#define _A_ 100
#define _B_ 0

Steps to reproduce

1. In test.c, hover symbol _A_ and the value 100 is correctly shown in the hover.
2. In test.h, modify the value of _A_ to 101, then save the file.
3. In test.c, hover symbol _A_.

Expected: value 101 is shown in the hover.
Actual: value 100 is shown in the hover.

Proposed fix

Treat a completed request as stale and start a new request when request.isDone() is true, even if the region is unchanged.

I tested the following patch (it also includes fixes for sonarlint warnings) locally and it fixes the issue for my case.

diff --git a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/hover/LSPTextHover.java b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/hover/LSPTextHover.java
index c191c3b..4984263 100644
--- a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/hover/LSPTextHover.java
+++ b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/hover/LSPTextHover.java
@@ -16,8 +16,6 @@
  *******************************************************************************/
 package org.eclipse.lsp4e.operations.hover;
 
-import static org.eclipse.lsp4e.internal.NullSafetyHelper.castNonNull;
-
 import java.util.List;
 import java.util.NoSuchElementException;
 import java.util.Objects;
@@ -100,10 +98,18 @@
 	}
 
 	public CompletableFuture<@Nullable String> getHoverInfoFuture(ITextViewer textViewer, IRegion hoverRegion) {
-		if (this.request == null || !textViewer.equals(this.lastViewer) || !hoverRegion.equals(this.lastRegion)) {
+		CompletableFuture<List<Hover>> locRequest = this.request;
+		if (locRequest == null || locRequest.isDone()
+				|| !textViewer.equals(this.lastViewer) || !hoverRegion.equals(this.lastRegion)) {
 			initiateHoverRequest(textViewer, hoverRegion.getOffset());
+			locRequest = this.request;
 		}
-		return castNonNull(request).thenApply(hoversList -> {
+
+		if (locRequest == null) {
+			return CompletableFuture.completedFuture(null);
+		}
+
+		return locRequest.thenApply(hoversList -> {
 			String result = hoversList.stream() //
 					.filter(Objects::nonNull) //
 					.map(LSPTextHover::getHoverString) //
@@ -115,9 +121,8 @@
 				Node document = parser.parse(result);
 				HtmlRenderer renderer = HtmlRenderer.builder().build();
 				return renderer.render(document);
-			} else {
-				return null;
 			}
+			return null;
 		});
 	}
 
@@ -154,9 +159,12 @@
 	@Override
 	public @Nullable IRegion getHoverRegion(ITextViewer textViewer, int offset) {
 		final var lastRegion = this.lastRegion;
-		if (this.request == null || lastRegion == null || !textViewer.equals(this.lastViewer)
-				|| offset < lastRegion.getOffset() || offset > lastRegion.getOffset() + lastRegion.getLength()) {
+
+		CompletableFuture<List<Hover>> request = this.request;
+		if (request == null || request.isDone() || lastRegion == null || !textViewer.equals(this.lastViewer)
+				|| offset < lastRegion.getOffset() || offset >= lastRegion.getOffset() + lastRegion.getLength()) {
 			initiateHoverRequest(textViewer, offset);
+			request = this.request;
 		}
 
 		final IDocument document = textViewer.getDocument();
@@ -164,18 +172,26 @@
 			return null;
 		}
 
+		if (request == null) {
+			return this.lastRegion = computeHeuristicRegion(document, offset);
+		}
+
 		try {
-			// Wait shortly for hover region result, fallback to heuristics if LS is laggy
-			Range range = castNonNull(this.request).get(GET_HOVER_REGION_TIMEOUT_MS, TimeUnit.MILLISECONDS).stream() //
+			List<Hover> hovers = request.get(GET_HOVER_REGION_TIMEOUT_MS, TimeUnit.MILLISECONDS);
+
+			Range range = hovers.stream() //
 					.filter(Objects::nonNull) //
 					.map(Hover::getRange) //
 					.filter(Objects::nonNull) //
 					.reduce((first, second) -> second) //
-					.get();
-			int regionStartOffset = Math.max(0,
-					LSPEclipseUtils.toOffset(range.getStart(), document));
-			int regionEndOffset = Math.min(document.getLength(),
-					LSPEclipseUtils.toOffset(range.getEnd(), document));
+					.orElse(null);
+
+			if (range == null) {
+				return this.lastRegion = computeHeuristicRegion(document, offset);
+			}
+
+			int regionStartOffset = Math.max(0, LSPEclipseUtils.toOffset(range.getStart(), document));
+			int regionEndOffset = Math.min(document.getLength(), LSPEclipseUtils.toOffset(range.getEnd(), document));
 			return this.lastRegion = new Region(regionStartOffset, regionEndOffset - regionStartOffset);
 		} catch (ExecutionException | BadLocationException e) {
 			if (!CancellationUtil.isRequestCancelledException(e)) {

Notes

This is especially relevant when hover data depends on external files or workspace metadata that can change while the editor remains open, which is especially important for me as an ISV incorporating a cross-probe feature.

I also tested this behaviour in vscode with clangd extension and it was not reproducible.

[betamaxbandit]

I can create a PR for this if wanted.

[rubenporras]

Yes, that would be great. Your description of the problem and the solution make sense.