Prevent Tactics hover provider from blocking at startup

[pepeiborra]

There's been a lot of work done on making hover and getDefinition immediately responsive at startup by using persisted data.

Unfortunately we didn't install tests to preserve this fragile property. We should add those tests to the func-test testsuite.

The problem here is that Tactics installs a hover handler that depends on the TypeCheck rule. Since there is no persistent provider for this rule, it blocks until the file can be typechecked. Since HLS does not implement partial responses (and neither do most LSP clients anyway), this blocks all the other hover providers.

The solution is to install a new build rule GetMetaprograms that depends on TypeCheck, install a persistent provider for it that returns the empty list of meta programs, and switch the hover provider to useWithStaleFast.

The downsides of doing this are negligible - the hover provider won't show any metaprogram specific info if used at startup, but it will work finely on a second attempt.

[jneira]

There's been a lot of work done on making hover and getDefinition immediately responsive at startup by using persisted data.
Unfortunately we didn't install tests to preserve this fragile property. We should add those tests to the func-test testsuite.

It sounds like it worths to open an issue about. How would looks like those tests? defs/completions are received before some threshold? coud be it (also) catch by benchmarks?

[wz1000]

#1626 also introduced a useE of GhcSession here in the default ghcide hover provider. We could either catch the failure there or implement a persistent rule for GhcSession. The latter doesn't seem very sound.

We do need some way of enforcing this. How about something like this?

@@ -246,7 +248,7 @@ getPluginConfig plugin = do
 -- This is called when we don't already have a result, or computing the rule failed.
 -- The result of this function will always be marked as 'stale', and a 'proper' rebuild of the rule will
 -- be queued if the rule hasn't run before.
-addPersistentRule :: IdeRule k v => k -> (NormalizedFilePath -> IdeAction (Maybe (v,PositionDelta,TextDocumentVersion))) -> Rules ()
+addPersistentRule :: PersistentRule k v => k -> (NormalizedFilePath -> IdeAction (Maybe (v,PositionDelta,TextDocumentVersion))) -> Rules ()
 addPersistentRule k getVal = do
   ShakeExtras{persistentKeys} <- getShakeExtrasRules
   void $ liftIO $ modifyVar' persistentKeys $ HMap.insert (Key k) (fmap (fmap (first3 toDyn)) . getVal)
@@ -380,6 +382,14 @@ type IdeRule k v =
   , NFData v
   )
 
+-- | 'IdeRule' but also persistent
+type PersistentRule k v =
+  ( IdeRule k v
+  , PersistentValue k
+  )
+
+class PersistentValue k where
+  ...
+
 -- | A live Shake session with the ability to enqueue Actions for running.
 --   Keeps the 'ShakeDatabase' open, so at most one 'ShakeSession' per database.
 newtype ShakeSession = ShakeSession
@@ -806,7 +816,7 @@ data FastResult a = FastResult { stale :: Maybe (a,PositionMapping), uptoDate ::
 -- | Lookup value in the database and return with the stale value immediately
 -- Will queue an action to refresh the value.
 -- Might block the first time the rule runs, but never blocks after that.
-useWithStaleFast :: IdeRule k v => k -> NormalizedFilePath -> IdeAction (Maybe (v, PositionMapping))
+useWithStaleFast :: PersistentRule k v => k -> NormalizedFilePath -> IdeAction (Maybe (v, PositionMapping))
 useWithStaleFast key file = stale <$> useWithStaleFast' key file
 
 -- | Same as useWithStaleFast but lets you wait for an up to date result

[wz1000]

Oh, we can't catch the failure because it blocks, we will need to somehow come up with an empty HscEnv or UnitState at least.

[isovector]

I don't 100% understand what's happening here, but I trust @pepeiborra !