Skip to content

Commit bdb31c9

Browse files
vidit-odvidit-od
committed
Adding testcases
1 parent aba72d9 commit bdb31c9

14 files changed

+182
-38
lines changed

‎plugins/hls-rename-plugin/src/Ide/Plugin/Rename.hs‎

Lines changed: 49 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import qualified Development.IDE.GHC.ExactPrint as E
4141
import Development.IDE.Plugin.CodeAction
4242
import Development.IDE.Spans.AtPoint
4343
import Development.IDE.Types.Location
44+
import GHC (isGoodSrcSpan)
4445
import GHC.Iface.Ext.Types (HieAST (..),
4546
HieASTs (..),
4647
NodeOrigin (..),
@@ -54,7 +55,9 @@ import Ide.Types
5455
import qualified Language.LSP.Protocol.Lens as L
5556
import Language.LSP.Protocol.Message
5657
import Language.LSP.Protocol.Types
57-
58+
#if MIN_VERSION_ghc(9,8,0)
59+
import qualified GHC.Types.Name.Occurrence as OccName
60+
#endif
5861
instance Hashable (Mod a) where hash n = hash (unMod n)
5962

6063
descriptor :: Recorder (WithPriority E.Log) -> PluginId -> PluginDescriptor IdeState
@@ -142,9 +145,21 @@ renameProvider state pluginId (RenameParams _prog (TextDocumentIdentifier uri) p
142145
-- Perform rename
143146
let newName = mkTcOcc $ T.unpack newNameText
144147
filesRefs = collectWith locToUri refs
148+
#if MIN_VERSION_ghc(9,8,0)
149+
-- GHC 9.8+ stores field labels and their selectors in different OccName
150+
-- namespaces, breaking equality checks. Expand to cover both variants.
151+
oldOccNames = HS.fromList $ concatMap (expandOcc . nameOccName) oldNames
152+
where
153+
expandOcc occ
154+
| occNameSpace occ == OccName.varName = [occ, mkOccNameFS (fieldName (occNameFS occ)) (occNameFS occ)]
155+
| isFieldNameSpace (occNameSpace occ) = [occ, mkOccNameFS OccName.varName (occNameFS occ)]
156+
| otherwise = [occ]
157+
#else
158+
oldOccNames = HS.fromList $ map nameOccName oldNames
159+
#endif
145160
getFileEdit (uri, locations) = do
146161
verTxtDocId <- liftIO $ runAction "rename: getVersionedTextDoc" state $ getVersionedTextDoc (TextDocumentIdentifier uri)
147-
getSrcEdit state verTxtDocId (replaceRefs newName locations)
162+
getSrcEdit state verTxtDocId (replaceRefs newName locations oldOccNames)
148163
fileEdits <- mapM getFileEdit filesRefs
149164
pure $ InL $ fold fileEdits
150165

@@ -214,10 +229,10 @@ getSrcEdit state verTxtDocId updatePs = do
214229
replaceRefs ::
215230
OccName ->
216231
HashSet Location ->
232+
HashSet OccName ->
217233
ParsedSource ->
218234
ParsedSource
219-
replaceRefs newName refs = everywhere $
220-
-- there has to be a better way...
235+
replaceRefs newName refs oldOccNames = everywhere $
221236
mkT (replaceLoc @AnnListItem) `extT`
222237
-- replaceLoc @AnnList `extT` -- not needed
223238
-- replaceLoc @AnnParen `extT` -- not needed
@@ -228,8 +243,11 @@ replaceRefs newName refs = everywhere $
228243
where
229244
replaceLoc :: forall an. LocatedAn an RdrName -> LocatedAn an RdrName
230245
replaceLoc (L srcSpan oldRdrName)
231-
| isRef (locA srcSpan) = L srcSpan $ replace oldRdrName
246+
| isRef (locA srcSpan)
247+
, isTarget oldRdrName
248+
= L srcSpan $ replace oldRdrName
232249
replaceLoc lOldRdrName = lOldRdrName
250+
233251
replace :: RdrName -> RdrName
234252
replace (Qual modName _) = Qual modName newName
235253
replace _ = Unqual newName
@@ -239,6 +257,10 @@ replaceRefs newName refs = everywhere $
239257
Just loc -> loc `HS.member` refs
240258
Nothing -> False
241259

260+
-- Only replace RdrNames whose OccName matches a rename target, preventing
261+
-- co-located field selectors from being incorrectly renamed.
262+
isTarget :: RdrName -> Bool
263+
isTarget rdrName = rdrNameOcc rdrName `HS.member` oldOccNames
242264
---------------------------------------------------------------------------------------------------
243265
-- Reference finding
244266

@@ -250,38 +272,28 @@ refsAtName ::
250272
Name ->
251273
ExceptT PluginError m [Location]
252274
refsAtName state nfp targetName = do
253-
-- Get local references from current file's HieAST
254-
ast <- handleGetHieAst state nfp
255-
let localRefs = nameLocs targetName ast
256-
257-
-- Query HieDb for global matches (by OccName)
258-
ShakeExtras{withHieDb} <- liftIO $ runAction "Rename.HieDb" state getShakeExtras
259-
dbCandidates <- liftIO $ withHieDb $ \hieDb ->
260-
fmap (mapMaybe rowToLoc) $
261-
case nameModule_maybe targetName of
262-
Just mod -> findReferences hieDb True (nameOccName targetName) (Just $ moduleName mod) (Just $ moduleUnit mod) []
263-
Nothing -> findReferences hieDb True (nameOccName targetName) Nothing Nothing []
264-
265-
-- Filter candidates by exact Name identity
266-
filteredDbRefs <- filterM (matchesExactName state targetName) dbCandidates
267-
pure $ localRefs ++ filteredDbRefs
268-
269-
matchesExactName ::
270-
MonadIO m =>
271-
IdeState ->
272-
Name ->
273-
Location ->
274-
ExceptT PluginError m Bool
275-
matchesExactName state targetName loc = do
276-
(file, pos) <- locToFilePos loc
277-
namesAtPos <- getNamesAtPos state file pos
278-
pure $ targetName `elem` namesAtPos
279-
280-
nameLocs :: Name -> HieAstResult -> [Location]
281-
nameLocs name (HAR _ _ rm _ _) =
282-
concatMap (map (realSrcSpanToLocation . fst))
283-
(M.lookup (Right name) rm)
284-
275+
HAR{refMap} <- handleGetHieAst state nfp
276+
277+
let localRefs =
278+
case M.lookup (Right targetName) refMap of
279+
Nothing -> []
280+
Just spans -> [ realSrcSpanToLocation sp | (sp, _) <- spans]
281+
282+
let defLoc = nameSrcSpan targetName
283+
defLocations = case srcSpanToLocation defLoc of
284+
Just loc | isGoodSrcSpan defLoc -> [loc]
285+
_ -> []
286+
287+
-- Only query HieDb if it's a global name
288+
globalRefs <-
289+
case nameModule_maybe targetName of
290+
Nothing -> pure []
291+
Just mod -> do
292+
ShakeExtras{withHieDb} <- liftIO $ runAction "Rename.HieDb" state getShakeExtras
293+
liftIO $ withHieDb $ \hieDb ->
294+
fmap (mapMaybe rowToLoc) $ findReferences hieDb True (nameOccName targetName) (Just $ moduleName mod) (Just $ moduleUnit mod) []
295+
296+
pure (defLocations ++ localRefs ++ globalRefs)
285297
---------------------------------------------------------------------------------------------------
286298
-- Util
287299

‎plugins/hls-rename-plugin/test/Main.hs‎

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,12 +111,78 @@ tests = testGroup "Rename"
111111

112112
-- Make sure renaming succeeds
113113
rename doc (Position 3 0) "foo'"
114+
, goldenWithCrossModuleRename True "Cross Module (Declaration)" "CrossMaster" "CrossFunctionClient" (Position 2 2) "fooRenamed"
115+
, goldenWithCrossModuleRename True "Cross Module (Referenced)" "CrossFunctionClient" "CrossMaster" (Position 4 11) "crossfooRenamed"
116+
, goldenWithCrossModuleRename True "Cross Module Qualified (Declaration)" "CrossMaster" "CrossQualifiedClient" (Position 2 2) "newFoo"
117+
, goldenWithCrossModuleRename True "Cross Module Qualified (Referenced)" "CrossQualifiedClient" "CrossMaster" (Position 4 22) "crossfooRenamed"
118+
, goldenWithCrossModuleSession True "Cross Error : No export list" "CrossMasterTwo" "CrossFunctionClientTwo" $ \masterDoc _clientDoc -> do
119+
let expectedError = TResponseError
120+
(InR ErrorCodes_InvalidParams)
121+
"rename: Invalid Params: Cannot rename symbol: module has no explicit export list and the symbol is referenced from other modules."
122+
Nothing
123+
124+
renameExpectError expectedError masterDoc (Position 2 0) "ImpossibleRename"
125+
, goldenWithCrossModuleSession False "Cross Error : Cross Moduel disabled" "CrossMaster" "CrossFunctionError" $ \masterDoc _clientDoc -> do
126+
let expectedError = TResponseError
127+
(InR ErrorCodes_InternalError)
128+
"rename: Internal Error: Cross-module rename is disabled."
129+
Nothing
130+
131+
renameExpectError expectedError masterDoc (Position 2 0) "ImpossibleRename"
114132
]
115133

116134
goldenWithRename :: TestName-> FilePath -> (TextDocumentIdentifier -> Session ()) -> TestTree
117135
goldenWithRename title path act =
118136
goldenWithHaskellDoc (def { plugins = M.fromList [("rename", def { plcConfig = "crossModule" .= True })] })
119-
renamePlugin title testDataDir path "expected" "hs" act
137+
renamePlugin title testDataDir path "expected" "hs"
138+
$ \_ -> do
139+
doc <- openDoc (path <.> "hs") "haskell"
140+
_ <- getDocumentSymbols doc
141+
_ <- waitForBuildQueue
142+
act doc
143+
144+
goldenWithCrossModuleRename :: Bool -> TestName -> FilePath -> FilePath -> Position -> String -> TestTree
145+
goldenWithCrossModuleRename crossModuleEnabled title primaryFile secondaryFile pos newName =
146+
goldenWithHaskellDoc
147+
(def { plugins = M.fromList [("rename", def { plcConfig = "crossModule" .= crossModuleEnabled })] })
148+
renamePlugin
149+
title
150+
testDataDir
151+
secondaryFile
152+
"expected"
153+
"hs"
154+
$ \_ -> do
155+
156+
primaryDoc <- openDoc (primaryFile <.> "hs") "haskell"
157+
secondaryDoc <- openDoc (secondaryFile <.> "hs") "haskell"
158+
159+
_ <- getDocumentSymbols primaryDoc
160+
_ <- getDocumentSymbols secondaryDoc
161+
_ <- waitForBuildQueue
162+
163+
164+
rename primaryDoc pos newName
165+
166+
goldenWithCrossModuleSession :: Bool -> TestName -> FilePath -> FilePath -> (TextDocumentIdentifier -> TextDocumentIdentifier -> Session ()) -> TestTree
167+
goldenWithCrossModuleSession crossModuleEnabled title primaryFile secondaryFile action =
168+
goldenWithHaskellDoc
169+
(def { plugins = M.fromList [("rename", def { plcConfig = "crossModule" .= crossModuleEnabled })] })
170+
renamePlugin
171+
title
172+
testDataDir
173+
secondaryFile
174+
"expected"
175+
"hs"
176+
$ \_ -> do
177+
178+
primaryDoc <- openDoc (primaryFile <.> "hs") "haskell"
179+
secondaryDoc <- openDoc (secondaryFile <.> "hs") "haskell"
180+
181+
_ <- getDocumentSymbols primaryDoc
182+
_ <- getDocumentSymbols secondaryDoc
183+
_ <- waitForBuildQueue
184+
185+
action primaryDoc secondaryDoc
120186

121187
renameExpectError :: TResponseError Method_TextDocumentRename -> TextDocumentIdentifier -> Position -> Text -> Session ()
122188
renameExpectError expectedError doc pos newName = do

‎plugins/hls-rename-plugin/test/testdata/CrossFunctionClient.expected.hs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
module CrossFunctionClient where
2+
3+
import CrossMaster
4+
5+
bar = fooRenamed

‎plugins/hls-rename-plugin/test/testdata/CrossFunctionClient.hs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
module CrossFunctionClient where
2+
3+
import CrossMaster
4+
5+
bar = crossfoo

‎plugins/hls-rename-plugin/test/testdata/CrossFunctionClientTwo.expected.hs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
module CrossFunctionClientTwo where
2+
3+
import CrossMasterTwo
4+
5+
bar = crossfooTwo

‎plugins/hls-rename-plugin/test/testdata/CrossFunctionClientTwo.hs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
module CrossFunctionClientTwo where
2+
3+
import CrossMasterTwo
4+
5+
bar = crossfooTwo

‎plugins/hls-rename-plugin/test/testdata/CrossFunctionError.expected.hs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
module CrossFunctionError where
2+
3+
import CrossMaster
4+
5+
bar = crossfoo

‎plugins/hls-rename-plugin/test/testdata/CrossFunctionError.hs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
module CrossFunctionError where
2+
3+
import CrossMaster
4+
5+
bar = crossfoo

‎plugins/hls-rename-plugin/test/testdata/CrossMaster.expected.hs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
module CrossMaster (crossfooRenamed) where
2+
3+
crossfooRenamed :: Int
4+
crossfooRenamed = 42

‎plugins/hls-rename-plugin/test/testdata/CrossMaster.hs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
module CrossMaster (crossfoo) where
2+
3+
crossfoo :: Int
4+
crossfoo = 42

0 commit comments

Comments
 (0)