Skip to content

Commit dc82156

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

14 files changed

+162
-46
lines changed

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

Lines changed: 35 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -108,16 +108,12 @@ renameProvider state pluginId (RenameParams _prog (TextDocumentIdentifier uri) p
108108
exportRefs <- exportNameLocs pm oldNames
109109
isExported <- or <$> mapM (isNameExplicitExported pm) oldNames
110110
let refs = HS.union refs' (HS.fromList exportRefs)
111-
currentModule = fmap unLoc $ hsmodName $ unLoc $ pm_parsed_source pm
111+
currentModuleFull = ms_mod $ pm_mod_summary pm
112112
isLocallyDefined name =
113-
case (nameModule_maybe name, currentModule) of
114-
(Just nameModule, Just curMod) -> moduleName nameModule == curMod
115-
-- No module means local
116-
(Nothing, _) -> True
117-
-- Has module but current has none = not local
118-
(Just _, Nothing) -> False
119-
renamingLocalDeclaration = not (null directOldNames) && not (null oldNames) && all isLocallyDefined oldNames
120-
113+
case nameModule_maybe name of
114+
Nothing -> True
115+
Just mod -> mod == currentModuleFull
116+
renamingLocalDeclaration = not (null directOldNames) && all isLocallyDefined directOldNames
121117
-- We have to show CrossModule Disabled error ONLY when
122118
-- 1. CrossModule is Disabled
123119
-- 2. User Tries to rename Exported variable
@@ -128,7 +124,7 @@ renameProvider state pluginId (RenameParams _prog (TextDocumentIdentifier uri) p
128124
-- if variable is imported somewhere else && No explicit export => ERROR
129125
-- if variable is locally used => No ERROR
130126
let hasExplicitExportList = isJust (hsmodExports (unLoc $ pm_parsed_source pm))
131-
refFiles <- forM (HS.toList refs) $ \loc -> do
127+
refFiles <- forM (HS.toList refs') $ \loc -> do
132128
(file, _) <- locToFilePos loc
133129
pure file
134130
let hasExternalRefs = any (/= nfp) refFiles
@@ -142,9 +138,10 @@ renameProvider state pluginId (RenameParams _prog (TextDocumentIdentifier uri) p
142138
-- Perform rename
143139
let newName = mkTcOcc $ T.unpack newNameText
144140
filesRefs = collectWith locToUri refs
141+
oldOccNames = HS.fromList $ map nameOccName oldNames
145142
getFileEdit (uri, locations) = do
146143
verTxtDocId <- liftIO $ runAction "rename: getVersionedTextDoc" state $ getVersionedTextDoc (TextDocumentIdentifier uri)
147-
getSrcEdit state verTxtDocId (replaceRefs newName locations)
144+
getSrcEdit state verTxtDocId (replaceRefs newName locations oldOccNames)
148145
fileEdits <- mapM getFileEdit filesRefs
149146
pure $ InL $ fold fileEdits
150147

@@ -214,10 +211,10 @@ getSrcEdit state verTxtDocId updatePs = do
214211
replaceRefs ::
215212
OccName ->
216213
HashSet Location ->
214+
HashSet OccName ->
217215
ParsedSource ->
218216
ParsedSource
219-
replaceRefs newName refs = everywhere $
220-
-- there has to be a better way...
217+
replaceRefs newName refs oldOccNames = everywhere $
221218
mkT (replaceLoc @AnnListItem) `extT`
222219
-- replaceLoc @AnnList `extT` -- not needed
223220
-- replaceLoc @AnnParen `extT` -- not needed
@@ -228,8 +225,11 @@ replaceRefs newName refs = everywhere $
228225
where
229226
replaceLoc :: forall an. LocatedAn an RdrName -> LocatedAn an RdrName
230227
replaceLoc (L srcSpan oldRdrName)
231-
| isRef (locA srcSpan) = L srcSpan $ replace oldRdrName
228+
| isRef (locA srcSpan)
229+
, isTarget oldRdrName
230+
= L srcSpan $ replace oldRdrName
232231
replaceLoc lOldRdrName = lOldRdrName
232+
233233
replace :: RdrName -> RdrName
234234
replace (Qual modName _) = Qual modName newName
235235
replace _ = Unqual newName
@@ -239,6 +239,10 @@ replaceRefs newName refs = everywhere $
239239
Just loc -> loc `HS.member` refs
240240
Nothing -> False
241241

242+
-- Only replace RdrNames whose OccName matches a rename target, preventing
243+
-- co-located field selectors from being incorrectly renamed.
244+
isTarget :: RdrName -> Bool
245+
isTarget rdrName = rdrNameOcc rdrName `HS.member` oldOccNames
242246
---------------------------------------------------------------------------------------------------
243247
-- Reference finding
244248

@@ -250,38 +254,23 @@ refsAtName ::
250254
Name ->
251255
ExceptT PluginError m [Location]
252256
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-
257+
HAR{refMap} <- handleGetHieAst state nfp
258+
259+
let localRefs =
260+
case M.lookup (Right targetName) refMap of
261+
Nothing -> []
262+
Just spans -> [ realSrcSpanToLocation sp | (sp, _) <- spans]
263+
264+
-- Only query HieDb if it's a global name
265+
globalRefs <-
266+
case nameModule_maybe targetName of
267+
Nothing -> pure []
268+
Just mod -> do
269+
ShakeExtras{withHieDb} <- liftIO $ runAction "Rename.HieDb" state getShakeExtras
270+
liftIO $ withHieDb $ \hieDb ->
271+
fmap (mapMaybe rowToLoc) $ findReferences hieDb True (nameOccName targetName) (Just $ moduleName mod) (Just $ moduleUnit mod) []
272+
273+
pure (localRefs ++ globalRefs)
285274
---------------------------------------------------------------------------------------------------
286275
-- Util
287276

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

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,13 +111,74 @@ 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 })] })
119137
renamePlugin title testDataDir path "expected" "hs" act
120138

139+
goldenWithCrossModuleRename :: Bool -> TestName -> FilePath -> FilePath -> Position -> String -> TestTree
140+
goldenWithCrossModuleRename crossModuleEnabled title primaryFile secondaryFile pos newName =
141+
goldenWithHaskellDoc
142+
(def { plugins = M.fromList [("rename", def { plcConfig = "crossModule" .= crossModuleEnabled })] })
143+
renamePlugin
144+
title
145+
testDataDir
146+
secondaryFile
147+
"expected"
148+
"hs"
149+
$ \_ -> do
150+
151+
primaryDoc <- openDoc (primaryFile <.> "hs") "haskell"
152+
secondaryDoc <- openDoc (secondaryFile <.> "hs") "haskell"
153+
154+
_ <- getDocumentSymbols primaryDoc
155+
_ <- getDocumentSymbols secondaryDoc
156+
_ <- waitForBuildQueue
157+
158+
159+
rename primaryDoc pos newName
160+
161+
goldenWithCrossModuleSession :: Bool -> TestName -> FilePath -> FilePath -> (TextDocumentIdentifier -> TextDocumentIdentifier -> Session ()) -> TestTree
162+
goldenWithCrossModuleSession crossModuleEnabled title primaryFile secondaryFile action =
163+
goldenWithHaskellDoc
164+
(def { plugins = M.fromList [("rename", def { plcConfig = "crossModule" .= crossModuleEnabled })] })
165+
renamePlugin
166+
title
167+
testDataDir
168+
secondaryFile
169+
"expected"
170+
"hs"
171+
$ \_ -> do
172+
173+
primaryDoc <- openDoc (primaryFile <.> "hs") "haskell"
174+
secondaryDoc <- openDoc (secondaryFile <.> "hs") "haskell"
175+
176+
_ <- getDocumentSymbols primaryDoc
177+
_ <- getDocumentSymbols secondaryDoc
178+
_ <- waitForBuildQueue
179+
180+
action primaryDoc secondaryDoc
181+
121182
renameExpectError :: TResponseError Method_TextDocumentRename -> TextDocumentIdentifier -> Position -> Text -> Session ()
122183
renameExpectError expectedError doc pos newName = do
123184
let params = RenameParams Nothing doc pos newName

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