LSP server development #14

koliyo · 2023-10-27T12:08:31Z

Support for serverside LSP development:

Split library in three parts

LanguageServerProtocol
- Common protocol code
LanguageServerProtocol-Client
- Client side functionality to communicate with existing LSP servers
LanguageServerProtocol-Server
- Functionality to build LSP server backends

Renaming of `Server` concept/protocol

Calling the clientside connection to the server makes sense in the client context, but makes things a bit confusing when working from the other side.

I have attempted to find a terminology that works well from both sides:

What was previously called Server, is now ServerConnection. Where a ServerConnection contains:

Event sequence from the server
Methods for sending notifications and requests to the server

And with matching ClientConnection protocol on the server side.

Serverside LSP developent

In addition to the ClientConnection protocol, the serverside library has additional support that can be reused for specific LSP implementations.

The same pattern should probably be implemented in the client side, but first lets make sure we can agree on a design that seems reasonable and support both sides of the LSP protocol.

ClientConnection
- Event sequence from the client
- Methods for sending notifications and requests to the client
JSONRPCClientConnection
- JSONRPC implementation for the ClientConnection protocol
RequestHandler
- Protocol for all LSP request methods
- Has default implementation that return not-implemented errors to client
  - We can not check this statically since this depends on registered server capabilities, but this way the server does not need to implement the full protocol for parts it does not need to use
- Pass request id to all request handlers
  - This is needed if the server want to support protocolCancelRequest
- Protocol methods use return value instead of response callback
  - This allows static compilation checking that the request handlers actually return a result to the caller
- Has a default request dispatcher to send request to proper method
NotificationHandler
- Protocol for all LSP notification methods
- Very similar to request handler, but notifications does not return results, and can not be cancelled
ErrorHandler
- Where internal errors are sent
EventHandler
- Convenience protocol which combines: RequestHandler, NotificationHandler, ErrorHandler
EventDispatcher
- Consumes client event stream and dispatch events to registered handlers

…development

koliyo · 2023-10-27T12:17:38Z

NOTE: This PR depends on #13 so that should be merged first. Or could potentially be merged as part of this if that makes it easier.

koliyo · 2023-10-27T12:22:54Z

In the Hylo LSP I have separated NotificationHandler and RequestHandler, but since the protocols compose it is also possible to implementing them in the same actor, which is more similar to the current ServerConnection (previously Server).

I to however think it would be best if the client side had similar setup as the server side.

…development

mattmassicotte · 2023-10-27T17:25:11Z

Ok #13 is now merged! I'm going to start looking at this more closely.

mattmassicotte · 2023-10-27T17:42:11Z

Sources/LanguageServerProtocol-Server/Server.ProtocolHandler.swift

+import Logging
+
+public protocol ProtocolHandler {
+  var connection: JSONRPCClientConnection { get }


This protocol gives me pause. A synchronous getter can be very problematic for an actor. I haven't yet looked closely enough to be sure, but it seems like it may be possible to make this { get async }.

This has been refactored and the protocol is not there anymore, so not sure if it is still something that needs fixing in the updated changeset. This is a bit beyond my swift knowledge atm!

mattmassicotte · 2023-10-27T17:44:03Z

Sources/LanguageServerProtocol-Server/Server.NotificationHandler.swift

+import Foundation
+import JSONRPC
+import LanguageServerProtocol
+import Logging


Is Logging still being used?

Logging usage has been removed

mattmassicotte · 2023-10-27T17:49:59Z

I explicitly do not squash to try to avoid conflict here, but it still happened...

mattmassicotte

I think this is pretty stylistic, but it is uncommon (though not unheard of) to have module names with "-" in them. But, I think it's ok to just go with "LSPClient" and "LSPServer". I usually like to avoid acronyms, but in this case its already getting long and I don't think anyone will be confused.

into feature/lsp-server-development

koliyo · 2023-10-28T12:44:46Z

Sounds good with rename. And I have removed Logger and ProtocolHandler.

Also, regarding client side, not sure it needs to follow the same pattern as the server protocols, client development will typically use LanguageClient and work on that abstraction level anyway.

But also, the splitting of this repo into server/client parts makes the separation between LSPClient and LanguageClient a bit more ambigious. Perhaps LanguageClient should just be moved into LSPClient, with preserved code? Not something that needs to be handled in this PR, just something to think about.

mattmassicotte · 2023-11-01T12:45:46Z

At first, when I read your comment I thought "yes! why is there a LanguageClient package at all?" But, exploring that idea quickly led to a very good reason: the LanguageClient package has 6 dependencies. I agree that this is an awkward arrangement. I'm just not sure how to best balance these trade-offs. But I agree that whatever change, if any, we decide to do, can be done separately from this work.

I'm going to give this another closer look this week.

koliyo · 2023-11-01T15:15:16Z

Ah, yes, additional dependencies makes sense as reason to keep it separate 👍

…oliyo/LanguageServerProtocol into feature/lsp-server-development

All requests need to respond, otherwise it is a notification, not a request

mattmassicotte

I found some very minor stuff.

I was worried, at first, about the introduction of the handler protocols. But, I think that was largely overblown from having such a hard time doing similar (but not identical) work when adopting swift concurrency.

But after looking more closely, I think this is going to be just fine. Thanks so much for all your hard work here. I really appreciate it.

mattmassicotte · 2023-11-06T18:17:20Z

Sources/LanguageServerProtocol/LanguageFeatures/LinkedEditingRange.swift

-	public init(ranges: [LSPRange], wordPattern: String? = nil) {
-		self.ranges = ranges
-		self.wordPattern = wordPattern
-	}


Removed on purpose?

Merge issue, restored now

mattmassicotte · 2023-11-06T18:18:16Z

Sources/LanguageServerProtocol/LanguageFeatures/Moniker.swift

@@ -2,7 +2,7 @@ import Foundation

 public typealias MonikerClientCapabilities = DynamicRegistrationClientCapabilities

-public struct MonikerParams: Codable, Hashable, Sendable {
+public struct MonkierParams: Codable, Hashable, Sendable {


mattmassicotte · 2023-11-06T18:19:33Z

Sources/LanguageServerProtocol/LanguageServerProtocol.swift


 public enum ClientNotification: Sendable, Hashable {
    public enum Method: String, Hashable, Sendable {
        case initialized
        case exit
 		case windowWorkDoneProgressCancel = "window/workDoneProgress/cancel"
-        case workspaceDidChangeWatchedFiles = "workspace/didChangeWatchedFiles"
+        case workspaceDidChangeWatchedFiles = "workspace/workspaceDidChangeWatchedFiles"


copy-paste error I bet

mattmassicotte · 2023-11-06T18:27:25Z

Sources/LSPClient/Client.JSONRPCServerConnection.swift

-enum ServerError: Error {
-	case unrecognizedMethod(String)
-	case missingParams
-	case unhandledRegisterationMethod(String)


typo "unhandledRegisterationMethod" -> unhandleRegistrationMethod"

koliyo · 2023-11-17T12:40:37Z

I've addressed the comments, let me know if I missed anything, or if you have additional comments!

mattmassicotte · 2023-11-17T18:06:50Z

I suspect we may still end up making some changes here and there. But, I think we're ready enough to make progress. Let's do it!

Thank you so much for all of your hard work and patience.

koliyo added 4 commits October 27, 2023 12:22

Merge branch 'feature/consistent-lsp-naming' into feature/lsp-server-…

8771e01

…development

Add server-side LSP development support

aeaabc4

Clean up ProtocolHandler

452d90d

Replace print with usage of error stream

81d9666

koliyo mentioned this pull request Oct 27, 2023

Serverside LSP development #8

Closed

Fix workflow

d7c206e

Merge branch 'feature/consistent-lsp-naming' into feature/lsp-server-…

e12604f

…development

mattmassicotte reviewed Oct 27, 2023

View reviewed changes

koliyo added 4 commits October 28, 2023 09:56

Merge branch 'main' of https://github.com/ChimeHQ/LanguageServerProtocol

869fd56

into feature/lsp-server-development

Remove logging dependency

83cbf9e

Remove ProtocolHandler

b27cd12

Rename to LSPClient/LSPServer

9da92f3

koliyo added 3 commits November 3, 2023 14:46

Merge branch 'feature/lsp-server-development' of https://github.com/k…

4d8f663

…oliyo/LanguageServerProtocol into feature/lsp-server-development

Remove unused task

c4954f7

Send empty result for requests without response

57d5c87

All requests need to respond, otherwise it is a notification, not a request

mattmassicotte reviewed Nov 6, 2023

View reviewed changes

JCWasmx86 mentioned this pull request Nov 7, 2023

[Dec 2023] Port to ChimeHQ infrastructure JCWasmx86/mesonlsp#29

Closed

koliyo added 4 commits November 17, 2023 13:30

Restore initializer

8dd776b

Fix spelling

bdcb751

Fix copy-paste error

5d4710a

Fix spelling

a137809

mattmassicotte merged commit cddb5bd into ChimeHQ:main Nov 17, 2023
3 checks passed

koliyo mentioned this pull request Dec 8, 2023

Hylo LSP proof-of-concept hylo-lang/hylo#1010

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LSP server development #14

LSP server development #14

koliyo commented Oct 27, 2023 •

edited

koliyo commented Oct 27, 2023

koliyo commented Oct 27, 2023

mattmassicotte commented Oct 27, 2023

mattmassicotte Oct 27, 2023

koliyo Nov 17, 2023

mattmassicotte Oct 27, 2023

koliyo Nov 17, 2023

mattmassicotte commented Oct 27, 2023

mattmassicotte left a comment

koliyo commented Oct 28, 2023 •

edited

mattmassicotte commented Nov 1, 2023

koliyo commented Nov 1, 2023

mattmassicotte left a comment

mattmassicotte Nov 6, 2023

koliyo Nov 17, 2023

mattmassicotte Nov 6, 2023

koliyo Nov 17, 2023

mattmassicotte Nov 6, 2023

koliyo Nov 17, 2023

mattmassicotte Nov 6, 2023

koliyo commented Nov 17, 2023

mattmassicotte commented Nov 17, 2023

LSP server development #14

LSP server development #14

Conversation

koliyo commented Oct 27, 2023 • edited

Split library in three parts

Renaming of Server concept/protocol

Serverside LSP developent

koliyo commented Oct 27, 2023

koliyo commented Oct 27, 2023

mattmassicotte commented Oct 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattmassicotte commented Oct 27, 2023

mattmassicotte left a comment

Choose a reason for hiding this comment

koliyo commented Oct 28, 2023 • edited

mattmassicotte commented Nov 1, 2023

koliyo commented Nov 1, 2023

mattmassicotte left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koliyo commented Nov 17, 2023

mattmassicotte commented Nov 17, 2023

koliyo commented Oct 27, 2023 •

edited

Renaming of `Server` concept/protocol

koliyo commented Oct 28, 2023 •

edited