Skip to content

Clean up unused LSPIDEServices class #1038

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.

Sign up for GitHub

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

Closed
toinehartman wants to merge 4 commits into from
Closed

Clean up unused LSPIDEServices class #1038

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
612210c
Extract monitor from otherwise unused `LSPIDEServices`
toinehartman Mar 30, 2026
ca2e716
Remove unused `LSPIDEServices`.
toinehartman Mar 30, 2026
1b4686f
Remove unused arguments and getters.
toinehartman Mar 30, 2026
26ce5f6
Merge remote-tracking branch 'origin/main' into fix/uncouple-and-cleanup
toinehartman Mar 30, 2026
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
182 changes: 0 additions & 182 deletions rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/LSPIDEServices.java
View file
Open in desktop

This file was deleted.

8 changes: 2 additions & 6 deletions ...p/src/main/java/org/rascalmpl/vscode/lsp/parametric/InterpretedLanguageContributions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,12 @@
import org.rascalmpl.values.functions.IFunction;
import org.rascalmpl.values.parsetrees.ITree;
import org.rascalmpl.values.parsetrees.TreeAdapter;
import org.rascalmpl.vscode.lsp.BaseWorkspaceService;
import org.rascalmpl.vscode.lsp.IBaseLanguageClient;
import org.rascalmpl.vscode.lsp.IBaseTextDocumentService;
import org.rascalmpl.vscode.lsp.RascalLSPMonitor;
import org.rascalmpl.vscode.lsp.parametric.LanguageRegistry.LanguageParameter;
import org.rascalmpl.vscode.lsp.parametric.model.RascalADTs.LanguageContributions;
import org.rascalmpl.vscode.lsp.rascal.conversion.KeywordParameter;
import org.rascalmpl.vscode.lsp.util.EvaluatorUtil;
import org.rascalmpl.vscode.lsp.util.EvaluatorUtil.LSPContext;
import org.rascalmpl.vscode.lsp.util.concurrent.InterruptibleFuture;

import io.usethesource.vallang.IBool;
Expand Down Expand Up @@ -128,7 +125,7 @@ public class InterpretedLanguageContributions implements ILanguageContributions
private final IBaseLanguageClient client;
private final RascalLSPMonitor monitor;

public InterpretedLanguageContributions(LanguageParameter lang, IBaseTextDocumentService docService, BaseWorkspaceService workspaceService, IBaseLanguageClient client, ExecutorService exec) {
public InterpretedLanguageContributions(LanguageParameter lang, IBaseLanguageClient client, ExecutorService exec) {
this.client = client;
this.name = lang.getName();
this.mainModule = lang.getMainModule();
Expand All @@ -140,8 +137,7 @@ public InterpretedLanguageContributions(LanguageParameter lang, IBaseTextDocumen

monitor = new RascalLSPMonitor(client, LogManager.getLogger(logger.getName() + "[" + lang.getName() + "]"), lang.getName() + ": ");

this.eval = EvaluatorUtil.makeFutureEvaluator(new LSPContext(exec, docService, workspaceService, client),
"evaluator for " + lang.getName(), monitor, pcfg, lang.getMainModule());
this.eval = EvaluatorUtil.makeFutureEvaluator("evaluator for " + lang.getName(), monitor, pcfg, exec, lang.getMainModule());
var contributions = EvaluatorUtil.runEvaluator(name + ": loading contributions", eval,
e -> loadContributions(e, lang),
ValueFactoryFactory.getValueFactory().set(),
Expand Down
9 changes: 1 addition & 8 deletions ...-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParametricTextDocumentService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,13 +294,6 @@ private String getRascalMetaCommandName() {
return BaseWorkspaceService.RASCAL_META_COMMAND;
}

private BaseWorkspaceService availableWorkspaceService() {
if (workspaceService == null) {
throw new IllegalStateException("Workspace Service has not been paired");
}
return workspaceService;
}

@Override
public void pair(BaseWorkspaceService workspaceService) {
this.workspaceService = workspaceService;
Expand Down Expand Up @@ -998,7 +991,7 @@ public synchronized void registerLanguage(LanguageParameter lang) {

var clientCopy = availableClient();
multiplexer.addContributor(buildContributionKey(lang),
new InterpretedLanguageContributions(lang, this, availableWorkspaceService(), (IBaseLanguageClient)clientCopy, exec));
new InterpretedLanguageContributions(lang, (IBaseLanguageClient)clientCopy, exec));

fact.reloadContributions();
fact.setClient(clientCopy);
Expand Down
9 changes: 3 additions & 6 deletions rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalLanguageServices.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@
import org.rascalmpl.vscode.lsp.IBaseLanguageClient;
import org.rascalmpl.vscode.lsp.RascalLSPMonitor;
import org.rascalmpl.vscode.lsp.util.EvaluatorUtil;
import org.rascalmpl.vscode.lsp.util.EvaluatorUtil.LSPContext;
import org.rascalmpl.vscode.lsp.util.RascalServices;
import org.rascalmpl.vscode.lsp.util.concurrent.InterruptibleFuture;
import org.rascalmpl.vscode.lsp.util.locations.Locations;
Expand Down Expand Up @@ -120,11 +119,9 @@
var pcfg = EvaluatorUtil.addLSPSources(new PathConfig(URIUtil.rootLocation("cwd")), true);
var compilerPcfg = EvaluatorUtil.addRascalCompilerSources(pcfg);

var context = new LSPContext(exec, docService, workspaceService, client);

shortRunningTaskEvaluator = makeFutureEvaluator(context, "Rascal tasks", monitor, pcfg, "lang::rascal::lsp::DocumentSymbols", "lang::rascal::lsp::Templates");
semanticEvaluator = makeFutureEvaluator(context, "Rascal semantics", monitor, compilerPcfg, "lang::rascalcore::check::Summary", "lang::rascal::lsp::refactor::Rename", "lang::rascal::lsp::Actions");
compilerEvaluator = makeFutureEvaluator(context, "Rascal compiler", monitor, compilerPcfg, "lang::rascal::lsp::IDECheckerWrapper");
shortRunningTaskEvaluator = makeFutureEvaluator("Rascal tasks", monitor, pcfg, exec, "lang::rascal::lsp::DocumentSymbols", "lang::rascal::lsp::Templates");
semanticEvaluator = makeFutureEvaluator("Rascal semantics", monitor, compilerPcfg, exec, "lang::rascalcore::check::Summary", "lang::rascal::lsp::refactor::Rename", "lang::rascal::lsp::Actions");

Check failure on line 123 in rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalLanguageServices.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Define a constant instead of duplicating this literal "lang::rascal::lsp::Actions" 3 times. 


See more on https://sonarcloud.io/project/issues?id=usethesource_rascal-language-servers&issues=AZ0_XzgS5bprnWAwcTx7&open=AZ0_XzgS5bprnWAwcTx7&pullRequest=1038
compilerEvaluator = makeFutureEvaluator("Rascal compiler", monitor, compilerPcfg, exec, "lang::rascal::lsp::IDECheckerWrapper");
actionStore = semanticEvaluator.thenApply(e -> ((ModuleEnvironment) e.getModule("lang::rascal::lsp::Actions")).getStore());
rascalTextDocumentService = docService;
this.workspaceService = workspaceService;
Expand Down
37 changes: 7 additions & 30 deletions rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/EvaluatorUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@
import org.eclipse.lsp4j.services.LanguageClient;
import org.rascalmpl.debug.IRascalMonitor;
import org.rascalmpl.exceptions.Throw;
import org.rascalmpl.ideservices.IDEServices;
import org.rascalmpl.interpreter.Evaluator;
import org.rascalmpl.interpreter.control_exceptions.InterruptException;
import org.rascalmpl.interpreter.staticErrors.StaticError;
Expand All @@ -75,13 +74,10 @@
import org.rascalmpl.shell.ShellEvaluatorFactory;
import org.rascalmpl.uri.URIUtil;
import org.rascalmpl.uri.jar.JarURIResolver;
import org.rascalmpl.vscode.lsp.BaseWorkspaceService;
import org.rascalmpl.vscode.lsp.IBaseLanguageClient;
import org.rascalmpl.vscode.lsp.IBaseTextDocumentService;
import org.rascalmpl.vscode.lsp.LSPIDEServices;
import org.rascalmpl.vscode.lsp.RascalLSPMonitor;
import org.rascalmpl.vscode.lsp.rascal.RascalLanguageServer;
import org.rascalmpl.vscode.lsp.util.concurrent.InterruptibleFuture;

import io.usethesource.vallang.IConstructor;
import io.usethesource.vallang.IString;
import io.usethesource.vallang.IValue;
Expand All @@ -104,9 +100,6 @@ public class EvaluatorUtil {
}
var monitor = actualEval.getMonitor();
// unwrap until we find the RascalLSPMonitor doing the heavy lifting
if (monitor instanceof LSPIDEServices) {
monitor = ((LSPIDEServices) monitor).getMonitor();
}
if (monitor instanceof RascalLSPMonitor) {
((RascalLSPMonitor) monitor).registerActiveFuture(task, self);
}
Expand Down Expand Up @@ -356,52 +349,36 @@ public static PathConfig addRascalCompilerSources(PathConfig pcfg) {
}
}

public static class LSPContext {
private final ExecutorService exec;
private final IBaseTextDocumentService docService;
private final BaseWorkspaceService workspaceService;
private final IBaseLanguageClient client;

public LSPContext(ExecutorService exec, IBaseTextDocumentService docService,
BaseWorkspaceService workspaceService, IBaseLanguageClient client) {
this.exec = exec;
this.docService = docService;
this.workspaceService = workspaceService;
this.client = client;
}
}

/**
* This function is used to construct evaluators used by LSP servers, not the terminal REPL
*/
public static CompletableFuture<Evaluator> makeFutureEvaluator(LSPContext context, String label, IRascalMonitor monitor, PathConfig pcfg, final String... imports) {
public static CompletableFuture<Evaluator> makeFutureEvaluator(String label, IRascalMonitor monitor, PathConfig pcfg, ExecutorService exec, final String... imports) {
return CompletableFuture.supplyAsync(() -> {
Logger customLog = LogManager.getLogger("Evaluator: " + label);
IDEServices services = new LSPIDEServices(context.client, context.docService, context.workspaceService, monitor);
boolean jobSuccess = false;
String jobName = "Loading " + label;
try {
services.jobStart(jobName, imports.length);
monitor.jobStart(jobName, imports.length);
var eval = ShellEvaluatorFactory.getDefaultEvaluatorForPathConfig(
pcfg.getProjectRoot(),
pcfg,
Reader.nullReader(),
logWriter(customLog, Level.INFO),
logWriter(customLog, Level.ERROR),
services
Copy link
Copy Markdown
Member

@DavyLandman DavyLandman Mar 30, 2026

Choose a reason for hiding this comment

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

I'm unsure if this is unused.

What if a DSL does edit from a VS Code action, how will that get wired through to VS Code?

I think the bird IDE contains an example like that.

It might also mean we have to grow our test set before we start working on #1010

Copy link
Copy Markdown
Member Author

@toinehartman toinehartman Mar 30, 2026

Choose a reason for hiding this comment

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

All the places where services have been replaced with monitor expected an IRascalMonitor, not a more specialized type like the removed LSPIDEServices. All of the IDE services have been moved in #856 IIUC. :edit is here:

rascal-language-servers/rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/terminal/RemoteIDEServicesServer.java

Line 74 in 26ce5f6

public CompletableFuture<Void> edit(ISourceLocation loc, int viewColumn) {

Copy link
Copy Markdown
Member Author

@toinehartman toinehartman Mar 30, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Extending the test coverage before rewiring all of this is definitely a good idea.

Copy link
Copy Markdown
Member Author

@toinehartman toinehartman Mar 31, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Btw, we already have a test for :edit:

rascal-language-servers/rascal-vscode-extension/src/test/vscode-suite/repl.test.ts

Lines 92 to 98 in 97f83de

it("edit call module via repl", async() => {
const repl = new RascalREPL(bench, driver);
await repl.start();
await repl.execute(":edit demo::lang::pico::LanguageServer", true, Delays.extremelySlow);
await driver.wait(async () => await (await bench.getEditorView().getActiveTab())?.getTitle() === "LanguageServer.rsc", Delays.slow, "LanguageServer should be opened");
});

Copy link
Copy Markdown
Member

@DavyLandman DavyLandman Mar 31, 2026

Choose a reason for hiding this comment

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

My concern is about the util::IDEServices::edit function. For which the evaluator casts the monitor to an IDEServices interface if it implements it.

So imagine a VS Code action that has as a side effect edit or perhaps applyDocumentsEdits. This can be both from a rascal evaluator, or a dsl (aka parametric) evaluator.

Copy link
Copy Markdown
Member Author

@toinehartman toinehartman Mar 31, 2026

Choose a reason for hiding this comment

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

I think this log from the failing test answers your question:

Warning: 1 07:42:29.631 [warning] [parametric-40] org.rascalmpl.vscode.lsp.parametric.InterpretedLanguageContributions[Pico] |unknown:///| : Could not execute FileSystemChange due to File system error: ENOPRO: No file system provider found for resource 'lsp
+file:/' data: "Unavailable" (@1774942949631 ms)

I'll investigate with @rodinaarssen whether we missed something in #856 or how we can solve this otherwise.

monitor
);

eval.addClassLoader(RascalLanguageServer.class.getClassLoader());

eval.doImport(services, imports);
eval.doImport(monitor, imports);

jobSuccess = true;
return eval;
}
finally {
services.jobEnd(jobName, jobSuccess);
monitor.jobEnd(jobName, jobSuccess);
}
}, context.exec);
}, exec);
}

private static PrintWriter logWriter(Logger customLog, Level level) {
Expand Down
Loading