From 6de4f75ecb4ccc077a97a78b18f56c9e860b1eda Mon Sep 17 00:00:00 2001 From: Kris De Volder Date: Wed, 10 May 2017 12:37:38 -0700 Subject: [PATCH] Workaround for vscode 1.12 completion sorting issue. --- .../VscodeCompletionEngineAdapter.java | 87 +++++++++++++++++-- .../util/SimpleLanguageServer.java | 6 +- .../YamlCompletionEngineOptions.java | 2 +- .../languageserver/testharness/Editor.java | 1 + .../testharness/LanguageServerHarness.java | 13 +-- .../concourse/ConcourseLanguageServer.java | 15 +++- .../ConcourseLanguageServerTest.java | 2 +- 7 files changed, 105 insertions(+), 21 deletions(-) diff --git a/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/completion/VscodeCompletionEngineAdapter.java b/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/completion/VscodeCompletionEngineAdapter.java index d4c6652a2..2cebb3046 100644 --- a/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/completion/VscodeCompletionEngineAdapter.java +++ b/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/completion/VscodeCompletionEngineAdapter.java @@ -8,13 +8,15 @@ * Contributors: * Pivotal, Inc. - initial API and implementation *******************************************************************************/ - package org.springframework.ide.vscode.commons.languageserver.completion; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.concurrent.CompletableFuture; +import java.util.function.Consumer; import org.eclipse.lsp4j.CompletionItem; import org.eclipse.lsp4j.CompletionList; @@ -28,6 +30,7 @@ import org.springframework.ide.vscode.commons.languageserver.completion.Document import org.springframework.ide.vscode.commons.languageserver.util.SimpleLanguageServer; import org.springframework.ide.vscode.commons.languageserver.util.SimpleTextDocumentService; import org.springframework.ide.vscode.commons.languageserver.util.SortKeys; +import org.springframework.ide.vscode.commons.util.Log; import org.springframework.ide.vscode.commons.util.Renderable; import org.springframework.ide.vscode.commons.util.StringUtil; import org.springframework.ide.vscode.commons.util.text.TextDocument; @@ -40,18 +43,73 @@ import reactor.core.scheduler.Schedulers; */ public class VscodeCompletionEngineAdapter implements VscodeCompletionEngine { + public static class LazyCompletionResolver { + private int nextId = 0; //Used to assign unique id to completion items. + + private Map> resolvers = new HashMap<>(); + + private String nextId() { + //Warning: it's tempting to return 'int' and use a Integer object as id but... + // Looks like that breaks things because the Integer becomes a Double after being + // serialized and deserialized to json. + return ""+(nextId++); + } + + public synchronized String resolveLater(ICompletionProposal completion, TextDocument doc) { + String id = nextId(); + resolvers.put(id, (unresolved) -> { + try { + resolveItem(doc, completion, unresolved); + } catch (Exception e) { + Log.log(e); + } + }); + return id; + } + + public synchronized void resolveNow(CompletionItem unresolved) { + Object id = unresolved.getData(); + if (id!=null) { + Consumer resolver = resolvers.get(id); + if (resolver!=null) { + resolver.accept(unresolved); + unresolved.setData(null); //No longer needed after item is resolved. + Log.info("Resolved completion: "+unresolved); + } else { + Log.warn("Couldn't resolve completion item. Did it already get flushed from the resolver's cache? "+unresolved); + } + } + } + + public synchronized void clear() { + resolvers.clear(); + } + } + private final static int DEFAULT_MAX_COMPLETIONS = 50; private int maxCompletions = DEFAULT_MAX_COMPLETIONS; //TODO: move this to CompletionEngineOptions. final static Logger logger = LoggerFactory.getLogger(VscodeCompletionEngineAdapter.class); private SimpleLanguageServer server; private ICompletionEngine engine; + private LazyCompletionResolver resolver = null; public VscodeCompletionEngineAdapter(SimpleLanguageServer server, ICompletionEngine engine) { this.server = server; this.engine = engine; } + /** + * By setting a non-null {@link LazyCompletionResolver} you can enable lazy completion resolution. + * By default lazy resolution is not implemented. + *

+ * The resolver is injected rather than created locally to allow sharing it between multiple + * engines. + */ + public void setLazyCompletionResolver(LazyCompletionResolver resolver) { + this.resolver = resolver; + } + public void setMaxCompletionsNumber(int maxCompletions) { this.maxCompletions = maxCompletions; } @@ -66,6 +124,11 @@ public class VscodeCompletionEngineAdapter implements VscodeCompletionEngine { TextDocument doc = documents.get(params).copy(); if (doc!=null) { return Mono.fromCallable(() -> { + if (resolver!=null) { + //Assumes we don't have more than one completion request in flight from the client. + // So when a new request arrives we can forget about the old unresolved items: + resolver.clear(); + } //TODO: This callable is a 'big lump of work' so can't be canceled in pieces. // Should we push using of reactive streams down further and compose this all // using reactive style? If not then this is overkill could just as well use @@ -105,19 +168,27 @@ public class VscodeCompletionEngineAdapter implements VscodeCompletionEngine { item.setSortText(sortkeys.next()); item.setFilterText(completion.getFilterText()); item.setDetail(completion.getDetail()); - item.setDocumentation(toMarkdown(completion.getDocumentation())); - adaptEdits(item, doc, completion.getTextEdit()); + if (resolver!=null) { + item.setData(resolver.resolveLater(completion, doc)); + } else { + resolveItem(doc, completion, item); + } return item; } - private String toMarkdown(Renderable r) { + private static void resolveItem(TextDocument doc, ICompletionProposal completion, CompletionItem item) throws Exception { + item.setDocumentation(toMarkdown(completion.getDocumentation())); + adaptEdits(item, doc, completion.getTextEdit()); + } + + private static String toMarkdown(Renderable r) { if (r!=null) { return r.toMarkdown(); } return null; } - private void adaptEdits(CompletionItem item, TextDocument doc, DocumentEdits edits) throws Exception { + private static void adaptEdits(CompletionItem item, TextDocument doc, DocumentEdits edits) throws Exception { TextReplace replaceEdit = edits.asReplacement(doc); if (replaceEdit==null) { //The original edit does nothing. @@ -134,7 +205,7 @@ public class VscodeCompletionEngineAdapter implements VscodeCompletionEngine { } } - private String vscodeIndentFix(TextDocument doc, Position start, String newText) { + private static String vscodeIndentFix(TextDocument doc, Position start, String newText) { //Vscode applies some magic indent to a multi-line edit text. We do everything ourself so we have adjust for the magic // and do some kind of 'inverse magic' here. //See here: https://github.com/Microsoft/language-server-protocol/issues/83 @@ -149,9 +220,7 @@ public class VscodeCompletionEngineAdapter implements VscodeCompletionEngine { @Override public CompletableFuture resolveCompletion(CompletionItem unresolved) { - //TODO: item is pre-resoved so we don't do anything, but we really should somehow defer some work, such as - // for example computing docs and edits to resolve time. - //The tricky part is that we have to probably remember infos about the unresolved elements somehow so we can resolve later. + resolver.resolveNow(unresolved); return CompletableFuture.completedFuture(unresolved); } } diff --git a/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/SimpleLanguageServer.java b/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/SimpleLanguageServer.java index 6ad04769c..7eb4b7a8c 100644 --- a/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/SimpleLanguageServer.java +++ b/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/SimpleLanguageServer.java @@ -209,7 +209,7 @@ public abstract class SimpleLanguageServer implements LanguageServer, LanguageCl c.setHoverProvider(true); CompletionOptions completionProvider = new CompletionOptions(); - completionProvider.setResolveProvider(false); + completionProvider.setResolveProvider(hasLazyCompletionResolver()); c.setCompletionProvider(completionProvider); if (hasQuickFixes()) { @@ -233,6 +233,10 @@ public abstract class SimpleLanguageServer implements LanguageServer, LanguageCl return c; } + public boolean hasLazyCompletionResolver() { + return false; + } + private boolean hasDocumentSymbolHandler() { return getTextDocumentService().hasDocumentSymbolHandler(); } diff --git a/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/completion/YamlCompletionEngineOptions.java b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/completion/YamlCompletionEngineOptions.java index f10db2a60..d5c74d5a2 100644 --- a/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/completion/YamlCompletionEngineOptions.java +++ b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/completion/YamlCompletionEngineOptions.java @@ -19,7 +19,7 @@ public interface YamlCompletionEngineOptions { default boolean includeDeindentedProposals() { //Disabled by default for now because of bug introduced in VSCode 1.12: //https://github.com/Microsoft/vscode/issues/26096 - return false; + return true; } YamlCompletionEngineOptions DEFAULT = new YamlCompletionEngineOptions() {}; diff --git a/headless-services/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/languageserver/testharness/Editor.java b/headless-services/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/languageserver/testharness/Editor.java index 5ca2ce00d..6754acb94 100644 --- a/headless-services/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/languageserver/testharness/Editor.java +++ b/headless-services/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/languageserver/testharness/Editor.java @@ -372,6 +372,7 @@ public class Editor { } public void apply(CompletionItem completion) throws Exception { + completion = harness.resolveCompletionItem(completion); TextEdit edit = completion.getTextEdit(); String docText = doc.getText(); if (edit!=null) { diff --git a/headless-services/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/languageserver/testharness/LanguageServerHarness.java b/headless-services/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/languageserver/testharness/LanguageServerHarness.java index 1c219cf38..bcef9462a 100644 --- a/headless-services/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/languageserver/testharness/LanguageServerHarness.java +++ b/headless-services/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/languageserver/testharness/LanguageServerHarness.java @@ -391,12 +391,15 @@ public class LanguageServerHarness { } - public CompletionItem resolveCompletionItem(CompletionItem unresolved) { - try { - return server.getTextDocumentService().resolveCompletionItem(unresolved).get(); - } catch (Exception e) { - throw new RuntimeException(e); + public CompletionItem resolveCompletionItem(CompletionItem maybeUnresolved) { + if (server.hasLazyCompletionResolver()) { + try { + return server.getTextDocumentService().resolveCompletionItem(maybeUnresolved).get(); + } catch (Exception e) { + throw new RuntimeException(e); + } } + return maybeUnresolved; } public List resolveCompletions(CompletionList completions) { diff --git a/headless-services/concourse-language-server/src/main/java/org/springframework/ide/vscode/concourse/ConcourseLanguageServer.java b/headless-services/concourse-language-server/src/main/java/org/springframework/ide/vscode/concourse/ConcourseLanguageServer.java index 5e81d46c8..3e4c9e621 100644 --- a/headless-services/concourse-language-server/src/main/java/org/springframework/ide/vscode/concourse/ConcourseLanguageServer.java +++ b/headless-services/concourse-language-server/src/main/java/org/springframework/ide/vscode/concourse/ConcourseLanguageServer.java @@ -16,6 +16,7 @@ import java.util.concurrent.CompletableFuture; import org.eclipse.lsp4j.CompletionList; import org.eclipse.lsp4j.DiagnosticSeverity; import org.springframework.ide.vscode.commons.languageserver.completion.VscodeCompletionEngineAdapter; +import org.springframework.ide.vscode.commons.languageserver.completion.VscodeCompletionEngineAdapter.LazyCompletionResolver; import org.springframework.ide.vscode.commons.languageserver.hover.HoverInfoProvider; import org.springframework.ide.vscode.commons.languageserver.hover.VscodeHoverEngineAdapter; import org.springframework.ide.vscode.commons.languageserver.reconcile.IReconcileEngine; @@ -51,6 +52,7 @@ public class ConcourseLanguageServer extends SimpleLanguageServer { private SchemaSpecificPieces forPipelines; private SchemaSpecificPieces forTasks; private final YamlQuickfixes yamlQuickfixes; + private final LazyCompletionResolver completionResolver = new LazyCompletionResolver(); //Set this to null to disable lazy completion resolving private class SchemaSpecificPieces { @@ -63,6 +65,7 @@ public class ConcourseLanguageServer extends SimpleLanguageServer { SchemaBasedYamlAssistContextProvider contextProvider = new SchemaBasedYamlAssistContextProvider(schema); YamlCompletionEngine yamlCompletionEngine = new YamlCompletionEngine(structureProvider, contextProvider, COMPLETION_OPTIONS); this.completionEngine = new VscodeCompletionEngineAdapter(ConcourseLanguageServer.this, yamlCompletionEngine); + this.completionEngine.setLazyCompletionResolver(completionResolver); HoverInfoProvider infoProvider = new YamlHoverInfoProvider(currentAsts, structureProvider, contextProvider); this.hoverEngine = new VscodeHoverEngineAdapter(ConcourseLanguageServer.this, infoProvider); @@ -80,6 +83,11 @@ public class ConcourseLanguageServer extends SimpleLanguageServer { } } + @Override + public boolean hasLazyCompletionResolver() { + return completionResolver!=null; + } + public ConcourseLanguageServer(YamlCompletionEngineOptions completionOptions) { super("vscode-concourse"); this.COMPLETION_OPTIONS = completionOptions; @@ -124,10 +132,9 @@ public class ConcourseLanguageServer extends SimpleLanguageServer { } return CompletableFuture.completedFuture(new CompletionList(false, ImmutableList.of())); }); - documents.onCompletionResolve(params -> { - //this is a bogus implementation. But its not currently used. - throw new IllegalStateException("Not implemented"); - + documents.onCompletionResolve(item -> { + completionResolver.resolveNow(item); + return CompletableFuture.completedFuture(item); }); documents.onHover(params -> { TextDocument doc = documents.get(params); diff --git a/headless-services/concourse-language-server/src/test/java/org/springframework/ide/vscode/concourse/ConcourseLanguageServerTest.java b/headless-services/concourse-language-server/src/test/java/org/springframework/ide/vscode/concourse/ConcourseLanguageServerTest.java index 8a48f5d08..c0418f3f5 100644 --- a/headless-services/concourse-language-server/src/test/java/org/springframework/ide/vscode/concourse/ConcourseLanguageServerTest.java +++ b/headless-services/concourse-language-server/src/test/java/org/springframework/ide/vscode/concourse/ConcourseLanguageServerTest.java @@ -45,7 +45,7 @@ public class ConcourseLanguageServerTest { } private void assertExpectedInitResult(InitializeResult initResult) { - assertThat(initResult.getCapabilities().getCompletionProvider().getResolveProvider()).isFalse(); + assertThat(initResult.getCapabilities().getCompletionProvider().getResolveProvider()).isTrue(); assertThat(initResult.getCapabilities().getTextDocumentSync().getLeft()).isEqualTo(TextDocumentSyncKind.Incremental); }