Skip to content

9.4 support for rename plugin #3417

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

Merged
merged 1 commit into from
Dec 27, 2022
Merged

9.4 support for rename plugin #3417

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ jobs:
name: Test hls-call-hierarchy-plugin test suite
run: cabal test hls-call-hierarchy-plugin --test-options="$TEST_OPTS" || LSP_TEST_LOG_COLOR=0 LSP_TEST_LOG_MESSAGES=true LSP_TEST_LOG_STDERR=true cabal test hls-call-hierarchy-plugin --test-options="$TEST_OPTS"

- if: matrix.test && matrix.os != 'windows-latest' && matrix.ghc != '9.4.2' && matrix.ghc != '9.4.3'
- if: matrix.test && matrix.os != 'windows-latest'
name: Test hls-rename-plugin test suite
run: cabal test hls-rename-plugin --test-options="$TEST_OPTS" || LSP_TEST_LOG_COLOR=0 LSP_TEST_LOG_MESSAGES=true LSP_TEST_LOG_STDERR=true cabal test hls-rename-plugin --test-options="$TEST_OPTS"

Expand Down
2 changes: 1 addition & 1 deletion haskell-language-server.cabal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ common refineImports
cpp-options: -Dhls_refineImports

common rename
if flag(rename) && (impl(ghc < 9.4.1) || flag(ignore-plugins-ghc-bounds))
if flag(rename)
build-depends: hls-rename-plugin ^>= 1.0
cpp-options: -Dhls_rename

Expand Down
9 changes: 1 addition & 8 deletions plugins/hls-rename-plugin/hls-rename-plugin.cabal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@ source-repository head
location: https://github.com/haskell/haskell-language-server.git

library
if impl(ghc >= 9.3)
buildable: False
else
buildable: True
exposed-modules: Ide.Plugin.Rename
hs-source-dirs: src
build-depends:
Expand All @@ -36,6 +32,7 @@ library
, ghcide ^>=1.8 || ^>= 1.9
, hashable
, hiedb
, hie-compat
, hls-plugin-api ^>= 1.3 || ^>=1.4 || ^>= 1.5 || ^>= 1.6
, hls-refactor-plugin
, lsp
Expand All @@ -49,10 +46,6 @@ library
default-language: Haskell2010

test-suite tests
if impl(ghc >= 9.3)
buildable: False
else
buildable: True
type: exitcode-stdio-1.0
default-language: Haskell2010
hs-source-dirs: test
Expand Down
25 changes: 23 additions & 2 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 @@ -8,6 +8,7 @@
{-# LANGUAGE RankNTypes #-}
{-# LANGUAGE ScopedTypeVariables #-}
{-# LANGUAGE TypeApplications #-}
{-# LANGUAGE RecordWildCards #-}

module Ide.Plugin.Rename (descriptor, E.Log) where

Expand All @@ -21,11 +22,13 @@ import Control.Monad.IO.Class
import Control.Monad.Trans.Class
import Control.Monad.Trans.Except
import Data.Generics
import Data.Bifunctor (first)
import Data.Hashable
import Data.HashSet (HashSet)
import qualified Data.HashSet as HS
import Data.List.Extra hiding (length)
import qualified Data.Map as M
import qualified Data.Set as S
import Data.Maybe
import Data.Mod.Word
import qualified Data.Text as T
Expand All @@ -51,6 +54,7 @@ import Ide.PluginUtils
import Ide.Types
import Language.LSP.Server
import Language.LSP.Types
import Compat.HieTypes

instance Hashable (Mod a) where hash n = hash (unMod n)

Expand All @@ -74,7 +78,10 @@ renameProvider state pluginId (RenameParams (TextDocumentIdentifier uri) pos _pr
See the `IndirectPuns` test for an example. -}
indirectOldNames <- concat . filter ((>1) . Prelude.length) <$>
mapM (uncurry (getNamesAtPos state) . locToFilePos) directRefs
let oldNames = indirectOldNames ++ directOldNames
let oldNames = (filter matchesDirect indirectOldNames) ++ directOldNames
matchesDirect n = occNameFS (nameOccName n) `elem` directFS
where
directFS = map (occNameFS. nameOccName) directOldNames
Comment on lines +81 to +84
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this necessary? Is there a case that we were missing?

The only case I am aware of for indirect names is with record field puns, but these should always "match" the direct names since they are, well, puns.

Looks good otherwise - many thanks

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If there are implicit dictionary names (eg $fShow for code like show 1) then we don't want to attempt to rename those.

Copy link
Contributor

Choose a reason for hiding this comment

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

You should mention that you need to filter stuff out since GHC-9.4 because now there are implicit dictionary names like $fShow which we don't want to attempt to rename.

refs <- HS.fromList . concat <$> mapM (refsAtName state nfp) oldNames

-- Validate rename
Expand Down Expand Up @@ -220,7 +227,21 @@ handleGetHieAst ::
ExceptT String m (HieAstResult, PositionMapping)
handleGetHieAst state nfp = handleMaybeM
("No AST for file: " ++ show nfp)
(liftIO $ runAction "Rename.GetHieAst" state $ useWithStale GetHieAst nfp)
(liftIO $ fmap (fmap (first removeGenerated)) $ runAction "Rename.GetHieAst" state $ useWithStale GetHieAst nfp)

-- | We don't want to rename in code generated by GHC as this gives false positives.
-- So we restrict the HIE file to remove all the generated code.
removeGenerated :: HieAstResult -> HieAstResult
removeGenerated HAR{..} = HAR{hieAst = go hieAst,..}
where
go :: HieASTs a -> HieASTs a
go hf =
#if MIN_VERSION_ghc(9,2,1)
HieASTs (fmap goAst (getAsts hf))
goAst (Node nsi sp xs) = Node (SourcedNodeInfo $ M.restrictKeys (getSourcedNodeInfo nsi) (S.singleton SourceInfo)) sp (map goAst xs)
#else
hf
#endif

handleUriToNfp :: (Monad m) => Uri -> ExceptT String m NormalizedFilePath
handleUriToNfp uri = handleMaybe
Expand Down