Type safe API, significantly simplify Core, Remove Capture, Update to 3.15, Split out features into modules, Use TH to generate datatypes, Add proper support for dynamic registration #244

wz1000 · 2020-05-08T14:44:00Z

This PR is a big, breaking change that has spent a long time in the oven. I am requesting reviews while I work to patch upstream packages.

The main motivation behind this is to provide a safe API for constructing and sending LSP requests and responses. The method field of request messages is no longer divorced from the param field,
so as to statically ensure that you provide the correct payload for your messages

The methods are represented on the type level by a GADT tagged by the methods provenance(FromServer | FromClient), and its message type (Request | Notification). At the value level, it is represented by a constructor of a singleton type for the method.

data Method (p :: Provenance) (t :: MethodType) where
  Initialize  :: Method FromClient Request
  Initialized :: Method FromClient Notification
  ...
data SMethod (m :: Method p t) where
  SInitialize  :: SMethod Initialize
  SInitialized :: SMethod Initialized
  ...

These methods are mapped to parameter and response types with the help of two type families,

type family MessageParams (m :: Method p t) :: Type where
  MessageParams Initialize  = InitializeParams
  MessageParams Initialized  = Maybe InitializedParams
  ...
type family ResponseParams (m :: Method p Request) :: Type where
  ResponseParams Initialize = InitializeResponseCapabilities
  ResponseParams Shutdown   = Empty
  ...

Using this encoding, you only need to check these two places to know everything
you need about a particular method type

As these are all listed in order and grouped by provenance, correlating with the LSP spec becomes easy.

We ensure the well-formedness of responses by tagging LspId with a phantom
method type parameter, which is used in ResponseMessage to ensure that
you are responding to a request with the correct params.

Major changes in haskell-lsp-types:

Method definitions moved to Language.Haskell.LSP.Types.Method
Definition of RequestMessage, NotificationMessage and ResponseMessage updated using the MessageParams and ResponseParams type families in order to be typesafe as described above
LspId moved to Language.Haskell.LSP.Types.LspId
Type synonyms moved to new module Language.Haskell.LSP.Types.Synonyms
New module Data.IxMap that helps in dealing with method-tagged ids and response handlers in a type-safe manner

Major changes in haskell-lsp:

Deleted Capture.hs
@bubba and I came to this conclusion after discussing the state of lsp-test.
The capturing mechanism in haskell-lsp is currently lacking in quite a few ways and even misses some communication with the client.
The plan is to add a lsp-test-conduit tool to lsp-test that wraps around language servers and accurately captures
information independent of haskell-lsp
Deleted Messages.hs: it is no longer required
Significantly reworked Core.hs
- The handlerMap mechanism was simplified:
  - The Handlers type was replaced by a function J.SClientMethod m -> Maybe (Handler m)
  - hh was slightly altered in order to be generalizable for all requests
  - handlerMap now calls hh to handle all request. Any special cases/extra work to be done by haskell-lsp for this request
    is uniformly handled by generalizing the changeVFS parameter that hh previously accepted.
- The sendFunc in LspFuncs was changed to be the typesafe ServerMessageFunc
  ServerMessageFunc is SomeServerMessageWithResponse -> IO ()
  SomeServerMessageWithResponse is a type safe way to specify both a message to
  be sent from the server to the client as well as forcing you to specify
  a handler/callback for the eventual response
- In order send responses to client messages, we include a
  ClientResponseHanlder callback with every message that you can call with a
  (well-typed) reponse to the message

Suggested reading order:

Language/Haskell/LSP/Types/Method.hs
Language/Haskell/LSP/Types/Message.hs
Language/Haskell/LSP/Types/LspId.hs

These should give you a basic overview of what this patch is trying to accomplish

After this, the changes in Language/Haskell/LSP/Core.hs are pretty significant

Lastly, the following modules contain some new utility code:

Data/IxMap.hs
Language/Haskell/LSP/Types/Utils.hs

The rest of the changes are pretty trivial.

TODO:

Update tests
Rework lsp-test
Patch upstream libraries

lukel97 · 2020-05-10T13:22:18Z

Great stuff! Going to try plugging this into lsp-test to dogfood this. At the very least this will require a major major version bump (i.e. haskell-lsp-types-1.0) or putting this into alternative library a-la haskell-lsp-types-safe if this type level stuff is too much overhead for users.

lukel97 · 2020-05-10T19:58:32Z

I'm currently working on migrating the example server

lukel97 · 2020-05-12T14:33:24Z

Remove handler for initialization and provide a different callback, since haskell-lsp modifies it anyway
Is there a way we can prevent the user from responding to a request twice?

wz1000 · 2020-05-13T07:54:41Z

src/Language/Haskell/LSP/Core.hs


          -- Send initial notification
-          liftIO $ sf $ J.fromServerNot $ J.NotificationMessage "2.0" J.SProgress $
+          mkSendNotFunc tvarCtx J.SProgress $


Couldn't these notifications be moved inside the Right () -> case branch above to fix the issue?

Unfortunately it needs to return the result of running the user's callback, which would have to happen inside the response handler. I think it might be possible with the use of some async

lukel97 · 2020-05-14T16:18:26Z

Make Registration's registerOptions field dependent on the ClientMethod

commit 6aa60a4 Author: Zubin Duggal <[email protected]> Date: Fri Jun 5 15:46:30 2020 +0530 Separate out static LanguageContextData from dynamic, introduce LspM, handle initialize specially commit 0cf607f Author: Zubin Duggal <[email protected]> Date: Fri Jun 5 00:48:06 2020 +0530 Eliminate duplication of parsing logic commit cb8bd35 Author: Zubin Duggal <[email protected]> Date: Thu Jun 4 23:01:05 2020 +0530 Import LSP types unqualified in Core and other cleanup commit bb7a3f8 Author: Zubin Duggal <[email protected]> Date: Thu Jun 4 19:16:09 2020 +0530 tweaks for lsp-test commit 58609e1 Author: Luke Lau <[email protected]> Date: Thu May 14 18:21:46 2020 +0100 Fix method type in Unregistration This could probably be backported commit 3f35d04 Author: Luke Lau <[email protected]> Date: Tue May 12 18:03:58 2020 +0100 Add back erroneous deletion commit 63b80f6 Author: Luke Lau <[email protected]> Date: Tue May 12 17:58:48 2020 +0100 Update the example server commit 5514e76 Author: Luke Lau <[email protected]> Date: Tue May 12 17:27:11 2020 +0100 Tidy up some stuff in Core, add getVersionedTextDoc When wanting to create textdocumentedits, you need to pass a versioned text doc identifier. But often times the request that you are working with only gives you an unversioned text doc identifier. This is a helper function to convert it to the appropriate version by looking it up in the VFS. commit aedaa2a Merge: 56a2c9e bce18bc Author: Luke Lau <[email protected]> Date: Tue May 12 17:34:29 2020 +0100 Merge branch 'singleton-methods' of github.com:wz1000/haskell-lsp into singleton-methods commit bce18bc Author: Luke Lau <[email protected]> Date: Tue May 12 16:37:21 2020 +0100 Hide LSP ids from the users This changes the send function into two separate functions, where parameters are passed instead of raw messages. That way haskell-lsp can do the bookkeeping work of figuring out the appropriate request id, and we can get rid of MessageFuncs since the user no longer needs to construct messages from scratch. commit 2d65114 Author: Luke Lau <[email protected]> Date: Mon May 11 16:40:55 2020 +0100 Rename old sendFunc -> sendMsg commit f08b599 Author: Luke Lau <[email protected]> Date: Sun May 10 20:43:25 2020 +0100 Fix haddock Was interpreting the pipe as a haddock commit 56a2c9e Author: Luke Lau <[email protected]> Date: Mon May 11 16:40:55 2020 +0100 Rename old sendFunc -> sendMsg commit be92b48 Author: Luke Lau <[email protected]> Date: Sun May 10 20:43:25 2020 +0100 Fix haddock Was interpreting the pipe as a haddock commit cafca98 Author: Luke Lau <[email protected]> Date: Sun May 10 17:44:45 2020 +0100 Make *ResponseHandler newtypes commit a4fbbcf Author: Luke Lau <[email protected]> Date: Sun May 10 17:16:01 2020 +0100 Add explicit type parameter to LookupFunc Needed to build on ghc-8.10 Also some whitespace fixes my editor seemed to throw in automatically commit 2c32c56 Author: Zubin Duggal <[email protected]> Date: Fri May 8 19:13:10 2020 +0530 Add utility functions for parsing server and client messages commit 2d67cb5 Author: Zubin Duggal <[email protected]> Date: Fri May 8 15:46:06 2020 +0530 Rename Message -> Method, Types -> Message, extract LspId into its own module commit c51dc57 Author: Zubin Duggal <[email protected]> Date: Fri May 8 15:03:03 2020 +0530 Make ResponseMessage use Either again commit 4b9341f Author: Zubin Duggal <[email protected]> Date: Fri May 8 14:11:56 2020 +0530 Safer API for IxMap commit f2c617c Author: Zubin Duggal <[email protected]> Date: Fri May 8 13:07:43 2020 +0530 Split out IxMap into new module commit ddd8555 Author: Zubin Duggal <[email protected]> Date: Fri May 8 12:04:35 2020 +0530 move synonyms to new module commit a7ffa3e Author: Zubin Duggal <[email protected]> Date: Fri May 8 11:34:42 2020 +0530 remove capturing and misc spring cleaning commit 8e91bdb Author: Zubin Duggal <[email protected]> Date: Fri May 8 11:07:14 2020 +0530 vastly improve the TH for generating FromJSON method instances commit f6403bf Author: Zubin Duggal <[email protected]> Date: Fri May 8 02:55:51 2020 +0530 Hack together some nasty looking TH to reduce SMethod FromJSON boilerplate commit b9ac067 Author: Zubin Duggal <[email protected]> Date: Fri May 8 01:36:21 2020 +0530 Cleanup Core.hs commit 0c15553 Author: Zubin Duggal <[email protected]> Date: Thu May 7 20:33:04 2020 +0530 fix clientMethodJSON/serverMethodJSON commit 3f28fb2 Merge: c84e4e0 2ff1129 Author: Zubin Duggal <[email protected]> Date: Thu May 7 18:14:23 2020 +0530 Merge branch 'master' of https://github.com/alanz/haskell-lsp into singleton-methods commit c84e4e0 Merge: c194d51 c19ed85 Author: Zubin Duggal <[email protected]> Date: Wed Apr 29 00:00:07 2020 +0530 Merge branch 'master' of https://github.com/alanz/haskell-lsp into singleton-methods commit c194d51 Author: Zubin Duggal <[email protected]> Date: Sat Apr 25 00:05:23 2020 +0530 Redesign methods once more commit bfb3b55 Author: Zubin Duggal <[email protected]> Date: Fri Dec 20 16:23:08 2019 +0530 Fix merge commit a098073 Merge: 4cc2902 5497e9f Author: Zubin Duggal <[email protected]> Date: Tue Dec 17 17:14:40 2019 +0530 Merge branch 'master' of https://github.com/alanz/haskell-lsp into singleton-methods commit 4cc2902 Author: Zubin Duggal <[email protected]> Date: Sat Jul 13 17:01:37 2019 +0530 Dependent types, type safe responses and methods

…ties

Currently chewing through the langauge features Done completion, completion resolve, hover, signature help Moving everything in clientcapabilities/servercapabilities into the respective modules. Rewriting things to use makeExtendingDatatype. Deleting a lot of the copied + pasted in parts of the speification as most of them are out of date

See haskell#210 and haskell#211 Also rename Progress to ProgressAmount to avoid clashing with Progress method in haskell-lsp-types Also fix haskell#252, wrapping sendProgress notifications in bracket Also add example func-test for said bug

alanz · 2020-09-28T16:31:17Z

One high-level comment: this PR has

data Method (p :: Provenance) (t :: MethodType) where

It is not immediately clear what Provenance is. Perhaps consider renaming it to something obvious, like From?

sir4ur0n

Granted, this may look weird for a total stranger to review this request 😅

Actually I was about to start working on a PR to add some documentation on this repo (because when I first tried to hack on HLS a few weeks ago, I got quickly frustrated by the lack of documentation on LSP types) and before starting, I checked open PRs (to ensure this was not already worked on) and I noticed this HUGE PR, which may very well induce many conflicts with any PR I could start hacking.

So long story short, I decided to review this PR, with 0 knowledge of this repo 😁

As such, my questions/remarks are mostly about documentation and style, I hope you don't mind 🙇

Cheers and good luck with this huge PR!

sir4ur0n · 2020-09-28T20:03:38Z

hie.yaml

@@ -0,0 +1,21 @@
+# This is a sample hie.yaml file for opening haskell-language-server


Thank you for providing an hie.yaml file!

However this leads to weird errors for non-Cabal users, would you mind moving it to hie.yaml.cbl and also provide a Stack equivalent hie.yaml.stack? Here's what it should contain:

cradle: stack: - path: "./haskell-lsp-types" component: "haskell-lsp-types:lib" - path: "./src" component: "haskell-lsp:lib" - path: "./test" component: "haskell-lsp:test:haskell-lsp-test" - path: "./func-test" component: "func-test:test:func-test" - path: "./example/Reactor.hs" component: "haskell-lsp:exe:lsp-demo-reactor-server" - path: "./example/Simple.hs" component: "haskell-lsp:exe:lsp-demo-simple-server"

I copied the hie.yaml.xxx names from https://github.com/haskell/haskell-language-server

sir4ur0n · 2020-09-28T20:13:06Z

haskell-lsp-types/src/Data/IxMap.hs

+{-# LANGUAGE FlexibleContexts #-}
+{-# LANGUAGE ViewPatterns     #-}
+
+module Data.IxMap where


Maybe this module deserves some documentation, in particular IxOrd and IxMap? At least what's their purpose, ideally an example of usage?

This module should be internal to haskell-lsp and haskell-lsp-types, maybe we should move it to Language.Haskell.LSP.Types.Internal.IxMap or something

I think it might be useful if users want to make maps keyed on LspId or RegistrationId.

haskell-lsp-types/src/Language/Haskell/LSP/Types/Common.hs

sir4ur0n · 2020-09-28T20:32:39Z

haskell-lsp-types/src/Language/Haskell/LSP/Types/Common.hs

+  parseJSON Null = return (List [])
+  parseJSON v      = List <$> parseJSON v
+
+data Empty = Empty deriving (Eq,Ord,Show)


Maybe this also deserves some documentation? If I understand correctly, its main use (compared to ()) is to change the JSON representation to null rather than an empty list []

sir4ur0n · 2020-09-28T20:39:15Z

haskell-lsp-types/src/Language/Haskell/LSP/Types/Location.hs


 -- ---------------------------------------------------------------------

-{-


General question: why remove documentation and links to LSP spec? I think, if anything, that this was pretty useful (though it deserves to be present on more types)

The inline comments were out of date for the most part unfortunately. What I would like to ultimately see however is the specification documentation as haddocks on the types themselves (which we might need https://gitlab.haskell.org/ghc/ghc/-/merge_requests/3330 first!)

haskell-lsp-types/src/Language/Haskell/LSP/Types/Message.hs

sir4ur0n · 2020-09-28T21:16:39Z

src/Language/Haskell/LSP/Core.hs

+      Just (fn, write) ->
+        let revMap = case uriToFilePath (fromNormalizedUri uri) of
+              Just uri_fp -> Map.insert fn uri_fp $ reverseMap vfs
+              -- TODO: Does the VFS make sense for URIs which are not files?


Is this a question for maintainers? Or just a reminder to solve this before merging?

Co-authored-by: Julien Debon <[email protected]>

wz1000 requested review from alanz and lukel97 May 8, 2020 14:44

wz1000 marked this pull request as draft May 8, 2020 15:04

wz1000 commented May 13, 2020

View reviewed changes

wz1000 force-pushed the singleton-methods branch from 6aa60a4 to fdf3a2f Compare August 4, 2020 12:19

lukel97 changed the title ~~Type safe API, significantly simplify Core, Remove Capture~~ Type safe API, significantly simplify Core, Remove Capture, Update to 3.15, Split out features into modules, Use TH to generate datatypes Aug 13, 2020

lukel97 force-pushed the singleton-methods branch from 1259b4a to 30044cb Compare August 18, 2020 19:20

lukel97 force-pushed the singleton-methods branch from 6850027 to 886c45c Compare August 27, 2020 14:03

wz1000 mentioned this pull request Aug 27, 2020

Add runWith for transport channels other than stdio #251

Merged

lukel97 mentioned this pull request Aug 28, 2020

Remove configuration aspect from onInitialConfiguration? #253

Closed

wz1000 marked this pull request as ready for review September 2, 2020 19:29

temyurchenko mentioned this pull request Sep 28, 2020

add support for Selection Range requests #256

Closed

wz1000 and others added 13 commits September 28, 2020 15:48

Reworking options and server capabilities WIP

bf4c2c9

Start makeExtendingDatatype and update Core.hs for new ServerCapabili…

aba7281

…ties

Implement go to declaration

04bcc0e

Update definition request

52fd481

Update typeDefinition request

dec10f7

Update implementation request

4abdede

Brush up references request

076842f

Update documentHighlight

1fa605c

Update documentSymbol

f26f464

Remove redundant TH $() in splices

065a307

Update codeAction

01a04fc

lukel97 and others added 5 commits September 28, 2020 15:56

Fix CompletionParams context

699faa1

Remove onInitialConfiguration

bfdc5bb

See haskell#210 and haskell#211 Also rename Progress to ProgressAmount to avoid clashing with Progress method in haskell-lsp-types Also fix haskell#252, wrapping sendProgress notifications in bracket Also add example func-test for said bug

Make WorksapceFolderSpec a func test

fbdc7ba

fix build

d70912d

Update stack files

8e6fbb7

lukel97 force-pushed the singleton-methods branch from 5011bc9 to 8e6fbb7 Compare September 28, 2020 15:01

wz1000 added 2 commits September 28, 2020 22:56

minor fixes

3e460c7

make Handlers a DMap

b393180

sir4ur0n reviewed Sep 28, 2020

View reviewed changes

wz1000 and others added 10 commits September 29, 2020 13:42

use unliftio instead of monad-control

1f49334

ord for SomeLspId

5b70230

add toEither

47b376c

Update haskell-lsp-types/src/Language/Haskell/LSP/Types/Common.hs

a29e68f

Co-authored-by: Julien Debon <[email protected]>

simplify pattern matching in Messages.hs

076f2f7

introduce MonadLsp

343998e

tweaks to types

37a7f0d

Rename Provenance to From

bb63d61

Prevent crash when log file is misconfigured

4879365

Merge branch 'master' into singleton-methods

d591f3d

lukel97 approved these changes Oct 9, 2020

View reviewed changes

lukel97 merged commit 9dc38a3 into haskell:master Oct 9, 2020

lukel97 mentioned this pull request Oct 9, 2020

Remove onInitialConfiguration from InitializeCallbacks #211

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Type safe API, significantly simplify Core, Remove Capture, Update to 3.15, Split out features into modules, Use TH to generate datatypes, Add proper support for dynamic registration #244

Type safe API, significantly simplify Core, Remove Capture, Update to 3.15, Split out features into modules, Use TH to generate datatypes, Add proper support for dynamic registration #244

wz1000 commented May 8, 2020 •

edited

Loading

lukel97 commented May 10, 2020

lukel97 commented May 10, 2020

lukel97 commented May 12, 2020 •

edited

Loading

wz1000 May 13, 2020

lukel97 May 13, 2020

lukel97 commented May 14, 2020 •

edited

Loading

alanz commented Sep 28, 2020

sir4ur0n left a comment

sir4ur0n Sep 28, 2020

sir4ur0n Sep 28, 2020

lukel97 Oct 2, 2020

wz1000 Oct 2, 2020

sir4ur0n Sep 28, 2020

sir4ur0n Sep 28, 2020

lukel97 Oct 2, 2020

sir4ur0n Sep 28, 2020

		@@ -0,0 +1,21 @@
		# This is a sample hie.yaml file for opening haskell-language-server


		-- ---------------------------------------------------------------------

		{-

Type safe API, significantly simplify Core, Remove Capture, Update to 3.15, Split out features into modules, Use TH to generate datatypes, Add proper support for dynamic registration #244

Type safe API, significantly simplify Core, Remove Capture, Update to 3.15, Split out features into modules, Use TH to generate datatypes, Add proper support for dynamic registration #244

Conversation

wz1000 commented May 8, 2020 • edited Loading

lukel97 commented May 10, 2020

lukel97 commented May 10, 2020

lukel97 commented May 12, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukel97 commented May 14, 2020 • edited Loading

alanz commented Sep 28, 2020

sir4ur0n 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

Choose a reason for hiding this comment

wz1000 commented May 8, 2020 •

edited

Loading

lukel97 commented May 12, 2020 •

edited

Loading

lukel97 commented May 14, 2020 •

edited

Loading