Skip to content

Commit

Permalink
fix: completion becomes unresponsive while completing class names ins…
Browse files Browse the repository at this point in the history
…ide an anonymous class

This bug is caused when the user requests completions for class names inside an anonymous class (`new View.OnClickListener() { ... } ` for example).
The completion provider succeeds in finding the completion candidates. However, for creating CompletionItem instance for classes, it needs to know if the class that is being suggested
is a nested class or not. This is checked by `IJavaCompletionProvider.findTopLevelDeclaration()` which however, fails the check the nullability of the parent elements while traversing.
This results in an infinite loop causing the `SynchronizedTask` to hold on to the `CompileTask` instance which further prevents any compilation related tasks.

As the `CompileTask` is held due to the infinite loop, the Java completion provider simply returns empty results on further completion requests.
  • Loading branch information
itsaky committed Mar 4, 2023
1 parent 66b7f57 commit bdd119d
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 12 deletions.
Binary file modified app/src/main/assets/data/common/tooling-api-all.jar
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,15 @@ void setTask(CompileTask task) {
this.task = task;
}

public synchronized boolean isCompiling() {
return isCompiling || semaphore.hasQueuedThreads();
public synchronized boolean isBusy() {
return isCompiling || semaphore.availablePermits() == 0;
}

/**
* <b>FOR INTERNAL USE ONLY!</b>
*/
public void logStats() {
LOG.warn("[SynchronizedTask] isCompiling =", isCompiling);
LOG.warn("[SynchronizedTask] queuedLength =", semaphore.getQueueLength());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,10 @@ public boolean canComplete(Path file) {
@NonNull
@Override
public CompletionResult complete(@NonNull CompletionParams params) {
if (compiler.getSynchronizedTask().isCompiling()) {
final var synchronizedTask = compiler.getSynchronizedTask();
if (synchronizedTask.isBusy()) {
LOG.error("Cannot complete, a compilation task is already in progress");
synchronizedTask.logStats();
return CompletionResult.EMPTY;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ import com.itsaky.androidide.lsp.models.MatchLevel
import com.itsaky.androidide.lsp.models.MethodCompletionData
import com.itsaky.androidide.progress.ProgressManager.Companion.abortIfCancelled
import com.itsaky.androidide.utils.ILogger
import openjdk.source.tree.Tree
import openjdk.source.util.TreePath
import java.nio.file.Path
import jdkx.lang.model.element.Element
import jdkx.lang.model.element.ElementKind.ANNOTATION_TYPE
Expand All @@ -67,6 +65,8 @@ import jdkx.lang.model.element.ElementKind.TYPE_PARAMETER
import jdkx.lang.model.element.ExecutableElement
import jdkx.lang.model.element.TypeElement
import jdkx.lang.model.element.VariableElement
import openjdk.source.tree.Tree
import openjdk.source.util.TreePath

/**
* Completion provider for Java source code.
Expand All @@ -83,7 +83,6 @@ abstract class IJavaCompletionProvider(
protected lateinit var filePackage: String
protected lateinit var fileImports: Set<String>

@Suppress("Since15")
open fun complete(
task: CompileTask,
path: TreePath,
Expand All @@ -102,11 +101,11 @@ abstract class IJavaCompletionProvider(
* Provide completions with the given data.
*
* @param task The compilation task. Subclasses are expected to use this compile task instead of
* starting another compilation process.
* starting another compilation process.
* @param path The [TreePath] defining the [Tree] at the current position.
* @param partial The partial identifier.
* @param endsWithParen `true` if the statement at cursor ends with a parenthesis. `false`
* otherwise.
* otherwise.
*/
protected abstract fun doComplete(
task: CompileTask,
Expand Down Expand Up @@ -337,13 +336,13 @@ abstract class IJavaCompletionProvider(
val parameterTypes = Array(element.parameters.size) { "" }
val erasedParameterTypes = Array(parameterTypes.size) { "" }
val plusOverloads = overloads - 1

for (i in element.parameters.indices) {
val p = element.parameters[i].asType()
parameterTypes[i] = p.toString()
erasedParameterTypes[i] = types.erasure(p).toString()
}

return MethodCompletionData(
element.simpleName.toString(),
getClassCompletionData(type),
Expand Down Expand Up @@ -373,11 +372,11 @@ abstract class IJavaCompletionProvider(

var element: TypeElement? = this
while (true) {
if (element?.enclosingElement?.kind == PACKAGE) {
if (element == null || element.enclosingElement?.kind == PACKAGE) {
break
}

element = element?.enclosingElement as? TypeElement
element = element.enclosingElement as? TypeElement
}

return element!!
Expand Down

0 comments on commit bdd119d

Please sign in to comment.