diff --git a/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/hover/HoverTest.java b/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/hover/HoverTest.java index deb3fd2a8..af1d9b84c 100644 --- a/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/hover/HoverTest.java +++ b/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/hover/HoverTest.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2016 Rogue Wave Software Inc. and others. + * Copyright (c) 2016, 2026 Rogue Wave Software Inc. and others. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -235,4 +235,55 @@ public void completed(ProgressEvent event) { shell.dispose(); } } + + @Test + public void testHoverRegionRefreshesForSameOffsetAfterCompletedRequest() throws Exception { + // Test for https://github.com/eclipse-lsp4e/lsp4e/issues/1514 + // Verifies that getHoverRegion refreshes for the same offset after a completed + // request, instead of reusing the previous completed hover range indefinitely. + final var firstHover = new Hover(List.of(Either.forLeft("FirstValue")), + new Range(new Position(0, 0), new Position(0, 5))); + final var secondHover = new Hover(List.of(Either.forLeft("SecondValue")), + new Range(new Position(0, 6), new Position(0, 10))); + + IFile file = TestUtils.createUniqueTestFile(project, "HoverRange Other Text"); + ITextViewer viewer = TestUtils.openTextViewer(file); + + MockLanguageServer.INSTANCE.setHover(firstHover); + assertEquals(new Region(0, 5), hover.getHoverRegion(viewer, 2)); + + MockLanguageServer.INSTANCE.setHover(secondHover); + assertEquals(new Region(6, 4), hover.getHoverRegion(viewer, 2)); + } + + @Test + public void testHoverInfoRefreshesForSameOffsetAfterCompletedRequest() throws Exception { + // Test for https://github.com/eclipse-lsp4e/lsp4e/issues/1514 + // Verifies that a second hover at the same offset recomputes the hover region + // and refreshes the hover content after the previous request completed. + final var firstHover = new Hover(List.of(Either.forLeft("FirstValue")), + new Range(new Position(0, 0), new Position(0, 5))); + final var secondHover = new Hover(List.of(Either.forLeft("SecondValue")), + new Range(new Position(0, 6), new Position(0, 10))); + + IFile file = TestUtils.createUniqueTestFile(project, "HoverRange Other Text"); + ITextViewer viewer = TestUtils.openTextViewer(file); + + MockLanguageServer.INSTANCE.setHover(firstHover); + Region firstRegion = (Region) hover.getHoverRegion(viewer, 2); + assertEquals(new Region(0, 5), firstRegion); + + String firstHtml = hover.getHoverInfoFuture(viewer, firstRegion).get(2, TimeUnit.SECONDS); + assertNotNull(firstHtml); + assertTrue(firstHtml.contains("FirstValue")); + + MockLanguageServer.INSTANCE.setHover(secondHover); + Region secondRegion = (Region) hover.getHoverRegion(viewer, 2); + assertEquals(new Region(6, 4), secondRegion); + + String secondHtml = hover.getHoverInfoFuture(viewer, secondRegion).get(2, TimeUnit.SECONDS); + assertNotNull(secondHtml); + assertTrue(secondHtml.contains("SecondValue")); + assertTrue(!secondHtml.contains("FirstValue")); + } } 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 5b87ba634..2ca162756 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 @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2016, 2023 Red Hat Inc. and others. + * Copyright (c) 2016, 2026 Red Hat Inc. and others. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -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; @@ -98,10 +96,18 @@ public class LSPTextHover implements ITextHover, ITextHoverExtension, ITextHover } public CompletableFuture<@Nullable String> getHoverInfoFuture(ITextViewer textViewer, IRegion hoverRegion) { - if (this.request == null || !textViewer.equals(this.lastViewer) || !hoverRegion.equals(this.lastRegion)) { + CompletableFuture> locRequest = this.request; + if (locRequest == null || locRequest.isDone() || !textViewer.equals(this.lastViewer) + || !hoverRegion.equals(this.lastRegion)) { initiateHoverRequest(textViewer, hoverRegion.getOffset()); + locRequest = this.request; + } + + if (locRequest == null) { + return CompletableFuture.completedFuture(null); } - return castNonNull(request).thenApply(hoversList -> { + + return locRequest.thenApply(hoversList -> { String result = hoversList.stream() // .filter(Objects::nonNull) // .map(LSPTextHover::getHoverString) // @@ -110,9 +116,8 @@ public class LSPTextHover implements ITextHover, ITextHoverExtension, ITextHover .trim(); if (!result.isEmpty()) { return MarkdownUtil.renderToHtml(result); - } else { - return null; } + return null; }); } @@ -149,9 +154,12 @@ public class LSPTextHover implements ITextHover, ITextHoverExtension, ITextHover @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> locRequest = this.request; + if (locRequest == null || locRequest.isDone() || lastRegion == null || !textViewer.equals(this.lastViewer) + || offset < lastRegion.getOffset() || offset >= lastRegion.getOffset() + lastRegion.getLength()) { initiateHoverRequest(textViewer, offset); + locRequest = this.request; } final IDocument document = textViewer.getDocument(); @@ -159,18 +167,27 @@ public class LSPTextHover implements ITextHover, ITextHoverExtension, ITextHover return null; } + if (locRequest == 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 hovers = locRequest.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)) {