Parametric completion #706

PieterOlivier · 2025-08-22T09:28:35Z

Design & implementation of completion support for the parametric language server.

This PR also includes support for dynamically registering capabilities.

…e completion protocol

toinehartman

I am excited to see this in action! Since we already discussed this design together, my comments mostly consider documentation and naming.

rascal-lsp/src/main/rascal/library/demo/lang/pico/LanguageServer.rsc

rascal-lsp/src/main/rascal/library/util/LanguageServer.rsc

DavyLandman

It's looking good, it's not a small API, but it's looking good.

rascal-lsp/src/main/rascal/library/util/LanguageServer.rsc

Co-authored-by: Toine Hartman <toinehartman@users.noreply.github.com>

…al-language-servers into completions/parametric

…ind.

rascal-lsp/src/main/rascal/library/demo/lang/pico/LanguageServer.rsc

…er.rsc Co-authored-by: Toine Hartman <toinehartman@users.noreply.github.com>

…al-language-servers into completions/parametric

rascal-lsp/src/main/rascal/library/util/LanguageServer.rsc

jurgenvinju · 2025-08-29T13:15:20Z

It would perhaps be good to test drive this contribution with a generic function that takes a reified grammar and produces a simplified syntax directed completion contribution function (based on error trees or normal trees). To be used as completion(syntaxDirectedCompletion(#start[Program])) for example for Pico?

That way we can see if the LSP side fits on the Rascal side with what we have in mind. Or if we still need to think more about the Rascal side.

DavyLandman · 2025-09-19T08:46:32Z

I've discussed the design choice of merging the 2 LSP enums into one ADT with @jurgenvinju and he agrees that in the end it's the cleanest solution. It's not 100% ideal, but the alternatives are worse for the end users.

He indicated he would take a final look at the rest of the API today, as he had some concerns and wanted to verify how they were addressed.

Co-authored-by: Toine Hartman <toinehartman@users.noreply.github.com>

…al-language-servers into completions/parametric

...-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/capabilities/DynamicCapabilities.java

rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/util/locations/Locations.java

toinehartman · 2025-12-17T16:53:29Z

rascal-lsp/src/test/resources/log4j2.xml

+<?xml version="1.0" encoding="UTF-8"?>
+<Configuration xmlns="https://logging.apache.org/xml/ns"
+               xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+               xsi:schemaLocation="
+                   https://logging.apache.org/xml/ns
+                   https://logging.apache.org/xml/ns/log4j-config-2.xsd">
+  <Appenders>
+    <Console name="CONSOLE">
+      <PatternLayout pattern="%d [%t] %p - %c %m%n"/>
+    </Console>
+  </Appenders>
+  <Loggers>
+    <Root level="DEBUG">
+      <AppenderRef ref="CONSOLE"/>
+    </Root>
+  </Loggers>
+</Configuration>


Currently we don't have any logging from LSP, since log4j is disabled. We do have a lot of logs coming from the Rascal monitor though.

DavyLandman

Looks a lot better, but I have some remaining concerns about the merging of capabilities.

...-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/capabilities/DynamicCapabilities.java

DavyLandman · 2025-12-18T09:02:08Z

...-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/capabilities/DynamicCapabilities.java

+    public CompletableFuture<Void> updateCapabilities(Collection<ILanguageContributions> contribs) {
+        // Copy the contributions so we know we are looking at a stable set of elements.
+        // If the contributions change, we expect our caller to call again.
+        var stableContribs = new LinkedHashSet<>(contribs);


Good protection against modifying contribs.

Is there a specific reason we're using LinkedHashSet here? and not List.copyOf for example? or ArrayList for that matter?

My rationale was that this should conceptually be a set (unique, unordered). But if we want that, we should enforce that higher up and shift responsibility to our caller.

...-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/capabilities/DynamicCapabilities.java

DavyLandman

This is a lot more readable, I have some small concerns about the timing of updates on the map, especially if there is a window where the map might run ahead of registered reality. I don't know what the trade-off is, but at least that is not documented in the code right now.

...-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/capabilities/DynamicCapabilities.java

DavyLandman · 2025-12-22T16:29:29Z

...-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/capabilities/DynamicCapabilities.java

+        // If our administration contains exactly this registration, remove it and inform the client
+        if (!currentRegistrations.remove(reg.getMethod(), reg)) {
+            return falsy.get();
+        }


I think this should only be done after the unregisterCapability has been completed succesfully?

(else this happens 3/4 statements after the .get)

See the answer here #706 (comment)

DavyLandman · 2025-12-22T16:38:38Z

...-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/capabilities/DynamicCapabilities.java

+        // If our administration contains no registration, inform the client
+        if (currentRegistrations.putIfAbsent(reg.getMethod(), reg) != null) {
+            return falsy.get();
+        }


not sure if we should do this here, or after a succesfull register.

The idea here (and similarly for unregister) is the following:

Initial assumption: the client and server have the same administration of registrations.

Atomically verify-and-update the state of the map with the new registration. If someone raced us and we lost, verification fails, no update will happen, and the next steps will be skipped.

Send registerCapability request.

If this succeeds: done.

If this fails: the capability is not registered on the client. Remove it from our map (verifying the exact value being removed again). Done.

The client and server (currentRegistrations) now have the same administration of registrations.

DavyLandman · 2025-12-22T16:39:56Z

...-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/capabilities/DynamicCapabilities.java

+                if (!b.booleanValue()) {
+                    return falsy.get();
+                }
+                return register(newRegistration);


this reads better, but what if the unregister fails, shouldn't we at least try to register? as in, what is the state we're leaving ourselves in. I don't know the right answer, but since there is no comment in the code wrt this trade-off, I'm confused about the guarantees.

Added a comment. At lest in VS Code, registration only explicitly fails when registering a capability that is not supported:
https://github.com/microsoft/vscode-languageserver-node/blob/00392bf903f6b6de92ae6f722408d41c8b921637/client/src/common/client.ts#L2187-L2199

In our case, that means programmer error, since AbstractDynamicCapability::isProvidedBy should check this.

Since we are talking about unregister specifically, this should never happen; we only unregister after a previous register succeeded (which requires the capability to be supported).

rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/parametric/ParametricTextDocumentService.java

sonarqubecloud · 2025-12-23T12:47:41Z

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
47.1% Coverage on New Code
0.1% Duplication on New Code

See analysis details on SonarQube Cloud

PieterOlivier added 4 commits August 22, 2025 11:27

First version of "completion" design

c8899ba

Completed first version of "completion for dsls" design.

392711b

Updated completion service design based on better understanding of th…

c1ab656

…e completion protocol

Fixed documentation

87000af

toinehartman requested changes Aug 26, 2025

View reviewed changes

DavyLandman reviewed Aug 26, 2025

View reviewed changes

rascal-lsp/src/main/rascal/library/util/LanguageServer.rsc Outdated Show resolved Hide resolved

rascal-lsp/src/main/rascal/library/util/LanguageServer.rsc Outdated Show resolved Hide resolved

rascal-lsp/src/main/rascal/library/util/LanguageServer.rsc Outdated Show resolved Hide resolved

PieterOlivier and others added 10 commits August 26, 2025 11:40

Update rascal-lsp/src/main/rascal/library/util/LanguageServer.rsc

92eef7b

Co-authored-by: Toine Hartman <toinehartman@users.noreply.github.com>

Update rascal-lsp/src/main/rascal/library/util/LanguageServer.rsc

11517b8

Co-authored-by: Toine Hartman <toinehartman@users.noreply.github.com>

Update rascal-lsp/src/main/rascal/library/util/LanguageServer.rsc

8dd1872

Co-authored-by: Toine Hartman <toinehartman@users.noreply.github.com>

Update rascal-lsp/src/main/rascal/library/util/LanguageServer.rsc

7ac0970

Co-authored-by: Toine Hartman <toinehartman@users.noreply.github.com>

Update rascal-lsp/src/main/rascal/library/util/LanguageServer.rsc

370169c

Co-authored-by: Toine Hartman <toinehartman@users.noreply.github.com>

Update rascal-lsp/src/main/rascal/library/util/LanguageServer.rsc

fa12957

Co-authored-by: Toine Hartman <toinehartman@users.noreply.github.com>

Update rascal-lsp/src/main/rascal/library/util/LanguageServer.rsc

9fd52b3

Co-authored-by: Toine Hartman <toinehartman@users.noreply.github.com>

Processed review comments on "completion for DSL design" PR.

90d3d71

Merge branch 'completions/parametric' of github.com:usethesource/rasc…

07916c0

…al-language-servers into completions/parametric

Documented differences between CompletionItemKind and DocumentSymbolK…

c329475

…ind.

toinehartman reviewed Aug 26, 2025

View reviewed changes

rascal-lsp/src/main/rascal/library/demo/lang/pico/LanguageServer.rsc Outdated Show resolved Hide resolved

PieterOlivier and others added 4 commits August 27, 2025 10:35

Update rascal-lsp/src/main/rascal/library/demo/lang/pico/LanguageServ…

5bd8b5d

…er.rsc Co-authored-by: Toine Hartman <toinehartman@users.noreply.github.com>

Replaced string command with Command.

75de3de

Merge branch 'completions/parametric' of github.com:usethesource/rasc…

bfac3d9

…al-language-servers into completions/parametric

Merge branch 'main' into completions/parametric

77b9990

DavyLandman reviewed Aug 28, 2025

View reviewed changes

rascal-lsp/src/main/rascal/library/util/LanguageServer.rsc Outdated Show resolved Hide resolved

PieterOlivier added 3 commits September 1, 2025 10:00

Brought completion design more in line with lsp spec

250b04e

Unified DocumentSymbolKind and CompletionItemKind again

9f16fa5

Updated Rascal completion example

f0f1468

PieterOlivier and others added 3 commits September 22, 2025 09:48

Update rascal-lsp/src/main/rascal/library/util/LanguageServer.rsc

e50b42d

Co-authored-by: Toine Hartman <toinehartman@users.noreply.github.com>

Added synopsis to new ADTs

d37a1df

Merge branch 'completions/parametric' of github.com:usethesource/rasc…

92758e2

…al-language-servers into completions/parametric

toinehartman added 6 commits December 17, 2025 11:30

Merge remote-tracking branch 'origin/main' into completions/parametric

d9f6aae

Merge definitions and fix documentation.

73faacc

Improve class docs.

52992b7

Do not mulxiplex completion contributions by trigger character.

c4fd6c0

Remove outdated comments.

2da77a4

Make sure no-one modifies our capabilities after we are created.

7f109db

toinehartman mentioned this pull request Dec 17, 2025

Always use ISourceLocation for Rascal locations #929

Merged

Use Locations library to convert ranges.

a784ae7

toinehartman approved these changes Dec 18, 2025

View reviewed changes

toinehartman added 2 commits December 18, 2025 09:30

Merge remote-tracking branch 'origin/main' into completions/parametric

f3b55da

Small fixes.

4198ebc

DavyLandman reviewed Dec 18, 2025

View reviewed changes

toinehartman and others added 9 commits December 18, 2025 10:44

Dynamic capability fixes based on review by @DavyLandman.

aecb382

Rewrite to separate request per capability.

7e28423

Reduce stream instead of list.

2e75130

Chaining with booleans.

28a734b

Ensure atomicity with parallel requests.

801deee

Merge remote-tracking branch 'origin/main' into completions/parametric

51aa42b

Fix SQ issues.

2c4c0bc

Gracefully handle failed registrations.

33b8a00

Merge branch 'main' into completions/parametric

c5f88fa

toinehartman requested a review from DavyLandman December 22, 2025 10:32

DavyLandman reviewed Dec 22, 2025

View reviewed changes

toinehartman added 5 commits December 23, 2025 13:38

Refactor streaming to not use pairs and improve reduction readability.

4bae2ee

Only construct true/false futures once and return true for no-op.

293bb86

Document registrations functions and logic.

a3cd8e6

Less strict reduction signature.

0535216

Merge remote-tracking branch 'origin/main' into completions/parametric

00ebc2b

Implement basic lock-free capability registration.

20d116c

Parametric completion #706

Are you sure you want to change the base?

Parametric completion #706

Uh oh!

Conversation

PieterOlivier commented Aug 22, 2025 • edited by toinehartman Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

toinehartman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DavyLandman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jurgenvinju commented Aug 29, 2025

Uh oh!

DavyLandman commented Sep 19, 2025

Uh oh!

Uh oh!

Uh oh!

toinehartman Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

DavyLandman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

DavyLandman Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

toinehartman Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DavyLandman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DavyLandman Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

toinehartman Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DavyLandman Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

toinehartman Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DavyLandman Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

PieterOlivier commented Aug 22, 2025 •

edited by toinehartman

Loading

toinehartman Dec 18, 2025 •

edited

Loading

toinehartman Dec 23, 2025 •

edited

Loading

toinehartman Dec 23, 2025 •

edited

Loading