Never compute the DocMap with stale data

As the regression test 'eps-pollution' demonstrates, changing the
imports of a module with class instances can poison the HLS session.

To fix this, we simply shouldn't use stale results for computing the
`DocMap`.

Frankly, there is no reason to use stale results when we are not even
using the `PositionMapping`, this sounds like a recipe for disaster in
general. Especially, since the source locations matter.

Author: fendor

ghcide-test/data/multi-unit-eps-pollution/c/C.hs

@@ -1,5 +1,7 @@
+{-# LANGUAGE TypeApplications #-}
+{-# OPTIONS_GHC -Wall #-}
 module C where
 import A
 
-cFoo :: AType -> String
-cFoo = myMethod
+-- Omit top-level signature so we have a warning we can check against
+cFoo = myMethod @AType

ghcide-test/exe/EpsPollutionTests.hs

@@ -7,8 +7,8 @@
 -- 'ClsInst' twice (once via @ie_global@ from the EPS, once via
 -- @ie_local@ from 'hptInstancesBelow').
 --
--- The pollution enters through 'Development.IDE.Spans.Documentation.mkDocMap'.
--- Its 'Rules.GetDocMap' rule reads three inputs via independent
+-- The pollution entered through 'Development.IDE.Spans.Documentation.mkDocMap'.
+-- Its 'Rules.GetDocMap' rule read three inputs via independent
 -- @useWithStale_@ calls: 'TypeCheck', 'GhcSessionDeps' and 'GetHieAst'.
 -- These three can diverge: an edit that merely changes imports lets
 -- 'GhcSessionDeps' re-evaluate (fresh, with a different HPT) while
@@ -21,28 +21,22 @@
 -- evicts anything, so the pollution is permanent for the session.
 module EpsPollutionTests (tests) where
 
-import           Config                      (runWithExtraFiles)
+import           Config                      (Expect (ExpectHoverText),
+                                              runWithExtraFiles)
 import           Control.Lens                ((^.))
-import           Control.Monad.IO.Class      (liftIO)
+import           Control.Monad               (void)
 import qualified Data.Text                   as T
 import           Development.IDE.GHC.Util    (readFileUtf8)
-import           Development.IDE.Test        (waitForTypecheck)
+import           Hover
 import qualified Language.LSP.Protocol.Lens  as L
 import           Language.LSP.Protocol.Types
 import           Language.LSP.Test
 import           System.FilePath
-import           Test.Hls                    (expectFailBecause,
-                                              waitForDiagnosticsFrom)
-import           Test.Tasty
-import           Test.Tasty.HUnit
+import           Test.Hls
 
 tests :: TestTree
 tests = testGroup "eps-pollution"
-    [ expectFailBecause
-        "The EPS gets polluted with a home-module instance via GetDocMap's \
-        \inconsistent stale-value snapshot; the next successful typecheck \
-        \sees the instance twice."
-        staleHieProvokesOverlapping
+    [ staleHieProvokesOverlapping
     ]
 
 -- The fixture at ghcide-test/data/multi-unit-eps-pollution/ sets up two
@@ -59,20 +53,22 @@ staleHieProvokesOverlapping =
     originalC <- liftIO $ readFileUtf8 cPath
     let brokenC = T.replace "import A\n" "" originalC
     cdoc <- openDoc cPath "haskell"
-    True <- waitForTypecheck cdoc
+    void $ waitForTypecheck cdoc
     -- Hovering triggers the hover pipeline, which forces GetDocMap.
     -- While C is healthy this populates GetHieAst with a RefMap
     -- referencing A's names -- the stale value we rely on below.
-    _ <- getHover cdoc (hoverOnMyMethod originalC)
+    hover <- getHover cdoc (hoverOnMyMethod originalC)
+    checkHover hover [ExpectHoverText ["myMethod", "MyClass"]]
+
     -- Break C's import of A. C fails to typecheck, but GhcSessionDeps
     -- re-evaluates successfully (it only needs the import list) with an
     -- HPT that no longer contains A. A further hover forces GetDocMap
     -- to run with the fresh GhcSessionDeps alongside the stale RefMap;
     -- loadSysInterface(A) then runs and pollutes the EPS.
     changeDoc cdoc [TextDocumentContentChangeEvent . InR .
       TextDocumentContentChangeWholeDocument $ brokenC]
-    _ <- getHover cdoc (hoverOnMyMethod brokenC)
-    _ <- waitForDiagnosticsFrom cdoc  -- let the broken state settle
+    void $ getHover cdoc (hoverOnMyMethod brokenC)
+    void $ waitForDiagnosticsFrom cdoc  -- let the broken state settle
     -- Repair C. The next typecheck legitimately has A in its HPT; with
     -- the polluted EPS it also has A's ClsInst in eps_inst_env, so
     -- instance resolution for 'myMethod x :: AType -> String' finds

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

@@ -579,11 +579,9 @@ getBindingsRule recorder =
 getDocMapRule :: Recorder (WithPriority Log) -> Rules ()
 getDocMapRule recorder =
     define (cmapWithPrio LogShake recorder) $ \GetDocMap file -> do
-      -- Stale data for the scenario where a broken module has previously typechecked
-      -- but we never generated a DocMap for it
-      (tmrTypechecked -> tc, _) <- useWithStale_ TypeCheck file
-      (hscEnv -> hsc, _)        <- useWithStale_ GhcSessionDeps file
-      (HAR{refMap=rf}, _)       <- useWithStale_ GetHieAst file
+      (tmrTypechecked -> tc) <- use_ TypeCheck file
+      (hscEnv -> hsc)        <- use_ GhcSessionDeps file
+      HAR{refMap=rf}         <- use_ GetHieAst file
       cfg <- getClientConfigAction
       dkMap <- liftIO $ mkDocMap hsc rf tc $ LinkTargets
                 { linkSource = linkSourceTo cfg