Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix LSP document synchronization #544

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
8c1b469
Redirecting source locations of files managed by LSP
rodinaarssen Dec 13, 2024
ac5a860
Make sure FallbackResolver is used while debugging
rodinaarssen Dec 13, 2024
e873a4a
Made sure source locations with lsp prefix don't leak to the outside
rodinaarssen Dec 13, 2024
ba53307
Fixed offsets in lsp-redirected source locations
rodinaarssen Dec 16, 2024
f4ddf7a
Keeping track of last modified timestamp for lsp editor states
rodinaarssen Dec 16, 2024
d202427
Added copyright notice
rodinaarssen Dec 16, 2024
c2da74f
Fixed indentation
rodinaarssen Dec 16, 2024
3cfc64b
Using correct container for lookup
rodinaarssen Dec 23, 2024
66d9d8e
Added some comments
rodinaarssen Dec 23, 2024
2b37aa9
Changed exception type
rodinaarssen Dec 23, 2024
8e4ec7b
Moved to thread-safe container
rodinaarssen Dec 23, 2024
963c5c8
Propagating timestamp of receiving a didChange message to TextDocumen…
rodinaarssen Jan 6, 2025
a83ed45
Propagint an additional timestamp to TextDocumentState
rodinaarssen Jan 6, 2025
99a6688
Rewrote method to reduce map lookups and avoid a race
rodinaarssen Jan 6, 2025
4acc254
Added a comment
rodinaarssen Jan 6, 2025
06175aa
Reduced unused imports
rodinaarssen Jan 6, 2025
0413b95
Removed redundant check
rodinaarssen Jan 8, 2025
eea2ddc
Relocated a .top()
rodinaarssen Jan 8, 2025
8da696d
Removed unnecessary qualifiers
rodinaarssen Jan 8, 2025
ac1fe33
Promoted comment to Javadoc
rodinaarssen Jan 8, 2025
6e0e0a8
TextDocumentState now consistently uses provided timestamps
rodinaarssen Jan 8, 2025
eaa0653
Added IDE test
rodinaarssen Jan 9, 2025
ff52937
Merge branch 'main' into lsp-open-files
rodinaarssen Jan 9, 2025
033c56e
Removed trailing space
rodinaarssen Jan 9, 2025
230ef75
Merge branch 'main' into lsp-open-files
rodinaarssen Jan 9, 2025
3d13c37
Fixed race
rodinaarssen Jan 10, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions rascal-lsp/.vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
"console": "internalConsole",
"vmArgs": [
"-Dlog4j2.level=TRACE",
"-Drascal.compilerClasspath=${workspaceFolder}/target/lib/rascal.jar"
"-Drascal.compilerClasspath=${workspaceFolder}/target/lib/rascal.jar",
"-Drascal.fallbackResolver=org.rascalmpl.vscode.lsp.uri.FallbackResolver"
]
},
{
Expand All @@ -22,7 +23,8 @@
"console": "internalConsole",
"vmArgs": [
"-Dlog4j2.level=TRACE",
"-Drascal.compilerClasspath=${workspaceFolder}/target/lib/rascal.jar"
"-Drascal.compilerClasspath=${workspaceFolder}/target/lib/rascal.jar",
"-Drascal.fallbackResolver=org.rascalmpl.vscode.lsp.uri.FallbackResolver"
]
}
]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
org.rascalmpl.vscode.lsp.uri.LSPOpenFileResolver
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ public interface IBaseTextDocumentService extends TextDocumentService {
void unregisterLanguage(LanguageParameter lang);
CompletableFuture<IValue> executeCommand(String languageName, String command);
LineColumnOffsetMap getColumnMap(ISourceLocation file);
TextDocumentState getDocumentState(ISourceLocation file);

boolean isManagingFile(ISourceLocation file);

default void didRenameFiles(RenameFilesParams params, Set<ISourceLocation> workspaceFolders) {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import org.rascalmpl.uri.URIResolverRegistry;
import org.rascalmpl.uri.URIUtil;
import org.rascalmpl.values.IRascalValueFactory;
import org.rascalmpl.vscode.lsp.util.locations.Locations;

import io.usethesource.vallang.ISourceLocation;
import io.usethesource.vallang.IValueFactory;
Expand All @@ -65,7 +66,7 @@ default CompletableFuture<SourceLocation> resolveLocation(SourceLocation loc) {
try {
ISourceLocation tmp = loc.toRascalLocation();

ISourceLocation resolved = reg.logicalToPhysical(tmp);
ISourceLocation resolved = Locations.toClientLocation(tmp);

if (resolved == null) {
return loc;
Expand All @@ -77,10 +78,6 @@ default CompletableFuture<SourceLocation> resolveLocation(SourceLocation loc) {
IRascalFileSystemServices__logger.warn("Could not resolve location {} due to {}.", loc, e.getMessage());
return loc;
}
catch (IOException e) {
// This is normal behavior (when its not a logical scheme)
return loc;
}
catch (Throwable e) {
IRascalFileSystemServices__logger.warn("Could not resolve location {} due to {}.", loc, e.getMessage());
return loc;
Expand Down Expand Up @@ -130,7 +127,7 @@ private static boolean readonly(ISourceLocation loc) throws IOException {
return false;
}
if (reg.getRegisteredLogicalSchemes().contains(loc.getScheme())) {
var resolved = reg.logicalToPhysical(loc);
var resolved = Locations.toClientLocation(loc);
if (resolved != null && resolved != loc) {
return readonly(resolved);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public void browse(URI uri, String title, int viewColumn) {
@Override
public void edit(ISourceLocation path) {
try {
ISourceLocation physical = URIResolverRegistry.getInstance().logicalToPhysical(path);
ISourceLocation physical = Locations.toClientLocation(path);
ShowDocumentParams params = new ShowDocumentParams(physical.getURI().toASCIIString());
params.setTakeFocus(true);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ public class TextDocumentState {
@SuppressWarnings("java:S3077") // we are use volatile correctly
private volatile CompletableFuture<Versioned<ITree>> currentTree;

public TextDocumentState(BiFunction<ISourceLocation, String, CompletableFuture<ITree>> parser, ISourceLocation file, int initialVersion, String initialContent) {
public TextDocumentState(BiFunction<ISourceLocation, String, CompletableFuture<ITree>> parser, ISourceLocation file, int initialVersion, String initialContent, long timestamp) {
this.parser = parser;
this.file = file;
this.currentContent = new Versioned<>(initialVersion, initialContent);
this.currentContent = new Versioned<>(initialVersion, initialContent, timestamp);
this.currentTree = newTreeAsync(initialVersion, initialContent);
}

Expand All @@ -72,8 +72,8 @@ public TextDocumentState(BiFunction<ISourceLocation, String, CompletableFuture<I
* Thus, callers of `getCurrentTreeAsync` are guaranteed to obtain a
* consistent <version, tree> pair.
*/
public CompletableFuture<Versioned<ITree>> update(int version, String content) {
currentContent = new Versioned<>(version, content);
public CompletableFuture<Versioned<ITree>> update(int version, String content, long timestamp) {
currentContent = new Versioned<>(version, content, timestamp);
rodinaarssen marked this conversation as resolved.
Show resolved Hide resolved
var newTree = newTreeAsync(version, content);
currentTree = newTree;
return newTree;
Expand Down Expand Up @@ -105,4 +105,8 @@ public ISourceLocation getLocation() {
public Versioned<String> getCurrentContent() {
return currentContent;
}

public long getLastModified() {
return currentContent.getTimestamp();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@
import org.rascalmpl.vscode.lsp.parametric.model.ParametricSummary;
import org.rascalmpl.vscode.lsp.parametric.model.ParametricSummary.SummaryLookup;
import org.rascalmpl.vscode.lsp.terminal.ITerminalIDEServer.LanguageParameter;
import org.rascalmpl.vscode.lsp.uri.FallbackResolver;
import org.rascalmpl.vscode.lsp.util.CodeActions;
import org.rascalmpl.vscode.lsp.util.Diagnostics;
import org.rascalmpl.vscode.lsp.util.FoldingRanges;
Expand Down Expand Up @@ -149,6 +150,9 @@ public class ParametricTextDocumentService implements IBaseTextDocumentService,
private final @Nullable LanguageParameter dedicatedLanguage;

public ParametricTextDocumentService(ExecutorService exec, @Nullable LanguageParameter dedicatedLanguage) {
// The following call ensures that URIResolverRegistry is initialized before FallbackResolver is accessed
URIResolverRegistry.getInstance();

this.ownExecuter = exec;
this.files = new ConcurrentHashMap<>();
this.columns = new ColumnMaps(this::getContents);
Expand All @@ -160,6 +164,7 @@ public ParametricTextDocumentService(ExecutorService exec, @Nullable LanguagePar
this.dedicatedLanguageName = dedicatedLanguage.getName();
this.dedicatedLanguage = dedicatedLanguage;
}
FallbackResolver.getInstance().registerTextDocumentService(this);
}

@Override
Expand Down Expand Up @@ -226,15 +231,17 @@ public void connect(LanguageClient client) {

@Override
public void didOpen(DidOpenTextDocumentParams params) {
var timestamp = System.currentTimeMillis();
logger.debug("Did Open file: {}", params.getTextDocument());
handleParsingErrors(open(params.getTextDocument()));
handleParsingErrors(open(params.getTextDocument(), timestamp));
triggerAnalyzer(params.getTextDocument(), Duration.ofMillis(800));
}

@Override
public void didChange(DidChangeTextDocumentParams params) {
var timestamp = System.currentTimeMillis();
logger.debug("Did Change file: {}", params.getTextDocument().getUri());
updateContents(params.getTextDocument(), last(params.getContentChanges()).getText());
updateContents(params.getTextDocument(), last(params.getContentChanges()).getText(), timestamp);
triggerAnalyzer(params.getTextDocument(), Duration.ofMillis(800));
}

Expand Down Expand Up @@ -275,10 +282,10 @@ private void triggerBuilder(TextDocumentIdentifier doc) {
fileFacts.calculateBuilder(location, getFile(doc).getCurrentTreeAsync());
}

private TextDocumentState updateContents(VersionedTextDocumentIdentifier doc, String newContents) {
private TextDocumentState updateContents(VersionedTextDocumentIdentifier doc, String newContents, long timestamp) {
TextDocumentState file = getFile(doc);
logger.trace("New contents for {}", doc);
handleParsingErrors(file, file.update(doc.getVersion(), newContents));
handleParsingErrors(file, file.update(doc.getVersion(), newContents, timestamp));
return file;
}

Expand Down Expand Up @@ -439,9 +446,9 @@ private ParametricFileFacts facts(String doc) {
throw new UnsupportedOperationException("Rascal Parametric LSP has no support for this file: " + doc);
}

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

Expand Down Expand Up @@ -683,4 +690,14 @@ public CompletableFuture<IValue> executeCommand(String languageName, String comm
return CompletableFuture.completedFuture(null);
}
}

@Override
public boolean isManagingFile(ISourceLocation file) {
return files.containsKey(file.top());
}

@Override
public TextDocumentState getDocumentState(ISourceLocation file) {
return files.get(file.top());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@
import org.rascalmpl.library.util.PathConfig;
import org.rascalmpl.parser.gtd.exception.ParseError;
import org.rascalmpl.uri.URIResolverRegistry;
import org.rascalmpl.uri.URIUtil;
import org.rascalmpl.values.parsetrees.ITree;
import org.rascalmpl.values.parsetrees.ProductionAdapter;
import org.rascalmpl.values.parsetrees.TreeAdapter;
Expand All @@ -113,6 +114,7 @@
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.uri.FallbackResolver;
import org.rascalmpl.vscode.lsp.util.CodeActions;
import org.rascalmpl.vscode.lsp.util.Diagnostics;
import org.rascalmpl.vscode.lsp.util.DocumentChanges;
Expand Down Expand Up @@ -143,9 +145,13 @@ public class RascalTextDocumentService implements IBaseTextDocumentService, Lang
private @MonotonicNonNull BaseWorkspaceService workspaceService;

public RascalTextDocumentService(ExecutorService exec) {
// The following call ensures that URIResolverRegistry is initialized before FallbackResolver is accessed
URIResolverRegistry.getInstance();

this.ownExecuter = exec;
this.documents = new ConcurrentHashMap<>();
this.columns = new ColumnMaps(this::getContents);
FallbackResolver.getInstance().registerTextDocumentService(this);
}

@Override
Expand Down Expand Up @@ -200,15 +206,17 @@ public void connect(LanguageClient client) {

@Override
public void didOpen(DidOpenTextDocumentParams params) {
var timestamp = System.currentTimeMillis();
logger.debug("Open: {}", params.getTextDocument());
TextDocumentState file = open(params.getTextDocument());
TextDocumentState file = open(params.getTextDocument(), timestamp);
handleParsingErrors(file);
}

@Override
public void didChange(DidChangeTextDocumentParams params) {
var timestamp = System.currentTimeMillis();
logger.trace("Change: {}", params.getTextDocument());
updateContents(params.getTextDocument(), last(params.getContentChanges()).getText());
updateContents(params.getTextDocument(), last(params.getContentChanges()).getText(), timestamp);
}

@Override
Expand All @@ -230,10 +238,10 @@ public void didSave(DidSaveTextDocumentParams params) {
}
}

private TextDocumentState updateContents(VersionedTextDocumentIdentifier doc, String newContents) {
private TextDocumentState updateContents(VersionedTextDocumentIdentifier doc, String newContents, long timestamp) {
TextDocumentState file = getFile(doc);
logger.trace("New contents for {}", doc);
handleParsingErrors(file, file.update(doc.getVersion(), newContents));
handleParsingErrors(file, file.update(doc.getVersion(), newContents, timestamp));
return file;
}

Expand Down Expand Up @@ -419,9 +427,9 @@ private static <T> T last(List<T> l) {
return l.get(l.size() - 1);
}

private TextDocumentState open(TextDocumentItem doc) {
private TextDocumentState open(TextDocumentItem doc, long timestamp) {
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, doc.getVersion(), doc.getText(), timestamp));
}

private TextDocumentState getFile(TextDocumentIdentifier doc) {
Expand Down Expand Up @@ -567,6 +575,16 @@ private static <T> CompletableFuture<T> recoverExceptions(CompletableFuture<T> f
});
}

@Override
public boolean isManagingFile(ISourceLocation file) {
return documents.containsKey(file.top());
}

@Override
public TextDocumentState getDocumentState(ISourceLocation file) {
return documents.get(file.top());
}

public @MonotonicNonNull FileFacts getFileFacts() {
return facts;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@
import org.rascalmpl.vscode.lsp.util.concurrent.LazyUpdateableReference;
import org.rascalmpl.vscode.lsp.util.concurrent.ReplaceableFuture;
import org.rascalmpl.vscode.lsp.util.locations.ColumnMaps;
import org.rascalmpl.vscode.lsp.util.locations.Locations;

import io.usethesource.vallang.ISourceLocation;

public class FileFacts {
Expand Down Expand Up @@ -87,7 +89,7 @@ public void reportParseErrors(ISourceLocation file, List<Diagnostic> msgs) {
private FileFact getFile(ISourceLocation l) {
ISourceLocation resolved = null;
try {
resolved = URIResolverRegistry.getInstance().logicalToPhysical(l);
resolved = Locations.toClientLocation(l);
if (resolved == null) {
resolved = l;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public void browse(URI uri, String title, int viewColumn) {
@Override
public void edit(ISourceLocation path) {
try {
ISourceLocation physical = URIResolverRegistry.getInstance().logicalToPhysical(path);
ISourceLocation physical = Locations.toClientLocation(path);
ShowDocumentParams params = new ShowDocumentParams(physical.getURI().toASCIIString());
params.setTakeFocus(true);

Expand Down
Loading
Loading