Skip to content

Cross Module renaming #4845

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vidit-od wants to merge 2 commits into
base: master
Choose a base branch
from
+313 −62
Open

Cross Module renaming #4845

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aba72d9
crossModule renaming
vidit-od Feb 14, 2026
bdb31c9
Adding testcases
vidit-od Feb 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
203 changes: 155 additions & 48 deletions plugins/hls-rename-plugin/src/Ide/Plugin/Rename.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ module Ide.Plugin.Rename (descriptor, E.Log) where

import Control.Lens ((^.))
import Control.Monad
import Control.Monad.Except (ExceptT, throwError)
import Control.Monad.Except (ExceptT, MonadError,
throwError)
import Control.Monad.IO.Class (MonadIO, liftIO)
import Control.Monad.Trans.Class (lift)
import Data.Either (rights)
Expand Down Expand Up @@ -45,9 +46,7 @@ import GHC.Iface.Ext.Types (HieAST (..),
NodeOrigin (..),
SourcedNodeInfo (..))
import GHC.Iface.Ext.Utils (generateReferencesMap)
import HieDb ((:.) (..))
import HieDb.Query
import HieDb.Types (RefRow (refIsGenerated))
import Ide.Plugin.Error
import Ide.Plugin.Properties
import Ide.PluginUtils
Expand Down Expand Up @@ -86,6 +85,8 @@ prepareRenameProvider state _pluginId (PrepareRenameParams (TextDocumentIdentifi
renameProvider :: PluginMethodHandler IdeState Method_TextDocumentRename
renameProvider state pluginId (RenameParams _prog (TextDocumentIdentifier uri) pos newNameText) = do
nfp <- getNormalizedFilePathE uri
crossModuleEnabled <- liftIO $ runAction "rename: config" state $ usePropertyAction #crossModule pluginId properties
pm <- runActionE "Rename.GetParsedModule" state (useE GetParsedModule nfp)
directOldNames <- getNamesAtPos state nfp pos
directRefs <- concat <$> mapM (refsAtName state nfp) directOldNames

Expand All @@ -99,16 +100,43 @@ renameProvider state pluginId (RenameParams _prog (TextDocumentIdentifier uri) p
where
matchesDirect n = occNameFS (nameOccName n) `elem` directFS
directFS = map (occNameFS . nameOccName) directOldNames

case oldNames of
-- There were no Names at given position (e.g. rename triggered within a comment or on a keyword)
[] -> throwError $ PluginInvalidParams "No symbol to rename at given position"
_ -> do
refs <- HS.fromList . concat <$> mapM (refsAtName state nfp) oldNames
refs' <- HS.fromList . concat <$> mapM (refsAtName state nfp) oldNames
exportRefs <- exportNameLocs pm oldNames
isExported <- or <$> mapM (isNameExplicitExported pm) oldNames
let refs = HS.union refs' (HS.fromList exportRefs)
currentModule = fmap unLoc $ hsmodName $ unLoc $ pm_parsed_source pm
Copy link
Collaborator

@wz1000 wz1000 Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get this from the ModSummary, will allow you to get rid of the Nothing case.

Copy link
Collaborator Author

@vidit-od vidit-od Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am not sure if this is logical / why this is happening but i tried doing what the review recommends by replacing the highlighted code with this code

currentModule = ms_mod $ pm_mod_summary pm
                isLocallyDefined name =
                    case nameModule_maybe name of
                        Nothing -> True
                        Just nameModule -> nameModule == currentModule

doing this made a lot of testcases flaky. i have tried running almost all of the testcases numerous times and find no logic to why they fail. below is the list of testcases that showed flaky nature;

  • "Data constructor with fields"
  • "Function name"
  • "Realigns do block indentation"

i dont know why this would happen but for all of the above cases we randomly get the below error
"Cannot rename symbol: module has no explicit export list and the symbol is referenced from other modules."

reverting this block to the older version made all of the cases work fine ( Confirmed this multiple times as well )
any guesses why this would happen ?

isLocallyDefined name =
case (nameModule_maybe name, currentModule) of
(Just nameModule, Just curMod) -> moduleName nameModule == curMod
-- No module means local
(Nothing, _) -> True
-- Has module but current has none = not local
(Just _, Nothing) -> False
renamingLocalDeclaration = not (null directOldNames) && not (null oldNames) && all isLocallyDefined oldNames

-- We have to show CrossModule Disabled error ONLY when
-- 1. CrossModule is Disabled
-- 2. User Tries to rename Exported variable
-- We still allow local variable renaming in Disabled CrossModule mode.
when (not crossModuleEnabled && ((not renamingLocalDeclaration) || isExported)) $ throwError $ PluginInternalError "Cross-module rename is disabled."

-- if CrossModule renaming requires Explicit Export list
-- if variable is imported somewhere else && No explicit export => ERROR
-- if variable is locally used => No ERROR
let hasExplicitExportList = isJust (hsmodExports (unLoc $ pm_parsed_source pm))
refFiles <- forM (HS.toList refs) $ \loc -> do
(file, _) <- locToFilePos loc
pure file
let hasExternalRefs = any (/= nfp) refFiles
when ( crossModuleEnabled && not hasExplicitExportList && hasExternalRefs && renamingLocalDeclaration ) $ throwError $ PluginInvalidParams
"Cannot rename symbol: module has no explicit export list and the symbol is referenced from other modules."
Comment on lines +133 to +139
Copy link
Collaborator Author

@vidit-od vidit-od Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are the conditions that define when to show the mentioned error.

CurrentModule -> isLocallyDefined -> renamingLocalDeclaration -> causing the error ?

could this be a possible reason ?


-- Validate rename
crossModuleEnabled <- liftIO $ runAction "rename: config" state $ usePropertyAction #crossModule pluginId properties
unless crossModuleEnabled $ failWhenImportOrExport state nfp refs oldNames
-- Indirect names are assumed safe once the direct ones are
when (any isBuiltInSyntax oldNames) $ throwError $ PluginInternalError "Invalid rename of built-in syntax"

-- Perform rename
Expand All @@ -120,25 +148,49 @@ renameProvider state pluginId (RenameParams _prog (TextDocumentIdentifier uri) p
fileEdits <- mapM getFileEdit filesRefs
pure $ InL $ fold fileEdits

-- | Limit renaming across modules.
failWhenImportOrExport ::
IdeState ->
NormalizedFilePath ->
HashSet Location ->
[Name] ->
ExceptT PluginError (HandlerM config) ()
failWhenImportOrExport state nfp refLocs names = do
pm <- runActionE "Rename.GetParsedModule" state
(useE GetParsedModule nfp)
-- | Check if a name is exported from the module
-- Crossmodule Renaming happens only if names are Explicit Exported
isNameExplicitExported ::
Monad m =>
ParsedModule ->
Name ->
ExceptT PluginError m Bool
isNameExplicitExported pm name = do
let hsMod = unLoc $ pm_parsed_source pm
case (unLoc <$> hsmodName hsMod, hsmodExports hsMod) of
(mbModName, _) | not $ any (\n -> nameIsLocalOrFrom (replaceModName n mbModName) n) names
-> throwError $ PluginInternalError "Renaming of an imported name is unsupported"
(_, Just (L _ exports)) | any ((`HS.member` refLocs) . unsafeSrcSpanToLoc . getLoc) exports
-> throwError $ PluginInternalError "Renaming of an exported name is unsupported"
(Just _, Nothing) -> throwError $ PluginInternalError "Explicit export list required for renaming"
_ -> pure ()

case hsmodExports hsMod of
Nothing -> pure False
Just exports -> do
let exportedOccNames = getExportedOccNames exports
nameOcc = nameOccName name
pure $ nameOcc `elem` exportedOccNames

-- | Extract all OccNames from an export list
getExportedOccNames ::
XRec GhcPs [LIE GhcPs] ->
[OccName]
getExportedOccNames exports =
concatMap extractFromExport (unLoc exports)
where
extractFromExport ::
LIE GhcPs ->
[OccName]
extractFromExport lie = case unLocA lie of
#if MIN_VERSION_ghc(9,10,0)
IEVar _ ieWrapped _ -> handle ieWrapped
IEThingAbs _ ieWrapped _ -> handle ieWrapped
IEThingAll _ ieWrapped _ -> handle ieWrapped
IEThingWith _ ieWrapped _ _ _ -> handle ieWrapped
#else
IEVar _ ieWrapped -> handle ieWrapped
IEThingAbs _ ieWrapped -> handle ieWrapped
IEThingAll _ ieWrapped -> handle ieWrapped
IEThingWith _ ieWrapped _ _ -> handle ieWrapped
#endif
IEModuleContents{} -> []
_ -> []
where
handle ieWrapped = maybeToList $ fmap rdrNameOcc $ unwrapIEWrappedName (unLoc ieWrapped)
---------------------------------------------------------------------------------------------------
-- Source renaming

Expand Down Expand Up @@ -183,7 +235,9 @@ replaceRefs newName refs = everywhere $
replace _ = Unqual newName

isRef :: SrcSpan -> Bool
isRef = (`HS.member` refs) . unsafeSrcSpanToLoc
isRef srcSpan = case srcSpanToLocation srcSpan of
Just loc -> loc `HS.member` refs
Nothing -> False

---------------------------------------------------------------------------------------------------
-- Reference finding
Expand All @@ -195,23 +249,33 @@ refsAtName ::
NormalizedFilePath ->
Name ->
ExceptT PluginError m [Location]
refsAtName state nfp name = do
ShakeExtras{withHieDb} <- liftIO $ runAction "Rename.HieDb" state getShakeExtras
refsAtName state nfp targetName = do
-- Get local references from current file's HieAST
ast <- handleGetHieAst state nfp
dbRefs <- case nameModule_maybe name of
Nothing -> pure []
Just mod -> liftIO $ mapMaybe rowToLoc <$> withHieDb (\hieDb ->
-- See Note [Generated references]
filter (\(refRow HieDb.:. _) -> refIsGenerated refRow) <$>
findReferences
hieDb
True
(nameOccName name)
(Just $ moduleName mod)
(Just $ moduleUnit mod)
[fromNormalizedFilePath nfp]
)
pure $ nameLocs name ast ++ dbRefs
let localRefs = nameLocs targetName ast

-- Query HieDb for global matches (by OccName)
ShakeExtras{withHieDb} <- liftIO $ runAction "Rename.HieDb" state getShakeExtras
dbCandidates <- liftIO $ withHieDb $ \hieDb ->
fmap (mapMaybe rowToLoc) $
case nameModule_maybe targetName of
Just mod -> findReferences hieDb True (nameOccName targetName) (Just $ moduleName mod) (Just $ moduleUnit mod) []
Nothing -> findReferences hieDb True (nameOccName targetName) Nothing Nothing []

-- Filter candidates by exact Name identity
filteredDbRefs <- filterM (matchesExactName state targetName) dbCandidates
pure $ localRefs ++ filteredDbRefs

matchesExactName ::
MonadIO m =>
IdeState ->
Name ->
Location ->
ExceptT PluginError m Bool
matchesExactName state targetName loc = do
(file, pos) <- locToFilePos loc
namesAtPos <- getNamesAtPos state file pos
pure $ targetName `elem` namesAtPos

nameLocs :: Name -> HieAstResult -> [Location]
nameLocs name (HAR _ _ rm _ _) =
Expand Down Expand Up @@ -272,19 +336,62 @@ getNamesAtPoint' hf pos =
locToUri :: Location -> Uri
locToUri (Location uri _) = uri

unsafeSrcSpanToLoc :: SrcSpan -> Location
unsafeSrcSpanToLoc srcSpan =
srcSpanToLocE :: MonadError PluginError m => SrcSpan -> m Location
srcSpanToLocE srcSpan =
case srcSpanToLocation srcSpan of
Nothing -> error "Invalid conversion from UnhelpfulSpan to Location"
Just location -> location
Nothing -> throwError $ PluginInternalError "Invalid SrcSpan conversion"
Just loc -> pure loc

locToFilePos :: Monad m => Location -> ExceptT PluginError m (NormalizedFilePath, Position)
locToFilePos (Location uri (Range pos _)) = (,pos) <$> getNormalizedFilePathE uri

replaceModName :: Name -> Maybe ModuleName -> Module
replaceModName name mbModName =
mkModule (moduleUnit $ nameModule name) (fromMaybe (mkModuleName "Main") mbModName)
-- | Collect locations of simple exported identifiers (IEVar / IEName).
-- Only supports variable exports; complex export forms are rejected.
exportNameLocs ::
ParsedModule ->
[Name] ->
ExceptT PluginError (HandlerM config) [Location]
exportNameLocs pm names = do
let hsMod = unLoc $ pm_parsed_source pm

case hsmodExports hsMod of
Nothing -> pure []
Just exports ->
fmap concat $ forM (unLoc exports) $ \export ->
case unLocA export of
#if MIN_VERSION_ghc(9,10,0)
IEVar _ ieWrapped _ -> matchWrapped (getLoc export) ieWrapped
#else
IEVar _ ieWrapped -> matchWrapped (getLoc export) ieWrapped
#endif
IEThingAll{} -> unsupported
IEThingWith{} -> unsupported
IEModuleContents{} -> unsupported
IEThingAbs{} -> unsupported
IEGroup{} -> unsupported
IEDoc{} -> unsupported
IEDocNamed{} -> unsupported
where
unsupported = throwError $ PluginInternalError "Renaming is unsupported for complex export forms"

matchWrapped :: SrcSpan -> LIEWrappedName GhcPs -> ExceptT PluginError (HandlerM config) [Location]
matchWrapped l ieWrapped =
case unwrapIEWrappedName (unLoc ieWrapped) of
Just rdr
| any (matchesRdr rdr) names
-> do
loc <- srcSpanToLocE l
pure [loc]
_ -> pure []

matchesRdr rdr name = occNameFS (rdrNameOcc rdr) == occNameFS (nameOccName name)

-- | Extract a RdrName from an IEWrappedName when possible.
unwrapIEWrappedName :: IEWrappedName GhcPs -> Maybe RdrName
unwrapIEWrappedName ie =
case ie of
IEName _ (L _ rdr) -> Just rdr
_ -> Nothing
---------------------------------------------------------------------------------------------------
-- Config

Expand Down
Loading