Skip to content

Commit

Permalink
fix: Infinite completionItem/resolve when documentation for completion
Browse files Browse the repository at this point in the history 
returns nothing

Fixes eclipse-lsp4j#182

Signed-off-by: azerr <[email protected]>
  • Loading branch information
@angelozerr
angelozerr committed Apr 17, 2024
1 parent 9c6b341 commit 3fc3d44
Show file tree
Hide file tree
Showing 10 changed files with 298 additions and 113 deletions.
13 changes: 13 additions & 0 deletions src/main/java/com/redhat/devtools/lsp4ij/LSPFileSupport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import com.redhat.devtools.lsp4ij.features.codeAction.intention.LSPIntentionCodeActionSupport;
import com.redhat.devtools.lsp4ij.features.codeLens.LSPCodeLensSupport;
import com.redhat.devtools.lsp4ij.features.color.LSPColorSupport;
import com.redhat.devtools.lsp4ij.features.completion.LSPCompletionSupport;
import com.redhat.devtools.lsp4ij.features.documentLink.LSPDocumentLinkSupport;
import com.redhat.devtools.lsp4ij.features.documentation.LSPHoverSupport;
import com.redhat.devtools.lsp4ij.features.foldingRange.LSPFoldingRangeSupport;
Expand Down Expand Up @@ -60,6 +61,8 @@ public class LSPFileSupport implements Disposable {

private final LSPRenameSupport renameSupport;

private final LSPCompletionSupport completionSupport;

private LSPFileSupport(@NotNull PsiFile file) {
this.file = file;
this.codeLensSupport = new LSPCodeLensSupport(file);
Expand All @@ -74,6 +77,7 @@ private LSPFileSupport(@NotNull PsiFile file) {
this.intentionCodeActionSupport = new LSPIntentionCodeActionSupport(file);
this.prepareRenameSupport = new LSPPrepareRenameSupport(file);
this.renameSupport = new LSPRenameSupport(file);
this.completionSupport = new LSPCompletionSupport(file);
file.putUserData(LSP_FILE_SUPPORT_KEY, this);
}

Expand All @@ -93,6 +97,7 @@ public void dispose() {
getIntentionCodeActionSupport().cancel();
getPrepareRenameSupport().cancel();
getRenameSupport().cancel();
getCompletionSupport().cancel();
}

/**
Expand Down Expand Up @@ -203,6 +208,14 @@ public LSPRenameSupport getRenameSupport() {
return renameSupport;
}

/**
* Returns the LSP completion support.
*
* @return the LSP completion support.
*/
public LSPCompletionSupport getCompletionSupport() {
return completionSupport;
}

/**
* Return the existing LSP file support for the given Psi file, or create a new one if necessary.
Expand Down
8 changes: 0 additions & 8 deletions src/main/java/com/redhat/devtools/lsp4ij/LSPIJUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -623,14 +623,6 @@ private static void applyWorkspaceEdit(Document document, List<TextEdit> edits)
return EditorFactory.getInstance().getEditors(document, project);
}

public static CompletionParams toCompletionParams(URI fileUri, int offset, Document document) {
Position start = toPosition(offset, document);
CompletionParams param = new CompletionParams();
param.setPosition(start);
param.setTextDocument(toTextDocumentIdentifier(fileUri));
return param;
}

public static TextDocumentIdentifier toTextDocumentIdentifier(VirtualFile file) {
return toTextDocumentIdentifier(toUri(file));
}
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/redhat/devtools/lsp4ij/LSPRequestConstants.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ public class LSPRequestConstants {
public static final String TEXT_DOCUMENT_SIGNATURE_HELP = "textDocument/signatureHelp";
public static final String TEXT_DOCUMENT_PREPARE_RENAME = "textDocument/prepareRename";
public static final String TEXT_DOCUMENT_RENAME = "textDocument/rename";
public static final String TEXT_DOCUMENT_DOCUMENT_COMPLETION = "textDocument/completion";

private LSPRequestConstants() {

Expand Down
29 changes: 29 additions & 0 deletions src/main/java/com/redhat/devtools/lsp4ij/features/completion/CompletionData.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*******************************************************************************
* Copyright (c) 2024 Red Hat, Inc.
* Distributed under license by Red Hat, Inc. All rights reserved.
* This program is made available under the terms of the
* Eclipse Public License v2.0 which accompanies this distribution,
* and is available at http://www.eclipse.org/legal/epl-v20.html
*
* Contributors:
* Red Hat, Inc. - initial API and implementation
******************************************************************************/
package com.redhat.devtools.lsp4ij.features.completion;

import com.redhat.devtools.lsp4ij.LanguageServerItem;
import org.eclipse.lsp4j.CompletionItem;
import org.eclipse.lsp4j.CompletionList;
import org.eclipse.lsp4j.jsonrpc.messages.Either;
import org.jetbrains.annotations.NotNull;

import java.util.List;

/**
* LSP completion data.
*
* @param completion the LSP completion result
* @param languageServer the language server which has created the completion result.
*/
record CompletionData(@NotNull Either<List<CompletionItem>, CompletionList> completion,
@NotNull LanguageServerItem languageServer) {
}
5 changes: 3 additions & 2 deletions src/main/java/com/redhat/devtools/lsp4ij/features/completion/CompletionPrefix.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ public Document getDocument() {
* @param item the completion item.
* @return the proper prefix from the given text range and label/filterText defined in the given completion item and null otherwise.
*/
public @Nullable String getPrefixFor(@NotNull Range textEditRange, CompletionItem item) {
public @Nullable String getPrefixFor(@NotNull Range textEditRange,
@NotNull CompletionItem item) {
// Try to get the computed prefix from the cache
String prefix = prefixCache.get(textEditRange);
if (prefix == null && !prefixCache.containsKey(textEditRange)) {
Expand Down Expand Up @@ -101,7 +102,7 @@ public Document getDocument() {
return prefix;
}

private static String getAccurateFilterText(CompletionItem item) {
private static String getAccurateFilterText(@NotNull CompletionItem item) {
String filterText = item.getFilterText();
if (StringUtils.isBlank(filterText)) {
return null;
Expand Down
118 changes: 44 additions & 74 deletions src/main/java/com/redhat/devtools/lsp4ij/features/completion/LSPCompletionContributor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,36 +14,33 @@
import com.intellij.codeInsight.completion.CompletionParameters;
import com.intellij.codeInsight.completion.CompletionResultSet;
import com.intellij.codeInsight.completion.PrioritizedLookupElement;
import com.intellij.codeInsight.lookup.LookupElement;
import com.intellij.codeInsight.lookup.LookupElementBuilder;
import com.intellij.openapi.editor.Document;
import com.intellij.openapi.editor.Editor;
import com.intellij.openapi.progress.ProcessCanceledException;
import com.intellij.openapi.progress.ProgressManager;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.Pair;
import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.psi.PsiFile;
import com.redhat.devtools.lsp4ij.LSPFileSupport;
import com.redhat.devtools.lsp4ij.LSPIJUtils;
import com.redhat.devtools.lsp4ij.LanguageServerItem;
import com.redhat.devtools.lsp4ij.LanguageServersRegistry;
import com.redhat.devtools.lsp4ij.LanguageServiceAccessor;
import com.redhat.devtools.lsp4ij.internal.CancellationSupport;
import com.redhat.devtools.lsp4ij.internal.StringUtils;
import com.redhat.devtools.lsp4ij.LSPRequestConstants;
import org.eclipse.lsp4j.*;
import org.eclipse.lsp4j.jsonrpc.messages.Either;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.net.URI;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.BlockingDeque;
import java.util.concurrent.CancellationException;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.LinkedBlockingDeque;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.ExecutionException;

import static com.redhat.devtools.lsp4ij.internal.CompletableFutures.isDoneNormally;
import static com.redhat.devtools.lsp4ij.internal.CompletableFutures.waitUntilDone;

/**
* LSP completion contributor.
Expand All @@ -65,65 +62,44 @@ public void fillCompletionVariants(@NotNull CompletionParameters parameters, @No

Editor editor = parameters.getEditor();
Document document = editor.getDocument();
Project project = psiFile.getProject();
int offset = parameters.getOffset();
URI uri = LSPIJUtils.toUri(file);

ProgressManager.checkCanceled();

final CancellationSupport cancellationSupport = new CancellationSupport();
// Get LSP completion items from cache or create them
LSPCompletionParams params = new LSPCompletionParams(LSPIJUtils.toTextDocumentIdentifier(file), LSPIJUtils.toPosition(offset, document), offset);
CompletableFuture<List<CompletionData>> future = LSPFileSupport.getSupport(psiFile)
.getCompletionSupport()
.getCompletions(params);
try {
CompletableFuture<List<LanguageServerItem>> completionLanguageServersFuture = initiateLanguageServers(file, project);
cancellationSupport.execute(completionLanguageServersFuture);
ProgressManager.checkCanceled();

/*
process the responses out of the completable loop as it may cause deadlock if user is typing
more characters as toProposals will require as read lock that this thread already have and
async processing is occuring on a separate thread.
*/
CompletionParams params = LSPIJUtils.toCompletionParams(uri, offset, document);
BlockingDeque<Pair<Either<List<CompletionItem>, CompletionList>, LanguageServerItem>> proposals = new LinkedBlockingDeque<>();

CompletableFuture<Void> future = completionLanguageServersFuture
.thenComposeAsync(languageServers -> cancellationSupport.execute(
CompletableFuture.allOf(languageServers.stream()
.map(languageServer ->
cancellationSupport.execute(languageServer.getServer()
.getTextDocumentService()
.completion(params), languageServer, LSPRequestConstants.TEXT_DOCUMENT_COMPLETION)
.thenAcceptAsync(completion -> {
if (completion != null) {
proposals.add(new Pair<>(completion, languageServer));
}
}))
.toArray(CompletableFuture[]::new))));
// Wait upon the future is finished and stop the wait if there are some ProcessCanceledException.
waitUntilDone(future, psiFile);
} catch (CancellationException | ProcessCanceledException e) {
return;
} catch (ExecutionException e) {
LOGGER.error("Error while consuming LSP 'textDocument/completion' request", e);
return;
}

ProgressManager.checkCanceled();
while (!future.isDone() || !proposals.isEmpty()) {
ProgressManager.checkCanceled();
Pair<Either<List<CompletionItem>, CompletionList>, LanguageServerItem> pair = proposals.poll(25, TimeUnit.MILLISECONDS);
if (pair != null) {
Either<List<CompletionItem>, CompletionList> completion = pair.getFirst();
if (completion != null) {
CompletionPrefix completionPrefix = new CompletionPrefix(offset, document);
addCompletionItems(psiFile, editor, completionPrefix, pair.getFirst(), pair.getSecond(), result, cancellationSupport);
}
ProgressManager.checkCanceled();
if (isDoneNormally(future)) {
List<CompletionData> data = future.getNow(Collections.emptyList());
if (!data.isEmpty()) {
CompletionPrefix completionPrefix = new CompletionPrefix(offset, document);
for(var item : data) {
ProgressManager.checkCanceled();
addCompletionItems(psiFile, editor, completionPrefix, item.completion(), item.languageServer(), result);
}
}
} catch (ProcessCanceledException cancellation) {
cancellationSupport.cancel();
throw cancellation;
} catch (RuntimeException | InterruptedException e) {
LOGGER.warn(e.getLocalizedMessage(), e);
result.addElement(createErrorProposal(offset, e));
}
}

private static final CompletionItemComparator completionProposalComparator = new CompletionItemComparator();

private void addCompletionItems(PsiFile file, Editor editor, CompletionPrefix completionPrefix, Either<List<CompletionItem>,
CompletionList> completion, LanguageServerItem languageServer, @NotNull CompletionResultSet result, CancellationSupport cancellationSupport) {
private void addCompletionItems(@NotNull PsiFile file,
@NotNull Editor editor,
@NotNull CompletionPrefix completionPrefix,
@NotNull Either<List<CompletionItem>, CompletionList> completion,
@NotNull LanguageServerItem languageServer,
@NotNull CompletionResultSet result) {
CompletionItemDefaults itemDefaults = null;
List<CompletionItem> items = new ArrayList<>();
if (completion.isLeft()) {
Expand All @@ -145,7 +121,7 @@ private void addCompletionItems(PsiFile file, Editor editor, CompletionPrefix co
// Invalid completion Item, ignore it
continue;
}
cancellationSupport.checkCanceled();
ProgressManager.checkCanceled();
// Create lookup item
var lookupItem = createLookupItem(file, editor, completionPrefix.getCompletionOffset(), item, itemDefaults, languageServer);

Expand All @@ -166,15 +142,19 @@ private void addCompletionItems(PsiFile file, Editor editor, CompletionPrefix co
}
}

private static LSPCompletionProposal createLookupItem(PsiFile file, Editor editor, int offset,
CompletionItem item,
CompletionItemDefaults itemDefaults, LanguageServerItem languageServer) {
private static LSPCompletionProposal createLookupItem(@NotNull PsiFile file,
@NotNull Editor editor,
int offset,
@NotNull CompletionItem item,
@Nullable CompletionItemDefaults itemDefaults,
@NotNull LanguageServerItem languageServer) {
// Update text edit range with item defaults if needed
updateWithItemDefaults(item, itemDefaults);
return new LSPCompletionProposal(file, editor, offset, item, languageServer);
}

private static void updateWithItemDefaults(CompletionItem item, CompletionItemDefaults itemDefaults) {
private static void updateWithItemDefaults(@NotNull CompletionItem item,
@Nullable CompletionItemDefaults itemDefaults) {
if (itemDefaults == null) {
return;
}
Expand All @@ -194,14 +174,4 @@ private static void updateWithItemDefaults(CompletionItem item, CompletionItemDe
}
}


private static LookupElement createErrorProposal(int offset, Exception ex) {
return LookupElementBuilder.create("Error while computing completion", "");
}

private static CompletableFuture<List<LanguageServerItem>> initiateLanguageServers(@NotNull VirtualFile file, @NotNull Project project) {
return LanguageServiceAccessor
.getInstance(project)
.getLanguageServers(file,LanguageServerItem::isCompletionSupported);
}
}
34 changes: 34 additions & 0 deletions src/main/java/com/redhat/devtools/lsp4ij/features/completion/LSPCompletionParams.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*******************************************************************************
* Copyright (c) 2024 Red Hat, Inc.
* Distributed under license by Red Hat, Inc. All rights reserved.
* This program is made available under the terms of the
* Eclipse Public License v2.0 which accompanies this distribution,
* and is available at https://www.eclipse.org/legal/epl-v20.html
*
* Contributors:
* Red Hat, Inc. - initial API and implementation
******************************************************************************/
package com.redhat.devtools.lsp4ij.features.completion;

import org.eclipse.lsp4j.CompletionParams;
import org.eclipse.lsp4j.Position;
import org.eclipse.lsp4j.TextDocumentIdentifier;

/**
* LSP completion parameters which hosts the offset where completion has been triggered.
*/
public class LSPCompletionParams extends CompletionParams {

// Use transient to avoid serializing the fields when GSON will be processed
private transient final int offset;

public LSPCompletionParams(TextDocumentIdentifier textDocument, Position position, int offset) {
super.setTextDocument(textDocument);
super.setPosition(position);
this.offset = offset;
}

public int getOffset() {
return offset;
}
}
Loading

0 comments on commit 3fc3d44

Please sign in to comment.