Skip to content

Commit 217573f

Browse files
Kobayashimergify[bot]
Kobayashi
andauthored
Improve haddock comments (#3207)
* remove 'buildable: False' in cabal * remove constraint on ghc-exactprint * wip * revert HaddockComments.hs * generate haddock comments for constructors * fix tests * restore constraints * make it compatible with ghc 9.0 * add more tests * add comments & fix dp calculation for inline case * add kokobd to codeowners of haddock-comments plugin * fix a comment * rephrase some comments to make them clearer Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
1 parent 6f9b435 commit 217573f

File tree

12 files changed

+294
-74
lines changed

12 files changed

+294
-74
lines changed

CODEOWNERS

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
/plugins/hls-floskell-plugin @Ailrun
1717
/plugins/hls-fourmolu-plugin @georgefst
1818
/plugins/hls-gadt-plugin @July541
19-
/plugins/hls-haddock-comments-plugin @berberman
19+
/plugins/hls-haddock-comments-plugin @berberman @kokobd
2020
/plugins/hls-hlint-plugin @jneira @eddiemundo
2121
/plugins/hls-module-name-plugin
2222
/plugins/hls-ormolu-plugin @georgefst

plugins/hls-haddock-comments-plugin/hls-haddock-comments-plugin.cabal

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,10 @@ library
2626
buildable: False
2727
else
2828
buildable: True
29-
exposed-modules: Ide.Plugin.HaddockComments
29+
exposed-modules:
30+
Ide.Plugin.HaddockComments
31+
Ide.Plugin.HaddockComments.Data
32+
Ide.Plugin.HaddockComments.Prelude
3033
hs-source-dirs: src
3134
ghc-options:
3235
-Wall -Wno-name-shadowing -Wredundant-constraints
@@ -43,6 +46,8 @@ library
4346
, lsp-types
4447
, text
4548
, unordered-containers
49+
, transformers
50+
, mtl
4651

4752
default-language: Haskell2010
4853
default-extensions:

plugins/hls-haddock-comments-plugin/src/Ide/Plugin/HaddockComments.hs

Lines changed: 33 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
{-# LANGUAGE CPP #-}
2+
{-# LANGUAGE DuplicateRecordFields #-}
23
{-# LANGUAGE ExistentialQuantification #-}
34
{-# LANGUAGE FlexibleContexts #-}
45
{-# LANGUAGE NamedFieldPuns #-}
@@ -8,8 +9,9 @@
89

910
module Ide.Plugin.HaddockComments (descriptor) where
1011

11-
import Control.Monad (join)
12+
import Control.Monad (join, when)
1213
import Control.Monad.IO.Class
14+
import Control.Monad.Trans.Class (lift)
1315
import qualified Data.HashMap.Strict as HashMap
1416
import qualified Data.Map as Map
1517
import qualified Data.Text as T
@@ -19,6 +21,8 @@ import Development.IDE.GHC.Compat.ExactPrint
1921
import Development.IDE.GHC.ExactPrint (GetAnnotatedParsedSource (..))
2022
import qualified Development.IDE.GHC.ExactPrint as E
2123
import Development.IDE.Plugin.CodeAction
24+
import Ide.Plugin.HaddockComments.Data (genForDataDecl)
25+
import Ide.Plugin.HaddockComments.Prelude
2226
import Ide.Types
2327
import Language.Haskell.GHC.ExactPrint
2428
import Language.Haskell.GHC.ExactPrint.Types hiding (GhcPs)
@@ -40,47 +44,49 @@ codeActionProvider ideState _pId (CodeActionParams _ _ (TextDocumentIdentifier u
4044
(join -> pm) <- liftIO $ runAction "HaddockComments.GetAnnotatedParsedSource" ideState $ use GetAnnotatedParsedSource `traverse` nfp
4145
let locDecls = hsmodDecls . unLoc . astA <$> pm
4246
anns = annsA <$> pm
43-
edits = [runGenComments gen locDecls anns range | noErr, gen <- genList]
47+
edits = [gen locDecls anns range | noErr, gen <- genList]
4448
return $ Right $ List [InR $ toAction title uri edit | (Just (title, edit)) <- edits]
4549

46-
genList :: [GenComments]
50+
genList :: [Maybe [LHsDecl GhcPs] -> Maybe Anns -> Range -> Maybe (T.Text, TextEdit)]
4751
genList =
48-
[ genForSig,
49-
genForRecord
52+
[ runGenCommentsSimple genForSig,
53+
runGenComments genForDataDecl
5054
]
5155

5256
-----------------------------------------------------------------------------
5357

54-
-- | Defines how to generate haddock comments by tweaking annotations of AST
55-
data GenComments = forall a.
56-
GenComments
57-
{ title :: T.Text,
58-
fromDecl :: HsDecl GhcPs -> Maybe a,
59-
collectKeys :: a -> [AnnKey],
60-
isFresh :: Annotation -> Bool,
61-
updateAnn :: Annotation -> Annotation,
62-
updateDeclAnn :: Annotation -> Annotation
63-
}
64-
6558
runGenComments :: GenComments -> Maybe [LHsDecl GhcPs] -> Maybe Anns -> Range -> Maybe (T.Text, TextEdit)
66-
runGenComments GenComments {..} mLocDecls mAnns range
59+
runGenComments GenComments{..} mLocDecls mAnns range
6760
| Just locDecls <- mLocDecls,
6861
Just anns <- mAnns,
69-
[(locDecl, src, x)] <- [(locDecl, l, x) | locDecl@(L l (fromDecl -> Just x)) <- locDecls, range `isIntersectWith` l],
70-
annKeys <- collectKeys x,
71-
not $ null annKeys,
72-
and $ maybe False isFresh . flip Map.lookup anns <$> annKeys,
73-
declKey <- mkAnnKey locDecl,
74-
anns' <- Map.adjust updateDeclAnn declKey $ foldr (Map.adjust updateAnn) anns annKeys,
62+
[(locDecl, src)] <- [(locDecl, l) | locDecl@(L l _) <- locDecls, range `isIntersectWith` l],
7563
Just range' <- toRange src,
76-
result <- T.strip . T.pack $ exactPrint locDecl anns' =
77-
Just (title, TextEdit range' result)
64+
Just (_, (anns', _), _) <- runTransformT anns (updateAnns locDecl),
65+
result <- T.strip . T.pack $ exactPrint locDecl anns'
66+
= Just (title, TextEdit range' result)
7867
| otherwise = Nothing
7968

69+
runGenCommentsSimple :: GenCommentsSimple -> Maybe [LHsDecl GhcPs] -> Maybe Anns -> Range -> Maybe (T.Text, TextEdit)
70+
runGenCommentsSimple GenCommentsSimple {..} = runGenComments GenComments {
71+
title = title,
72+
updateAnns = updateAnns
73+
}
74+
where
75+
updateAnns :: LHsDecl GhcPs -> TransformT Maybe ()
76+
updateAnns locDecl@(L _ decl) = do
77+
x <- lift $ fromDecl decl
78+
let annKeys = collectKeys x
79+
anns <- getAnnsT
80+
when (null annKeys || not (and $ maybe False isFresh . flip Map.lookup anns <$> annKeys)) $
81+
lift Nothing
82+
let declKey = mkAnnKey locDecl
83+
anns' = Map.adjust updateDeclAnn declKey $ foldr (Map.adjust updateAnn) anns annKeys
84+
putAnnsT anns'
85+
8086
-----------------------------------------------------------------------------
8187

82-
genForSig :: GenComments
83-
genForSig = GenComments {..}
88+
genForSig :: GenCommentsSimple
89+
genForSig = GenCommentsSimple {..}
8490
where
8591
title = "Generate signature comments"
8692

@@ -102,30 +108,6 @@ genForSig = GenComments {..}
102108
#endif
103109
dp = [(AnnComment comment, DP (0, 1)), (G AnnRarrow, DP (1, 2))]
104110

105-
genForRecord :: GenComments
106-
genForRecord = GenComments {..}
107-
where
108-
title = "Generate fields comments"
109-
110-
fromDecl (TyClD _ DataDecl {tcdDataDefn = HsDataDefn {dd_cons = cons}}) =
111-
Just [x | (L _ ConDeclH98 {con_args = x}) <- cons]
112-
fromDecl _ = Nothing
113-
114-
updateAnn x = x {annEntryDelta = DP (1, 2), annPriorComments = [(comment, DP (1, 2))]}
115-
updateDeclAnn = cleanPriorComments
116-
117-
isFresh Ann {annPriorComments} = null annPriorComments
118-
119-
collectKeys = keyFromCon
120-
121-
#if MIN_VERSION_ghc(9,2,0)
122-
comment = mkComment "-- | " (spanAsAnchor noSrcSpan)
123-
#elif MIN_VERSION_ghc(9,0,0)
124-
comment = mkComment "-- | " badRealSrcSpan
125-
#else
126-
comment = mkComment "-- | " noSrcSpan
127-
#endif
128-
129111
-----------------------------------------------------------------------------
130112

131113
toAction :: T.Text -> Uri -> TextEdit -> CodeAction
@@ -176,7 +158,4 @@ keyFromTyVar dep (L _ (HsParTy _ x)) = keyFromTyVar (succ dep) x
176158
keyFromTyVar dep (L _ (HsBangTy _ _ x)) = keyFromTyVar dep x
177159
keyFromTyVar _ _ = []
178160

179-
keyFromCon :: [HsConDeclDetails GhcPs] -> [AnnKey]
180-
keyFromCon cons = mconcat [mkAnnKey <$> xs | (RecCon (L _ xs)) <- cons]
181-
182161
-----------------------------------------------------------------------------
Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
{-# LANGUAGE CPP #-}
2+
{-# LANGUAGE DuplicateRecordFields #-}
3+
{-# LANGUAGE LambdaCase #-}
4+
{-# LANGUAGE OverloadedStrings #-}
5+
6+
module Ide.Plugin.HaddockComments.Data
7+
( genForDataDecl
8+
) where
9+
10+
import Control.Monad (unless, when)
11+
import Control.Monad.Trans.Class (lift)
12+
import Data.Data (Data)
13+
import Data.Foldable (for_)
14+
import Data.List (isPrefixOf)
15+
import qualified Data.Map.Strict as Map
16+
import Data.Maybe (fromMaybe, isJust)
17+
import Development.IDE (realSpan)
18+
import Development.IDE.GHC.Compat
19+
import Development.IDE.GHC.ExactPrint
20+
import Ide.Plugin.HaddockComments.Prelude
21+
import Language.Haskell.GHC.ExactPrint
22+
import Language.Haskell.GHC.ExactPrint.Types hiding (GhcPs)
23+
import Language.Haskell.GHC.ExactPrint.Utils (mkComment)
24+
25+
genForDataDecl :: GenComments
26+
genForDataDecl =
27+
GenComments {
28+
title = "Generate haddock comments",
29+
updateAnns = updateDataAnns
30+
}
31+
32+
updateDataAnns :: LHsDecl GhcPs -> TransformT Maybe ()
33+
updateDataAnns decl@(L declLoc (TyClD _ DataDecl {tcdDataDefn = HsDataDefn { dd_cons = cons }})) = do
34+
-- skip if all constructors and fields already have a haddock comment
35+
getAnnsT >>= (\anns -> unless (missingSomeHaddock anns cons) (lift Nothing))
36+
37+
-- visit each constructor and field
38+
addHaddockCommentsToList True declLoc (G AnnVbar) cons
39+
for_ cons $ \case
40+
L conLoc ConDeclH98 { con_args = RecCon (L _ fields) } -> addHaddockCommentsToList False conLoc (G AnnComma) fields
41+
_ -> pure ()
42+
modifyAnnsT $ Map.adjust (\ann -> ann {annPriorComments = []}) (mkAnnKey decl)
43+
updateDataAnns _ = lift Nothing
44+
45+
-- | Add haddock comments to a list of nodes
46+
addHaddockCommentsToList
47+
:: (Data a, Monad m)
48+
=> Bool -- ^ If true, for each node, use previous node in the list as the anchor. Otherwise, use the outer node
49+
-> SrcSpan -- ^ The outer node
50+
-> KeywordId -- ^ The seperator between adjacent nodes
51+
-> [Located a] -- ^ The list of nodes. Haddock comments will be added to each of them
52+
-> TransformT m ()
53+
addHaddockCommentsToList usePrevNodeAsAnchor outerLoc seperator nodes =
54+
-- If you want to understand this function, please first read this page carefully:
55+
-- https://hackage.haskell.org/package/ghc-exactprint-0.6.4/docs/Language-Haskell-GHC-ExactPrint-Delta.html
56+
-- The important part is that for DP(r,c), if r is zero, c is the offset start from the end of the previous node.
57+
-- However, if r is greater than zero, c is the offset start from the 'anchor'.
58+
-- Generally speaking, the 'anchor' is the node that "enclose" the current node. But it's not always the case.
59+
-- Sometimes 'anchor' is just the previous node. It depends on the the syntactic structure.
60+
-- For constructors, the anchor is the previous node (if there is any).
61+
-- For record fields, the anchor is always the constructor they belong to.
62+
for_ (zip nodes (Nothing: fmap Just nodes)) $ \(node, prevNode) -> do
63+
addHaddockCommentToCurrentNode <- fmap (not . fromMaybe True . flip hasHaddock node) getAnnsT
64+
-- We don't add new haddock comments to nodes with existing ones.
65+
when addHaddockCommentToCurrentNode $ do
66+
-- 'sameLineAsPrev' is a flag to determine the inline case, for example:
67+
-- data T = A { a :: Int, b :: String } | B { b :: Double }
68+
-- Note that it's a 'Maybe (Located a)', containing the previous node if the current node
69+
-- and the previous node are on the same line.
70+
--
71+
-- For the multiline case (which is the most common), we keep the original indentation of each constructor
72+
-- and field.
73+
--
74+
-- For the inline case, we use the first construcotr/field as the base, and align all following items
75+
-- to them.
76+
let sameLineAsPrev = prevNode >>= (
77+
\prevNode' -> if notSeperatedByLineEnding prevNode' node
78+
then pure prevNode'
79+
else Nothing
80+
)
81+
-- For the inline case, we need to move the seperator to the next line.
82+
-- For constructors, it's vertical bar; for fields, it's comma.
83+
-- The seperator is passed in as function argument.
84+
when (isJust sameLineAsPrev) $ modifyAnnsT $ \anns ->
85+
let newSepCol :: Annotation -> Int
86+
newSepCol ann =
87+
if usePrevNodeAsAnchor then 0 else deltaColumn (annEntryDelta ann)
88+
updateSepAnn :: Annotation -> Annotation
89+
updateSepAnn ann = ann {annsDP =
90+
Map.toList . Map.adjust (const $ DP (1, newSepCol ann)) seperator . Map.fromList $ annsDP ann}
91+
in flip (maybe anns) prevNode $ \prevNode' -> Map.adjust updateSepAnn (mkAnnKey prevNode') anns
92+
-- Calculate the real column of the anchor
93+
let anchorCol = maybe 0 srcSpanStartCol . realSpan . maybe outerLoc getLoc $
94+
if usePrevNodeAsAnchor then prevNode else Nothing
95+
-- 'dpCol' is what we will use for the current node's entry delta's column
96+
dpCol <- flip fmap getAnnsT $ \anns ->
97+
case sameLineAsPrev of
98+
Just prevNode' ->
99+
-- If the previous node is the anchor, using 0 as column will make current code align with
100+
-- the previous one.
101+
-- Otherwise, use the column of entry delta of the previous node.
102+
-- The map lookup should not fail. '2' is used as a fallback value to make sure the syntax
103+
-- is correct after the changes.
104+
if usePrevNodeAsAnchor then 0 else maybe 2 (deltaColumn . annEntryDelta)
105+
$ anns Map.!? mkAnnKey prevNode'
106+
-- We subtract the real column to get dp column.
107+
Nothing -> (maybe 2 srcSpanStartCol . realSpan $ getLoc node) - anchorCol
108+
-- Modify the current node
109+
modifyAnnsT $
110+
let updateCurrent :: Annotation -> Annotation
111+
updateCurrent ann = ann {
112+
-- If there exist non-haddock comments, we simply inherit the first one's delta pos,
113+
-- and move them two lines below, to seperate them from our newly added haddock comments
114+
-- Otherwise, inherit the node's entry delta pos.
115+
annPriorComments = case annPriorComments ann of
116+
(c, dp) : rem -> (emptyPriorHaddockComment, dp) : (c, DP (2,0)) : rem
117+
_ -> [(emptyPriorHaddockComment, annEntryDelta ann)],
118+
annEntryDelta = DP (1, dpCol)
119+
}
120+
in Map.adjust updateCurrent (mkAnnKey node)
121+
122+
-- | Determine if a list of constructor declarations is missing some haddock comments.
123+
missingSomeHaddock :: Anns -> [LConDecl GhcPs] -> Bool
124+
missingSomeHaddock anns = any $ \lcon@(L _ conDecl) -> case conDecl of
125+
ConDeclH98 { con_args = RecCon (L _ fields) } ->
126+
elem (Just False) $ hasHaddock anns lcon : fmap (hasHaddock anns) fields
127+
_ -> False -- GADT is not supported yet
128+
129+
-- | Returns 'True' if the end of the first node and the start of the second node are on the same line.
130+
notSeperatedByLineEnding :: Located a
131+
-> Located a
132+
-> Bool
133+
notSeperatedByLineEnding (L (RealSrcSpan x _) _) (L (RealSrcSpan y _) _) =
134+
srcLocLine (realSrcSpanEnd x) == srcLocLine (realSrcSpanStart y)
135+
notSeperatedByLineEnding _ _ = False
136+
137+
-- | Empty haddock, suitable for being added to 'annPriorComments'
138+
emptyPriorHaddockComment :: Comment
139+
emptyPriorHaddockComment = mkComment "-- |"
140+
#if MIN_VERSION_ghc(9,0,0)
141+
badRealSrcSpan
142+
#else
143+
noSrcSpan
144+
#endif
145+
146+
-- | Determines the given node has haddock comments attached to it.
147+
hasHaddock :: Data a => Anns -> Located a -> Maybe Bool
148+
hasHaddock anns node = fmap annHasHaddock (anns Map.!? key)
149+
where
150+
key = mkAnnKey node
151+
annHasHaddock ann =
152+
any (matchCommentPrefix priorCommentPrefix . fst) (annPriorComments ann)
153+
|| any (matchCommentPrefix followingCommentPrefix . fst) (annFollowingComments ann)
154+
|| any (keywordIdIsHaddockComment . fst) (annsDP ann)
155+
156+
-- | Checks if the given 'KeywordId' is a comment, and specifically, a haddock comment.
157+
keywordIdIsHaddockComment :: KeywordId -> Bool
158+
keywordIdIsHaddockComment (AnnComment comment) = any (`isPrefixOf` commentContents comment) (priorCommentPrefix ++ followingCommentPrefix)
159+
keywordIdIsHaddockComment _ = False
160+
161+
priorCommentPrefix :: [String]
162+
priorCommentPrefix = ["-- |", "{-|", "{- |"]
163+
164+
followingCommentPrefix :: [String]
165+
followingCommentPrefix = ["-- ^", "{-^", "{- ^"]
166+
167+
matchCommentPrefix :: [String] -> Comment -> Bool
168+
matchCommentPrefix prefix comment = any (`isPrefixOf` commentContents comment) prefix
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
{-# LANGUAGE DuplicateRecordFields #-}
2+
{-# LANGUAGE ExistentialQuantification #-}
3+
4+
module Ide.Plugin.HaddockComments.Prelude where
5+
import qualified Data.Text as T
6+
import Development.IDE.GHC.Compat
7+
import Development.IDE.GHC.ExactPrint
8+
import Language.Haskell.GHC.ExactPrint (AnnKey, Annotation)
9+
10+
-- | A more generic comments generator
11+
data GenComments = GenComments
12+
{ title :: T.Text,
13+
-- | Use 'Maybe' monad to exit early. 'Nothing' means a code action for haddock comments
14+
-- in the given context is not possible.
15+
updateAnns :: LHsDecl GhcPs -> TransformT Maybe ()
16+
}
17+
18+
-- | Defines how to generate haddock comments by tweaking annotations of AST
19+
--
20+
-- This is left here for compatibility reason, so that we don't break the existing code.
21+
data GenCommentsSimple = forall a.
22+
GenCommentsSimple
23+
{ title :: T.Text,
24+
fromDecl :: HsDecl GhcPs -> Maybe a,
25+
collectKeys :: a -> [AnnKey],
26+
isFresh :: Annotation -> Bool,
27+
updateAnn :: Annotation -> Annotation,
28+
updateDeclAnn :: Annotation -> Annotation
29+
}

plugins/hls-haddock-comments-plugin/test/Main.hs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,11 @@ tests =
3030
goldenWithHaddockComments "MultivariateFunction" Signature 4 8,
3131
goldenWithHaddockComments "QualFunction" Signature 2 10,
3232
goldenWithHaddockComments "Record" Record 7 2,
33+
goldenWithHaddockComments "Record2" Record 3 6,
34+
goldenWithHaddockComments "InlineRecord" Record 3 20,
3335
expectedNothing "ConstFunction" Signature 2 2,
3436
expectedNothing "StaleFunction" Signature 3 3,
35-
expectedNothing "StaleRecord" Record 3 12
37+
expectedNothing "StaleRecord" Record 4 9
3638
]
3739

3840
goldenWithHaddockComments :: FilePath -> GenCommentsType -> UInt -> UInt -> TestTree
@@ -54,7 +56,7 @@ data GenCommentsType = Signature | Record
5456

5557
toTitle :: GenCommentsType -> Text
5658
toTitle Signature = "Generate signature comments"
57-
toTitle Record = "Generate fields comments"
59+
toTitle Record = "Generate haddock comments"
5860

5961
caTitle :: (Command |? CodeAction) -> Maybe Text
6062
caTitle (InR CodeAction {_title}) = Just _title

0 commit comments

Comments
 (0)