tricktron / frege-lsp-server

Frege Language Server Using the Language Server Protocol
MIT License
1 stars 0 forks source link

Replace StateT with Reader #52

Closed tricktron closed 2 years ago

tricktron commented 2 years ago

Fixes #49.

The pure core only reads the Global state. As a result, I don't need the State monad and can replace it with the Reader monad to clearly reflect this requirement in the type system as well.

This provides an interesting design space with the following three options:

No Transformers, just Thread/Passthrough the Global argument:

getTypeOnHover :: Global  -> Position -> Maybe Hover
getTypeOnHover gl pos = do
    token      <- findToken gl pos
    qname      <- tokenToQName gl token
    symbol     <- findSymbol gl qname
    symbolType  = getSymbolType gl symbol
    pure        $ Hover 
        { 
            range   = tokenToRange token, 
            content = FregeCodeBlock symbolType
        }

MaybeT Reader Global a

The code can be found on the f-readerT branch.

In order to combine it with the Maybe Monad, I opted for the MaybeT transformer monad. The liftMaybe function is responsible to lift Maybe values into the MaybeT monad and allows to combine the various search functions, which return Maybe values:

https://github.com/tricktron/frege-lsp-server/blob/a7241931652560324f8d8a4d585f3219081ebe10/src/main/frege/ch/fhnw/thga/fregelanguageserver/hover/Hover.fr#L39-L40

https://github.com/tricktron/frege-lsp-server/blob/a7241931652560324f8d8a4d585f3219081ebe10/src/main/frege/ch/fhnw/thga/fregelanguageserver/hover/Hover.fr#L74-L84

ReaderT Maybe Global a a.k.a Kleisli Maybe Global a

The code can be found on the f-readerT-arrow branch.

This stack has the same pros and cons as the MaybeT Reader Global a approach. However, it has some subtle differences:

Kleisli m a b is an instance of the Arrow type class and the monad type class in Frege. This is a major difference to Haskell where theReaderT is not an Arrow instance. According to the Haskell Typeclassopedia an Arrow and a Monad is not the same but Monad and ArrowApply are the same.

I have never used the Arrow type class, so the following may be completely wrong and inaccurate:

Since Kleisli is not an instance of MonadTrans we cannot use the standard lift function, e.g:

findSymbol :: QName -> ReaderT Maybe Global Symbol
findSymbol qname = do
    global     <- ask
    lift       $ Global.find global qname -- fails because Kleisli Maybe is not an instance of MonadTrans

In Haskell this works because there is a MonadTrans instance of ReaderT. I did not manage to write a MonadTrans (ReaderT) instance in Frege. Instead I wrote another helper function liftKleisli:

https://github.com/tricktron/frege-lsp-server/blob/7f790e28a9a9af3f1d4e5ee3b884250d94fd1fd0/src/main/frege/ch/fhnw/thga/fregelanguageserver/hover/Hover.fr#L34-L35

The high-level getTypeOnHover function then looks like this:

https://github.com/tricktron/frege-lsp-server/blob/7f790e28a9a9af3f1d4e5ee3b884250d94fd1fd0/src/main/frege/ch/fhnw/thga/fregelanguageserver/hover/Hover.fr#L65-L75

Instead of lift I used the arr function to lift a function into Kleisli m. This has the advantage that I can keep the getSymbolType function even simpler without making it a Reader:

Using Arrow with Kleisli: https://github.com/tricktron/frege-lsp-server/blob/7f790e28a9a9af3f1d4e5ee3b884250d94fd1fd0/src/main/frege/ch/fhnw/thga/fregelanguageserver/hover/Hover.fr#L37-L38

Vs. Using Monad with MaybeT: https://github.com/tricktron/frege-lsp-server/blob/a7241931652560324f8d8a4d585f3219081ebe10/src/main/frege/ch/fhnw/thga/fregelanguageserver/hover/Hover.fr#L42-L45

tricktron commented 2 years ago

After some more thinking and some discussions I decided to remove the monad transformers again and just passthrough the Global argument because

the key to performance is elegance, not battalions of special cases. The terrible temptation to tweak should be resisted unless the payoff is really noticeable. -- Jon Bentley and Doug McIlroy.

Let's resist the temptation of monad transformers because the payoff is not worth it.