Skip to content

Commit 416ca36

Browse files
authored
Merge pull request #635 from jneira/apply-refact-exts
Leverage last apply-refact improvements in hlint plugin (include getParsedModuleWithComments in ghcide)
2 parents a4627e5 + a0b3e4d commit 416ca36

19 files changed

+227
-70
lines changed

.github/workflows/bench.yml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,11 @@ jobs:
2626
path: |
2727
~/.cabal/packages
2828
~/.cabal/store
29-
key: ${{ runner.os }}-${{ matrix.ghc }}-bench-${{ hashFiles('cabal.project') }}
29+
key: v2-${{ runner.os }}-${{ matrix.ghc }}-bench-${{ hashFiles('cabal.project') }}
3030
restore-keys: |
31-
${{ runner.os }}-${{ matrix.ghc }}-build-${{ hashFiles('cabal.project') }}
32-
${{ runner.os }}-${{ matrix.ghc }}-bench-
33-
${{ runner.os }}-${{ matrix.ghc }}
31+
v2-${{ runner.os }}-${{ matrix.ghc }}-build-${{ hashFiles('cabal.project') }}
32+
v2-${{ runner.os }}-${{ matrix.ghc }}-bench-
33+
v2-${{ runner.os }}-${{ matrix.ghc }}
3434
3535
- run: cabal update
3636

.github/workflows/test.yml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,11 +79,11 @@ jobs:
7979
path: |
8080
${{ env.CABAL_PKGS_DIR }}
8181
${{ env.CABAL_STORE_DIR }}
82-
key: ${{ runner.os }}-${{ matrix.ghc }}-build-${{ hashFiles('cabal.project') }}
82+
key: v2-${{ runner.os }}-${{ matrix.ghc }}-build-${{ hashFiles('cabal.project') }}
8383
restore-keys: |
84-
${{ runner.os }}-${{ matrix.ghc }}-bench-${{ hashFiles('cabal.project') }}
85-
${{ runner.os }}-${{ matrix.ghc }}-build-
86-
${{ runner.os }}-${{ matrix.ghc }}
84+
v2-${{ runner.os }}-${{ matrix.ghc }}-bench-${{ hashFiles('cabal.project') }}
85+
v2-${{ runner.os }}-${{ matrix.ghc }}-build-
86+
v2-${{ runner.os }}-${{ matrix.ghc }}
8787
8888
- run: cabal update
8989

cabal.project

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@ packages:
1313
./plugins/hls-haddock-comments-plugin
1414
./plugins/hls-splice-plugin
1515

16+
source-repository-package
17+
type: git
18+
location: https://github.com/mpickering/apply-refact.git
19+
tag: 4fbd3a3a9b408bd31080848feb6b78e13c3eeb6d
20+
1621
tests: true
1722

1823
package *
@@ -25,7 +30,7 @@ package ghcide
2530

2631
write-ghc-environment-files: never
2732

28-
index-state: 2021-01-07T18:06:52Z
33+
index-state: 2021-01-14T12:49:26Z
2934

3035
allow-newer:
3136
active:base,

ghcide/src/Development/IDE/Core/RuleTypes.hs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,10 @@ data LinkableType = ObjectLinkable | BCOLinkable
5454
-- | The parse tree for the file using GetFileContents
5555
type instance RuleResult GetParsedModule = ParsedModule
5656

57+
-- | The parse tree for the file using GetFileContents,
58+
-- all comments included using Opt_KeepRawTokenStream
59+
type instance RuleResult GetParsedModuleWithComments = ParsedModule
60+
5761
-- | The dependency information produced by following the imports recursively.
5862
-- This rule will succeed even if there is an error, e.g., a module could not be located,
5963
-- a module could not be parsed or an import cycle.
@@ -302,6 +306,12 @@ instance Hashable GetParsedModule
302306
instance NFData GetParsedModule
303307
instance Binary GetParsedModule
304308

309+
data GetParsedModuleWithComments = GetParsedModuleWithComments
310+
deriving (Eq, Show, Typeable, Generic)
311+
instance Hashable GetParsedModuleWithComments
312+
instance NFData GetParsedModuleWithComments
313+
instance Binary GetParsedModuleWithComments
314+
305315
data GetLocatedImports = GetLocatedImports
306316
deriving (Eq, Show, Typeable, Generic)
307317
instance Hashable GetLocatedImports

ghcide/src/Development/IDE/Core/Rules.hs

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,13 @@ module Development.IDE.Core.Rules(
2929
highlightAtPoint,
3030
getDependencies,
3131
getParsedModule,
32+
getParsedModuleWithComments,
3233
getClientConfigAction,
3334
-- * Rules
3435
CompiledLinkables(..),
3536
IsHiFileStable(..),
3637
getParsedModuleRule,
38+
getParsedModuleWithCommentsRule,
3739
getLocatedImportsRule,
3840
getDependencyInformationRule,
3941
reportImportCyclesRule,
@@ -268,9 +270,14 @@ getPackageHieFile ide mod file = do
268270
_ -> MaybeT $ return Nothing
269271
_ -> MaybeT $ return Nothing
270272

271-
-- | Parse the contents of a daml file.
273+
-- | Parse the contents of a haskell file.
272274
getParsedModule :: NormalizedFilePath -> Action (Maybe ParsedModule)
273-
getParsedModule file = use GetParsedModule file
275+
getParsedModule = use GetParsedModule
276+
277+
-- | Parse the contents of a haskell file,
278+
-- ensuring comments are preserved in annotations
279+
getParsedModuleWithComments :: NormalizedFilePath -> Action (Maybe ParsedModule)
280+
getParsedModuleWithComments = use GetParsedModuleWithComments
274281

275282
------------------------------------------------------------
276283
-- Rules
@@ -285,12 +292,15 @@ priorityGenerateCore = Priority (-1)
285292
priorityFilesOfInterest :: Priority
286293
priorityFilesOfInterest = Priority (-2)
287294

288-
-- | IMPORTANT FOR HLINT INTEGRATION:
295+
-- | WARNING:
289296
-- We currently parse the module both with and without Opt_Haddock, and
290297
-- return the one with Haddocks if it -- succeeds. However, this may not work
291-
-- for hlint, and we might need to save the one without haddocks too.
298+
-- for hlint or any client code that might need the parsed source with all
299+
-- annotations, including comments.
300+
-- For that use case you might want to use `getParsedModuleWithCommentsRule`
292301
-- See https://github.com/haskell/ghcide/pull/350#discussion_r370878197
293302
-- and https://github.com/mpickering/ghcide/pull/22#issuecomment-625070490
303+
-- GHC wiki about: https://gitlab.haskell.org/ghc/ghc/-/wikis/api-annotations
294304
getParsedModuleRule :: Rules ()
295305
getParsedModuleRule = defineEarlyCutoff $ \GetParsedModule file -> do
296306
(ms, _) <- use_ GetModSummary file
@@ -333,8 +343,10 @@ getParsedModuleRule = defineEarlyCutoff $ \GetParsedModule file -> do
333343
pure res
334344

335345
withOptHaddock :: ModSummary -> ModSummary
336-
withOptHaddock ms = ms{ms_hspp_opts= gopt_set (ms_hspp_opts ms) Opt_Haddock}
346+
withOptHaddock = withOption Opt_Haddock
337347

348+
withOption :: GeneralFlag -> ModSummary -> ModSummary
349+
withOption opt ms = ms{ms_hspp_opts= gopt_set (ms_hspp_opts ms) opt}
338350

339351
-- | Given some normal parse errors (first) and some from Haddock (second), merge them.
340352
-- Ignore Haddock errors that are in both. Demote Haddock-only errors to warnings.
@@ -348,6 +360,19 @@ mergeParseErrorsHaddock normal haddock = normal ++
348360
fixMessage x | "parse error " `T.isPrefixOf` x = "Haddock " <> x
349361
| otherwise = "Haddock: " <> x
350362

363+
-- | This rule provides a ParsedModule preserving all annotations,
364+
-- including keywords, punctuation and comments.
365+
-- So it is suitable for use cases where you need a perfect edit.
366+
getParsedModuleWithCommentsRule :: Rules ()
367+
getParsedModuleWithCommentsRule = defineEarlyCutoff $ \GetParsedModuleWithComments file -> do
368+
(ms, _) <- use_ GetModSummary file
369+
sess <- use_ GhcSession file
370+
opt <- getIdeOptions
371+
372+
let ms' = withOption Opt_KeepRawTokenStream ms
373+
374+
liftIO $ getParsedModuleDefinition (hscEnv sess) opt file ms'
375+
351376
getParsedModuleDefinition :: HscEnv -> IdeOptions -> NormalizedFilePath -> ModSummary -> IO (Maybe ByteString, ([FileDiagnostic], Maybe ParsedModule))
352377
getParsedModuleDefinition packageState opt file ms = do
353378
let fp = fromNormalizedFilePath file
@@ -974,6 +999,7 @@ mainRule = do
974999
linkables <- liftIO $ newVar emptyModuleEnv
9751000
addIdeGlobal $ CompiledLinkables linkables
9761001
getParsedModuleRule
1002+
getParsedModuleWithCommentsRule
9771003
getLocatedImportsRule
9781004
getDependencyInformationRule
9791005
reportImportCyclesRule

plugins/hls-hlint-plugin/hls-hlint-plugin.cabal

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ library
3838
, directory
3939
, extra
4040
, filepath
41+
, ghc-exactprint
4142
, ghcide
4243
, hashable
4344
, haskell-lsp
@@ -61,7 +62,7 @@ library
6162
, ghc-lib ^>= 8.10.2.20200916
6263
, ghc-lib-parser-ex ^>= 8.10
6364

64-
cpp-options: -DGHC_LIB
65+
cpp-options: -DHLINT_ON_GHC_LIB
6566

6667
ghc-options: -Wall -Wredundant-constraints -Wno-name-shadowing
6768

plugins/hls-hlint-plugin/src/Ide/Plugin/Hlint.hs

Lines changed: 79 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
{-# LANGUAGE ScopedTypeVariables #-}
12
{-# LANGUAGE CPP #-}
23
{-# LANGUAGE DeriveAnyClass #-}
34
{-# LANGUAGE DeriveGeneric #-}
@@ -33,18 +34,25 @@ import qualified Data.Text as T
3334
import qualified Data.Text.IO as T
3435
import Data.Typeable
3536
import Development.IDE
36-
import Development.IDE.Core.Rules (defineNoFile)
37+
import Development.IDE.Core.Rules (getParsedModuleWithComments, defineNoFile)
3738
import Development.IDE.Core.Shake (getDiagnostics)
3839

39-
#ifdef GHC_LIB
40+
#ifdef HLINT_ON_GHC_LIB
4041
import Data.List (nub)
41-
import "ghc-lib" GHC hiding (DynFlags(..))
42+
import "ghc-lib" GHC hiding (DynFlags(..), ms_hspp_opts)
43+
import "ghc-lib-parser" GHC.LanguageExtensions (Extension)
4244
import "ghc" GHC as RealGHC (DynFlags(..))
43-
import "ghc" HscTypes as RealGHC.HscTypes (hsc_dflags)
45+
import "ghc" HscTypes as RealGHC.HscTypes (hsc_dflags, ms_hspp_opts)
4446
import qualified "ghc" EnumSet as EnumSet
4547
import Language.Haskell.GhclibParserEx.GHC.Driver.Session as GhclibParserEx (readExtension)
48+
import System.FilePath (takeFileName)
49+
import System.IO (hPutStr, noNewlineTranslation, hSetNewlineMode, utf8, hSetEncoding, IOMode(WriteMode), withFile, hClose)
50+
import System.IO.Temp
4651
#else
4752
import Development.IDE.GHC.Compat hiding (DynFlags(..))
53+
import Language.Haskell.GHC.ExactPrint.Parsers (postParseTransform)
54+
import Language.Haskell.GHC.ExactPrint.Delta (deltaOptions)
55+
import Language.Haskell.GHC.ExactPrint.Types (Rigidity(..))
4856
#endif
4957

5058
import Ide.Logger
@@ -53,12 +61,12 @@ import Ide.Plugin.Config
5361
import Ide.PluginUtils
5462
import Language.Haskell.HLint as Hlint
5563
import Language.Haskell.LSP.Core
64+
( LspFuncs(withIndefiniteProgress),
65+
ProgressCancellable(Cancellable) )
5666
import Language.Haskell.LSP.Types
5767
import qualified Language.Haskell.LSP.Types as LSP
5868
import qualified Language.Haskell.LSP.Types.Lens as LSP
59-
import System.FilePath (takeFileName)
60-
import System.IO (hPutStr, noNewlineTranslation, hSetNewlineMode, utf8, hSetEncoding, IOMode(WriteMode), withFile, hClose)
61-
import System.IO.Temp
69+
6270
import Text.Regex.TDFA.Text()
6371
import GHC.Generics (Generic)
6472

@@ -176,7 +184,14 @@ getIdeas nfp = do
176184
fmap applyHints' (moduleEx flags)
177185

178186
where moduleEx :: ParseFlags -> Action (Maybe (Either ParseError ModuleEx))
179-
#ifdef GHC_LIB
187+
#ifndef HLINT_ON_GHC_LIB
188+
moduleEx _flags = do
189+
mbpm <- getParsedModule nfp
190+
return $ createModule <$> mbpm
191+
where createModule pm = Right (createModuleEx anns modu)
192+
where anns = pm_annotations pm
193+
modu = pm_parsed_source pm
194+
#else
180195
moduleEx flags = do
181196
mbpm <- getParsedModule nfp
182197
-- If ghc was not able to parse the module, we disable hlint diagnostics
@@ -190,20 +205,21 @@ getIdeas nfp = do
190205
Just <$> (liftIO $ parseModuleEx flags' fp contents')
191206

192207
setExtensions flags = do
193-
hsc <- hscEnv <$> use_ GhcSession nfp
194-
let dflags = hsc_dflags hsc
195-
let hscExts = EnumSet.toList (extensionFlags dflags)
196-
let hscExts' = mapMaybe (GhclibParserEx.readExtension . show) hscExts
197-
let hlintExts = nub $ enabledExtensions flags ++ hscExts'
208+
hlintExts <- getExtensions flags nfp
198209
logm $ "hlint:getIdeas:setExtensions:" ++ show hlintExts
199210
return $ flags { enabledExtensions = hlintExts }
200-
#else
201-
moduleEx _flags = do
202-
mbpm <- getParsedModule nfp
203-
return $ createModule <$> mbpm
204-
where createModule pm = Right (createModuleEx anns modu)
205-
where anns = pm_annotations pm
206-
modu = pm_parsed_source pm
211+
212+
getExtensions :: ParseFlags -> NormalizedFilePath -> Action [Extension]
213+
getExtensions pflags nfp = do
214+
dflags <- getFlags
215+
let hscExts = EnumSet.toList (extensionFlags dflags)
216+
let hscExts' = mapMaybe (GhclibParserEx.readExtension . show) hscExts
217+
let hlintExts = nub $ enabledExtensions pflags ++ hscExts'
218+
return hlintExts
219+
where getFlags :: Action DynFlags
220+
getFlags = do
221+
(modsum, _) <- use_ GetModSummary nfp
222+
return $ ms_hspp_opts modsum
207223
#endif
208224

209225
-- ---------------------------------------------------------------------
@@ -334,10 +350,18 @@ applyOneCmd lf ide (AOP uri pos title) = do
334350
applyHint :: IdeState -> NormalizedFilePath -> Maybe OneHint -> IO (Either String WorkspaceEdit)
335351
applyHint ide nfp mhint =
336352
runExceptT $ do
337-
ideas <- bimapExceptT showParseError id $ ExceptT $ liftIO $ runAction "applyHint" ide $ getIdeas nfp
353+
let runAction' :: Action a -> IO a
354+
runAction' = runAction "applyHint" ide
355+
let errorHandlers = [ Handler $ \e -> return (Left (show (e :: IOException)))
356+
, Handler $ \e -> return (Left (show (e :: ErrorCall)))
357+
]
358+
ideas <- bimapExceptT showParseError id $ ExceptT $ runAction' $ getIdeas nfp
338359
let ideas' = maybe ideas (`filterIdeas` ideas) mhint
339-
let commands = map (show &&& ideaRefactoring) ideas'
360+
let commands = map ideaRefactoring ideas'
340361
liftIO $ logm $ "applyHint:apply=" ++ show commands
362+
let fp = fromNormalizedFilePath nfp
363+
(_, mbOldContent) <- liftIO $ runAction' $ getFileContents nfp
364+
oldContent <- maybe (liftIO $ T.readFile fp) return mbOldContent
341365
-- set Nothing as "position" for "applyRefactorings" because
342366
-- applyRefactorings expects the provided position to be _within_ the scope
343367
-- of each refactoring it will apply.
@@ -353,27 +377,48 @@ applyHint ide nfp mhint =
353377
-- If we provide "applyRefactorings" with "Just (1,13)" then
354378
-- the "Redundant bracket" hint will never be executed
355379
-- because SrcSpan (1,20,??,??) doesn't contain position (1,13).
356-
let fp = fromNormalizedFilePath nfp
357-
(_, mbOldContent) <- liftIO $ runAction "hlint" ide $ getFileContents nfp
358-
oldContent <- maybe (liftIO $ T.readFile fp) return mbOldContent
359-
-- We need to save a file with last edited contents cause `apply-refact`
360-
-- doesn't expose a function taking directly contents instead a file path.
361-
-- Ideally we should try to expose that function upstream and remove this.
362-
res <- liftIO $ withSystemTempFile (takeFileName fp) $ \temp h -> do
380+
#ifdef HLINT_ON_GHC_LIB
381+
let writeFileUTF8NoNewLineTranslation file txt =
382+
withFile file WriteMode $ \h -> do
383+
hSetEncoding h utf8
384+
hSetNewlineMode h noNewlineTranslation
385+
hPutStr h (T.unpack txt)
386+
res <-
387+
liftIO $ withSystemTempFile (takeFileName fp) $ \temp h -> do
363388
hClose h
364389
writeFileUTF8NoNewLineTranslation temp oldContent
365-
(Right <$> applyRefactorings Nothing commands temp) `catches`
366-
[ Handler $ \e -> return (Left (show (e :: IOException)))
367-
, Handler $ \e -> return (Left (show (e :: ErrorCall)))
368-
]
390+
(pflags, _, _) <- runAction' $ useNoFile_ GetHlintSettings
391+
exts <- runAction' $ getExtensions pflags nfp
392+
-- We have to reparse extensions to remove the invalid ones
393+
let (enabled, disabled, _invalid) = parseExtensions $ map show exts
394+
let refactExts = map show $ enabled ++ disabled
395+
(Right <$> applyRefactorings Nothing commands temp refactExts)
396+
`catches` errorHandlers
397+
#else
398+
mbParsedModule <- liftIO $ runAction' $ getParsedModuleWithComments nfp
399+
res <-
400+
case mbParsedModule of
401+
Nothing -> throwE "Apply hint: error parsing the module"
402+
Just pm -> do
403+
let anns = pm_annotations pm
404+
let modu = pm_parsed_source pm
405+
(modsum, _) <- liftIO $ runAction' $ use_ GetModSummary nfp
406+
let dflags = ms_hspp_opts modsum
407+
-- apply-refact uses RigidLayout
408+
let rigidLayout = deltaOptions RigidLayout
409+
(anns', modu') <-
410+
ExceptT $ return $ postParseTransform (Right (anns, [], dflags, modu)) rigidLayout
411+
liftIO $ (Right <$> applyRefactorings' Nothing commands anns' modu')
412+
`catches` errorHandlers
413+
#endif
369414
case res of
370415
Right appliedFile -> do
371416
let uri = fromNormalizedUri (filePathToUri' nfp)
372417
let wsEdit = diffText' True (uri, oldContent) (T.pack appliedFile) IncludeDeletions
373418
liftIO $ logm $ "hlint:applyHint:diff=" ++ show wsEdit
374419
ExceptT $ return (Right wsEdit)
375420
Left err ->
376-
throwE (show err)
421+
throwE err
377422
where
378423
-- | If we are only interested in applying a particular hint then
379424
-- let's filter out all the irrelevant ideas
@@ -396,10 +441,3 @@ bimapExceptT f g (ExceptT m) = ExceptT (fmap h m) where
396441
h (Left e) = Left (f e)
397442
h (Right a) = Right (g a)
398443
{-# INLINE bimapExceptT #-}
399-
400-
writeFileUTF8NoNewLineTranslation :: FilePath -> T.Text -> IO()
401-
writeFileUTF8NoNewLineTranslation file txt =
402-
withFile file WriteMode $ \h -> do
403-
hSetEncoding h utf8
404-
hSetNewlineMode h noNewlineTranslation
405-
hPutStr h (T.unpack txt)

stack-8.10.1.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,15 @@ ghc-options:
1919
"$everything": -haddock
2020

2121
extra-deps:
22+
- git: https://github.com/mpickering/apply-refact.git
23+
commit: 4fbd3a3a9b408bd31080848feb6b78e13c3eeb6d
2224
- brittany-0.13.1.0
2325
- Cabal-3.0.2.0
2426
- clock-0.7.2
2527
- data-tree-print-0.1.0.2@rev:2
2628
- floskell-0.10.4
2729
- fourmolu-0.3.0.0
30+
- ghc-exactprint-0.6.3.3
2831
- ghc-lib-8.10.3.20201220
2932
- ghc-lib-parser-8.10.3.20201220
3033
- heapsize-0.3.0

0 commit comments

Comments
 (0)