Skip to content
This repository has been archived by the owner on Oct 18, 2024. It is now read-only.

Commit

Permalink
fix(lsp): make signature help requests cancellable (fixes #1393)
Browse files Browse the repository at this point in the history
  • Loading branch information
@itsaky
itsaky committed Nov 11, 2023
1 parent 5d4eac3 commit 5373698
Show file tree
Hide file tree
Showing 6 changed files with 122 additions and 48 deletions.
24 changes: 15 additions & 9 deletions common/src/main/java/com/itsaky/androidide/tasks/coroutineUtils.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,20 @@ import java.io.InterruptedIOException
import kotlin.coroutines.CoroutineContext
import kotlin.coroutines.EmptyCoroutineContext

/**
* An [ICancelChecker] which when cancelled, cancels the corresponding [Job].
*/
class JobCancelChecker @JvmOverloads constructor(
var job: Job? = null
) : ICancelChecker.Default() {

override fun cancel() {
job?.cancel("Cancelled by user")
job = null
super.cancel()
}
}

/**
* Calls [CoroutineScope.cancel] only if a job is active in the scope.
*
Expand Down Expand Up @@ -69,15 +83,7 @@ inline fun CoroutineScope.launchAsyncWithProgress(
crossinline action: suspend CoroutineScope.(flashbar: Flashbar, cancelChecker: ICancelChecker) -> Unit
): Job? {

val cancelChecker = object : ICancelChecker.Default() {
var job: Job? = null

override fun cancel() {
job?.cancel("Cancelled by user")
job = null
super.cancel()
}
}
val cancelChecker = JobCancelChecker()

return flashProgress({
configureFlashbar(this, cancelChecker)
Expand Down
22 changes: 21 additions & 1 deletion editor/src/main/java/com/itsaky/androidide/editor/ui/IDEEditor.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import com.itsaky.androidide.eventbus.events.editor.DocumentSelectedEvent
import com.itsaky.androidide.flashbar.Flashbar
import com.itsaky.androidide.lsp.api.ILanguageClient
import com.itsaky.androidide.lsp.api.ILanguageServer
import com.itsaky.androidide.lsp.java.utils.CancelChecker
import com.itsaky.androidide.lsp.models.Command
import com.itsaky.androidide.lsp.models.DefinitionParams
import com.itsaky.androidide.lsp.models.DefinitionResult
Expand All @@ -67,6 +68,7 @@ import com.itsaky.androidide.preferences.internal.visiblePasswordFlag
import com.itsaky.androidide.progress.ICancelChecker
import com.itsaky.androidide.syntax.colorschemes.DynamicColorScheme
import com.itsaky.androidide.syntax.colorschemes.SchemeAndroidIDE
import com.itsaky.androidide.tasks.JobCancelChecker
import com.itsaky.androidide.tasks.cancelIfActive
import com.itsaky.androidide.tasks.launchAsyncWithProgress
import com.itsaky.androidide.utils.DocumentUtils
Expand Down Expand Up @@ -107,6 +109,7 @@ open class IDEEditor @JvmOverloads constructor(
private val editorFeatures: EditorFeatures = EditorFeatures()
) : CodeEditor(context, attrs, defStyleAttr, defStyleRes), IEditor by editorFeatures, ILspEditor {

@Suppress("PropertyName")
internal var _file: File? = null

private var _actionsMenu: EditorActionsMenu? = null
Expand All @@ -132,6 +135,8 @@ open class IDEEditor @JvmOverloads constructor(
protected val editorScope = CoroutineScope(Dispatchers.Default)
protected val eventDispatcher = EditorEventDispatcher()

private var sigHelpCancelChecker: ICancelChecker? = null

var languageServer: ILanguageServer? = null
private set

Expand Down Expand Up @@ -259,13 +264,28 @@ open class IDEEditor @JvmOverloads constructor(

this.languageClient ?: return

sigHelpCancelChecker?.also { it.cancel() }

val cancelChecker = JobCancelChecker().also {
this.sigHelpCancelChecker = it
}

editorScope.launch {
val params = SignatureHelpParams(file.toPath(), cursorLSPPosition)
cancelChecker.job = coroutineContext[Job]
val params = SignatureHelpParams(file.toPath(), cursorLSPPosition, cancelChecker)
val help = languageServer.signatureHelp(params)

withContext(Dispatchers.Main) {
showSignatureHelp(help)
}
}.invokeOnCompletion { error ->
if (error != null) {
if (!CancelChecker.isCancelled(error)) {
log.error("Failed to compute signature help", error)
} else {
log.debug("Signature help request cancelled")
}
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion lsp/java/src/main/java/com/itsaky/androidide/lsp/java/JavaLanguageServer.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ class JavaLanguageServer : ILanguageServer {
val compiler = getCompiler(params.file)
return if (!settings.signatureHelpEnabled()) {
SignatureHelp(emptyList(), -1, -1)
} else SignatureProvider(compiler).signatureHelp(params)
} else SignatureProvider(compiler, params.cancelChecker).signatureHelp(params)
}

override suspend fun analyze(file: Path): DiagnosticResult {
Expand Down
105 changes: 70 additions & 35 deletions lsp/java/src/main/java/com/itsaky/androidide/lsp/java/providers/SignatureProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,9 @@
package com.itsaky.androidide.lsp.java.providers;

import androidx.annotation.NonNull;

import com.itsaky.androidide.lsp.java.compiler.CompileTask;
import com.itsaky.androidide.lsp.java.compiler.CompilerProvider;
import com.itsaky.androidide.lsp.java.compiler.SynchronizedTask;
import com.itsaky.androidide.lsp.java.parser.ParseTask;
import com.itsaky.androidide.lsp.java.utils.FindHelper;
import com.itsaky.androidide.lsp.java.utils.MarkdownHelper;
import com.itsaky.androidide.lsp.java.utils.ScopeHelper;
Expand All @@ -32,29 +30,13 @@
import com.itsaky.androidide.lsp.models.SignatureHelp;
import com.itsaky.androidide.lsp.models.SignatureHelpParams;
import com.itsaky.androidide.lsp.models.SignatureInformation;
import openjdk.source.doctree.DocCommentTree;
import openjdk.source.tree.CompilationUnitTree;
import openjdk.source.tree.ExpressionTree;
import openjdk.source.tree.IdentifierTree;
import openjdk.source.tree.MemberSelectTree;
import openjdk.source.tree.MethodInvocationTree;
import openjdk.source.tree.MethodTree;
import openjdk.source.tree.NewClassTree;
import openjdk.source.tree.Scope;
import openjdk.source.tree.VariableTree;
import openjdk.source.util.DocTrees;
import openjdk.source.util.SourcePositions;
import openjdk.source.util.TreePath;
import openjdk.source.util.Trees;

import com.itsaky.androidide.progress.ICancelChecker;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.StringJoiner;
import java.util.function.Predicate;

import jdkx.lang.model.element.Element;
import jdkx.lang.model.element.ElementKind;
import jdkx.lang.model.element.ExecutableElement;
Expand All @@ -67,15 +49,28 @@
import jdkx.lang.model.type.PrimitiveType;
import jdkx.lang.model.type.TypeMirror;
import jdkx.lang.model.type.TypeVariable;
import jdkx.tools.JavaFileObject;
import openjdk.source.tree.CompilationUnitTree;
import openjdk.source.tree.ExpressionTree;
import openjdk.source.tree.IdentifierTree;
import openjdk.source.tree.MemberSelectTree;
import openjdk.source.tree.MethodInvocationTree;
import openjdk.source.tree.MethodTree;
import openjdk.source.tree.NewClassTree;
import openjdk.source.tree.Scope;
import openjdk.source.tree.VariableTree;
import openjdk.source.util.DocTrees;
import openjdk.source.util.SourcePositions;
import openjdk.source.util.TreePath;
import openjdk.source.util.Trees;

public class SignatureProvider {
public class SignatureProvider extends CancelableServiceProvider {

public static final SignatureHelp NOT_SUPPORTED =
new SignatureHelp(Collections.emptyList(), -1, -1);
private final CompilerProvider compiler;

public SignatureProvider(CompilerProvider compiler) {
public SignatureProvider(CompilerProvider compiler, ICancelChecker cancelChecker) {
super(cancelChecker);
this.compiler = compiler;
}

Expand All @@ -94,11 +89,14 @@ public SignatureHelp signatureHelp(Path file, int l, int c) {

// TODO prune
SynchronizedTask synchronizedTask = compiler.compile(file);
abortIfCancelled();
return synchronizedTask.get(
task -> {
long cursor = task.root().getLineMap().getPosition(line, column);
TreePath path = new FindInvocationAt(task.task).scan(task.root(), cursor);
if (path == null) return NOT_SUPPORTED;
TreePath path = new FindInvocationAt(task.task, this).scan(task.root(), cursor);
if (path == null) {
return NOT_SUPPORTED;
}
if (path.getLeaf() instanceof MethodInvocationTree) {
MethodInvocationTree invoke = (MethodInvocationTree) path.getLeaf();
List<ExecutableElement> overloads = methodOverloads(task, invoke);
Expand Down Expand Up @@ -133,6 +131,7 @@ public SignatureHelp signatureHelp(Path file, int l, int c) {

private List<ExecutableElement> methodOverloads(
CompileTask task, @NonNull MethodInvocationTree method) {
abortIfCancelled();
if (method.getMethodSelect() instanceof IdentifierTree) {
IdentifierTree id = (IdentifierTree) method.getMethodSelect();
return scopeOverloads(task, id);
Expand All @@ -146,6 +145,7 @@ private List<ExecutableElement> methodOverloads(

@NonNull
private List<ExecutableElement> scopeOverloads(@NonNull CompileTask task, IdentifierTree method) {
abortIfCancelled();
Trees trees = Trees.instance(task.task);
TreePath path = trees.getPath(task.root(), method);
Scope scope = trees.getScope(path);
Expand All @@ -163,6 +163,7 @@ private List<ExecutableElement> scopeOverloads(@NonNull CompileTask task, Identi
@NonNull
private List<ExecutableElement> memberOverloads(
@NonNull CompileTask task, @NonNull MemberSelectTree method) {
abortIfCancelled();
Trees trees = Trees.instance(task.task);
TreePath path = trees.getPath(task.root(), method.getExpression());
boolean isStatic = trees.getElement(path) instanceof TypeElement;
Expand All @@ -175,16 +176,25 @@ private List<ExecutableElement> memberOverloads(

List<ExecutableElement> list = new ArrayList<>();
for (Element member : task.task.getElements().getAllMembers(type)) {
if (member.getKind() != ElementKind.METHOD) continue;
if (!member.getSimpleName().contentEquals(method.getIdentifier())) continue;
if (isStatic != member.getModifiers().contains(Modifier.STATIC)) continue;
if (!trees.isAccessible(scope, member, (DeclaredType) type.asType())) continue;
if (member.getKind() != ElementKind.METHOD) {
continue;
}
if (!member.getSimpleName().contentEquals(method.getIdentifier())) {
continue;
}
if (isStatic != member.getModifiers().contains(Modifier.STATIC)) {
continue;
}
if (!trees.isAccessible(scope, member, (DeclaredType) type.asType())) {
continue;
}
list.add((ExecutableElement) member);
}
return list;
}

private TypeElement typeElement(TypeMirror type) {
abortIfCancelled();
if (type instanceof DeclaredType) {
DeclaredType declared = (DeclaredType) type;
return (TypeElement) declared.asElement();
Expand All @@ -199,21 +209,27 @@ private TypeElement typeElement(TypeMirror type) {
@NonNull
private List<ExecutableElement> constructorOverloads(
@NonNull CompileTask task, @NonNull NewClassTree method) {
abortIfCancelled();
Trees trees = Trees.instance(task.task);
TreePath path = trees.getPath(task.root(), method.getIdentifier());
Scope scope = trees.getScope(path);
TypeElement type = (TypeElement) trees.getElement(path);
List<ExecutableElement> list = new ArrayList<>();
for (Element member : task.task.getElements().getAllMembers(type)) {
if (member.getKind() != ElementKind.CONSTRUCTOR) continue;
if (!trees.isAccessible(scope, member, (DeclaredType) type.asType())) continue;
if (member.getKind() != ElementKind.CONSTRUCTOR) {
continue;
}
if (!trees.isAccessible(scope, member, (DeclaredType) type.asType())) {
continue;
}
list.add((ExecutableElement) member);
}
return list;
}

@NonNull
private SignatureInformation info(@NonNull ExecutableElement method) {
abortIfCancelled();
SignatureInformation info = new SignatureInformation();
info.setLabel(method.getSimpleName().toString());
if (method.getKind() == ElementKind.CONSTRUCTOR) {
Expand All @@ -225,6 +241,7 @@ private SignatureInformation info(@NonNull ExecutableElement method) {

@NonNull
private List<ParameterInformation> parameters(@NonNull ExecutableElement method) {
abortIfCancelled();
List<ParameterInformation> list = new ArrayList<>();
for (VariableElement p : method.getParameters()) {
list.add(parameter(p));
Expand All @@ -234,6 +251,7 @@ private List<ParameterInformation> parameters(@NonNull ExecutableElement method)

@NonNull
private ParameterInformation parameter(@NonNull VariableElement p) {
abortIfCancelled();
ParameterInformation info = new ParameterInformation();
info.setLabel(ShortTypePrinter.NO_PACKAGE.print(p.asType()));
return info;
Expand All @@ -244,6 +262,7 @@ private void addSourceInfo(
@NonNull ExecutableElement method,
@NonNull SignatureInformation info
) {
abortIfCancelled();
final var type = (TypeElement) method.getEnclosingElement();
final var className = type.getQualifiedName().toString();
final var methodName = method.getSimpleName().toString();
Expand Down Expand Up @@ -271,6 +290,7 @@ private void addSourceInfo(
}

private void addFancyLabel(@NonNull SignatureInformation info) {
abortIfCancelled();
StringJoiner join = new StringJoiner(", ");
for (ParameterInformation p : info.getParameters()) {
join.add(p.getLabel());
Expand All @@ -280,6 +300,7 @@ private void addFancyLabel(@NonNull SignatureInformation info) {

@NonNull
private List<ParameterInformation> parametersFromSource(MethodTree source) {
abortIfCancelled();
List<ParameterInformation> list = new ArrayList<>();
for (VariableTree p : source.getParameters()) {
ParameterInformation info = new ParameterInformation();
Expand All @@ -291,6 +312,7 @@ private List<ParameterInformation> parametersFromSource(MethodTree source) {

private int activeParameter(
@NonNull CompileTask task, @NonNull List<? extends ExpressionTree> arguments, long cursor) {
abortIfCancelled();
SourcePositions pos = Trees.instance(task.task).getSourcePositions();
CompilationUnitTree root = task.root();
for (int i = 0; i < arguments.size(); i++) {
Expand All @@ -307,6 +329,7 @@ private int activeSignature(
TreePath invocation,
List<? extends ExpressionTree> arguments,
List<ExecutableElement> overloads) {
abortIfCancelled();
for (int i = 0; i < overloads.size(); i++) {
if (isCompatible(task, invocation, arguments, overloads.get(i))) {
return i;
Expand All @@ -320,33 +343,45 @@ private boolean isCompatible(
TreePath invocation,
List<? extends ExpressionTree> arguments,
ExecutableElement overload) {
if (arguments.size() > overload.getParameters().size()) return false;
abortIfCancelled();
if (arguments.size() > overload.getParameters().size()) {
return false;
}
for (int i = 0; i < arguments.size(); i++) {
ExpressionTree argument = arguments.get(i);
TypeMirror argumentType =
Trees.instance(task.task).getTypeMirror(new TreePath(invocation, argument));
TypeMirror parameterType = overload.getParameters().get(i).asType();
if (!isCompatible(task, argumentType, parameterType)) return false;
if (!isCompatible(task, argumentType, parameterType)) {
return false;
}
}
return true;
}

private boolean isCompatible(CompileTask task, TypeMirror argument, TypeMirror parameter) {
if (argument instanceof ErrorType) return true;
abortIfCancelled();
if (argument instanceof ErrorType) {
return true;
}
if (argument instanceof PrimitiveType) {
argument = task.task.getTypes().boxedClass((PrimitiveType) argument).asType();
}
if (parameter instanceof PrimitiveType) {
parameter = task.task.getTypes().boxedClass((PrimitiveType) parameter).asType();
}
if (argument instanceof ArrayType) {
if (!(parameter instanceof ArrayType)) return false;
if (!(parameter instanceof ArrayType)) {
return false;
}
ArrayType argumentA = (ArrayType) argument;
ArrayType parameterA = (ArrayType) parameter;
return isCompatible(task, argumentA.getComponentType(), parameterA.getComponentType());
}
if (argument instanceof DeclaredType) {
if (!(parameter instanceof DeclaredType)) return false;
if (!(parameter instanceof DeclaredType)) {
return false;
}
argument = task.task.getTypes().erasure(argument);
parameter = task.task.getTypes().erasure(parameter);
return argument.toString().equals(parameter.toString());
Expand Down
Loading

0 comments on commit 5373698

Please sign in to comment.