Adapt to lsp changes for workspace/configuration (#3773)

* Adapt to lsp changes for workspace/configuration

This has a few substantive changes and a lot of messing with tests.

- We now tell `lsp` our config section, and parse just that section.
- We move the logic for updating the shake build rules for client config
  from a `workspace/didChangeConfiguration` handler to the new `lsp`
  callback, which will ensure it gets called in all circumstances that
  can be relevant.

The test changes are more annoying:
- We ignore config and logging messages by default now, so we have to
  stop doing that when we care about it.
- Many tests didn't really need to _change_ the config, but rather just
  to set it once at the beginning. I adjusted a lot of test functions to
  allow passing in the initial config for this reason.

* don't reduce the message timeout for wingman

* Fix stan plugin

* Doh

Author: michaelpj

cabal.project

@@ -55,7 +55,7 @@ package *
 
 write-ghc-environment-files: never
 
-index-state: 2023-08-09T10:37:15Z
+index-state: 2023-08-25T00:00:00Z
 
 constraints:
   -- For GHC 9.4, older versions of entropy fail to build on Windows
@@ -101,4 +101,3 @@ if impl(ghc >= 9.5)
     -- ghc-9.6
     ekg-core:ghc-prim,
     stm-hamt:transformers,
-

exe/Wrapper.hs

@@ -283,7 +283,8 @@ launchErrorLSP recorder errorMsg = do
 
   outH <- Main.argsHandleOut defaultArguments
 
-  let onConfigurationChange cfg _ = Right cfg
+  let parseConfig cfg _ = Right cfg
+      onConfigChange _ = pure ()
 
   let setup clientMsgVar = do
         -- Forcefully exit
@@ -311,7 +312,8 @@ launchErrorLSP recorder errorMsg = do
     inH
     outH
     (Main.argsDefaultHlsConfig defaultArguments)
-    onConfigurationChange
+    parseConfig
+    onConfigChange
     setup
 
 exitHandler :: IO () -> LSP.Handlers (ErrorLSPM c)

flake.lock

@@ -119,7 +119,7 @@ test-suite test
         base,
         extra,
         ghcide-bench,
-        lsp-test ^>= 0.15.0.1,
+        lsp-test ^>= 0.16,
         tasty,
         tasty-hunit >= 0.10,
         tasty-rerun,

ghcide-bench/ghcide-bench.cabal

@@ -363,8 +363,8 @@ runBenchmarksFun dir allBenchmarks = do
       createDirectoryIfMissing True eventlogDir
 
   lspConfig <- if Experiments.Types.lspConfig ?config
-    then either error Just . eitherDecodeStrict' <$> BS.getContents
-    else return Nothing
+    then either error id . eitherDecodeStrict' <$> BS.getContents
+    else return mempty
 
   let conf = defaultConfig
         { logStdErr = verbose ?config,
@@ -512,7 +512,7 @@ waitForProgressStart :: Session ()
 waitForProgressStart = void $ do
     skipManyTill anyMessage $ satisfy $ \case
       FromServerMess SMethod_WindowWorkDoneProgressCreate _ -> True
-      _                                              -> False
+      _                                                     -> False
 
 -- | Wait for all progress to be done
 -- Needs at least one progress done notification to return

ghcide-bench/src/Experiments.hs

@@ -74,8 +74,8 @@ library
         lens,
         list-t,
         hiedb == 0.4.3.*,
-        lsp-types ^>= 2.0.1.0,
-        lsp ^>= 2.1.0.0 ,
+        lsp-types ^>= 2.0.2.0,
+        lsp ^>= 2.2.0.0 ,
         mtl,
         optparse-applicative,
         parallel,
@@ -371,7 +371,7 @@ test-suite ghcide-tests
         hls-plugin-api,
         lens,
         list-t,
-        lsp-test ^>= 0.15.0.1,
+        lsp-test ^>= 0.16.0.0,
         mtl,
         monoid-subclasses,
         network-uri,

ghcide/ghcide.cabal

@@ -475,7 +475,8 @@ data GetModSummary = GetModSummary
 instance Hashable GetModSummary
 instance NFData   GetModSummary
 
--- | Get the vscode client settings stored in the ide state
+-- See Note [Client configuration in Rules]
+-- | Get the client config stored in the ide state
 data GetClientSettings = GetClientSettings
     deriving (Eq, Show, Typeable, Generic)
 instance Hashable GetClientSettings
@@ -510,3 +511,33 @@ instance NFData   GhcSessionIO
 makeLensesWith
     (lensRules & lensField .~ mappingNamer (pure . (++ "L")))
     ''Splices
+
+{- Note [Client configuration in Rules]
+The LSP client configuration is stored by `lsp` for us, and is accesible in
+handlers through the LspT monad.
+
+This is all well and good, but what if we want to write a Rule that depends
+on the configuration? For example, we might have a plugin that provides
+diagnostics - if the configuration changes to turn off that plugin, then
+we need to invalidate the Rule producing the diagnostics so that they go
+away. More broadly, any time we define a Rule that really depends on the
+configuration, such that the dependency needs to be tracked and the Rule
+invalidated when the configuration changes, we have a problem.
+
+The solution is that we have to mirror the configuration into the state
+that our build system knows about. That means that:
+- We have a parallel record of the state in 'IdeConfiguration'
+- We install a callback so that when the config changes we can update the
+'IdeConfiguration' and mark the rule as dirty.
+
+Then we can define a Rule that gets the configuration, and build Actions
+on top of that that behave properly. However, these should really only
+be used if you need the dependency tracking - for normal usage in handlers
+the config can simply be accessed directly from LspT.
+
+TODO(michaelpj): this is me writing down what I think the logic is, but
+it doesn't make much sense to me. In particular, we *can* get the LspT
+in an Action. So I don't know why we need to store it twice. We would
+still need to invalidate the Rule otherwise we won't know it's changed,
+though. See https://github.com/haskell/ghcide/pull/731 for some context.
+-}

ghcide/src/Development/IDE/Core/RuleTypes.hs

@@ -1095,6 +1095,7 @@ writeCoreFileIfNeeded se hsc (Just _) getGuts tmr = do
       (diags', !res) <- liftIO $ mkHiFileResultCompile se hsc tmr guts
       pure (diags++diags', res)
 
+-- See Note [Client configuration in Rules]
 getClientSettingsRule :: Recorder (WithPriority Log) -> Rules ()
 getClientSettingsRule recorder = defineEarlyCutOffNoFile (cmapWithPrio LogShake recorder) $ \GetClientSettings -> do
   alwaysRerun

ghcide/src/Development/IDE/Core/Rules.hs

@@ -325,6 +325,7 @@ getShakeExtrasRules = do
       -- This will actually crash HLS
       Nothing -> liftIO $ fail "missing ShakeExtras"
 
+-- See Note [Client configuration in Rules]
 -- | Returns the client configuration, creating a build dependency.
 --   You should always use this function when accessing client configuration
 --   from build rules.

ghcide/src/Development/IDE/Core/Shake.hs

@@ -90,32 +90,33 @@ runLanguageServer
     -> Handle -- output
     -> config
     -> (config -> Value -> Either T.Text config)
+    -> (config -> m config ())
     -> (MVar ()
         -> IO (LSP.LanguageContextEnv config -> TRequestMessage Method_Initialize -> IO (Either ResponseError (LSP.LanguageContextEnv config, a)),
                LSP.Handlers (m config),
                (LanguageContextEnv config, a) -> m config <~> IO))
     -> IO ()
-runLanguageServer recorder options inH outH defaultConfig onConfigurationChange setup = do
+runLanguageServer recorder options inH outH defaultConfig parseConfig onConfigChange setup = do
     -- This MVar becomes full when the server thread exits or we receive exit message from client.
     -- LSP server will be canceled when it's full.
     clientMsgVar <- newEmptyMVar
 
     (doInitialize, staticHandlers, interpretHandler) <- setup clientMsgVar
 
     let serverDefinition = LSP.ServerDefinition
-            { LSP.onConfigurationChange = onConfigurationChange
+            { LSP.parseConfig = parseConfig
+            , LSP.onConfigChange = onConfigChange
             , LSP.defaultConfig = defaultConfig
+            -- TODO: magic string
+            , LSP.configSection = "haskell"
             , LSP.doInitialize = doInitialize
             , LSP.staticHandlers = (const staticHandlers)
             , LSP.interpretHandler = interpretHandler
             , LSP.options = modifyOptions options
             }
 
     let lspCologAction :: MonadIO m2 => Colog.LogAction m2 (Colog.WithSeverity LspServerLog)
-        lspCologAction = toCologActionWithPrio $ cfilter
-            -- filter out bad logs in lsp, see: https://github.com/haskell/lsp/issues/447
-            (\msg -> priority msg >= Info)
-            (cmapWithPrio LogLspServer recorder)
+        lspCologAction = toCologActionWithPrio (cmapWithPrio LogLspServer recorder)
 
     void $ untilMVar clientMsgVar $
           void $ LSP.runServerWithHandles