Skip to content

Commit 64e1158

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

14 files changed

+177
-37
lines changed

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

Lines changed: 44 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,13 @@ 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 (..),
4748
SourcedNodeInfo (..))
4849
import GHC.Iface.Ext.Utils (generateReferencesMap)
50+
import qualified GHC.Types.Name.Occurrence as OccName
4951
import HieDb.Query
5052
import Ide.Plugin.Error
5153
import Ide.Plugin.Properties
@@ -142,9 +144,18 @@ renameProvider state pluginId (RenameParams _prog (TextDocumentIdentifier uri) p
142144
-- Perform rename
143145
let newName = mkTcOcc $ T.unpack newNameText
144146
filesRefs = collectWith locToUri refs
147+
148+
-- GHC 9.8+ stores field labels and their selectors in different OccName
149+
-- namespaces, breaking equality checks. Expand to cover both variants.
150+
oldOccNames = HS.fromList $ concatMap (expandOcc . nameOccName) oldNames
151+
where
152+
expandOcc occ
153+
| occNameSpace occ == OccName.varName = [occ, mkOccNameFS (fieldName (occNameFS occ)) (occNameFS occ)]
154+
| isFieldNameSpace (occNameSpace occ) = [occ, mkOccNameFS OccName.varName (occNameFS occ)]
155+
| otherwise = [occ]
145156
getFileEdit (uri, locations) = do
146157
verTxtDocId <- liftIO $ runAction "rename: getVersionedTextDoc" state $ getVersionedTextDoc (TextDocumentIdentifier uri)
147-
getSrcEdit state verTxtDocId (replaceRefs newName locations)
158+
getSrcEdit state verTxtDocId (replaceRefs newName locations oldOccNames)
148159
fileEdits <- mapM getFileEdit filesRefs
149160
pure $ InL $ fold fileEdits
150161

@@ -214,10 +225,10 @@ getSrcEdit state verTxtDocId updatePs = do
214225
replaceRefs ::
215226
OccName ->
216227
HashSet Location ->
228+
HashSet OccName ->
217229
ParsedSource ->
218230
ParsedSource
219-
replaceRefs newName refs = everywhere $
220-
-- there has to be a better way...
231+
replaceRefs newName refs oldOccNames = everywhere $
221232
mkT (replaceLoc @AnnListItem) `extT`
222233
-- replaceLoc @AnnList `extT` -- not needed
223234
-- replaceLoc @AnnParen `extT` -- not needed
@@ -228,8 +239,11 @@ replaceRefs newName refs = everywhere $
228239
where
229240
replaceLoc :: forall an. LocatedAn an RdrName -> LocatedAn an RdrName
230241
replaceLoc (L srcSpan oldRdrName)
231-
| isRef (locA srcSpan) = L srcSpan $ replace oldRdrName
242+
| isRef (locA srcSpan)
243+
, isTarget oldRdrName
244+
= L srcSpan $ replace oldRdrName
232245
replaceLoc lOldRdrName = lOldRdrName
246+
233247
replace :: RdrName -> RdrName
234248
replace (Qual modName _) = Qual modName newName
235249
replace _ = Unqual newName
@@ -239,6 +253,10 @@ replaceRefs newName refs = everywhere $
239253
Just loc -> loc `HS.member` refs
240254
Nothing -> False
241255

256+
-- Only replace RdrNames whose OccName matches a rename target, preventing
257+
-- co-located field selectors from being incorrectly renamed.
258+
isTarget :: RdrName -> Bool
259+
isTarget rdrName = rdrNameOcc rdrName `HS.member` oldOccNames
242260
---------------------------------------------------------------------------------------------------
243261
-- Reference finding
244262

@@ -250,38 +268,28 @@ refsAtName ::
250268
Name ->
251269
ExceptT PluginError m [Location]
252270
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-
271+
HAR{refMap} <- handleGetHieAst state nfp
272+
273+
let localRefs =
274+
case M.lookup (Right targetName) refMap of
275+
Nothing -> []
276+
Just spans -> [ realSrcSpanToLocation sp | (sp, _) <- spans]
277+
278+
let defLoc = nameSrcSpan targetName
279+
defLocations = case srcSpanToLocation defLoc of
280+
Just loc | isGoodSrcSpan defLoc -> [loc]
281+
_ -> []
282+
283+
-- Only query HieDb if it's a global name
284+
globalRefs <-
285+
case nameModule_maybe targetName of
286+
Nothing -> pure []
287+
Just mod -> do
288+
ShakeExtras{withHieDb} <- liftIO $ runAction "Rename.HieDb" state getShakeExtras
289+
liftIO $ withHieDb $ \hieDb ->
290+
fmap (mapMaybe rowToLoc) $ findReferences hieDb True (nameOccName targetName) (Just $ moduleName mod) (Just $ moduleUnit mod) []
291+
292+
pure (defLocations ++ localRefs ++ globalRefs)
285293
---------------------------------------------------------------------------------------------------
286294
-- Util
287295

‎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)