Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Process parse errors (including error nodes) in TextDocumentState #492

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
252 changes: 94 additions & 158 deletions rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,27 +27,36 @@
package org.rascalmpl.vscode.lsp;

import java.time.Duration;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
import java.util.concurrent.atomic.AtomicStampedReference;
import java.util.List;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.Executor;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.CompletionException;
import java.util.function.BiFunction;
import java.util.function.Function;
import java.util.function.Supplier;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.eclipse.lsp4j.Diagnostic;
import org.eclipse.lsp4j.DiagnosticSeverity;
import org.eclipse.lsp4j.Position;
import org.eclipse.lsp4j.Range;
import org.rascalmpl.library.util.ErrorRecovery;
import org.rascalmpl.parser.gtd.exception.ParseError;
import org.rascalmpl.values.RascalValueFactory;
import org.rascalmpl.values.ValueFactoryFactory;
import org.rascalmpl.values.parsetrees.ITree;
import org.rascalmpl.vscode.lsp.util.Diagnostics;
import org.rascalmpl.vscode.lsp.util.Versioned;
import org.rascalmpl.vscode.lsp.util.concurrent.Debouncer;
import org.rascalmpl.vscode.lsp.util.locations.ColumnMaps;

import io.usethesource.vallang.IList;
import io.usethesource.vallang.ISourceLocation;
import io.usethesource.vallang.IValue;

/**
* TextDocumentState encapsulates the current contents of every open file editor,
Expand All @@ -59,30 +68,32 @@
* and ParametricTextDocumentService.
*/
public class TextDocumentState {

private static final ErrorRecovery RECOVERY =
new ErrorRecovery((RascalValueFactory) ValueFactoryFactory.getValueFactory());
private static final Logger logger = LogManager.getLogger(TextDocumentState.class);

private final BiFunction<ISourceLocation, String, CompletableFuture<ITree>> parser;
private final ISourceLocation location;
private final ColumnMaps columns;

@SuppressWarnings("java:S3077") // Visibility of writes is enough
private volatile Update current;
private final Debouncer<Versioned<ITree>> currentTreeAsyncDebouncer;
private final Debouncer<Update> currentAsyncParseDebouncer;
sungshik marked this conversation as resolved.
Show resolved Hide resolved

private final AtomicReference<@MonotonicNonNull Versioned<ITree>> lastWithoutErrors;
private final AtomicReference<@MonotonicNonNull Versioned<ITree>> last;

public TextDocumentState(
BiFunction<ISourceLocation, String, CompletableFuture<ITree>> parser,
ISourceLocation location, int initialVersion, String initialContent) {
ISourceLocation location, ColumnMaps columns,
int initialVersion, String initialContent) {

this.parser = parser;
this.location = location;
this.columns = columns;

this.current = new Update(initialVersion, initialContent);
this.currentTreeAsyncDebouncer = new Debouncer<>(50,
this::getCurrentTreeAsyncIfParsing, this::getCurrentTreeAsync);
this.currentAsyncParseDebouncer = new Debouncer<>(50,
this::getCurrentAsyncIfParsing,
this::parseAndGetCurrentAsync);

this.lastWithoutErrors = new AtomicReference<>();
this.last = new AtomicReference<>();
Expand All @@ -95,7 +106,7 @@ public ISourceLocation getLocation() {
public void update(int version, String content) {
current = new Update(version, content);
// The creation of the `Update` object doesn't trigger the parser yet.
// This happens only when the tree is requested.
// This happens only when the tree or diagnostics are requested.
}

public Versioned<String> getCurrentContent() {
Expand All @@ -107,17 +118,21 @@ public CompletableFuture<Versioned<ITree>> getCurrentTreeAsync() {
}

public CompletableFuture<Versioned<ITree>> getCurrentTreeAsync(Duration delay) {
return currentTreeAsyncDebouncer.get(delay);
return currentAsyncParseDebouncer
.get(delay)
.thenApply(Update::getTreeAsync)
.thenCompose(Function.identity());
sungshik marked this conversation as resolved.
Show resolved Hide resolved
}

public @Nullable CompletableFuture<Versioned<ITree>> getCurrentTreeAsyncIfParsing() {
var update = current;
return update.isParsing() ? update.getTreeAsync() : null;
public CompletableFuture<Versioned<List<Diagnostic>>> getCurrentDiagnosticsAsync() {
return current.getDiagnosticsAsync(); // Triggers the parser
}

public CompletableFuture<Versioned<List<Diagnostic>>> getCurrentDiagnostics() {
throw new UnsupportedOperationException();
// TODO: In a separate PR
public CompletableFuture<Versioned<List<Diagnostic>>> getCurrentDiagnosticsAsync(Duration delay) {
return currentAsyncParseDebouncer
.get(delay)
.thenApply(Update::getDiagnosticsAsync)
.thenCompose(Function.identity());
}

public @MonotonicNonNull Versioned<ITree> getLastTree() {
Expand All @@ -128,16 +143,29 @@ public CompletableFuture<Versioned<List<Diagnostic>>> getCurrentDiagnostics() {
return lastWithoutErrors.get();
}

private @Nullable CompletableFuture<Update> getCurrentAsyncIfParsing() {
var update = current;
return update.isParsing() ? CompletableFuture.completedFuture(update) : null;
}

private CompletableFuture<Update> parseAndGetCurrentAsync() {
var update = current;
update.parseIfNotParsing();
return CompletableFuture.completedFuture(update);
}

private class Update {
private final int version;
private final String content;
private final CompletableFuture<Versioned<ITree>> treeAsync;
private final CompletableFuture<Versioned<List<Diagnostic>>> diagnosticsAsync;
private final AtomicBoolean parsing;

public Update(int version, String content) {
this.version = version;
this.content = content;
this.treeAsync = new CompletableFuture<>();
this.diagnosticsAsync = new CompletableFuture<>();
this.parsing = new AtomicBoolean(false);
}

Expand All @@ -150,6 +178,11 @@ public CompletableFuture<Versioned<ITree>> getTreeAsync() {
return treeAsync;
}

public CompletableFuture<Versioned<List<Diagnostic>>> getDiagnosticsAsync() {
parseIfNotParsing();
return diagnosticsAsync;
}

public boolean isParsing() {
return parsing.get();
}
Expand All @@ -158,154 +191,57 @@ private void parseIfNotParsing() {
if (parsing.compareAndSet(false, true)) {
parser
.apply(location, content)
.thenApply(t -> new Versioned<>(version, t))
.whenComplete((t, error) -> {
if (t != null) {
var errors = RECOVERY.findAllErrors(t.get());
if (errors.isEmpty()) {
Versioned.replaceIfNewer(lastWithoutErrors, t);
.whenComplete((t, e) -> {

// Prepare result values for futures
var tree = new Versioned<>(version, t);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a <tree,diagnostics> pair seem to belong together (not suprisingly), would ik make sense to have one Versioned object with a version and a pair of <tree,diagnostics>?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In principle, yes, it would. However, in many places in the project (including the Parametric... classes), a Versioned<ITree> is currently expected, so it seems quite an invasive change to supply a Versioned<Pair<ITree,List<Diagnostic>> instead. I'm not worried about the complicated type parameter btw (i.e., we could hide the structure with an auxiliary class). But it's just a lot of changes to places that we don't really need/hope to touch right now 😉.

At the same time, I imagine the reporting of diagnostics can be streamlined a bit more if the diagnostics become accessible in some places where currently there's only a parse tree. Tt requires a bit more thought, though. I think I'd prefer to do that in a separate PR.

var diagnostics = new Versioned<>(version, toDiagnostics(t, e));

// Complete future to get the tree
if (t == null) {
treeAsync.completeExceptionally(e);
} else {
treeAsync.complete(tree);
Versioned.replaceIfNewer(last, tree);
if (diagnostics.get().isEmpty()) {
Versioned.replaceIfNewer(lastWithoutErrors, tree);
}
Versioned.replaceIfNewer(last, t);
treeAsync.complete(t);
}
if (error != null) {
treeAsync.completeExceptionally(error);
}

// Complete future to get diagnostics
diagnosticsAsync.complete(diagnostics);
});
}
}
}
}

/**
* A *debouncer* is an object to get a *resource* from an *underlying resource
* provider* with a certain delay. From the perspective of the debouncer, the
* underlying resource provider has two states: initialized and not-initialized.
*
* 1. While the underlying resource provider is not-initialized (e.g., the
* computation of a parse tree has not yet started), the debouncer waits
* until the delay is over.
*
* 2. When the underlying resource provider becomes initialized (e.g., the
* computation of a parse tree has started, but possibly not yet finished),
* the debouncer returns a future for the resource.
*
* 3. When the underlying resource provider is not-initialized, but the delay
* is over, the debouncer forcibly initializes the resource (e.g., it starts
* the asynchronous computation of a parse tree) and returns a future for
* the resource.
*/
class Debouncer<T> {

// A debouncer is implemented using a *delayed executor* as `scheduler`. The
// idea is to *periodically* check the state of the underlying resource
// provider. More precisely, each time when the resource is requested,
// immediately check if case 2 or case 3 (above) are applicable. If so,
// return. If not, schedule a *delayed future* to retry the request, to be
// completed after a small `period` (e.g., 50 milliseconds).
//
// The reason why multiple futures are scheduled in small periods, instead
// of a single future for the entire large delay, is that futures (of type
// `CompletableFuture`) cannot be interrupted.

private final int period; // Milliseconds
private final Executor scheduler;

// At any point in time, only one delayed future to retry the request for
// the resource should be `scheduled`, tied with the total remaining delay.
// For bookkeeping, a *stamped reference* is used. The reference is the
// delayed future, while the stamp is the remaining delay *upon completion
// of the delayed future*.

private final AtomicStampedReference<@Nullable CompletableFuture<T>> scheduled;

// The underlying resource provider is represented abstractly in terms of
// two suppliers, each of which corresponds with a state of the underlying
// resource provider. `getIfInitialized` should return `null` iff the
// underlying resource provider is not-initialized.

private final Supplier<@Nullable CompletableFuture<T>> getIfInitialized;
private final Supplier<CompletableFuture<T>> initializeAndGet;

public Debouncer(Duration period,
Supplier<@Nullable CompletableFuture<T>> getIfInitialized,
Supplier<CompletableFuture<T>> initializeAndGet) {

this(Math.toIntExact(period.toMillis()), getIfInitialized, initializeAndGet);
}

public Debouncer(int period,
Supplier<@Nullable CompletableFuture<T>> getIfInitialized,
Supplier<CompletableFuture<T>> initializeAndGet) {

this.period = period;
this.scheduler = CompletableFuture.delayedExecutor(period, TimeUnit.MILLISECONDS);
this.scheduled = new AtomicStampedReference<>(null, 0);
this.getIfInitialized = getIfInitialized;
this.initializeAndGet = initializeAndGet;
}

public CompletableFuture<T> get(Duration delay) {
return get(Math.toIntExact(delay.toMillis()));
}

public CompletableFuture<T> get(int delay) {
return schedule(delay, false);
}
private List<Diagnostic> toDiagnostics(ITree tree, Throwable excp) {
sungshik marked this conversation as resolved.
Show resolved Hide resolved
List<Diagnostic> parseErrors = new ArrayList<>();

private CompletableFuture<T> schedule(int delay, boolean reschedule) {

// Get a consistent old stamp and old reference
var oldRef = scheduled.getReference();
var oldStamp = scheduled.getStamp();
while (!scheduled.compareAndSet(oldRef, oldRef, oldStamp, oldStamp));

// Compute a new reference (= delayed future to retry this method)
var delayArg = new CompletableFuture<Integer>();
var newRef = delayArg
.thenApplyAsync(this::reschedule, scheduler)
.thenCompose(Function.identity());

// Compute a new stamp
var delayRemaining = Math.max(oldStamp, delay);
var newStamp = delayRemaining - period;

// If the underlying resource provider is initialized, then return the
// future to get the resource
var future = getIfInitialized.get();
if (future != null && scheduled.compareAndSet(oldRef, null, oldStamp, 0)) {
return future;
}
if (excp instanceof CompletionException) {
excp = excp.getCause();
}

// Otherwise, if the delay is over already, then initialize the
// underlying resource provider and return the future to get the
// resource
if (delayRemaining <= 0 && scheduled.compareAndSet(oldRef, null, oldStamp, 0)) {
return initializeAndGet.get();
}
if (excp instanceof ParseError) {
parseErrors.add(Diagnostics.translateDiagnostic((ParseError)excp, columns));
} else if (excp != null) {
logger.error("Parsing crashed", excp);
parseErrors.add(new Diagnostic(
new Range(new Position(0,0), new Position(0,1)),
"Parsing failed: " + excp.getMessage(),
DiagnosticSeverity.Error,
"Rascal Parser"));
}

// Otherwise (i.e., the delay isn't over yet), if a delayed future to
// retry this method hasn't been scheduled yet, or if it must be
// rescheduled regardless, then schedule it
if ((oldRef == null || reschedule) && scheduled.compareAndSet(oldRef, newRef, oldStamp, newStamp)) {
delayArg.complete(newStamp);
return newRef;
}
if (tree != null) {
RascalValueFactory valueFactory = (RascalValueFactory) ValueFactoryFactory.getValueFactory();
IList errors = new ErrorRecovery(valueFactory).findAllErrors(tree);
for (IValue error : errors) {
ITree errorTree = (ITree) error;
parseErrors.add(Diagnostics.translateErrorRecoveryDiagnostic(errorTree, columns));
}
}

// Otherwise (i.e, the delay is not yet over, but a delayed future has
// been scheduled already), then update the remaining delay; it will be
// used by the already-scheduled delayed future.
if (scheduled.attemptStamp(oldRef, newStamp)) {
return oldRef;
return parseErrors;
}

// When this point is reached, concurrent modifications to the stamp or
// the reference in `scheduled` have happened. In that case, retry
// immediately.
return schedule(delay, reschedule);
}

private CompletableFuture<T> reschedule(int delay) {
return schedule(delay, true);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,7 @@ private ParametricFileFacts facts(String doc) {

private TextDocumentState open(TextDocumentItem doc) {
return files.computeIfAbsent(Locations.toLoc(doc),
l -> new TextDocumentState(contributions(doc)::parseSourceFile, l, doc.getVersion(), doc.getText())
l -> new TextDocumentState(contributions(doc)::parseSourceFile, l, columns, doc.getVersion(), doc.getText())
);
}

Expand Down Expand Up @@ -638,7 +638,7 @@ public CompletableFuture<List<Either<Command, CodeAction>>> codeAction(CodeActio

private CompletableFuture<IList> computeCodeActions(final ILanguageContributions contribs, final int startLine, final int startColumn, ITree tree) {
IList focus = TreeSearch.computeFocusList(tree, startLine, startColumn);

if (!focus.isEmpty()) {
return contribs.codeActions(focus).get();
}
Expand Down
Loading
Loading