From 73ff997d05a9063a3921d1b66f2f3cb8cbade2b1 Mon Sep 17 00:00:00 2001 From: Will Lauer Date: Tue, 7 Jan 2025 14:22:46 -0600 Subject: [PATCH] Ignoring failures when checking for hyperlinks Adding LanguageServers.addAllSuccessful as an alternative to LanguageServers.addAll so that exceptions from one of the requests being combined can be ignored. Then using this new method in OpenDeclarationHyperlinkDetector when combining the results from definitions, declarations, typeDefinitions, and implementions because some LSPs return errors instead of empty lists for some of these calls. Instead, we combine only the successful results when building the list of hyperlinks, ignoring the errors. This should resolve https://github.com/eclipse-lsp4e/lsp4e/issues/1169 --- org.eclipse.lsp4e/.project | 6 +++ .../org/eclipse/lsp4e/LanguageServers.java | 44 ++++++++++++++----- .../OpenDeclarationHyperlinkDetector.java | 20 ++++----- 3 files changed, 50 insertions(+), 20 deletions(-) diff --git a/org.eclipse.lsp4e/.project b/org.eclipse.lsp4e/.project index c4e8a56b8..dd4a3ac39 100644 --- a/org.eclipse.lsp4e/.project +++ b/org.eclipse.lsp4e/.project @@ -30,10 +30,16 @@ + + org.whitesource.eclipse.plugin.WSbuilder + + + org.eclipse.m2e.core.maven2Nature org.eclipse.pde.PluginNature org.eclipse.jdt.core.javanature + org.whitesource.eclipse.plugin.WSNature diff --git a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/LanguageServers.java b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/LanguageServers.java index a66d71312..6c2e20e06 100644 --- a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/LanguageServers.java +++ b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/LanguageServers.java @@ -12,6 +12,16 @@ *******************************************************************************/ package org.eclipse.lsp4e; +import org.eclipse.core.resources.IProject; +import org.eclipse.core.runtime.Assert; +import org.eclipse.jdt.annotation.Nullable; +import org.eclipse.jface.text.IDocument; +import org.eclipse.lsp4e.LanguageServersRegistry.LanguageServerDefinition; +import org.eclipse.lsp4e.internal.ArrayUtil; +import org.eclipse.lsp4j.ServerCapabilities; +import org.eclipse.lsp4j.jsonrpc.messages.Either; +import org.eclipse.lsp4j.services.LanguageServer; + import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -27,16 +37,6 @@ import java.util.function.Predicate; import java.util.stream.Stream; -import org.eclipse.core.resources.IProject; -import org.eclipse.core.runtime.Assert; -import org.eclipse.jdt.annotation.Nullable; -import org.eclipse.jface.text.IDocument; -import org.eclipse.lsp4e.LanguageServersRegistry.LanguageServerDefinition; -import org.eclipse.lsp4e.internal.ArrayUtil; -import org.eclipse.lsp4j.ServerCapabilities; -import org.eclipse.lsp4j.jsonrpc.messages.Either; -import org.eclipse.lsp4j.services.LanguageServer; - /** * Main entry point for accessors to run requests on the language servers, and some utilities * for manipulating the asynchronous response objects in streams @@ -410,6 +410,30 @@ public static CompletableFuture> addAll(CompletableFuture> a return res; } + /** + * Combines two async lists of results into a single list by adding all the elements of the second list to the first one, + * ignoring abnormal results and only including succeses. + * + * @param Result type + * @param futures Async results + * @return Async combined result + */ + public static CompletableFuture> addAllSuccessful(CompletableFuture>... futures) { + CompletableFuture> res = CompletableFuture.allOf(futures) + .thenApply( v -> { + List results = new ArrayList<>(futures.length); + for(CompletableFuture> f : futures) { + if (!f.isCompletedExceptionally()) { + results.addAll(f.join()); + } + } + return results; + }); + forwardCancellation(res, futures); + return res; + } + + /** * Retrieves the initialized servers and apply the given query. *

The query must ideally be a direct query to the language server diff --git a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/declaration/OpenDeclarationHyperlinkDetector.java b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/declaration/OpenDeclarationHyperlinkDetector.java index f20d1d66f..0dfdfc19a 100644 --- a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/declaration/OpenDeclarationHyperlinkDetector.java +++ b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/declaration/OpenDeclarationHyperlinkDetector.java @@ -13,15 +13,6 @@ *******************************************************************************/ package org.eclipse.lsp4e.operations.declaration; -import java.util.Collection; -import java.util.Collections; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Objects; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; - import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; import org.eclipse.jface.text.BadLocationException; @@ -43,6 +34,15 @@ import org.eclipse.lsp4j.TextDocumentPositionParams; import org.eclipse.lsp4j.jsonrpc.messages.Either; +import java.util.Collection; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Objects; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + public class OpenDeclarationHyperlinkDetector extends AbstractHyperlinkDetector { @Override @@ -68,7 +68,7 @@ public class OpenDeclarationHyperlinkDetector extends AbstractHyperlinkDetector .collectAll(ls -> ls.getTextDocumentService().typeDefinition(LSPEclipseUtils.toTypeDefinitionParams(params)).thenApply(l -> Pair.of(Messages.typeDefinitionHyperlinkLabel, l))); var implementations = LanguageServers.forDocument(document).withCapability(ServerCapabilities::getImplementationProvider) .collectAll(ls -> ls.getTextDocumentService().implementation(LSPEclipseUtils.toImplementationParams(params)).thenApply(l -> Pair.of(Messages.implementationHyperlinkLabel, l))); - LanguageServers.addAll(LanguageServers.addAll(LanguageServers.addAll(definitions, declarations), typeDefinitions), implementations) + LanguageServers.addAllSuccessful(definitions, declarations, typeDefinitions, implementations) .get(800, TimeUnit.MILLISECONDS) .stream().flatMap(locations -> toHyperlinks(document, region, locations.first(), locations.second()).stream()) .forEach(link -> allLinks.putIfAbsent(link.getLocation(), link));