diff --git a/plugins/hls-rename-plugin/src/Ide/Plugin/Rename.hs b/plugins/hls-rename-plugin/src/Ide/Plugin/Rename.hs index 6d051d96de..6d7dbea5d5 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,28 @@ 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 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. + See the `IndirectPuns` test for an example. -} + indirectOldNames <- concat . filter ((>1) . Prelude.length) <$> + mapM (uncurry (getNamesAtPos state) . locToFilePos) directRefs + let oldNames = 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 +95,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 +123,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 +139,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 +160,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 +183,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 +198,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 +240,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) diff --git a/plugins/hls-rename-plugin/test/Main.hs b/plugins/hls-rename-plugin/test/Main.hs index 66bcea6222..21151dec1a 100644 --- a/plugins/hls-rename-plugin/test/Main.hs +++ b/plugins/hls-rename-plugin/test/Main.hs @@ -15,12 +15,19 @@ 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] recordConstructorIssue $ + 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 +40,9 @@ tests = testGroup "Rename" rename doc (Position 3 8) "baz" , goldenWithRename "Import hiding" "ImportHiding" $ \doc -> rename doc (Position 0 22) "hiddenFoo" + , ignoreForGhcVersions [GHC90, GHC92] recordConstructorIssue $ + 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 -> @@ -43,7 +53,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 [GHC90, GHC92] recordConstructorIssue $ + goldenWithRename "Record field" "RecordField" $ \doc -> rename doc (Position 6 9) "number" , goldenWithRename "Shadowed name" "ShadowedName" $ \doc -> rename doc (Position 1 1) "baz" 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"