From d7162249a10a98051c958950497c40a8d1095bac Mon Sep 17 00:00:00 2001
From: Oliver Madine <30090176+OliverMadine@users.noreply.github.com>
Date: Sun, 3 Jul 2022 04:41:05 +0100
Subject: [PATCH 01/10] test: add tests for record puns

---
 plugins/hls-rename-plugin/test/Main.hs                    | 4 ++++
 .../hls-rename-plugin/test/testdata/FieldPuns.expected.hs | 8 ++++++++
 plugins/hls-rename-plugin/test/testdata/FieldPuns.hs      | 8 ++++++++
 .../test/testdata/IndirectPuns.expected.hs                | 8 ++++++++
 plugins/hls-rename-plugin/test/testdata/IndirectPuns.hs   | 8 ++++++++
 plugins/hls-rename-plugin/test/testdata/hie.yaml          | 2 ++
 6 files changed, 38 insertions(+)
 create mode 100644 plugins/hls-rename-plugin/test/testdata/FieldPuns.expected.hs
 create mode 100644 plugins/hls-rename-plugin/test/testdata/FieldPuns.hs
 create mode 100644 plugins/hls-rename-plugin/test/testdata/IndirectPuns.expected.hs
 create mode 100644 plugins/hls-rename-plugin/test/testdata/IndirectPuns.hs

diff --git a/plugins/hls-rename-plugin/test/Main.hs b/plugins/hls-rename-plugin/test/Main.hs
index 66bcea6222..874310d1d7 100644
--- a/plugins/hls-rename-plugin/test/Main.hs
+++ b/plugins/hls-rename-plugin/test/Main.hs
@@ -21,6 +21,8 @@ tests = testGroup "Rename"
         rename doc (Position 0 15) "Op"
     , goldenWithRename "Exported function" "ExportedFunction" $ \doc ->
         rename doc (Position 2 1) "quux"
+    , goldenWithRename "Field Puns" "FieldPuns" $ \doc ->
+        rename doc (Position 7 13) "bleh"
     , goldenWithRename "Function argument" "FunctionArgument" $ \doc ->
         rename doc (Position 3 4) "y"
     , goldenWithRename "Function name" "FunctionName" $ \doc ->
@@ -33,6 +35,8 @@ tests = testGroup "Rename"
         rename doc (Position 3 8) "baz"
     , goldenWithRename "Import hiding" "ImportHiding" $ \doc ->
         rename doc (Position 0 22) "hiddenFoo"
+    , goldenWithRename "Indirect Puns" "IndirectPuns" $ \doc ->
+        rename doc (Position 4 23) "blah"
     , goldenWithRename "Let expression" "LetExpression" $ \doc ->
         rename doc (Position 5 11) "foobar"
     , goldenWithRename "Qualified as" "QualifiedAs" $ \doc ->
diff --git a/plugins/hls-rename-plugin/test/testdata/FieldPuns.expected.hs b/plugins/hls-rename-plugin/test/testdata/FieldPuns.expected.hs
new file mode 100644
index 0000000000..f6618927b0
--- /dev/null
+++ b/plugins/hls-rename-plugin/test/testdata/FieldPuns.expected.hs
@@ -0,0 +1,8 @@
+{-# LANGUAGE NamedFieldPuns #-}
+
+module FieldPun () where
+
+newtype Foo = Foo { bleh :: Int }
+
+unFoo :: Foo -> Int
+unFoo Foo{bleh} = bleh
diff --git a/plugins/hls-rename-plugin/test/testdata/FieldPuns.hs b/plugins/hls-rename-plugin/test/testdata/FieldPuns.hs
new file mode 100644
index 0000000000..2cd53d0026
--- /dev/null
+++ b/plugins/hls-rename-plugin/test/testdata/FieldPuns.hs
@@ -0,0 +1,8 @@
+{-# LANGUAGE NamedFieldPuns #-}
+
+module FieldPun () where
+
+newtype Foo = Foo { field :: Int }
+
+unFoo :: Foo -> Int
+unFoo Foo{field} = field
diff --git a/plugins/hls-rename-plugin/test/testdata/IndirectPuns.expected.hs b/plugins/hls-rename-plugin/test/testdata/IndirectPuns.expected.hs
new file mode 100644
index 0000000000..cf181c7215
--- /dev/null
+++ b/plugins/hls-rename-plugin/test/testdata/IndirectPuns.expected.hs
@@ -0,0 +1,8 @@
+{-# LANGUAGE NamedFieldPuns #-}
+
+module IndirectPuns () where
+
+newtype Foo = Foo { blah :: Int }
+
+unFoo :: Foo -> Int
+unFoo Foo{blah} = blah
diff --git a/plugins/hls-rename-plugin/test/testdata/IndirectPuns.hs b/plugins/hls-rename-plugin/test/testdata/IndirectPuns.hs
new file mode 100644
index 0000000000..c823126a76
--- /dev/null
+++ b/plugins/hls-rename-plugin/test/testdata/IndirectPuns.hs
@@ -0,0 +1,8 @@
+{-# LANGUAGE NamedFieldPuns #-}
+
+module IndirectPuns () where
+
+newtype Foo = Foo { field :: Int }
+
+unFoo :: Foo -> Int
+unFoo Foo{field} = field
diff --git a/plugins/hls-rename-plugin/test/testdata/hie.yaml b/plugins/hls-rename-plugin/test/testdata/hie.yaml
index 4c184b3c33..892a7d675f 100644
--- a/plugins/hls-rename-plugin/test/testdata/hie.yaml
+++ b/plugins/hls-rename-plugin/test/testdata/hie.yaml
@@ -3,6 +3,7 @@ cradle:
     arguments:
       - "DataConstructor"
       - "ExportedFunction"
+      - "FieldPuns"
       - "Foo"
       - "FunctionArgument"
       - "FunctionName"
@@ -10,6 +11,7 @@ cradle:
       - "HiddenFunction"
       - "ImportHiding"
       - "ImportedFunction"
+      - "IndirectPuns"
       - "LetExpression"
       - "QualifiedAs"
       - "QualifiedFunction"

From 6659bf3e4507817a978e9d162291a0c6706e83e9 Mon Sep 17 00:00:00 2001
From: Oliver Madine <30090176+OliverMadine@users.noreply.github.com>
Date: Sun, 3 Jul 2022 04:43:44 +0100
Subject: [PATCH 02/10] feat: rename indirect references refactor: remove
 "safe" from function names

---
 .../src/Ide/Plugin/Rename.hs                  | 77 +++++++++++--------
 1 file changed, 43 insertions(+), 34 deletions(-)

diff --git a/plugins/hls-rename-plugin/src/Ide/Plugin/Rename.hs b/plugins/hls-rename-plugin/src/Ide/Plugin/Rename.hs
index 6d051d96de..6d6e5b7ccc 100644
--- a/plugins/hls-rename-plugin/src/Ide/Plugin/Rename.hs
+++ b/plugins/hls-rename-plugin/src/Ide/Plugin/Rename.hs
@@ -24,7 +24,7 @@ import           Data.Generics
 import           Data.Hashable
 import           Data.HashSet                          (HashSet)
 import qualified Data.HashSet                          as HS
-import           Data.List.Extra
+import           Data.List.Extra                       hiding (length)
 import qualified Data.Map                              as M
 import           Data.Maybe
 import           Data.Mod.Word
@@ -42,7 +42,6 @@ import           Development.IDE.GHC.ExactPrint
 import           Development.IDE.Spans.AtPoint
 import           Development.IDE.Types.Location
 import           HieDb.Query
-import           Ide.Plugin.Config
 import           Ide.Plugin.Properties
 import           Ide.PluginUtils
 import           Ide.Types
@@ -65,16 +64,25 @@ descriptor pluginId = (defaultPluginDescriptor pluginId)
 renameProvider :: PluginMethodHandler IdeState TextDocumentRename
 renameProvider state pluginId (RenameParams (TextDocumentIdentifier uri) pos _prog newNameText) =
     pluginResponse $ do
-        nfp <- safeUriToNfp uri
-        oldName <- getNameAtPos state nfp pos
-        refLocs <- refsAtName state nfp oldName
+        nfp <- handleUriToNfp uri
+        directOldNames <- getNamesAtPos state nfp pos
+        directRefs <- concat <$> mapM (refsAtName state nfp) directOldNames
+
+        -- References of references to account for punned names (Indirect references)
+        indirectOldNames <- find ((>1) . Prelude.length) <$>
+            mapM (uncurry (getNamesAtPos state) . locToFilePos) directRefs
+        let oldNames = fromMaybe [] indirectOldNames ++ directOldNames
+        refs <- HS.fromList . concat <$> mapM (refsAtName state nfp) oldNames
+
+        -- Validate rename
         crossModuleEnabled <- lift $ usePropertyLsp #crossModule pluginId properties
-        unless crossModuleEnabled $ failWhenImportOrExport state nfp refLocs oldName
-        when (isBuiltInSyntax oldName) $
-            throwE ("Invalid rename of built-in syntax: \"" ++ showName oldName ++ "\"")
+        unless crossModuleEnabled $ failWhenImportOrExport state nfp refs oldNames
+        when (any isBuiltInSyntax oldNames) $ throwE "Invalid rename of built-in syntax"
+
+        -- Perform rename
         let newName = mkTcOcc $ T.unpack newNameText
-            filesRefs = collectWith locToUri refLocs
-            getFileEdit = flip $ getSrcEdit state . renameRefs newName
+            filesRefs = collectWith locToUri refs
+            getFileEdit = flip $ getSrcEdit state . replaceRefs newName
         fileEdits <- mapM (uncurry getFileEdit) filesRefs
         pure $ foldl' (<>) mempty fileEdits
 
@@ -84,16 +92,16 @@ failWhenImportOrExport ::
     IdeState ->
     NormalizedFilePath ->
     HashSet Location ->
-    Name ->
+    [Name] ->
     ExceptT String m ()
-failWhenImportOrExport state nfp refLocs name = do
+failWhenImportOrExport state nfp refLocs names = do
     pm <- handleMaybeM ("No parsed module for: " ++ show nfp) $ liftIO $ runAction
         "Rename.GetParsedModule"
         state
         (use GetParsedModule nfp)
     let hsMod = unLoc $ pm_parsed_source pm
     case (unLoc <$> hsmodName hsMod, hsmodExports hsMod) of
-        (mbModName, _) | not $ nameIsLocalOrFrom (replaceModName name mbModName) name
+        (mbModName, _) | not $ any (\n -> nameIsLocalOrFrom (replaceModName n mbModName) n) names
             -> throwE "Renaming of an imported name is unsupported"
         (_, Just (L _ exports)) | any ((`HS.member` refLocs) . unsafeSrcSpanToLoc . getLoc) exports
             -> throwE "Renaming of an exported name is unsupported"
@@ -112,7 +120,7 @@ getSrcEdit ::
     ExceptT String m WorkspaceEdit
 getSrcEdit state updatePs uri = do
     ccs <- lift getClientCapabilities
-    nfp <- safeUriToNfp uri
+    nfp <- handleUriToNfp uri
     annAst <- handleMaybeM ("No parsed source for: " ++ show nfp) $ liftIO $ runAction
         "Rename.GetAnnotatedParsedSource"
         state
@@ -128,13 +136,13 @@ getSrcEdit state updatePs uri = do
     pure $ diffText ccs (uri, src) res IncludeDeletions
 
 -- | Replace names at every given `Location` (in a given `ParsedSource`) with a given new name.
-renameRefs ::
+replaceRefs ::
     OccName ->
     HashSet Location ->
     ParsedSource ->
     ParsedSource
 #if MIN_VERSION_ghc(9,2,1)
-renameRefs newName refs = everywhere $
+replaceRefs newName refs = everywhere $
     -- there has to be a better way...
     mkT (replaceLoc @AnnListItem) `extT`
     -- replaceLoc @AnnList `extT` -- not needed
@@ -149,14 +157,13 @@ renameRefs newName refs = everywhere $
             | isRef (locA srcSpan) = L srcSpan $ replace oldRdrName
         replaceLoc lOldRdrName = lOldRdrName
 #else
-renameRefs newName refs = everywhere $ mkT replaceLoc
+replaceRefs newName refs = everywhere $ mkT replaceLoc
     where
         replaceLoc :: Located RdrName -> Located RdrName
         replaceLoc (L srcSpan oldRdrName)
             | isRef srcSpan = L srcSpan $ replace oldRdrName
         replaceLoc lOldRdrName = lOldRdrName
 #endif
-
         replace :: RdrName -> RdrName
         replace (Qual modName _) = Qual modName newName
         replace _                = Unqual newName
@@ -173,10 +180,10 @@ refsAtName ::
     IdeState ->
     NormalizedFilePath ->
     Name ->
-    ExceptT String m (HashSet Location)
+    ExceptT String m [Location]
 refsAtName state nfp name = do
     ShakeExtras{withHieDb} <- liftIO $ runAction "Rename.HieDb" state getShakeExtras
-    ast <- safeGetHieAst state nfp
+    ast <- handleGetHieAst state nfp
     dbRefs <- case nameModule_maybe name of
         Nothing -> pure []
         Just mod -> liftIO $ mapMaybe rowToLoc <$> withHieDb (\hieDb ->
@@ -188,32 +195,32 @@ refsAtName state nfp name = do
                 (Just $ moduleUnit mod)
                 [fromNormalizedFilePath nfp]
             )
-    pure $ HS.fromList $ getNameLocs name ast ++ dbRefs
+    pure $ nameLocs name ast ++ dbRefs
 
-getNameLocs :: Name -> (HieAstResult, PositionMapping) -> [Location]
-getNameLocs name (HAR _ _ rm _ _, pm) =
+nameLocs :: Name -> (HieAstResult, PositionMapping) -> [Location]
+nameLocs name (HAR _ _ rm _ _, pm) =
     mapMaybe (toCurrentLocation pm . realSrcSpanToLocation . fst)
              (concat $ M.lookup (Right name) rm)
 
 ---------------------------------------------------------------------------------------------------
 -- Util
 
-getNameAtPos :: IdeState -> NormalizedFilePath -> Position -> ExceptT String (LspT Config IO) Name
-getNameAtPos state nfp pos = do
-    (HAR{hieAst}, pm) <- safeGetHieAst state nfp
-    handleMaybe ("No name at " ++ showPos pos) $ listToMaybe $ getNamesAtPoint hieAst pos pm
+getNamesAtPos :: MonadIO m => IdeState -> NormalizedFilePath -> Position -> ExceptT String m [Name]
+getNamesAtPos state nfp pos = do
+    (HAR{hieAst}, pm) <- handleGetHieAst state nfp
+    pure $ getNamesAtPoint hieAst pos pm
 
-safeGetHieAst ::
+handleGetHieAst ::
     MonadIO m =>
     IdeState ->
     NormalizedFilePath ->
     ExceptT String m (HieAstResult, PositionMapping)
-safeGetHieAst state nfp = handleMaybeM
+handleGetHieAst state nfp = handleMaybeM
     ("No AST for file: " ++ show nfp)
     (liftIO $ runAction "Rename.GetHieAst" state $ useWithStale GetHieAst nfp)
 
-safeUriToNfp :: (Monad m) => Uri -> ExceptT String m NormalizedFilePath
-safeUriToNfp uri = handleMaybe
+handleUriToNfp :: (Monad m) => Uri -> ExceptT String m NormalizedFilePath
+handleUriToNfp uri = handleMaybe
     ("No filepath for uri: " ++ show uri)
     (toNormalizedFilePath <$> uriToFilePath uri)
 
@@ -230,15 +237,17 @@ nfpToUri = filePathToUri . fromNormalizedFilePath
 showName :: Name -> String
 showName = occNameString . getOccName
 
-showPos :: Position -> String
-showPos Position{_line, _character} = "line: " ++ show _line ++ " - character: " ++ show _character
-
 unsafeSrcSpanToLoc :: SrcSpan -> Location
 unsafeSrcSpanToLoc srcSpan =
     case srcSpanToLocation srcSpan of
         Nothing       -> error "Invalid conversion from UnhelpfulSpan to Location"
         Just location -> location
 
+locToFilePos :: Location -> (NormalizedFilePath, Position)
+locToFilePos (Location uri (Range pos _)) = (nfp, pos)
+    where
+        Just nfp = (uriToNormalizedFilePath . toNormalizedUri) uri
+
 replaceModName :: Name -> Maybe ModuleName -> Module
 replaceModName name mbModName =
     mkModule (moduleUnit $ nameModule name) (fromMaybe (mkModuleName "Main") mbModName)

From d8a5b232a2d1b49f11aead238305238c745682d3 Mon Sep 17 00:00:00 2001
From: Oliver Madine <30090176+OliverMadine@users.noreply.github.com>
Date: Sun, 3 Jul 2022 13:51:17 +0100
Subject: [PATCH 03/10] test: ignore record field tests for ghc92 (#2915)

---
 plugins/hls-rename-plugin/test/Main.hs | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/plugins/hls-rename-plugin/test/Main.hs b/plugins/hls-rename-plugin/test/Main.hs
index 874310d1d7..b0ed6bcd41 100644
--- a/plugins/hls-rename-plugin/test/Main.hs
+++ b/plugins/hls-rename-plugin/test/Main.hs
@@ -35,7 +35,8 @@ tests = testGroup "Rename"
         rename doc (Position 3 8) "baz"
     , goldenWithRename "Import hiding" "ImportHiding" $ \doc ->
         rename doc (Position 0 22) "hiddenFoo"
-    , goldenWithRename "Indirect Puns" "IndirectPuns" $ \doc ->
+    , ignoreForGhcVersions [GHC92] "Record field refs broken in ghc92" $
+      goldenWithRename "Indirect Puns" "IndirectPuns" $ \doc ->
         rename doc (Position 4 23) "blah"
     , goldenWithRename "Let expression" "LetExpression" $ \doc ->
         rename doc (Position 5 11) "foobar"
@@ -47,7 +48,8 @@ tests = testGroup "Rename"
         rename doc (Position 3 12) "baz"
     , goldenWithRename "Realigns do block indentation" "RealignDo" $ \doc ->
         rename doc (Position 0 2) "fooBarQuux"
-    , goldenWithRename "Record field" "RecordField" $ \doc ->
+    , ignoreForGhcVersions [GHC92] "Record field refs broken in ghc92" $
+      goldenWithRename "Record field" "RecordField" $ \doc ->
         rename doc (Position 6 9) "number"
     , goldenWithRename "Shadowed name" "ShadowedName" $ \doc ->
         rename doc (Position 1 1) "baz"

From 80c329062e758856810625ae9dd660e8118a78e0 Mon Sep 17 00:00:00 2001
From: Oliver Madine <30090176+OliverMadine@users.noreply.github.com>
Date: Sun, 3 Jul 2022 17:36:11 +0100
Subject: [PATCH 04/10] test: ignore record field tests for ghc90 (#2915)

---
 plugins/hls-rename-plugin/test/Main.hs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/plugins/hls-rename-plugin/test/Main.hs b/plugins/hls-rename-plugin/test/Main.hs
index b0ed6bcd41..0f066d056d 100644
--- a/plugins/hls-rename-plugin/test/Main.hs
+++ b/plugins/hls-rename-plugin/test/Main.hs
@@ -35,7 +35,7 @@ tests = testGroup "Rename"
         rename doc (Position 3 8) "baz"
     , goldenWithRename "Import hiding" "ImportHiding" $ \doc ->
         rename doc (Position 0 22) "hiddenFoo"
-    , ignoreForGhcVersions [GHC92] "Record field refs broken in ghc92" $
+    , ignoreForGhcVersions [GHC90, GHC92] "Record field refs broken in ghc92" $
       goldenWithRename "Indirect Puns" "IndirectPuns" $ \doc ->
         rename doc (Position 4 23) "blah"
     , goldenWithRename "Let expression" "LetExpression" $ \doc ->
@@ -48,7 +48,7 @@ tests = testGroup "Rename"
         rename doc (Position 3 12) "baz"
     , goldenWithRename "Realigns do block indentation" "RealignDo" $ \doc ->
         rename doc (Position 0 2) "fooBarQuux"
-    , ignoreForGhcVersions [GHC92] "Record field refs broken in ghc92" $
+    , ignoreForGhcVersions [GHC90, GHC92] "Record field refs broken in ghc92" $
       goldenWithRename "Record field" "RecordField" $ \doc ->
         rename doc (Position 6 9) "number"
     , goldenWithRename "Shadowed name" "ShadowedName" $ \doc ->

From d24d1e6fb6e72e6997ae217b3faa19b8876df4f8 Mon Sep 17 00:00:00 2001
From: Oliver Madine <30090176+OliverMadine@users.noreply.github.com>
Date: Sun, 3 Jul 2022 17:56:51 +0100
Subject: [PATCH 05/10] fix: update record field test ignore message

---
 plugins/hls-rename-plugin/test/Main.hs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/plugins/hls-rename-plugin/test/Main.hs b/plugins/hls-rename-plugin/test/Main.hs
index 0f066d056d..ee5487669e 100644
--- a/plugins/hls-rename-plugin/test/Main.hs
+++ b/plugins/hls-rename-plugin/test/Main.hs
@@ -35,7 +35,7 @@ tests = testGroup "Rename"
         rename doc (Position 3 8) "baz"
     , goldenWithRename "Import hiding" "ImportHiding" $ \doc ->
         rename doc (Position 0 22) "hiddenFoo"
-    , ignoreForGhcVersions [GHC90, GHC92] "Record field refs broken in ghc92" $
+    , ignoreForGhcVersions [GHC90, GHC92] "Record field refs broken in ghc9" $
       goldenWithRename "Indirect Puns" "IndirectPuns" $ \doc ->
         rename doc (Position 4 23) "blah"
     , goldenWithRename "Let expression" "LetExpression" $ \doc ->
@@ -48,7 +48,7 @@ tests = testGroup "Rename"
         rename doc (Position 3 12) "baz"
     , goldenWithRename "Realigns do block indentation" "RealignDo" $ \doc ->
         rename doc (Position 0 2) "fooBarQuux"
-    , ignoreForGhcVersions [GHC90, GHC92] "Record field refs broken in ghc92" $
+    , ignoreForGhcVersions [GHC90, GHC92] "Record field refs broken in ghc9" $
       goldenWithRename "Record field" "RecordField" $ \doc ->
         rename doc (Position 6 9) "number"
     , goldenWithRename "Shadowed name" "ShadowedName" $ \doc ->

From 0ff2617e12ffb460e7f8f04ab9f11c4947ac058b Mon Sep 17 00:00:00 2001
From: Oliver Madine <30090176+OliverMadine@users.noreply.github.com>
Date: Mon, 4 Jul 2022 14:47:26 +0100
Subject: [PATCH 06/10] expand comment about indirect reference renaming

---
 plugins/hls-rename-plugin/src/Ide/Plugin/Rename.hs | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/plugins/hls-rename-plugin/src/Ide/Plugin/Rename.hs b/plugins/hls-rename-plugin/src/Ide/Plugin/Rename.hs
index 6d6e5b7ccc..cb064f16a6 100644
--- a/plugins/hls-rename-plugin/src/Ide/Plugin/Rename.hs
+++ b/plugins/hls-rename-plugin/src/Ide/Plugin/Rename.hs
@@ -68,7 +68,9 @@ renameProvider state pluginId (RenameParams (TextDocumentIdentifier uri) pos _pr
         directOldNames <- getNamesAtPos state nfp pos
         directRefs <- concat <$> mapM (refsAtName state nfp) directOldNames
 
-        -- References of references to account for punned names (Indirect references)
+        {- References in HieDB are not necessarily transitive. With `NamedFieldPuns`, we can have
+           indirect references through punned names. To find the transitive closure, we do a pass of
+           the direct references to find the references for any punned names. -}
         indirectOldNames <- find ((>1) . Prelude.length) <$>
             mapM (uncurry (getNamesAtPos state) . locToFilePos) directRefs
         let oldNames = fromMaybe [] indirectOldNames ++ directOldNames

From 517a648c5bf0ac31b799254d4638aff1f6770c4a Mon Sep 17 00:00:00 2001
From: Oliver Madine <30090176+OliverMadine@users.noreply.github.com>
Date: Tue, 5 Jul 2022 14:08:44 +0100
Subject: [PATCH 07/10] fix: find all punned references

---
 plugins/hls-rename-plugin/src/Ide/Plugin/Rename.hs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/plugins/hls-rename-plugin/src/Ide/Plugin/Rename.hs b/plugins/hls-rename-plugin/src/Ide/Plugin/Rename.hs
index cb064f16a6..f47895b05c 100644
--- a/plugins/hls-rename-plugin/src/Ide/Plugin/Rename.hs
+++ b/plugins/hls-rename-plugin/src/Ide/Plugin/Rename.hs
@@ -71,9 +71,9 @@ renameProvider state pluginId (RenameParams (TextDocumentIdentifier uri) pos _pr
         {- References in HieDB are not necessarily transitive. With `NamedFieldPuns`, we can have
            indirect references through punned names. To find the transitive closure, we do a pass of
            the direct references to find the references for any punned names. -}
-        indirectOldNames <- find ((>1) . Prelude.length) <$>
+        indirectOldNames <- concat . filter ((>1) . Prelude.length) <$>
             mapM (uncurry (getNamesAtPos state) . locToFilePos) directRefs
-        let oldNames = fromMaybe [] indirectOldNames ++ directOldNames
+        let oldNames = indirectOldNames ++ directOldNames
         refs <- HS.fromList . concat <$> mapM (refsAtName state nfp) oldNames
 
         -- Validate rename

From 923587e419cf57f9c403919f22bd5ce751b13a2a Mon Sep 17 00:00:00 2001
From: Oliver Madine <30090176+OliverMadine@users.noreply.github.com>
Date: Tue, 5 Jul 2022 17:54:37 +0100
Subject: [PATCH 08/10] test: ignore record field pun test for ghc > 9

---
 plugins/hls-rename-plugin/test/Main.hs | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/plugins/hls-rename-plugin/test/Main.hs b/plugins/hls-rename-plugin/test/Main.hs
index ee5487669e..478996e467 100644
--- a/plugins/hls-rename-plugin/test/Main.hs
+++ b/plugins/hls-rename-plugin/test/Main.hs
@@ -21,7 +21,8 @@ tests = testGroup "Rename"
         rename doc (Position 0 15) "Op"
     , goldenWithRename "Exported function" "ExportedFunction" $ \doc ->
         rename doc (Position 2 1) "quux"
-    , goldenWithRename "Field Puns" "FieldPuns" $ \doc ->
+    , ignoreForGhcVersions [GHC90, GHC92] "Record field refs broken for ghc > 9" $
+      goldenWithRename "Field Puns" "FieldPuns" $ \doc ->
         rename doc (Position 7 13) "bleh"
     , goldenWithRename "Function argument" "FunctionArgument" $ \doc ->
         rename doc (Position 3 4) "y"
@@ -35,7 +36,7 @@ tests = testGroup "Rename"
         rename doc (Position 3 8) "baz"
     , goldenWithRename "Import hiding" "ImportHiding" $ \doc ->
         rename doc (Position 0 22) "hiddenFoo"
-    , ignoreForGhcVersions [GHC90, GHC92] "Record field refs broken in ghc9" $
+    , ignoreForGhcVersions [GHC90, GHC92] "Record field refs broken for ghc > 9" $
       goldenWithRename "Indirect Puns" "IndirectPuns" $ \doc ->
         rename doc (Position 4 23) "blah"
     , goldenWithRename "Let expression" "LetExpression" $ \doc ->
@@ -48,7 +49,7 @@ tests = testGroup "Rename"
         rename doc (Position 3 12) "baz"
     , goldenWithRename "Realigns do block indentation" "RealignDo" $ \doc ->
         rename doc (Position 0 2) "fooBarQuux"
-    , ignoreForGhcVersions [GHC90, GHC92] "Record field refs broken in ghc9" $
+    , ignoreForGhcVersions [GHC90, GHC92] "Record field refs broken for ghc > 9" $
       goldenWithRename "Record field" "RecordField" $ \doc ->
         rename doc (Position 6 9) "number"
     , goldenWithRename "Shadowed name" "ShadowedName" $ \doc ->

From 37f817670b2b22e5d44befa9a734200392fb9ab6 Mon Sep 17 00:00:00 2001
From: Oliver Madine <30090176+OliverMadine@users.noreply.github.com>
Date: Wed, 6 Jul 2022 15:24:04 +0100
Subject: [PATCH 09/10] docs: mention test in indirect pun explaination

---
 plugins/hls-rename-plugin/src/Ide/Plugin/Rename.hs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/plugins/hls-rename-plugin/src/Ide/Plugin/Rename.hs b/plugins/hls-rename-plugin/src/Ide/Plugin/Rename.hs
index f47895b05c..6d7dbea5d5 100644
--- a/plugins/hls-rename-plugin/src/Ide/Plugin/Rename.hs
+++ b/plugins/hls-rename-plugin/src/Ide/Plugin/Rename.hs
@@ -70,7 +70,8 @@ renameProvider state pluginId (RenameParams (TextDocumentIdentifier uri) pos _pr
 
         {- References in HieDB are not necessarily transitive. With `NamedFieldPuns`, we can have
            indirect references through punned names. To find the transitive closure, we do a pass of
-           the direct references to find the references for any punned names. -}
+           the direct references to find the references for any punned names.
+           See the `IndirectPuns` test for an example. -}
         indirectOldNames <- concat . filter ((>1) . Prelude.length) <$>
             mapM (uncurry (getNamesAtPos state) . locToFilePos) directRefs
         let oldNames = indirectOldNames ++ directOldNames

From 797cea05683262eeb2fad5de69f539f214858fd5 Mon Sep 17 00:00:00 2001
From: Oliver Madine <30090176+OliverMadine@users.noreply.github.com>
Date: Wed, 6 Jul 2022 15:25:25 +0100
Subject: [PATCH 10/10] link issue for ignored record field rename tests

---
 plugins/hls-rename-plugin/test/Main.hs | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/plugins/hls-rename-plugin/test/Main.hs b/plugins/hls-rename-plugin/test/Main.hs
index 478996e467..21151dec1a 100644
--- a/plugins/hls-rename-plugin/test/Main.hs
+++ b/plugins/hls-rename-plugin/test/Main.hs
@@ -15,13 +15,17 @@ main = defaultTestRunner tests
 renamePlugin :: PluginDescriptor IdeState
 renamePlugin = Rename.descriptor "rename"
 
+-- See https://github.com/wz1000/HieDb/issues/45
+recordConstructorIssue :: String
+recordConstructorIssue = "HIE references for record fields incorrect with GHC versions >= 9"
+
 tests :: TestTree
 tests = testGroup "Rename"
     [ goldenWithRename "Data constructor" "DataConstructor" $ \doc ->
         rename doc (Position 0 15) "Op"
     , goldenWithRename "Exported function" "ExportedFunction" $ \doc ->
         rename doc (Position 2 1) "quux"
-    , ignoreForGhcVersions [GHC90, GHC92] "Record field refs broken for ghc > 9" $
+    , ignoreForGhcVersions [GHC90, GHC92] recordConstructorIssue $
       goldenWithRename "Field Puns" "FieldPuns" $ \doc ->
         rename doc (Position 7 13) "bleh"
     , goldenWithRename "Function argument" "FunctionArgument" $ \doc ->
@@ -36,7 +40,7 @@ tests = testGroup "Rename"
         rename doc (Position 3 8) "baz"
     , goldenWithRename "Import hiding" "ImportHiding" $ \doc ->
         rename doc (Position 0 22) "hiddenFoo"
-    , ignoreForGhcVersions [GHC90, GHC92] "Record field refs broken for ghc > 9" $
+    , ignoreForGhcVersions [GHC90, GHC92] recordConstructorIssue $
       goldenWithRename "Indirect Puns" "IndirectPuns" $ \doc ->
         rename doc (Position 4 23) "blah"
     , goldenWithRename "Let expression" "LetExpression" $ \doc ->
@@ -49,7 +53,7 @@ tests = testGroup "Rename"
         rename doc (Position 3 12) "baz"
     , goldenWithRename "Realigns do block indentation" "RealignDo" $ \doc ->
         rename doc (Position 0 2) "fooBarQuux"
-    , ignoreForGhcVersions [GHC90, GHC92] "Record field refs broken for ghc > 9" $
+    , ignoreForGhcVersions [GHC90, GHC92] recordConstructorIssue $
       goldenWithRename "Record field" "RecordField" $ \doc ->
         rename doc (Position 6 9) "number"
     , goldenWithRename "Shadowed name" "ShadowedName" $ \doc ->