From 20948c6e3678f463acaf98aea443bb75d96a9ef9 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Tue, 8 Oct 2024 12:58:21 +0200 Subject: [PATCH 01/28] Update `parse` call to use error recovery --- .../vscode/lsp/util/RascalServices.java | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/RascalServices.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/RascalServices.java index 93cafecce..a80506e00 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/RascalServices.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/RascalServices.java @@ -27,19 +27,39 @@ package org.rascalmpl.vscode.lsp.util; import org.rascalmpl.library.lang.rascal.syntax.RascalParser; +import org.rascalmpl.library.util.ErrorRecovery; import org.rascalmpl.parser.Parser; -import org.rascalmpl.parser.gtd.result.action.IActionExecutor; import org.rascalmpl.parser.gtd.result.out.DefaultNodeFlattener; +import org.rascalmpl.parser.gtd.util.StackNodeIdDispenser; import org.rascalmpl.parser.uptr.UPTRNodeFactory; import org.rascalmpl.parser.uptr.action.NoActionExecutor; +import org.rascalmpl.parser.uptr.recovery.ToTokenRecoverer; +import org.rascalmpl.values.RascalValueFactory; +import org.rascalmpl.values.ValueFactoryFactory; import org.rascalmpl.values.parsetrees.ITree; +import io.usethesource.vallang.IBool; import io.usethesource.vallang.ISourceLocation; public class RascalServices { + + private static final RascalValueFactory VALUE_FACTORY = (RascalValueFactory) ValueFactoryFactory.getValueFactory(); + private static final IBool TRUE = VALUE_FACTORY.bool(true); + public static ITree parseRascalModule(ISourceLocation loc, char[] input) { - IActionExecutor actions = new NoActionExecutor(); - return new RascalParser().parse(Parser.START_MODULE, loc.getURI(), input, actions, - new DefaultNodeFlattener<>(), new UPTRNodeFactory(true)); + // TODO: Which of these objects are stateless and can be reused? + + // Parse + RascalParser parser = new RascalParser(); + ITree tree = parser.parse( + Parser.START_MODULE, loc.getURI(), input, + new NoActionExecutor(), + new DefaultNodeFlattener<>(), + new UPTRNodeFactory(true), + new ToTokenRecoverer(loc.getURI(), parser, new StackNodeIdDispenser(parser))); + + // Recover + ErrorRecovery recoverer = new ErrorRecovery(VALUE_FACTORY); + return (ITree) recoverer.disambiguateErrors(tree, TRUE); } } From ea87d9e30ccff61c07a968f1e029b5b3400581bc Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Tue, 8 Oct 2024 12:59:44 +0200 Subject: [PATCH 02/28] Update `pom.xml` for development --- rascal-lsp/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rascal-lsp/pom.xml b/rascal-lsp/pom.xml index 5739f1f68..3543c27db 100644 --- a/rascal-lsp/pom.xml +++ b/rascal-lsp/pom.xml @@ -65,7 +65,7 @@ org.rascalmpl rascal - 0.40.7 + 0.40.9-SNAPSHOT org.rascalmpl From 6fa7a8262a33bb843530bc8311a09fd9cc16dccc Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Tue, 8 Oct 2024 14:33:04 +0200 Subject: [PATCH 03/28] Add reporting of diagnostics for error nodes --- .../lsp/rascal/RascalTextDocumentService.java | 33 ++++++++++++++----- .../vscode/lsp/util/Diagnostics.java | 14 +++++++- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java index 5e41f55ad..2958a6915 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java @@ -84,8 +84,11 @@ import org.eclipse.lsp4j.services.LanguageClient; import org.eclipse.lsp4j.services.LanguageClientAware; import org.rascalmpl.library.Prelude; +import org.rascalmpl.library.util.ErrorRecovery; import org.rascalmpl.parser.gtd.exception.ParseError; import org.rascalmpl.uri.URIResolverRegistry; +import org.rascalmpl.values.RascalValueFactory; +import org.rascalmpl.values.ValueFactoryFactory; import org.rascalmpl.values.parsetrees.ITree; import org.rascalmpl.vscode.lsp.BaseWorkspaceService; import org.rascalmpl.vscode.lsp.IBaseLanguageClient; @@ -105,6 +108,7 @@ import org.rascalmpl.vscode.lsp.util.locations.LineColumnOffsetMap; import org.rascalmpl.vscode.lsp.util.locations.Locations; +import io.usethesource.vallang.IList; import io.usethesource.vallang.ISourceLocation; import io.usethesource.vallang.IValue; @@ -215,26 +219,37 @@ private TextDocumentState updateContents(VersionedTextDocumentIdentifier doc, St private void handleParsingErrors(TextDocumentState file, CompletableFuture> futureTree) { futureTree.handle((tree, excp) -> { - Diagnostic newParseError = null; - if (excp != null && excp instanceof CompletionException) { + List parseErrors = new ArrayList<>(); + + if (excp instanceof CompletionException) { excp = excp.getCause(); } + if (excp instanceof ParseError) { - newParseError = Diagnostics.translateDiagnostic((ParseError)excp, columns); + parseErrors.add(Diagnostics.translateDiagnostic((ParseError)excp, columns)); } - else if (excp != null) { + + if (excp != null) { logger.error("Parsing crashed", excp); - newParseError = new Diagnostic( + parseErrors.add(new Diagnostic( new Range(new Position(0,0), new Position(0,1)), "Parsing failed: " + excp.getMessage(), DiagnosticSeverity.Error, - "Rascal Parser"); + "Rascal Parser")); + } + + if (tree != null) { + RascalValueFactory VALUE_FACTORY = (RascalValueFactory) ValueFactoryFactory.getValueFactory(); + IList errors = new ErrorRecovery(VALUE_FACTORY).findAllErrors(tree.get()); + for (IValue error : errors) { + ITree errorTree = (ITree) error; + parseErrors.add(Diagnostics.translateErrorRecoveryDiagnostic(errorTree, columns)); + } } - logger.trace("Finished parsing tree, reporting new parse error: {} for: {}", newParseError, file.getLocation()); + logger.trace("Finished parsing tree, reporting new parse errors: {} for: {}", parseErrors, file.getLocation()); if (facts != null) { - facts.reportParseErrors(file.getLocation(), - newParseError == null ? Collections.emptyList() : Collections.singletonList(newParseError)); + facts.reportParseErrors(file.getLocation(), parseErrors); } return null; }); diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Diagnostics.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Diagnostics.java index f6d9e07a5..d45206cf7 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Diagnostics.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Diagnostics.java @@ -42,6 +42,8 @@ import org.eclipse.lsp4j.Range; import org.rascalmpl.parser.gtd.exception.ParseError; import org.rascalmpl.values.ValueFactoryFactory; +import org.rascalmpl.values.parsetrees.ITree; +import org.rascalmpl.values.parsetrees.TreeAdapter; import org.rascalmpl.vscode.lsp.IBaseTextDocumentService; import org.rascalmpl.vscode.lsp.util.locations.ColumnMaps; import org.rascalmpl.vscode.lsp.util.locations.LineColumnOffsetMap; @@ -74,6 +76,9 @@ public static Diagnostic translateDiagnostic(ParseError e, ColumnMaps cm) { return new Diagnostic(toRange(e, cm), e.getMessage(), DiagnosticSeverity.Error, "parser"); } + public static Diagnostic translateErrorRecoveryDiagnostic(ITree errorTree, ColumnMaps cm) { + return new Diagnostic(toRange(errorTree, cm), "Parse error", DiagnosticSeverity.Error, "parser"); + } public static Diagnostic translateRascalParseError(IValue e, ColumnMaps cm) { if (e instanceof IConstructor) { @@ -121,8 +126,15 @@ public static Diagnostic translateDiagnostic(IConstructor d, Range range) { return result; } + private static Range toRange(ITree t, ColumnMaps cm) { + return toRange(TreeAdapter.getLocation(t), cm); + } + private static Range toRange(ParseError pe, ColumnMaps cm) { - ISourceLocation loc = pe.getLocation(); + return toRange(pe.getLocation(), cm); + } + + private static Range toRange(ISourceLocation loc, ColumnMaps cm) { if (loc.getBeginLine() == loc.getEndLine() && loc.getBeginColumn() == loc.getEndColumn()) { // zero width parse error is not something LSP likes, so we make it one char wider loc = ValueFactoryFactory.getValueFactory().sourceLocation(loc, From 1aa3634c330fb451d5597907d32e51c3e59efad5 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Tue, 8 Oct 2024 14:43:15 +0200 Subject: [PATCH 04/28] Make error recovery backward-compatible --- .../vscode/lsp/TextDocumentState.java | 27 ++++++++++++++----- .../ParametricTextDocumentService.java | 2 +- .../lsp/rascal/RascalTextDocumentService.java | 10 +++---- 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java index 73690847b..3f50366cf 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java @@ -30,9 +30,13 @@ import java.util.function.BiFunction; import org.checkerframework.checker.nullness.qual.MonotonicNonNull; +import org.rascalmpl.library.util.ErrorRecovery; +import org.rascalmpl.values.RascalValueFactory; +import org.rascalmpl.values.ValueFactoryFactory; import org.rascalmpl.values.parsetrees.ITree; import org.rascalmpl.vscode.lsp.util.Versioned; +import io.usethesource.vallang.IList; import io.usethesource.vallang.ISourceLocation; /** @@ -51,7 +55,9 @@ public class TextDocumentState { @SuppressWarnings("java:S3077") // we are use volatile correctly private volatile Versioned currentContent; @SuppressWarnings("java:S3077") // we are use volatile correctly - private volatile @MonotonicNonNull Versioned lastFullTree; + private volatile @MonotonicNonNull Versioned lastTree; + @SuppressWarnings("java:S3077") // we are use volatile correctly + private volatile @MonotonicNonNull Versioned lastTreeWithoutErrors; @SuppressWarnings("java:S3077") // we are use volatile correctly private volatile CompletableFuture> currentTree; @@ -83,9 +89,14 @@ public CompletableFuture> update(int version, String content) { private CompletableFuture> newTreeAsync(int version, String content) { return parser.apply(file, content) .thenApply(t -> new Versioned(version, t)) - .whenComplete((r, t) -> { - if (r != null) { - lastFullTree = r; + .whenComplete((t, error) -> { + if (t != null) { + RascalValueFactory valueFactory = (RascalValueFactory) ValueFactoryFactory.getValueFactory(); + IList errors = new ErrorRecovery(valueFactory).findAllErrors(t.get()); + if (errors.isEmpty()) { + lastTreeWithoutErrors = t; + } + lastTree = t; } }); } @@ -94,8 +105,12 @@ public CompletableFuture> getCurrentTreeAsync() { return currentTree; } - public @MonotonicNonNull Versioned getMostRecentTree() { - return lastFullTree; + public @MonotonicNonNull Versioned getLastTree() { + return lastTree; + } + + public @MonotonicNonNull Versioned getLastTreeWithoutErrors() { + return lastTreeWithoutErrors; } public ISourceLocation getLocation() { diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParametricTextDocumentService.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParametricTextDocumentService.java index 328222020..1032f9bc4 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParametricTextDocumentService.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParametricTextDocumentService.java @@ -341,7 +341,7 @@ public CompletableFuture> inlayHint(InlayHintParams params) { final TextDocumentState file = getFile(params.getTextDocument()); final ILanguageContributions contrib = contributions(params.getTextDocument()); return recoverExceptions( - recoverExceptions(file.getCurrentTreeAsync(), file::getMostRecentTree) + recoverExceptions(file.getCurrentTreeAsync(), file::getLastTreeWithoutErrors) .thenApply(Versioned::get) .thenApply(contrib::inlayHint) .thenCompose(InterruptibleFuture::get) diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java index 2958a6915..05d17627b 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java @@ -239,8 +239,8 @@ private void handleParsingErrors(TextDocumentState file, CompletableFuture, List (t == null ? (file.getMostRecentTree().get()) : t)) + .handle((t, r) -> (t == null ? (file.getLastTreeWithoutErrors().get()) : t)) .thenCompose(tr -> rascalServices.getOutline(tr).get()) .thenApply(c -> Outline.buildOutline(c, columns.get(file.getLocation()))) ; @@ -300,7 +300,7 @@ public CompletableFuture rename(RenameParams params) { return file.getCurrentTreeAsync() .thenApply(Versioned::get) - .handle((t, r) -> (t == null ? (file.getMostRecentTree().get()) : t)) + .handle((t, r) -> (t == null ? (file.getLastTreeWithoutErrors().get()) : t)) .thenCompose(tr -> rascalServices.getRename(tr, params.getPosition(), workspaceFolders, facts::getPathConfig, params.getNewName(), columns).get()) .thenApply(c -> new WorkspaceEdit(DocumentChanges.translateDocumentChanges(this, c))) ; @@ -410,7 +410,7 @@ public CompletableFuture> codeLens(CodeLensParams param .handle((r, e) -> { // fallback to tree if a parsing error occurred. if (r == null) { - r = f.getMostRecentTree(); + r = f.getLastTreeWithoutErrors(); } if (r == null) { throw new RuntimeException(e); From 35a3b0212f0764bef41a0a23b7b9072ba0992328 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Tue, 8 Oct 2024 15:13:55 +0200 Subject: [PATCH 05/28] Add flag to `loadParser` call to set allowRecovery to false for now --- .../rascalmpl/vscode/lsp/parametric/ParserOnlyContribution.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParserOnlyContribution.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParserOnlyContribution.java index 666b2d0ef..c25f37506 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParserOnlyContribution.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParserOnlyContribution.java @@ -94,7 +94,7 @@ private static Either loadParser(ParserSpecification spec) try { logger.debug("Loading parser {} at {}", reifiedType, spec.getParserLocation()); // this hides all the loading and instantiation details of Rascal-generated parsers - var parser = vf.loadParser(reifiedType, spec.getParserLocation(), VF.bool(spec.getAllowAmbiguity()), VF.bool(false), VF.bool(false), vf.set()); + var parser = vf.loadParser(reifiedType, spec.getParserLocation(), VF.bool(spec.getAllowAmbiguity()), VF.bool(false), VF.bool(false), VF.bool(false), vf.set()); logger.debug("Got parser: {}", parser); return Either.forLeft(parser); } From ea9919f65bd5f530abaed44ac9f4fd03a1685b6e Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Wed, 9 Oct 2024 15:58:28 +0200 Subject: [PATCH 06/28] Fix a bug that adds irrelevant diagnostics --- .../vscode/lsp/rascal/RascalTextDocumentService.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java index 05d17627b..87ec56d4e 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java @@ -227,9 +227,7 @@ private void handleParsingErrors(TextDocumentState file, CompletableFuture Date: Wed, 9 Oct 2024 15:59:57 +0200 Subject: [PATCH 07/28] Add exception handler when the location of a parse error is malformed --- .../org/rascalmpl/vscode/lsp/util/Diagnostics.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Diagnostics.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Diagnostics.java index d45206cf7..714b05775 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Diagnostics.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Diagnostics.java @@ -137,10 +137,16 @@ private static Range toRange(ParseError pe, ColumnMaps cm) { private static Range toRange(ISourceLocation loc, ColumnMaps cm) { if (loc.getBeginLine() == loc.getEndLine() && loc.getBeginColumn() == loc.getEndColumn()) { // zero width parse error is not something LSP likes, so we make it one char wider - loc = ValueFactoryFactory.getValueFactory().sourceLocation(loc, - loc.getOffset(), loc.getLength() + 1, - loc.getBeginLine(), loc.getBeginColumn(), - loc.getEndLine(), loc.getEndColumn() + 1); + try { + loc = ValueFactoryFactory.getValueFactory().sourceLocation(loc, + loc.getOffset(), loc.getLength() + 1, + loc.getBeginLine(), loc.getBeginColumn(), + loc.getEndLine(), loc.getEndColumn() + 1); + } catch (Throwable t) { + logger.trace("Cannot extend 0-width location for parse error: " + t.getMessage()); + loc = ValueFactoryFactory.getValueFactory().sourceLocation( + loc, 0, 1, 1, 1, 0, 1); + } } return Locations.toRange(loc, cm); } From 964385273d779f75d5c288ecbd081664b1cd87c2 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Fri, 11 Oct 2024 17:12:03 +0200 Subject: [PATCH 08/28] Add first version parse debouncing --- .../vscode/lsp/TextDocumentState.java | 138 +++++++++++++----- .../ParametricTextDocumentService.java | 3 +- .../lsp/rascal/RascalTextDocumentService.java | 3 +- .../rascalmpl/vscode/lsp/util/Versioned.java | 2 +- 4 files changed, 103 insertions(+), 43 deletions(-) diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java index 3f50366cf..1e07cfcd8 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java @@ -26,17 +26,23 @@ */ package org.rascalmpl.vscode.lsp; +import java.time.Duration; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Executor; +import java.util.concurrent.TimeUnit; import java.util.function.BiFunction; import org.checkerframework.checker.nullness.qual.MonotonicNonNull; +import org.checkerframework.checker.nullness.qual.NonNull; import org.rascalmpl.library.util.ErrorRecovery; +import org.rascalmpl.values.IRascalValueFactory; import org.rascalmpl.values.RascalValueFactory; import org.rascalmpl.values.ValueFactoryFactory; import org.rascalmpl.values.parsetrees.ITree; import org.rascalmpl.vscode.lsp.util.Versioned; -import io.usethesource.vallang.IList; import io.usethesource.vallang.ISourceLocation; /** @@ -50,25 +56,25 @@ */ public class TextDocumentState { private final BiFunction> parser; - private final ISourceLocation file; - @SuppressWarnings("java:S3077") // we are use volatile correctly - private volatile Versioned currentContent; - @SuppressWarnings("java:S3077") // we are use volatile correctly - private volatile @MonotonicNonNull Versioned lastTree; - @SuppressWarnings("java:S3077") // we are use volatile correctly - private volatile @MonotonicNonNull Versioned lastTreeWithoutErrors; - @SuppressWarnings("java:S3077") // we are use volatile correctly - private volatile CompletableFuture> currentTree; - - public TextDocumentState(BiFunction> parser, ISourceLocation file, int initialVersion, String initialContent) { + private final AtomicReference<@NonNull Update> inProgress; + private final AtomicReference<@MonotonicNonNull Versioned> lastWithoutErrors; + private final AtomicReference<@MonotonicNonNull Versioned> last; + + public TextDocumentState( + BiFunction> parser, + ISourceLocation file, int initialVersion, String initialContent) { + this.parser = parser; this.file = file; - this.currentContent = new Versioned<>(initialVersion, initialContent); - this.currentTree = newTreeAsync(initialVersion, initialContent); + this.inProgress = new AtomicReference<>(new Update(initialVersion, initialContent)); + this.lastWithoutErrors = new AtomicReference<>(); + this.last = new AtomicReference<>(); } /** + * WARNING: OUTDATED DOCUMENTATION + * * The current call of this method guarantees that, until the next call, * each intermediate call of `getCurrentTreeAsync` returns (a future for) a * *correct* versioned tree. This means that: @@ -78,46 +84,98 @@ public TextDocumentState(BiFunction pair. */ - public CompletableFuture> update(int version, String content) { - currentContent = new Versioned<>(version, content); - var newTree = newTreeAsync(version, content); - currentTree = newTree; - return newTree; + public void update(int version, String content) { + DEBOUNCER.debounce(() -> { + var u = new Update(version, content); + replaceIfNewer(inProgress, u); + }); } - @SuppressWarnings("java:S1181") // we want to catch all Java exceptions from the parser - private CompletableFuture> newTreeAsync(int version, String content) { - return parser.apply(file, content) - .thenApply(t -> new Versioned(version, t)) - .whenComplete((t, error) -> { - if (t != null) { - RascalValueFactory valueFactory = (RascalValueFactory) ValueFactoryFactory.getValueFactory(); - IList errors = new ErrorRecovery(valueFactory).findAllErrors(t.get()); - if (errors.isEmpty()) { - lastTreeWithoutErrors = t; - } - lastTree = t; - } - }); + public ISourceLocation getLocation() { + return file; + } + + public Versioned getCurrentContent() { + return inProgress.get().content; } public CompletableFuture> getCurrentTreeAsync() { - return currentTree; + return inProgress.get().tree; } public @MonotonicNonNull Versioned getLastTree() { - return lastTree; + return last.get(); } public @MonotonicNonNull Versioned getLastTreeWithoutErrors() { - return lastTreeWithoutErrors; + return lastWithoutErrors.get(); } - public ISourceLocation getLocation() { - return file; + private static final IRascalValueFactory VALUES = (RascalValueFactory) ValueFactoryFactory.getValueFactory(); + private static final ErrorRecovery RECOVERY = new ErrorRecovery(VALUES); + + private class Update { + public final Versioned content; + public final CompletableFuture> tree; + + public Update(int version, String content) { + this.content = new Versioned<>(version, content); + this.tree = parser + .apply(file, 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); + } + Versioned.replaceIfNewer(last, t); + } + // Add diagnostics + }) + ; + } + + public int getVersion() { + return content.version(); + } } - public Versioned getCurrentContent() { - return currentContent; + private static boolean replaceIfNewer(AtomicReference ref, Update newValue) { + Update oldValue = ref.get(); + if (oldValue == null || oldValue.getVersion() < newValue.getVersion()) { + return ref.compareAndSet(oldValue, newValue) || replaceIfNewer(ref, newValue); + } else { + return false; + } } + + private static class Debouncer { + private final Executor executor; + private final AtomicInteger numberOfDebounces; + + private Debouncer(Duration delay) { + this.executor = CompletableFuture.delayedExecutor(delay.toMillis(), TimeUnit.MILLISECONDS); + this.numberOfDebounces = new AtomicInteger(0); + } + + private void debounce(Runnable doContinue) { + debounce(doContinue, () -> {}); + } + + private void debounce(Runnable doContinue, Runnable doBreak) { + var n = numberOfDebounces.incrementAndGet(); + CompletableFuture.runAsync(() -> { + if (numberOfDebounces.get() == n) { + doContinue.run(); + } else { + doBreak.run(); + } + }, executor); + } + } + + private static final Duration DELAY = Duration.ofMillis(500); + private static final Debouncer DEBOUNCER = new Debouncer(DELAY); } diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParametricTextDocumentService.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParametricTextDocumentService.java index 1032f9bc4..ef26a10ce 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParametricTextDocumentService.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParametricTextDocumentService.java @@ -285,7 +285,8 @@ private void triggerBuilder(TextDocumentIdentifier doc) { private TextDocumentState updateContents(VersionedTextDocumentIdentifier doc, String newContents) { TextDocumentState file = getFile(doc); logger.trace("New contents for {}", doc); - handleParsingErrors(file, file.update(doc.getVersion(), newContents)); + file.update(doc.getVersion(), newContents); + handleParsingErrors(file, file.getCurrentTreeAsync()); // Warning: Might be a later version (when a concurrent update happened) return file; } diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java index 87ec56d4e..5882776cc 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java @@ -213,7 +213,8 @@ public void didSave(DidSaveTextDocumentParams params) { private TextDocumentState updateContents(VersionedTextDocumentIdentifier doc, String newContents) { TextDocumentState file = getFile(doc); logger.trace("New contents for {}", doc); - handleParsingErrors(file, file.update(doc.getVersion(), newContents)); + file.update(doc.getVersion(), newContents); + handleParsingErrors(file, file.getCurrentTreeAsync()); // Warning: Might be a later version (when a concurrent update happened) return file; } diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Versioned.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Versioned.java index c9a7b7189..fc387bb3e 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Versioned.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Versioned.java @@ -57,7 +57,7 @@ public static AtomicReference> atomic(int version, T object) { public static boolean replaceIfNewer(AtomicReference> current, Versioned maybeNewer) { while (true) { var old = current.get(); - if (old.version() < maybeNewer.version()) { + if (old == null || old.version() < maybeNewer.version()) { if (current.compareAndSet(old, maybeNewer)) { return true; } From 04eaedd9a64cfd06d2f5aed0a4e295f68e5c29d0 Mon Sep 17 00:00:00 2001 From: Pieter Olivier Date: Tue, 15 Oct 2024 11:22:01 +0200 Subject: [PATCH 09/28] Revert "Add first version parse debouncing" This reverts commit 964385273d779f75d5c288ecbd081664b1cd87c2. --- .../vscode/lsp/TextDocumentState.java | 138 +++++------------- .../ParametricTextDocumentService.java | 3 +- .../lsp/rascal/RascalTextDocumentService.java | 3 +- .../rascalmpl/vscode/lsp/util/Versioned.java | 2 +- 4 files changed, 43 insertions(+), 103 deletions(-) diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java index 1e07cfcd8..3f50366cf 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java @@ -26,23 +26,17 @@ */ package org.rascalmpl.vscode.lsp; -import java.time.Duration; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.Executor; -import java.util.concurrent.TimeUnit; import java.util.function.BiFunction; import org.checkerframework.checker.nullness.qual.MonotonicNonNull; -import org.checkerframework.checker.nullness.qual.NonNull; import org.rascalmpl.library.util.ErrorRecovery; -import org.rascalmpl.values.IRascalValueFactory; import org.rascalmpl.values.RascalValueFactory; import org.rascalmpl.values.ValueFactoryFactory; import org.rascalmpl.values.parsetrees.ITree; import org.rascalmpl.vscode.lsp.util.Versioned; +import io.usethesource.vallang.IList; import io.usethesource.vallang.ISourceLocation; /** @@ -56,25 +50,25 @@ */ public class TextDocumentState { private final BiFunction> parser; - private final ISourceLocation file; - private final AtomicReference<@NonNull Update> inProgress; - private final AtomicReference<@MonotonicNonNull Versioned> lastWithoutErrors; - private final AtomicReference<@MonotonicNonNull Versioned> last; - - public TextDocumentState( - BiFunction> parser, - ISourceLocation file, int initialVersion, String initialContent) { + private final ISourceLocation file; + @SuppressWarnings("java:S3077") // we are use volatile correctly + private volatile Versioned currentContent; + @SuppressWarnings("java:S3077") // we are use volatile correctly + private volatile @MonotonicNonNull Versioned lastTree; + @SuppressWarnings("java:S3077") // we are use volatile correctly + private volatile @MonotonicNonNull Versioned lastTreeWithoutErrors; + @SuppressWarnings("java:S3077") // we are use volatile correctly + private volatile CompletableFuture> currentTree; + + public TextDocumentState(BiFunction> parser, ISourceLocation file, int initialVersion, String initialContent) { this.parser = parser; this.file = file; - this.inProgress = new AtomicReference<>(new Update(initialVersion, initialContent)); - this.lastWithoutErrors = new AtomicReference<>(); - this.last = new AtomicReference<>(); + this.currentContent = new Versioned<>(initialVersion, initialContent); + this.currentTree = newTreeAsync(initialVersion, initialContent); } /** - * WARNING: OUTDATED DOCUMENTATION - * * The current call of this method guarantees that, until the next call, * each intermediate call of `getCurrentTreeAsync` returns (a future for) a * *correct* versioned tree. This means that: @@ -84,98 +78,46 @@ public TextDocumentState( * Thus, callers of `getCurrentTreeAsync` are guaranteed to obtain a * consistent pair. */ - public void update(int version, String content) { - DEBOUNCER.debounce(() -> { - var u = new Update(version, content); - replaceIfNewer(inProgress, u); - }); + public CompletableFuture> update(int version, String content) { + currentContent = new Versioned<>(version, content); + var newTree = newTreeAsync(version, content); + currentTree = newTree; + return newTree; } - public ISourceLocation getLocation() { - return file; - } - - public Versioned getCurrentContent() { - return inProgress.get().content; + @SuppressWarnings("java:S1181") // we want to catch all Java exceptions from the parser + private CompletableFuture> newTreeAsync(int version, String content) { + return parser.apply(file, content) + .thenApply(t -> new Versioned(version, t)) + .whenComplete((t, error) -> { + if (t != null) { + RascalValueFactory valueFactory = (RascalValueFactory) ValueFactoryFactory.getValueFactory(); + IList errors = new ErrorRecovery(valueFactory).findAllErrors(t.get()); + if (errors.isEmpty()) { + lastTreeWithoutErrors = t; + } + lastTree = t; + } + }); } public CompletableFuture> getCurrentTreeAsync() { - return inProgress.get().tree; + return currentTree; } public @MonotonicNonNull Versioned getLastTree() { - return last.get(); + return lastTree; } public @MonotonicNonNull Versioned getLastTreeWithoutErrors() { - return lastWithoutErrors.get(); - } - - private static final IRascalValueFactory VALUES = (RascalValueFactory) ValueFactoryFactory.getValueFactory(); - private static final ErrorRecovery RECOVERY = new ErrorRecovery(VALUES); - - private class Update { - public final Versioned content; - public final CompletableFuture> tree; - - public Update(int version, String content) { - this.content = new Versioned<>(version, content); - this.tree = parser - .apply(file, 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); - } - Versioned.replaceIfNewer(last, t); - } - // Add diagnostics - }) - ; - } - - public int getVersion() { - return content.version(); - } + return lastTreeWithoutErrors; } - private static boolean replaceIfNewer(AtomicReference ref, Update newValue) { - Update oldValue = ref.get(); - if (oldValue == null || oldValue.getVersion() < newValue.getVersion()) { - return ref.compareAndSet(oldValue, newValue) || replaceIfNewer(ref, newValue); - } else { - return false; - } + public ISourceLocation getLocation() { + return file; } - private static class Debouncer { - private final Executor executor; - private final AtomicInteger numberOfDebounces; - - private Debouncer(Duration delay) { - this.executor = CompletableFuture.delayedExecutor(delay.toMillis(), TimeUnit.MILLISECONDS); - this.numberOfDebounces = new AtomicInteger(0); - } - - private void debounce(Runnable doContinue) { - debounce(doContinue, () -> {}); - } - - private void debounce(Runnable doContinue, Runnable doBreak) { - var n = numberOfDebounces.incrementAndGet(); - CompletableFuture.runAsync(() -> { - if (numberOfDebounces.get() == n) { - doContinue.run(); - } else { - doBreak.run(); - } - }, executor); - } + public Versioned getCurrentContent() { + return currentContent; } - - private static final Duration DELAY = Duration.ofMillis(500); - private static final Debouncer DEBOUNCER = new Debouncer(DELAY); } diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParametricTextDocumentService.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParametricTextDocumentService.java index 161322514..cd7e47234 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParametricTextDocumentService.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParametricTextDocumentService.java @@ -286,8 +286,7 @@ private void triggerBuilder(TextDocumentIdentifier doc) { private TextDocumentState updateContents(VersionedTextDocumentIdentifier doc, String newContents) { TextDocumentState file = getFile(doc); logger.trace("New contents for {}", doc); - file.update(doc.getVersion(), newContents); - handleParsingErrors(file, file.getCurrentTreeAsync()); // Warning: Might be a later version (when a concurrent update happened) + handleParsingErrors(file, file.update(doc.getVersion(), newContents)); return file; } diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java index 5882776cc..87ec56d4e 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java @@ -213,8 +213,7 @@ public void didSave(DidSaveTextDocumentParams params) { private TextDocumentState updateContents(VersionedTextDocumentIdentifier doc, String newContents) { TextDocumentState file = getFile(doc); logger.trace("New contents for {}", doc); - file.update(doc.getVersion(), newContents); - handleParsingErrors(file, file.getCurrentTreeAsync()); // Warning: Might be a later version (when a concurrent update happened) + handleParsingErrors(file, file.update(doc.getVersion(), newContents)); return file; } diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Versioned.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Versioned.java index fc387bb3e..c9a7b7189 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Versioned.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Versioned.java @@ -57,7 +57,7 @@ public static AtomicReference> atomic(int version, T object) { public static boolean replaceIfNewer(AtomicReference> current, Versioned maybeNewer) { while (true) { var old = current.get(); - if (old == null || old.version() < maybeNewer.version()) { + if (old.version() < maybeNewer.version()) { if (current.compareAndSet(old, maybeNewer)) { return true; } From a3dbfbde33ece334234cedd6c49d87e41edf1599 Mon Sep 17 00:00:00 2001 From: Pieter Olivier Date: Wed, 16 Oct 2024 08:45:44 +0200 Subject: [PATCH 10/28] Show skipped part and use error tree for outlining --- .../vscode/lsp/rascal/RascalTextDocumentService.java | 2 +- .../main/java/org/rascalmpl/vscode/lsp/util/Diagnostics.java | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java index 87ec56d4e..47c6d5819 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java @@ -280,7 +280,7 @@ public CompletableFuture, List (t == null ? (file.getLastTreeWithoutErrors().get()) : t)) + .handle((t, r) -> (t == null ? (file.getLastTree().get()) : t)) .thenCompose(tr -> rascalServices.getOutline(tr).get()) .thenApply(c -> Outline.buildOutline(c, columns.get(file.getLocation()))) ; diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Diagnostics.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Diagnostics.java index 714b05775..2e404baee 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Diagnostics.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Diagnostics.java @@ -77,7 +77,9 @@ public static Diagnostic translateDiagnostic(ParseError e, ColumnMaps cm) { } public static Diagnostic translateErrorRecoveryDiagnostic(ITree errorTree, ColumnMaps cm) { - return new Diagnostic(toRange(errorTree, cm), "Parse error", DiagnosticSeverity.Error, "parser"); + IList args = TreeAdapter.getArgs(errorTree); + ITree skipped = (ITree) args.get(args.size()-1); + return new Diagnostic(toRange(skipped, cm), "Parse error", DiagnosticSeverity.Error, "parser"); } public static Diagnostic translateRascalParseError(IValue e, ColumnMaps cm) { From 17bcab1bff75555e34da6648a7e17628380b160b Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Thu, 24 Oct 2024 12:51:44 +0200 Subject: [PATCH 11/28] Add debounce to parsing in `TextDocumentState` --- .../vscode/lsp/TextDocumentState.java | 298 ++++++++++++++---- .../ParametricTextDocumentService.java | 3 +- .../lsp/rascal/RascalTextDocumentService.java | 4 +- .../rascalmpl/vscode/lsp/util/Versioned.java | 2 +- 4 files changed, 249 insertions(+), 58 deletions(-) diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java index 3f50366cf..880ae3d37 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java @@ -26,17 +26,27 @@ */ package org.rascalmpl.vscode.lsp; +import java.time.Duration; +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.function.BiFunction; +import java.util.function.Function; +import java.util.function.Supplier; import org.checkerframework.checker.nullness.qual.MonotonicNonNull; +import org.checkerframework.checker.nullness.qual.Nullable; +import org.eclipse.lsp4j.Diagnostic; import org.rascalmpl.library.util.ErrorRecovery; import org.rascalmpl.values.RascalValueFactory; import org.rascalmpl.values.ValueFactoryFactory; import org.rascalmpl.values.parsetrees.ITree; import org.rascalmpl.vscode.lsp.util.Versioned; -import io.usethesource.vallang.IList; import io.usethesource.vallang.ISourceLocation; /** @@ -49,75 +59,253 @@ * and ParametricTextDocumentService. */ public class TextDocumentState { + + private static final ErrorRecovery RECOVERY = + new ErrorRecovery((RascalValueFactory) ValueFactoryFactory.getValueFactory()); + private final BiFunction> parser; + private final ISourceLocation location; + + @SuppressWarnings("java:S3077") // Visibility of writes is enough + private volatile Update current; + private final Debouncer> currentTreeAsyncDebouncer; + + private final AtomicReference<@MonotonicNonNull Versioned> lastWithoutErrors; + private final AtomicReference<@MonotonicNonNull Versioned> last; + + public TextDocumentState( + BiFunction> parser, + ISourceLocation location, int initialVersion, String initialContent) { - private final ISourceLocation file; - @SuppressWarnings("java:S3077") // we are use volatile correctly - private volatile Versioned currentContent; - @SuppressWarnings("java:S3077") // we are use volatile correctly - private volatile @MonotonicNonNull Versioned lastTree; - @SuppressWarnings("java:S3077") // we are use volatile correctly - private volatile @MonotonicNonNull Versioned lastTreeWithoutErrors; - @SuppressWarnings("java:S3077") // we are use volatile correctly - private volatile CompletableFuture> currentTree; - - public TextDocumentState(BiFunction> parser, ISourceLocation file, int initialVersion, String initialContent) { this.parser = parser; - this.file = file; - this.currentContent = new Versioned<>(initialVersion, initialContent); - this.currentTree = newTreeAsync(initialVersion, initialContent); - } - - /** - * The current call of this method guarantees that, until the next call, - * each intermediate call of `getCurrentTreeAsync` returns (a future for) a - * *correct* versioned tree. This means that: - * - the version of the tree is parameter `version`; - * - the tree is produced by parsing parameter `content`. - * - * Thus, callers of `getCurrentTreeAsync` are guaranteed to obtain a - * consistent pair. - */ - public CompletableFuture> update(int version, String content) { - currentContent = new Versioned<>(version, content); - var newTree = newTreeAsync(version, content); - currentTree = newTree; - return newTree; - } - - @SuppressWarnings("java:S1181") // we want to catch all Java exceptions from the parser - private CompletableFuture> newTreeAsync(int version, String content) { - return parser.apply(file, content) - .thenApply(t -> new Versioned(version, t)) - .whenComplete((t, error) -> { - if (t != null) { - RascalValueFactory valueFactory = (RascalValueFactory) ValueFactoryFactory.getValueFactory(); - IList errors = new ErrorRecovery(valueFactory).findAllErrors(t.get()); - if (errors.isEmpty()) { - lastTreeWithoutErrors = t; - } - lastTree = t; - } - }); + this.location = location; + + this.current = new Update(initialVersion, initialContent); + this.currentTreeAsyncDebouncer = new Debouncer<>(50, + this::getCurrentTreeAsyncIfParsing, this::getCurrentTreeAsync); + + this.lastWithoutErrors = new AtomicReference<>(); + this.last = new AtomicReference<>(); + } + + public ISourceLocation getLocation() { + return location; + } + + 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. + } + + public Versioned getCurrentContent() { + return current.getContent(); } public CompletableFuture> getCurrentTreeAsync() { - return currentTree; + return current.getTreeAsync(); // Triggers the parser + } + + public CompletableFuture> getCurrentTreeAsync(Duration delay) { + return currentTreeAsyncDebouncer.get(delay); + } + + public @Nullable CompletableFuture> getCurrentTreeAsyncIfParsing() { + var update = current; + return update.isParsing() ? update.getTreeAsync() : null; + } + + public CompletableFuture>> getCurrentDiagnostics() { + throw new UnsupportedOperationException(); + // TODO: In a separate PR } public @MonotonicNonNull Versioned getLastTree() { - return lastTree; + return last.get(); } public @MonotonicNonNull Versioned getLastTreeWithoutErrors() { - return lastTreeWithoutErrors; + return lastWithoutErrors.get(); } - public ISourceLocation getLocation() { - return file; + private class Update { + private final int version; + private final String content; + private final CompletableFuture> treeAsync; + private final AtomicBoolean parsing; + + public Update(int version, String content) { + this.version = version; + this.content = content; + this.treeAsync = new CompletableFuture<>(); + this.parsing = new AtomicBoolean(false); + } + + public Versioned getContent() { + return new Versioned<>(version, content); + } + + public CompletableFuture> getTreeAsync() { + parseIfNotParsing(); + return treeAsync; + } + + public boolean isParsing() { + return parsing.get(); + } + + 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); + } + Versioned.replaceIfNewer(last, t); + treeAsync.complete(t); + } + if (error != null) { + treeAsync.completeExceptionally(error); + } + }); + } + } } +} - public Versioned getCurrentContent() { - return currentContent; +/** + * 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 { + + // 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 1 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> 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 provided is not-initialized. + + private final Supplier<@Nullable CompletableFuture> getIfInitialized; + private final Supplier> initializeAndGet; + + public Debouncer(Duration period, + Supplier<@Nullable CompletableFuture> getIfInitialized, + Supplier> initializeAndGet) { + + this(Math.toIntExact(period.toMillis()), getIfInitialized, initializeAndGet); + } + + public Debouncer(int period, + Supplier<@Nullable CompletableFuture> getIfInitialized, + Supplier> 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 get(Duration delay) { + return get(Math.toIntExact(delay.toMillis())); + } + + public CompletableFuture get(int delay) { + return schedule(delay, false); + } + + private CompletableFuture schedule(int delay, boolean reschedule) { + + // Get a consistent old stamp and old reference + var oldRef = scheduled.getReference(); + var oldStamp = scheduled.getStamp(); + while (!scheduled.weakCompareAndSet(oldRef, oldRef, oldStamp, oldStamp)); + + // Compute a new reference (delayed future to retry this method) + var delayArg = new CompletableFuture(); + 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.weakCompareAndSet(oldRef, null, oldStamp, 0)) { + return future; + } + + // 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.weakCompareAndSet(oldRef, null, oldStamp, 0)) { + return initializeAndGet.get(); + } + + // 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.weakCompareAndSet(oldRef, newRef, oldStamp, newStamp)) { + delayArg.complete(newStamp); + return newRef; + } + + // 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 to complete the already-scheduled delayed future. + if (scheduled.attemptStamp(oldRef, newStamp)) { + return oldRef; + } + + // 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 reschedule(int delay) { + return schedule(delay, true); } } diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParametricTextDocumentService.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParametricTextDocumentService.java index 1032f9bc4..ef26a10ce 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParametricTextDocumentService.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParametricTextDocumentService.java @@ -285,7 +285,8 @@ private void triggerBuilder(TextDocumentIdentifier doc) { private TextDocumentState updateContents(VersionedTextDocumentIdentifier doc, String newContents) { TextDocumentState file = getFile(doc); logger.trace("New contents for {}", doc); - handleParsingErrors(file, file.update(doc.getVersion(), newContents)); + file.update(doc.getVersion(), newContents); + handleParsingErrors(file, file.getCurrentTreeAsync()); // Warning: Might be a later version (when a concurrent update happened) return file; } diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java index 87ec56d4e..d157aceeb 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java @@ -28,6 +28,7 @@ import java.io.IOException; import java.io.Reader; +import java.time.Duration; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -213,7 +214,8 @@ public void didSave(DidSaveTextDocumentParams params) { private TextDocumentState updateContents(VersionedTextDocumentIdentifier doc, String newContents) { TextDocumentState file = getFile(doc); logger.trace("New contents for {}", doc); - handleParsingErrors(file, file.update(doc.getVersion(), newContents)); + file.update(doc.getVersion(), newContents); + handleParsingErrors(file, file.getCurrentTreeAsync(Duration.ofMillis(800))); return file; } diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Versioned.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Versioned.java index c9a7b7189..fc387bb3e 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Versioned.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Versioned.java @@ -57,7 +57,7 @@ public static AtomicReference> atomic(int version, T object) { public static boolean replaceIfNewer(AtomicReference> current, Versioned maybeNewer) { while (true) { var old = current.get(); - if (old.version() < maybeNewer.version()) { + if (old == null || old.version() < maybeNewer.version()) { if (current.compareAndSet(old, maybeNewer)) { return true; } From 6b35288d6e8abe8bf29d6d922cc3ce3682763491 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Thu, 24 Oct 2024 17:37:44 +0200 Subject: [PATCH 12/28] Use `compareAndSet` instead of `weak...` --- .../java/org/rascalmpl/vscode/lsp/TextDocumentState.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java index 880ae3d37..919253fca 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java @@ -258,7 +258,7 @@ private CompletableFuture schedule(int delay, boolean reschedule) { // Get a consistent old stamp and old reference var oldRef = scheduled.getReference(); var oldStamp = scheduled.getStamp(); - while (!scheduled.weakCompareAndSet(oldRef, oldRef, oldStamp, oldStamp)); + while (!scheduled.compareAndSet(oldRef, oldRef, oldStamp, oldStamp)); // Compute a new reference (delayed future to retry this method) var delayArg = new CompletableFuture(); @@ -273,21 +273,21 @@ private CompletableFuture schedule(int delay, boolean reschedule) { // If the underlying resource provider is initialized, then return the // future to get the resource var future = getIfInitialized.get(); - if (future != null && scheduled.weakCompareAndSet(oldRef, null, oldStamp, 0)) { + if (future != null && scheduled.compareAndSet(oldRef, null, oldStamp, 0)) { return future; } // 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.weakCompareAndSet(oldRef, null, oldStamp, 0)) { + if (delayRemaining <= 0 && scheduled.compareAndSet(oldRef, null, oldStamp, 0)) { return initializeAndGet.get(); } // 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.weakCompareAndSet(oldRef, newRef, oldStamp, newStamp)) { + if ((oldRef == null || reschedule) && scheduled.compareAndSet(oldRef, newRef, oldStamp, newStamp)) { delayArg.complete(newStamp); return newRef; } From 2749e4985521067a50b7664d36fc13e715bc8e18 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Thu, 24 Oct 2024 20:36:34 +0200 Subject: [PATCH 13/28] Fix typos in documentation --- .../java/org/rascalmpl/vscode/lsp/TextDocumentState.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java index 919253fca..c4205bcea 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java @@ -200,7 +200,7 @@ class Debouncer { // 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 1 or case 3 (above) are applicable. If so, + // 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). // @@ -222,7 +222,7 @@ class Debouncer { // 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 provided is not-initialized. + // underlying resource provider is not-initialized. private final Supplier<@Nullable CompletableFuture> getIfInitialized; private final Supplier> initializeAndGet; @@ -260,7 +260,7 @@ private CompletableFuture schedule(int delay, boolean reschedule) { var oldStamp = scheduled.getStamp(); while (!scheduled.compareAndSet(oldRef, oldRef, oldStamp, oldStamp)); - // Compute a new reference (delayed future to retry this method) + // Compute a new reference (= delayed future to retry this method) var delayArg = new CompletableFuture(); var newRef = delayArg .thenApplyAsync(this::reschedule, scheduler) @@ -294,7 +294,7 @@ private CompletableFuture schedule(int delay, boolean reschedule) { // 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 to complete the already-scheduled delayed future. + // used by the already-scheduled delayed future. if (scheduled.attemptStamp(oldRef, newStamp)) { return oldRef; } From b947d04e97310d74a17f52fb609739e608c40200 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Fri, 25 Oct 2024 12:29:15 +0200 Subject: [PATCH 14/28] Upgrade `pom.xml` and `package.sh` --- package.sh | 16 +++++++++++++++- rascal-lsp/pom.xml | 2 +- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/package.sh b/package.sh index e2d6db648..862c6595a 100755 --- a/package.sh +++ b/package.sh @@ -2,5 +2,19 @@ set -euxo pipefail -(cd rascal-vscode-extension && npx vsce package ) +cd rascal-vscode-extension +TEMP1=$(mktemp) +TEMP2=$(mktemp) +cp package.json $TEMP1 +cp package-lock.json $TEMP2 + +VERSION=$(node -p "require('./package.json').version") +PREFIX=$(echo $VERSION | cut -d "-" -f 1) +SUFFIX=$(git log --pretty=format:'%h' -n 1) +npx vsce package $PREFIX-$SUFFIX + +cp $TEMP1 package.json +cp $TEMP2 package-lock.json + +cd - \ No newline at end of file diff --git a/rascal-lsp/pom.xml b/rascal-lsp/pom.xml index e4883b97d..78f02be69 100644 --- a/rascal-lsp/pom.xml +++ b/rascal-lsp/pom.xml @@ -65,7 +65,7 @@ org.rascalmpl rascal - 0.40.9-SNAPSHOT + 0.40.13-SNAPSHOT org.rascalmpl From 073b6753d9191c235aa4439572f3a70a781ea84d Mon Sep 17 00:00:00 2001 From: Pieter Olivier Date: Fri, 25 Oct 2024 13:25:51 +0200 Subject: [PATCH 15/28] Both outline and codelenses support can now handle error trees --- .../lsp/rascal/RascalLanguageServices.java | 11 ++- .../main/rascal/lang/rascal/lsp/Outline.rsc | 72 ++++++++++++------- 2 files changed, 56 insertions(+), 27 deletions(-) diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalLanguageServices.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalLanguageServices.java index 2ace1ba1d..466ce195f 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalLanguageServices.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalLanguageServices.java @@ -51,6 +51,7 @@ import org.eclipse.lsp4j.jsonrpc.messages.ResponseErrorCode; import org.rascalmpl.exceptions.Throw; import org.rascalmpl.interpreter.Evaluator; +import org.rascalmpl.library.util.ErrorRecovery; import org.rascalmpl.library.util.PathConfig; import org.rascalmpl.values.IRascalValueFactory; import org.rascalmpl.values.functions.IFunction; @@ -76,6 +77,7 @@ public class RascalLanguageServices { private static final IValueFactory VF = IRascalValueFactory.getInstance(); + private static final ErrorRecovery RECOVERY = new ErrorRecovery(IRascalValueFactory.getInstance()); private static final Logger logger = LogManager.getLogger(RascalLanguageServices.class); @@ -236,8 +238,13 @@ public List locateCodeLenses(ITree tree) { List result = new ArrayList<>(2); result.add(new CodeLensSuggestion(module, "Import in new Rascal terminal", "rascalmpl.importModule", moduleName)); - for (IValue topLevel : TreeAdapter - .getListASTArgs(TreeAdapter.getArg(TreeAdapter.getArg(tree, "body"), "toplevels"))) { + ITree body = TreeAdapter.getArg(tree, "body"); + ITree toplevels = TreeAdapter.getArg(body, "toplevels"); + for (IValue topLevel : TreeAdapter.getListASTArgs(toplevels)) { + if (RECOVERY.hasErrors((ITree) topLevel)) { + continue; + } + ITree decl = TreeAdapter.getArg((ITree) topLevel, "declaration"); if ("function".equals(TreeAdapter.getConstructorName(decl))) { ITree signature = TreeAdapter.getArg(TreeAdapter.getArg(decl, "functionDeclaration"), "signature"); diff --git a/rascal-lsp/src/main/rascal/lang/rascal/lsp/Outline.rsc b/rascal-lsp/src/main/rascal/lang/rascal/lsp/Outline.rsc index d7c6a91c4..9457630fc 100644 --- a/rascal-lsp/src/main/rascal/lang/rascal/lsp/Outline.rsc +++ b/rascal-lsp/src/main/rascal/lang/rascal/lsp/Outline.rsc @@ -31,38 +31,58 @@ import String; import ParseTree; import lang::rascal::\syntax::Rascal; import util::LanguageServer; +import util::ErrorRecovery; list[DocumentSymbol] outlineRascalModule(start[Module] \mod) { m= \mod.top; - children = []; - - top-down-break visit (m) { - case (Declaration) ` <{Variable ","}+ vars>;`: - children += [symbol(clean(""), variable(), v@\loc, detail="variable ") | v <- vars]; - - case (Declaration) ` anno @;`: - children += [symbol(clean(""), field(), t@\loc, detail="anno ")]; - case (Declaration) ` alias = ;`: - children += [symbol(clean(""), struct(), u@\loc, detail=" = ")]; + if (!(m has header) || hasErrors(m.header)) { + return []; + } - case (Declaration) ` tag on <{Type ","}+ ts>;`: - children += [symbol(clean(""), \key(), name@\loc, detail="tag on ")]; + children = []; - case (Declaration) ` data ;`: { - kwlist = [symbol(".", property(), k@\loc, detail="") | kws is present, KeywordFormal k <- kws.keywordFormalList]; - children += [symbol("", struct(), u@\loc, detail="data ", children=kwlist)]; + top-down-break visit (m) { + case decl: (Declaration) ` <{Variable ","}+ vars>;`: + if (!hasErrors(decl)) { + children += [symbol(clean(""), variable(), v@\loc, detail="variable ") | v <- vars, !hasErrors(v)]; + } + + case decl: (Declaration) ` anno @;`: + if (!hasErrors(decl)) { + children += [symbol(clean(""), field(), t@\loc, detail="anno ")]; + } + + case decl: (Declaration) ` alias = ;`: + if (!hasErrors(decl)) { + children += [symbol(clean(""), struct(), u@\loc, detail=" = ")]; + } + + case decl: (Declaration) ` tag on <{Type ","}+ ts>;`: + if (!hasErrors(decl)) { + children += [symbol(clean(""), \key(), name@\loc, detail="tag on ")]; + } + + case decl: (Declaration) ` data ;`: { + if (!hasErrors(decl)) { + kwlist = [symbol(".", property(), k@\loc, detail="") | kws is present, KeywordFormal k <- kws.keywordFormalList]; + children += [symbol("", struct(), u@\loc, detail="data ", children=kwlist)]; + } } - case (Declaration) ` data = <{Variant "|"}+ variants>;` : { - kwlist = [symbol(".", property(), k@\loc, detail="") | kws is present, KeywordFormal k <- kws.keywordFormalList]; - variantlist = [symbol(clean(""), \constructor(), v@\loc) | v <- variants]; + case decl: (Declaration) ` data = <{Variant "|"}+ variants>;` : { + if (!hasErrors(decl)) { + kwlist = [symbol(".", property(), k@\loc, detail="") | kws is present, KeywordFormal k <- kws.keywordFormalList]; + variantlist = [symbol(clean(""), \constructor(), v@\loc) | v <- variants]; - children += [symbol("", struct(), u@\loc, detail="data ", children=kwlist + variantlist)]; + children += [symbol("", struct(), u@\loc, detail="data ", children=kwlist + variantlist)]; + } } case FunctionDeclaration func : - children += [symbol("", \function(), (func.signature)@\loc, detail="")]; + if (!hasErrors(func)) { + children += [symbol("", \function(), (func.signature)@\loc, detail="")]; + } /* case (Import) `extend ;` : @@ -76,11 +96,13 @@ list[DocumentSymbol] outlineRascalModule(start[Module] \mod) { */ case SyntaxDefinition def : { - rs = [symbol(clean(" "), \function(), p@\loc) - | /Prod p := def.production, p is labeled || p is unlabeled, - str prefix := (p is labeled ? ": " : "") - ]; - children += [symbol(clean(""), \struct(), def@\loc, children=rs)]; + if (!hasErrors(def)) { + rs = [symbol(clean(" "), \function(), p@\loc) + | /Prod p := def.production, !hasErrors(p) && (p is labeled || p is unlabeled), + str prefix := (p is labeled ? ": " : "") + ]; + children += [symbol(clean(""), \struct(), def@\loc, children=rs)]; + } } } From 5d60fde48e12e5e1655b4ee61b8eec286890104a Mon Sep 17 00:00:00 2001 From: Pieter Olivier Date: Fri, 25 Oct 2024 14:43:00 +0200 Subject: [PATCH 16/28] Removed spurious hasErrors --- rascal-lsp/src/main/rascal/lang/rascal/lsp/Outline.rsc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rascal-lsp/src/main/rascal/lang/rascal/lsp/Outline.rsc b/rascal-lsp/src/main/rascal/lang/rascal/lsp/Outline.rsc index 9457630fc..2c4f6186f 100644 --- a/rascal-lsp/src/main/rascal/lang/rascal/lsp/Outline.rsc +++ b/rascal-lsp/src/main/rascal/lang/rascal/lsp/Outline.rsc @@ -45,7 +45,7 @@ list[DocumentSymbol] outlineRascalModule(start[Module] \mod) { top-down-break visit (m) { case decl: (Declaration) ` <{Variable ","}+ vars>;`: if (!hasErrors(decl)) { - children += [symbol(clean(""), variable(), v@\loc, detail="variable ") | v <- vars, !hasErrors(v)]; + children += [symbol(clean(""), variable(), v@\loc, detail="variable ") | v <- vars]; } case decl: (Declaration) ` anno @;`: From a9d5a06fc07508a5ce44cb9b395546fc9d515661 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Fri, 25 Oct 2024 14:56:07 +0200 Subject: [PATCH 17/28] Move parse error processing (including error nodes) completely to `TextDocumentState` --- .../vscode/lsp/TextDocumentState.java | 126 ++++++++++++++---- .../ParametricTextDocumentService.java | 4 +- .../lsp/rascal/RascalTextDocumentService.java | 43 +----- 3 files changed, 107 insertions(+), 66 deletions(-) diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java index c4205bcea..a072e6f4a 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java @@ -27,27 +27,39 @@ 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.CompletionException; import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; 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.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, @@ -59,30 +71,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> parser; private final ISourceLocation location; + private final ColumnMaps columns; @SuppressWarnings("java:S3077") // Visibility of writes is enough private volatile Update current; - private final Debouncer> currentTreeAsyncDebouncer; + private final Debouncer currentAsyncParseDebouncer; private final AtomicReference<@MonotonicNonNull Versioned> lastWithoutErrors; private final AtomicReference<@MonotonicNonNull Versioned> last; public TextDocumentState( BiFunction> 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<>(); @@ -95,7 +109,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 getCurrentContent() { @@ -107,17 +121,21 @@ public CompletableFuture> getCurrentTreeAsync() { } public CompletableFuture> getCurrentTreeAsync(Duration delay) { - return currentTreeAsyncDebouncer.get(delay); + return currentAsyncParseDebouncer + .get(delay) + .thenApply(Update::getTreeAsync) + .thenCompose(Function.identity()); } - public @Nullable CompletableFuture> getCurrentTreeAsyncIfParsing() { - var update = current; - return update.isParsing() ? update.getTreeAsync() : null; + public CompletableFuture>> getCurrentDiagnosticsAsync() { + return current.getDiagnosticsAsync(); // Triggers the parser } - public CompletableFuture>> getCurrentDiagnostics() { - throw new UnsupportedOperationException(); - // TODO: In a separate PR + public CompletableFuture>> getCurrentDiagnosticsAsync(Duration delay) { + return currentAsyncParseDebouncer + .get(delay) + .thenApply(Update::getDiagnosticsAsync) + .thenCompose(Function.identity()); } public @MonotonicNonNull Versioned getLastTree() { @@ -128,16 +146,29 @@ public CompletableFuture>> getCurrentDiagnostics() { return lastWithoutErrors.get(); } + private @Nullable CompletableFuture getCurrentAsyncIfParsing() { + var update = current; + return update.isParsing() ? CompletableFuture.completedFuture(update) : null; + } + + private CompletableFuture parseAndGetCurrentAsync() { + var update = current; + update.parseIfNotParsing(); + return CompletableFuture.completedFuture(update); + } + private class Update { private final int version; private final String content; private final CompletableFuture> treeAsync; + private final CompletableFuture>> 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); } @@ -150,6 +181,11 @@ public CompletableFuture> getTreeAsync() { return treeAsync; } + public CompletableFuture>> getDiagnosticsAsync() { + parseIfNotParsing(); + return diagnosticsAsync; + } + public boolean isParsing() { return parsing.get(); } @@ -158,22 +194,58 @@ 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); + 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); }); } } + + private List toDiagnostics(ITree tree, Throwable excp) { + List parseErrors = new ArrayList<>(); + + if (excp instanceof CompletionException) { + excp = excp.getCause(); + } + + 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")); + } + + 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)); + } + } + + return parseErrors; + } } } diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParametricTextDocumentService.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParametricTextDocumentService.java index 161322514..79eee19c8 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParametricTextDocumentService.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParametricTextDocumentService.java @@ -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()) ); } @@ -638,7 +638,7 @@ public CompletableFuture>> codeAction(CodeActio private CompletableFuture 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(); } diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java index d157aceeb..a59b7a0f8 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java @@ -183,7 +183,7 @@ public void connect(LanguageClient client) { public void didOpen(DidOpenTextDocumentParams params) { logger.debug("Open file: {}", params.getTextDocument()); TextDocumentState file = open(params.getTextDocument()); - handleParsingErrors(file); + handleParsingErrors(file, file.getCurrentDiagnosticsAsync()); // No debounce } @Override @@ -215,51 +215,20 @@ private TextDocumentState updateContents(VersionedTextDocumentIdentifier doc, St TextDocumentState file = getFile(doc); logger.trace("New contents for {}", doc); file.update(doc.getVersion(), newContents); - handleParsingErrors(file, file.getCurrentTreeAsync(Duration.ofMillis(800))); + handleParsingErrors(file, file.getCurrentDiagnosticsAsync(Duration.ofMillis(800))); return file; } - private void handleParsingErrors(TextDocumentState file, CompletableFuture> futureTree) { - futureTree.handle((tree, excp) -> { - List parseErrors = new ArrayList<>(); - - if (excp instanceof CompletionException) { - excp = excp.getCause(); - } - - 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")); - } - - if (tree != null) { - RascalValueFactory valueFactory = (RascalValueFactory) ValueFactoryFactory.getValueFactory(); - IList errors = new ErrorRecovery(valueFactory).findAllErrors(tree.get()); - for (IValue error : errors) { - ITree errorTree = (ITree) error; - parseErrors.add(Diagnostics.translateErrorRecoveryDiagnostic(errorTree, columns)); - } - } - + private void handleParsingErrors(TextDocumentState file, CompletableFuture>> diagnosticsAsync) { + diagnosticsAsync.thenAccept(diagnostics -> { + List parseErrors = diagnostics.get(); logger.trace("Finished parsing tree, reporting new parse errors: {} for: {}", parseErrors, file.getLocation()); if (facts != null) { facts.reportParseErrors(file.getLocation(), parseErrors); } - return null; }); } - private void handleParsingErrors(TextDocumentState file) { - handleParsingErrors(file,file.getCurrentTreeAsync()); - } - - @Override public CompletableFuture, List>> definition(DefinitionParams params) { logger.debug("Definition: {} at {}", params.getTextDocument(), params.getPosition()); @@ -342,7 +311,7 @@ private static T last(List l) { private TextDocumentState open(TextDocumentItem doc) { return documents.computeIfAbsent(Locations.toLoc(doc), - l -> new TextDocumentState((loc, input) -> rascalServices.parseSourceFile(loc, input), l, doc.getVersion(), doc.getText())); + l -> new TextDocumentState((loc, input) -> rascalServices.parseSourceFile(loc, input), l, columns, doc.getVersion(), doc.getText())); } private TextDocumentState getFile(TextDocumentIdentifier doc) { From aeb1a9fa9486696d653bc70a892f855a9035b883 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Fri, 25 Oct 2024 15:06:06 +0200 Subject: [PATCH 18/28] Refine parse error messages --- .../main/java/org/rascalmpl/vscode/lsp/util/Diagnostics.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Diagnostics.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Diagnostics.java index 2e404baee..60c1c9db4 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Diagnostics.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/Diagnostics.java @@ -73,13 +73,14 @@ public static Map> groupByKey(Stream> diagnostics) } public static Diagnostic translateDiagnostic(ParseError e, ColumnMaps cm) { - return new Diagnostic(toRange(e, cm), e.getMessage(), DiagnosticSeverity.Error, "parser"); + var message = e.getMessage() + " (irrecoverable)"; + return new Diagnostic(toRange(e, cm), message, DiagnosticSeverity.Error, "parser"); } public static Diagnostic translateErrorRecoveryDiagnostic(ITree errorTree, ColumnMaps cm) { IList args = TreeAdapter.getArgs(errorTree); ITree skipped = (ITree) args.get(args.size()-1); - return new Diagnostic(toRange(skipped, cm), "Parse error", DiagnosticSeverity.Error, "parser"); + return new Diagnostic(toRange(skipped, cm), "Parse error (recoverable)", DiagnosticSeverity.Error, "parser"); } public static Diagnostic translateRascalParseError(IValue e, ColumnMaps cm) { From 90ab10c31721c73e879b4eb87b9cbc844fccec61 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Mon, 28 Oct 2024 16:32:35 +0100 Subject: [PATCH 19/28] Remove unused imports --- .../vscode/lsp/rascal/RascalTextDocumentService.java | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java index 9dc61b5f9..d1a1d69d3 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java @@ -35,7 +35,6 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionException; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutorService; import java.util.stream.Collectors; @@ -49,7 +48,6 @@ import org.eclipse.lsp4j.Command; import org.eclipse.lsp4j.DefinitionParams; import org.eclipse.lsp4j.Diagnostic; -import org.eclipse.lsp4j.DiagnosticSeverity; import org.eclipse.lsp4j.DidChangeTextDocumentParams; import org.eclipse.lsp4j.DidCloseTextDocumentParams; import org.eclipse.lsp4j.DidOpenTextDocumentParams; @@ -63,8 +61,6 @@ import org.eclipse.lsp4j.Location; import org.eclipse.lsp4j.LocationLink; import org.eclipse.lsp4j.MarkupContent; -import org.eclipse.lsp4j.Position; -import org.eclipse.lsp4j.Range; import org.eclipse.lsp4j.RenameParams; import org.eclipse.lsp4j.SemanticTokens; import org.eclipse.lsp4j.SemanticTokensDelta; @@ -85,12 +81,7 @@ import org.eclipse.lsp4j.services.LanguageClient; import org.eclipse.lsp4j.services.LanguageClientAware; import org.rascalmpl.library.Prelude; -import org.rascalmpl.library.util.ErrorRecovery; -import org.rascalmpl.parser.gtd.exception.ParseError; import org.rascalmpl.uri.URIResolverRegistry; -import org.rascalmpl.values.RascalValueFactory; -import org.rascalmpl.values.ValueFactoryFactory; -import org.rascalmpl.values.parsetrees.ITree; import org.rascalmpl.vscode.lsp.BaseWorkspaceService; import org.rascalmpl.vscode.lsp.IBaseLanguageClient; import org.rascalmpl.vscode.lsp.IBaseTextDocumentService; @@ -99,7 +90,6 @@ import org.rascalmpl.vscode.lsp.rascal.model.FileFacts; import org.rascalmpl.vscode.lsp.rascal.model.SummaryBridge; import org.rascalmpl.vscode.lsp.terminal.ITerminalIDEServer.LanguageParameter; -import org.rascalmpl.vscode.lsp.util.Diagnostics; import org.rascalmpl.vscode.lsp.util.DocumentChanges; import org.rascalmpl.vscode.lsp.util.FoldingRanges; import org.rascalmpl.vscode.lsp.util.Outline; @@ -109,7 +99,6 @@ import org.rascalmpl.vscode.lsp.util.locations.LineColumnOffsetMap; import org.rascalmpl.vscode.lsp.util.locations.Locations; -import io.usethesource.vallang.IList; import io.usethesource.vallang.ISourceLocation; import io.usethesource.vallang.IValue; From d86c6593ff95119254c92f2fb70178c03ab6f3ee Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Mon, 28 Oct 2024 16:33:10 +0100 Subject: [PATCH 20/28] Move class `Debouncer` to its own file --- .../vscode/lsp/TextDocumentState.java | 138 +------------- .../vscode/lsp/util/concurrent/Debouncer.java | 170 ++++++++++++++++++ 2 files changed, 171 insertions(+), 137 deletions(-) create mode 100644 rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/concurrent/Debouncer.java diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java index a072e6f4a..b392ba044 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java @@ -31,14 +31,10 @@ 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.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; -import java.util.concurrent.Executor; -import java.util.concurrent.TimeUnit; 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; @@ -55,6 +51,7 @@ 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; @@ -248,136 +245,3 @@ private List toDiagnostics(ITree tree, Throwable excp) { } } } - -/** - * 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 { - - // 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> 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> getIfInitialized; - private final Supplier> initializeAndGet; - - public Debouncer(Duration period, - Supplier<@Nullable CompletableFuture> getIfInitialized, - Supplier> initializeAndGet) { - - this(Math.toIntExact(period.toMillis()), getIfInitialized, initializeAndGet); - } - - public Debouncer(int period, - Supplier<@Nullable CompletableFuture> getIfInitialized, - Supplier> 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 get(Duration delay) { - return get(Math.toIntExact(delay.toMillis())); - } - - public CompletableFuture get(int delay) { - return schedule(delay, false); - } - - private CompletableFuture 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(); - 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; - } - - // 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(); - } - - // 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; - } - - // 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; - } - - // 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 reschedule(int delay) { - return schedule(delay, true); - } -} diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/concurrent/Debouncer.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/concurrent/Debouncer.java new file mode 100644 index 000000000..84bb3e0a0 --- /dev/null +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/concurrent/Debouncer.java @@ -0,0 +1,170 @@ +/* + * Copyright (c) 2018-2023, NWO-I CWI and Swat.engineering + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + */ +package org.rascalmpl.vscode.lsp.util.concurrent; + +import java.time.Duration; +import java.util.concurrent.atomic.AtomicStampedReference; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Executor; +import java.util.concurrent.TimeUnit; +import java.util.function.Function; +import java.util.function.Supplier; + +import org.checkerframework.checker.nullness.qual.Nullable; + +/** + * 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. + */ +public class Debouncer { + + // 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> 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> getIfInitialized; + private final Supplier> initializeAndGet; + + public Debouncer(Duration period, + Supplier<@Nullable CompletableFuture> getIfInitialized, + Supplier> initializeAndGet) { + + this(Math.toIntExact(period.toMillis()), getIfInitialized, initializeAndGet); + } + + public Debouncer(int period, + Supplier<@Nullable CompletableFuture> getIfInitialized, + Supplier> 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 get(Duration delay) { + return get(Math.toIntExact(delay.toMillis())); + } + + public CompletableFuture get(int delay) { + return schedule(delay, false); + } + + private CompletableFuture 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(); + 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; + } + + // 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(); + } + + // 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; + } + + // 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; + } + + // 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 reschedule(int delay) { + return schedule(delay, true); + } +} From 92cc9130d5e340ee4118aefd983bdc77a44e62d9 Mon Sep 17 00:00:00 2001 From: Pieter Olivier Date: Tue, 5 Nov 2024 15:42:03 +0100 Subject: [PATCH 21/28] Remove function that should have been deleted during merge from main --- .../vscode/lsp/rascal/RascalTextDocumentService.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java index 485b2ec2a..b65ff0f2b 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalTextDocumentService.java @@ -237,10 +237,6 @@ private void handleParsingErrors(TextDocumentState file, CompletableFuture, List>> definition(DefinitionParams params) { logger.debug("textDocument/definition: {} at {}", params.getTextDocument(), params.getPosition()); From 066c5df782bf489cd47754133163131f80a7c5cc Mon Sep 17 00:00:00 2001 From: Pieter Olivier Date: Wed, 6 Nov 2024 07:43:55 +0100 Subject: [PATCH 22/28] Bumped rascal version number --- rascal-lsp/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rascal-lsp/pom.xml b/rascal-lsp/pom.xml index 21bc9fa7a..5e173d187 100644 --- a/rascal-lsp/pom.xml +++ b/rascal-lsp/pom.xml @@ -65,7 +65,7 @@ org.rascalmpl rascal - 0.40.14-SNAPSHOT + 0.40.15-SNAPSHOT org.rascalmpl From ed3dd8744d05e24f9ceaec21a0eb1b44169dbf79 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Fri, 8 Nov 2024 12:52:35 +0100 Subject: [PATCH 23/28] Simplify debouncer (joint with @PieterOlivier) --- .../vscode/lsp/TextDocumentState.java | 39 ++-- .../util/concurrent/DebouncedSupplier.java | 69 +++++++ .../vscode/lsp/util/concurrent/Debouncer.java | 170 ------------------ 3 files changed, 88 insertions(+), 190 deletions(-) create mode 100644 rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/concurrent/DebouncedSupplier.java delete mode 100644 rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/concurrent/Debouncer.java diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java index b392ba044..0e2297826 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java @@ -33,13 +33,13 @@ import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; +import java.util.concurrent.TimeUnit; import java.util.function.BiFunction; import java.util.function.Function; 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; @@ -51,7 +51,7 @@ 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.concurrent.DebouncedSupplier; import org.rascalmpl.vscode.lsp.util.locations.ColumnMaps; import io.usethesource.vallang.IList; @@ -76,7 +76,7 @@ public class TextDocumentState { @SuppressWarnings("java:S3077") // Visibility of writes is enough private volatile Update current; - private final Debouncer currentAsyncParseDebouncer; + private final DebouncedSupplier parseCurrentDebouncer; private final AtomicReference<@MonotonicNonNull Versioned> lastWithoutErrors; private final AtomicReference<@MonotonicNonNull Versioned> last; @@ -91,10 +91,7 @@ public TextDocumentState( this.columns = columns; this.current = new Update(initialVersion, initialContent); - this.currentAsyncParseDebouncer = new Debouncer<>(50, - this::getCurrentAsyncIfParsing, - this::parseAndGetCurrentAsync); - + this.parseCurrentDebouncer = new DebouncedSupplier<>(this::parseCurrent); this.lastWithoutErrors = new AtomicReference<>(); this.last = new AtomicReference<>(); } @@ -114,23 +111,21 @@ public Versioned getCurrentContent() { } public CompletableFuture> getCurrentTreeAsync() { - return current.getTreeAsync(); // Triggers the parser + return getCurrentTreeAsync(Duration.ZERO); } public CompletableFuture> getCurrentTreeAsync(Duration delay) { - return currentAsyncParseDebouncer - .get(delay) + return parseCurrent(delay) .thenApply(Update::getTreeAsync) .thenCompose(Function.identity()); } public CompletableFuture>> getCurrentDiagnosticsAsync() { - return current.getDiagnosticsAsync(); // Triggers the parser + return getCurrentDiagnosticsAsync(Duration.ZERO); } public CompletableFuture>> getCurrentDiagnosticsAsync(Duration delay) { - return currentAsyncParseDebouncer - .get(delay) + return parseCurrent(delay) .thenApply(Update::getDiagnosticsAsync) .thenCompose(Function.identity()); } @@ -143,15 +138,17 @@ public CompletableFuture>> getCurrentDiagnosticsAsync return lastWithoutErrors.get(); } - private @Nullable CompletableFuture getCurrentAsyncIfParsing() { - var update = current; - return update.isParsing() ? CompletableFuture.completedFuture(update) : null; + private CompletableFuture parseCurrent() { + return current.parseIfNotParsing(); } - private CompletableFuture parseAndGetCurrentAsync() { + private CompletableFuture parseCurrent(Duration delay) { var update = current; - update.parseIfNotParsing(); - return CompletableFuture.completedFuture(update); + if (update.isParsing()) { + return update.parseIfNotParsing(); + } else { + return parseCurrentDebouncer.get(delay.toMillis(), TimeUnit.MILLISECONDS); + } } private class Update { @@ -187,7 +184,7 @@ public boolean isParsing() { return parsing.get(); } - private void parseIfNotParsing() { + private CompletableFuture parseIfNotParsing() { if (parsing.compareAndSet(false, true)) { parser .apply(location, content) @@ -212,6 +209,8 @@ private void parseIfNotParsing() { diagnosticsAsync.complete(diagnostics); }); } + + return CompletableFuture.completedFuture(this); } private List toDiagnostics(ITree tree, Throwable excp) { diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/concurrent/DebouncedSupplier.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/concurrent/DebouncedSupplier.java new file mode 100644 index 000000000..0e29f8b78 --- /dev/null +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/concurrent/DebouncedSupplier.java @@ -0,0 +1,69 @@ +/* + * Copyright (c) 2018-2023, NWO-I CWI and Swat.engineering + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + */ +package org.rascalmpl.vscode.lsp.util.concurrent; + +import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; +import java.util.function.Function; +import java.util.function.Supplier; + +import org.checkerframework.checker.nullness.qual.Nullable; + +public class DebouncedSupplier { + private final Supplier> supplier; + private final AtomicReference>> latest; + + public DebouncedSupplier(Supplier> supplier) { + this.supplier = supplier; + this.latest = new AtomicReference<>(CompletableFuture.completedFuture(null)); + } + + /** + * Gets the result of `supplier` with a debouncing delay. Specifically: + * - if the next get *doesn't* happen before the given timeout, then the + * current get completes with the result of `supplier` upon timeout; + * - if the next get *does* happen before the given timeout, then the + * current get completes with its result upon availability. + */ + public CompletableFuture get(long timeout, TimeUnit unit) { + var newLatest = new CompletableFuture>(); + var oldLatest = latest.getAndSet(newLatest); + + var newLatestResult = newLatest + .thenApply(result -> result == null ? supplier.get() : result) + .thenCompose(Function.identity()); + + // Complete the current get with the result of `supplier` upon timeout. + // Complete the previous get with the result of the current get upon + // availability. + newLatest.completeOnTimeout(null, timeout, unit); + oldLatest.complete(newLatestResult); + + return newLatestResult; + } +} diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/concurrent/Debouncer.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/concurrent/Debouncer.java deleted file mode 100644 index 84bb3e0a0..000000000 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/concurrent/Debouncer.java +++ /dev/null @@ -1,170 +0,0 @@ -/* - * Copyright (c) 2018-2023, NWO-I CWI and Swat.engineering - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * - * 1. Redistributions of source code must retain the above copyright notice, - * this list of conditions and the following disclaimer. - * - * 2. Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following disclaimer in the documentation - * and/or other materials provided with the distribution. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" - * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE - * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR - * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF - * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS - * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN - * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) - * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE - * POSSIBILITY OF SUCH DAMAGE. - */ -package org.rascalmpl.vscode.lsp.util.concurrent; - -import java.time.Duration; -import java.util.concurrent.atomic.AtomicStampedReference; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.Executor; -import java.util.concurrent.TimeUnit; -import java.util.function.Function; -import java.util.function.Supplier; - -import org.checkerframework.checker.nullness.qual.Nullable; - -/** - * 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. - */ -public class Debouncer { - - // 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> 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> getIfInitialized; - private final Supplier> initializeAndGet; - - public Debouncer(Duration period, - Supplier<@Nullable CompletableFuture> getIfInitialized, - Supplier> initializeAndGet) { - - this(Math.toIntExact(period.toMillis()), getIfInitialized, initializeAndGet); - } - - public Debouncer(int period, - Supplier<@Nullable CompletableFuture> getIfInitialized, - Supplier> 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 get(Duration delay) { - return get(Math.toIntExact(delay.toMillis())); - } - - public CompletableFuture get(int delay) { - return schedule(delay, false); - } - - private CompletableFuture 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(); - 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; - } - - // 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(); - } - - // 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; - } - - // 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; - } - - // 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 reschedule(int delay) { - return schedule(delay, true); - } -} From 7f16037a66d7f7884191b97163805df4053ce72b Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Fri, 8 Nov 2024 15:37:34 +0100 Subject: [PATCH 24/28] Improve API of `DebouncedSupplier` --- .../vscode/lsp/TextDocumentState.java | 2 +- .../lsp/util/concurrent/DebouncedSupplier.java | 18 +++++++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java index 0e2297826..f2d229162 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java @@ -147,7 +147,7 @@ private CompletableFuture parseCurrent(Duration delay) { if (update.isParsing()) { return update.parseIfNotParsing(); } else { - return parseCurrentDebouncer.get(delay.toMillis(), TimeUnit.MILLISECONDS); + return parseCurrentDebouncer.get(delay); } } diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/concurrent/DebouncedSupplier.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/concurrent/DebouncedSupplier.java index 0e29f8b78..6faae6fe1 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/concurrent/DebouncedSupplier.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/concurrent/DebouncedSupplier.java @@ -27,6 +27,7 @@ package org.rascalmpl.vscode.lsp.util.concurrent; import java.util.concurrent.atomic.AtomicReference; +import java.time.Duration; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; import java.util.function.Function; @@ -44,7 +45,22 @@ public DebouncedSupplier(Supplier> supplier) { } /** - * Gets the result of `supplier` with a debouncing delay. Specifically: + * Gets the result of `supplier` immediately. Previous uncompleted gets are + * completed. + */ + public CompletableFuture get() { + return get(Duration.ZERO); + } + + /** + * Gets the result of `supplier` with the given debouncing delay. + */ + public CompletableFuture get(Duration delay) { + return get(delay.toMillis(), TimeUnit.MILLISECONDS); + } + + /** + * Gets the result of `supplier` with the given timeout: * - if the next get *doesn't* happen before the given timeout, then the * current get completes with the result of `supplier` upon timeout; * - if the next get *does* happen before the given timeout, then the From 761ab06d0cb8a6318d104de0fb7de8114606c900 Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Fri, 8 Nov 2024 15:39:58 +0100 Subject: [PATCH 25/28] Add tests for `DebouncedSupplier` --- .../concurrent/DebouncedSupplierTests.java | 243 ++++++++++++++++++ 1 file changed, 243 insertions(+) create mode 100644 rascal-lsp/src/test/java/org/rascalmpl/vscode/lsp/util/concurrent/DebouncedSupplierTests.java diff --git a/rascal-lsp/src/test/java/org/rascalmpl/vscode/lsp/util/concurrent/DebouncedSupplierTests.java b/rascal-lsp/src/test/java/org/rascalmpl/vscode/lsp/util/concurrent/DebouncedSupplierTests.java new file mode 100644 index 000000000..1927955d7 --- /dev/null +++ b/rascal-lsp/src/test/java/org/rascalmpl/vscode/lsp/util/concurrent/DebouncedSupplierTests.java @@ -0,0 +1,243 @@ +/* + * Copyright (c) 2018-2023, NWO-I CWI and Swat.engineering + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + */ +package org.rascalmpl.vscode.lsp.util.concurrent; + +import java.time.Duration; +import java.util.ArrayList; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.atomic.AtomicReference; + +import org.junit.Assert; +import org.junit.Test; + +public class DebouncedSupplierTests { + private static final Duration EXTRA_SMALL_DELAY = Duration.ofMillis(250); + private static final Duration SMALL_DELAY = Duration.ofMillis(500); + private static final Duration MEDIUM_DELAY = Duration.ofMillis(1000); + private static final Duration LARGE_DELAY = Duration.ofMillis(2000); + + private static final Duration DEFAULT_SUPPLY_TIME = LARGE_DELAY; + + // Default tolerance when comparing expected/actual execution times + private static final Duration DELTA = EXTRA_SMALL_DELAY; + + private static DebouncedSupplier newDebouncedSupplier( + AtomicReference input) { + return newDebouncedSupplier(input, DEFAULT_SUPPLY_TIME); + } + + private static DebouncedSupplier newDebouncedSupplier( + AtomicReference input, Duration supplyTime) { + + return new DebouncedSupplier<>(() -> { + return CompletableFuture.supplyAsync(() -> { + var output = input.get(); + sleep(supplyTime); // Simulate supplier + return output; + }); + }); + } + + private static void sleep(Duration d) { + try { + Thread.sleep(d.toMillis()); + } catch (InterruptedException e) { + e.printStackTrace(); + } + } + + private static long timeMillis(Runnable r) { + var begin = System.currentTimeMillis(); + r.run(); + var end = System.currentTimeMillis(); + return end - begin; + } + + @Test + public void testNoDelay() { + var input = new AtomicReference<>(""); + var supplier = newDebouncedSupplier(input); + + var expectedTimeMillis = DEFAULT_SUPPLY_TIME.toMillis(); + var actualTimeMillis = timeMillis(() -> { + input.set("foo"); + supplier + .get() + .thenAccept(output -> Assert.assertEquals("foo", output)) + .join(); + }); + + Assert.assertEquals(expectedTimeMillis, actualTimeMillis, DELTA.toMillis()); + } + + @Test + public void testNoDelayAfterDelay() { + var input = new AtomicReference<>(""); + var supplier = newDebouncedSupplier(input); + + var expectedTimeMillis = SMALL_DELAY.plus(DEFAULT_SUPPLY_TIME).toMillis(); + + var actualTimeMillis = timeMillis(() -> { + input.set("foo1"); + var future1 = supplier + .get(MEDIUM_DELAY) + .thenAccept(output -> Assert.assertEquals("bar", output)); + + sleep(SMALL_DELAY); + + input.set("foo2"); + var future2 = supplier + .get() + .thenAccept(output -> Assert.assertEquals("bar", output)); + + input.set("bar"); + future1.join(); + future2.join(); + }); + + Assert.assertEquals(expectedTimeMillis, actualTimeMillis, DELTA.toMillis()); + + } + + @Test + public void testFewSequential() { + var input = new AtomicReference<>(""); + var supplier = newDebouncedSupplier(input); + + var expectedTimeMillis = Duration.ZERO + .plus(MEDIUM_DELAY).plus(DEFAULT_SUPPLY_TIME) // First get + .plus(MEDIUM_DELAY).plus(DEFAULT_SUPPLY_TIME) // Second get + .toMillis(); + + var actualTimeMillis = timeMillis(() -> { + input.set("foo1"); + supplier + .get(MEDIUM_DELAY) + .thenAccept(output -> Assert.assertEquals("foo1", output)) + .join(); + + input.set("foo2"); + supplier + .get(MEDIUM_DELAY) + .thenAccept(output -> Assert.assertEquals("foo2", output)) + .join(); + }); + + Assert.assertEquals(expectedTimeMillis, actualTimeMillis, DELTA.toMillis()); + } + + @Test + public void testManySequential() { + var input = new AtomicReference<>(""); + var supplier = newDebouncedSupplier(input); + var n = 5; + + var expectedTimeMillis = Duration.ZERO + .plus(MEDIUM_DELAY).plus(DEFAULT_SUPPLY_TIME).multipliedBy(n) // First get, ..., last get + .toMillis(); + + var actualTimeMillis = timeMillis(() -> { + for (var i = 0; i < n; i++) { + var fooi = "foo" + i; + input.set(fooi); + supplier + .get(MEDIUM_DELAY) + .thenAccept(output -> Assert.assertEquals(fooi, output)) + .join(); + } + }); + + Assert.assertEquals(expectedTimeMillis, actualTimeMillis, DELTA.toMillis()); + } + + @Test + public void testFewConcurrent() { + var input = new AtomicReference<>(""); + var supplier = newDebouncedSupplier(input); + + var expectedTimeMillis = Duration.ZERO + .plus(SMALL_DELAY) // First get until second get + .plus(MEDIUM_DELAY).plus(DEFAULT_SUPPLY_TIME) // Second get + .toMillis(); + + var actualTimeMillis = timeMillis(() -> { + input.set("foo1"); + var future1 = supplier + .get(MEDIUM_DELAY) + .thenAccept(output -> Assert.assertEquals("bar", output)); + + sleep(SMALL_DELAY); + + input.set("foo2"); + var future2 = supplier + .get(MEDIUM_DELAY) + .thenAccept(output -> Assert.assertEquals("bar", output)); + + input.set("bar"); + future1.join(); + future2.join(); + }); + + Assert.assertEquals(expectedTimeMillis, actualTimeMillis, DELTA.toMillis()); + } + + @Test + public void testManyConcurrent() { + var input = new AtomicReference<>(""); + var supplier = newDebouncedSupplier(input); + var n = 10; + + var expectedTimeMillis = Duration.ZERO + .plus(SMALL_DELAY).multipliedBy(n - 1) // First get until second get, ..., (n-1)th get until last get + .plus(MEDIUM_DELAY).plus(DEFAULT_SUPPLY_TIME) // Last get + .toMillis(); + + var actualTimeMillis = timeMillis(() -> { + var futures = new ArrayList>(n); + + for (var i = 0; i < n; i++) { + if (i > 0) { + sleep(SMALL_DELAY); + } + + input.set("foo" + i); + var future = supplier + .get(MEDIUM_DELAY) + .thenAccept(output -> Assert.assertEquals("bar", output)); + + futures.add(future); + } + + input.set("bar"); + for (var f : futures) { + f.join(); + } + }); + + Assert.assertEquals(expectedTimeMillis, actualTimeMillis, DELTA.toMillis()); + } +} From 15aaa0f41103e10842d94c1c58c98e3efe79ccfa Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Fri, 8 Nov 2024 15:59:11 +0100 Subject: [PATCH 26/28] Improve documentations of `DebouncedSupplier` --- .../vscode/lsp/util/concurrent/DebouncedSupplier.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/concurrent/DebouncedSupplier.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/concurrent/DebouncedSupplier.java index 6faae6fe1..b34f1177c 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/concurrent/DebouncedSupplier.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/concurrent/DebouncedSupplier.java @@ -60,11 +60,12 @@ public CompletableFuture get(Duration delay) { } /** - * Gets the result of `supplier` with the given timeout: + * Gets the result of `supplier` with the given timeout, as follows: * - if the next get *doesn't* happen before the given timeout, then the * current get completes with the result of `supplier` upon timeout; * - if the next get *does* happen before the given timeout, then the - * current get completes with its result upon availability. + * current get completes with the result of the next get upon + * availability. */ public CompletableFuture get(long timeout, TimeUnit unit) { var newLatest = new CompletableFuture>(); From b81dfc4f5573522cecc44a832e3d9e6bc8e3c1ff Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Fri, 8 Nov 2024 16:13:23 +0100 Subject: [PATCH 27/28] Revert signature change to `parseIfNotParsing` --- .../org/rascalmpl/vscode/lsp/TextDocumentState.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java index f2d229162..f235b9bd1 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java @@ -139,13 +139,15 @@ public CompletableFuture>> getCurrentDiagnosticsAsync } private CompletableFuture parseCurrent() { - return current.parseIfNotParsing(); + var update = current; + update.parseIfNotParsing(); + return CompletableFuture.completedFuture(update); } private CompletableFuture parseCurrent(Duration delay) { var update = current; if (update.isParsing()) { - return update.parseIfNotParsing(); + return CompletableFuture.completedFuture(update); } else { return parseCurrentDebouncer.get(delay); } @@ -184,7 +186,7 @@ public boolean isParsing() { return parsing.get(); } - private CompletableFuture parseIfNotParsing() { + private void parseIfNotParsing() { if (parsing.compareAndSet(false, true)) { parser .apply(location, content) @@ -209,8 +211,6 @@ private CompletableFuture parseIfNotParsing() { diagnosticsAsync.complete(diagnostics); }); } - - return CompletableFuture.completedFuture(this); } private List toDiagnostics(ITree tree, Throwable excp) { From 87e5959315d00e4a562bcb0ab7f40b554c9797dd Mon Sep 17 00:00:00 2001 From: Sung-Shik Jongmans Date: Fri, 8 Nov 2024 16:17:34 +0100 Subject: [PATCH 28/28] Rename methods to make the names more precise --- .../rascalmpl/vscode/lsp/TextDocumentState.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java index f235b9bd1..150296d82 100644 --- a/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java +++ b/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/TextDocumentState.java @@ -76,7 +76,7 @@ public class TextDocumentState { @SuppressWarnings("java:S3077") // Visibility of writes is enough private volatile Update current; - private final DebouncedSupplier parseCurrentDebouncer; + private final DebouncedSupplier parseAndGetCurrentDebouncer; private final AtomicReference<@MonotonicNonNull Versioned> lastWithoutErrors; private final AtomicReference<@MonotonicNonNull Versioned> last; @@ -91,7 +91,7 @@ public TextDocumentState( this.columns = columns; this.current = new Update(initialVersion, initialContent); - this.parseCurrentDebouncer = new DebouncedSupplier<>(this::parseCurrent); + this.parseAndGetCurrentDebouncer = new DebouncedSupplier<>(this::parseAndGetCurrent); this.lastWithoutErrors = new AtomicReference<>(); this.last = new AtomicReference<>(); } @@ -115,7 +115,7 @@ public CompletableFuture> getCurrentTreeAsync() { } public CompletableFuture> getCurrentTreeAsync(Duration delay) { - return parseCurrent(delay) + return parseAndGetCurrent(delay) .thenApply(Update::getTreeAsync) .thenCompose(Function.identity()); } @@ -125,7 +125,7 @@ public CompletableFuture>> getCurrentDiagnosticsAsync } public CompletableFuture>> getCurrentDiagnosticsAsync(Duration delay) { - return parseCurrent(delay) + return parseAndGetCurrent(delay) .thenApply(Update::getDiagnosticsAsync) .thenCompose(Function.identity()); } @@ -138,18 +138,18 @@ public CompletableFuture>> getCurrentDiagnosticsAsync return lastWithoutErrors.get(); } - private CompletableFuture parseCurrent() { + private CompletableFuture parseAndGetCurrent() { var update = current; update.parseIfNotParsing(); return CompletableFuture.completedFuture(update); } - private CompletableFuture parseCurrent(Duration delay) { + private CompletableFuture parseAndGetCurrent(Duration delay) { var update = current; if (update.isParsing()) { return CompletableFuture.completedFuture(update); } else { - return parseCurrentDebouncer.get(delay); + return parseAndGetCurrentDebouncer.get(delay); } }