From 2bcb20b054258554e992b2e43c101f5703a07047 Mon Sep 17 00:00:00 2001
From: Pepe Iborra <pepeiborra@gmail.com>
Date: Tue, 17 Aug 2021 12:55:05 +0200
Subject: [PATCH] Fix filepath identity in cradle dependencies

When doing reactive change tracking we track the set of dirty keys as opposed to conservative change tracking where we check every key on every build.

Filepath identity becomes even more important in this setting, opening a new type of bug where the filepath used in the dirty key is equivalent but not structurally equal to the filepath used in the dependency graph.

This can happen e.g. with cradle dependencies where hie-bios gives us a set of relative paths and we create GetFileModification keys for them as dependencies of the GhcSession, but the LSP client raises FileWatched notifications using absolute paths that we use to create dirty keys.

To prevent this class of bugs, we would need to strengthen the notion of NormalizedFilePath to make it always absolute/relative. In practice, since 99% of the paths are introduced by the LSP client, we just need to make sure that the other 1% (introduced by e.g. hie-bios) follow the same convention.

This commit fixes the hie-bios paths to make them absolute before creating rule keys, following the LSP convention
---
 ghcide/bench/lib/Experiments.hs          | 12 ++++++------
 ghcide/src/Development/IDE/Core/Rules.hs |  9 ++++++---
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/ghcide/bench/lib/Experiments.hs b/ghcide/bench/lib/Experiments.hs
index 80035ba119..ed2c4e6473 100644
--- a/ghcide/bench/lib/Experiments.hs
+++ b/ghcide/bench/lib/Experiments.hs
@@ -153,10 +153,10 @@ experiments =
                 forM_ identifierP $ \p -> changeDoc doc [charEdit p]
         )
         ( \docs -> do
-            Just hieYaml <- uriToFilePath <$> getDocUri "hie.yaml"
-            liftIO $ appendFile hieYaml "##\n"
+            hieYamlUri <- getDocUri "hie.yaml"
+            liftIO $ appendFile (fromJust $ uriToFilePath hieYamlUri) "##\n"
             sendNotification SWorkspaceDidChangeWatchedFiles $ DidChangeWatchedFilesParams $
-                List [ FileEvent (filePathToUri "hie.yaml") FcChanged ]
+                List [ FileEvent hieYamlUri FcChanged ]
             forM_ docs $ \DocumentPositions{..} ->
               changeDoc doc [charEdit stringLiteralP]
             waitForProgressDone
@@ -168,10 +168,10 @@ experiments =
       bench
         "hover after cradle edit"
         (\docs -> do
-            Just hieYaml <- uriToFilePath <$> getDocUri "hie.yaml"
-            liftIO $ appendFile hieYaml "##\n"
+            hieYamlUri <- getDocUri "hie.yaml"
+            liftIO $ appendFile (fromJust $ uriToFilePath hieYamlUri) "##\n"
             sendNotification SWorkspaceDidChangeWatchedFiles $ DidChangeWatchedFilesParams $
-                List [ FileEvent (filePathToUri "hie.yaml") FcChanged ]
+                List [ FileEvent hieYamlUri FcChanged ]
             flip allWithIdentifierPos docs $ \DocumentPositions{..} -> isJust <$> getHover doc (fromJust identifierP)
         ),
       ---------------------------------------------------------------------------------------
diff --git a/ghcide/src/Development/IDE/Core/Rules.hs b/ghcide/src/Development/IDE/Core/Rules.hs
index c4cbab0d02..0d4c082931 100644
--- a/ghcide/src/Development/IDE/Core/Rules.hs
+++ b/ghcide/src/Development/IDE/Core/Rules.hs
@@ -138,7 +138,7 @@ import qualified Language.LSP.Server                          as LSP
 import           Language.LSP.Types                           (SMethod (SCustomMethod))
 import           Language.LSP.VFS
 import           Module
-import           System.Directory                             (canonicalizePath)
+import           System.Directory                             (canonicalizePath, makeAbsolute)
 import           TcRnMonad                                    (tcg_dependent_files)
 
 import           Control.Applicative
@@ -674,9 +674,12 @@ loadGhcSession = do
 
         -- add the deps to the Shake graph
         let addDependency fp = do
-                let nfp = toNormalizedFilePath' fp
+                -- VSCode uses absolute paths in its filewatch notifications
+                afp <- liftIO $ makeAbsolute fp
+                let nfp = toNormalizedFilePath' afp
                 itExists <- getFileExists nfp
-                when itExists $ void $ use_ GetModificationTime nfp
+                when itExists $ void $ do
+                  use_ GetModificationTime nfp
         mapM_ addDependency deps
 
         opts <- getIdeOptions