From 19b170b57ede3d3616a2dd8a25c72b3a976b1f58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Porras=20Campo?= Date: Mon, 26 Aug 2024 14:58:23 +0200 Subject: [PATCH] refactor: minor cleanups Remove synchronized methods in favor of volatile variables, since mergeTriggers is an static method, a volatile variable is enough. Remove catch of a CancellationException which is not thrown by CompletableFuture#cancel and use CancellationSupport. Use Either#map instead of ternary expression. --- .../lsp4e/internal/CancellationSupport.java | 2 +- .../completion/LSContentAssistProcessor.java | 86 ++++++++----------- 2 files changed, 37 insertions(+), 51 deletions(-) diff --git a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/internal/CancellationSupport.java b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/internal/CancellationSupport.java index 93c1e96ee..e18eb2e61 100644 --- a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/internal/CancellationSupport.java +++ b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/internal/CancellationSupport.java @@ -20,7 +20,7 @@ /** * LSP cancellation support hosts the list of LSP requests to cancel when a - * process is canceled (ex: when completion is re-triggered, when hover is give + * process is cancelled (ex: when completion is re-triggered, when hover is give * up, etc) * * @see > completionLanguageServersFuture; - private final Object completionTriggerCharsSemaphore = new Object(); - private char[] completionTriggerChars = NO_CHARS; + private volatile char[] completionTriggerChars = NO_CHARS; private @Nullable CompletableFuture> contextInformationLanguageServersFuture; - private final Object contextTriggerCharsSemaphore = new Object(); - private char[] contextTriggerChars = NO_CHARS; + private volatile char[] contextTriggerChars = NO_CHARS; private final boolean incompleteAsCompletionItem; /** * The cancellation support used to cancel previous LSP requests * 'textDocument/completion' when completion is retriggered */ - private CancellationSupport cancellationSupport; + private CancellationSupport completionCancellationSupport; + /** + * The cancellation support used to cancel previous LSP requests + * for fetching the trigger characters + */ + private CancellationSupport triggerCharsCancellationSupport; public LSContentAssistProcessor() { this(true); @@ -99,7 +101,8 @@ public LSContentAssistProcessor(boolean errorAsCompletionItem) { public LSContentAssistProcessor(boolean errorAsCompletionItem, boolean incompleteAsCompletionItem) { this.errorAsCompletionItem = errorAsCompletionItem; - this.cancellationSupport = new CancellationSupport(); + this.completionCancellationSupport = new CancellationSupport(); + this.triggerCharsCancellationSupport = new CancellationSupport(); this.incompleteAsCompletionItem = incompleteAsCompletionItem; } @@ -133,14 +136,14 @@ public LSContentAssistProcessor(boolean errorAsCompletionItem, boolean incomplet try { // Cancel the previous LSP requests 'textDocument/completions' and // completionLanguageServersFuture - this.cancellationSupport.cancel(); + this.completionCancellationSupport.cancel(); // Initialize a new cancel support to register: // - LSP requests 'textDocument/completions' // - completionLanguageServersFuture final var cancellationSupport = new CancellationSupport(); - final var completionLanguageServersFuture = this.completionLanguageServersFuture = LanguageServers - .forDocument(document).withFilter(capabilities -> capabilities.getCompletionProvider() != null) // + final var completionLanguageServersFuture = this.completionLanguageServersFuture = cancellationSupport.execute( + LanguageServers.forDocument(document).withFilter(capabilities -> capabilities.getCompletionProvider() != null) // .collectAll((w, ls) -> cancellationSupport.execute(ls.getTextDocumentService().completion(param)) // .thenAccept(completion -> { boolean isIncomplete = completion != null && completion.isRight() @@ -156,9 +159,8 @@ public LSContentAssistProcessor(boolean errorAsCompletionItem, boolean incomplet .formatted(w.serverDefinition.label), t); } return null; - })); - cancellationSupport.execute(completionLanguageServersFuture); - this.cancellationSupport = cancellationSupport; + }))); + this.completionCancellationSupport = cancellationSupport; // Wait for the result of all LSP requests 'textDocument/completions', this // future will be canceled with the next completion @@ -216,43 +218,28 @@ private String createErrorMessage(int offset, Exception ex) { private void initiateLanguageServers(IDocument document) { if (currentDocument != document) { - this.currentDocument = document; - if (this.completionLanguageServersFuture != null) { - try { - this.completionLanguageServersFuture.cancel(true); - } catch (CancellationException ex) { - // nothing - } - } - if (this.contextInformationLanguageServersFuture != null) { - try { - this.contextInformationLanguageServersFuture.cancel(true); - } catch (CancellationException ex) { - // nothing - } - } - this.completionTriggerChars = NO_CHARS; - this.contextTriggerChars = NO_CHARS; + currentDocument = document; + triggerCharsCancellationSupport.cancel(); + + completionTriggerChars = NO_CHARS; + contextTriggerChars = NO_CHARS; - this.completionLanguageServersFuture = LanguageServers.forDocument(document) + completionLanguageServersFuture = triggerCharsCancellationSupport.execute(// + LanguageServers.forDocument(document) .withFilter(capabilities -> capabilities.getCompletionProvider() != null) // .collectAll((w, ls) -> { - CompletionOptions provider = castNonNull(w.getServerCapabilities()).getCompletionProvider(); - synchronized (completionTriggerCharsSemaphore) { - completionTriggerChars = mergeTriggers(completionTriggerChars, - provider.getTriggerCharacters()); - } + List triggerChars = castNonNull(w.getServerCapabilities()).getCompletionProvider().getTriggerCharacters(); + completionTriggerChars = mergeTriggers(completionTriggerChars,triggerChars); return CompletableFuture.completedFuture(null); - }); - this.contextInformationLanguageServersFuture = LanguageServers.forDocument(document) + })); + contextInformationLanguageServersFuture = triggerCharsCancellationSupport.execute(// + LanguageServers.forDocument(document) .withFilter(capabilities -> capabilities.getSignatureHelpProvider() != null) // .collectAll((w, ls) -> { - SignatureHelpOptions provider = castNonNull(w.getServerCapabilities()).getSignatureHelpProvider(); - synchronized (contextTriggerCharsSemaphore) { - contextTriggerChars = mergeTriggers(contextTriggerChars, provider.getTriggerCharacters()); - } + List triggerChars = castNonNull(w.getServerCapabilities()).getSignatureHelpProvider().getTriggerCharacters(); + contextTriggerChars = mergeTriggers(contextTriggerChars, triggerChars); return CompletableFuture.completedFuture(null); - }); + })); } } @@ -276,16 +263,15 @@ private static List toProposals(IDocument document, int off // Stop the compute of ICompletionProposal if the completion has been cancelled cancelChecker.checkCanceled(); CompletionItemDefaults defaults = completionList.map(o -> null, CompletionList::getItemDefaults); - List items = completionList.isLeft() ? completionList.getLeft() - : completionList.getRight().getItems(); - return items.stream() // - .filter(Objects::nonNull).map(item -> new LSCompletionProposal(document, offset, item, defaults, - languageServerWrapper, isIncomplete)) + return completionList.map( Functions.identity(), CompletionList::getItems).stream() // + .filter(Objects::nonNull) // + .map(item -> new LSCompletionProposal(document, offset, item, defaults, languageServerWrapper, isIncomplete)) .filter(proposal -> { // Stop the compute of ICompletionProposal if the completion has been cancelled cancelChecker.checkCanceled(); return true; - }).filter(proposal -> proposal.validate(document, offset, null)).map(ICompletionProposal.class::cast) + }).filter(proposal -> proposal.validate(document, offset, null)) // + .map(ICompletionProposal.class::cast) .toList(); }