on master help sign in
Prevent incorrect detection of continuations of comment series
commit991bf8e24d9adf17a8461521ff62a6d4bd0709d5 authorMark Karpov <markkarpov92@gmail.com> dateThu 30 Apr 2020, 13:44 (6 years ago) parent22839ed
10 files changed+116−58
CHANGELOG.md+4 data/examples/declaration/data/with-comment-out.hs+4 data/examples/declaration/data/with-comment.hs+5 default.nix−1 expected-failures/brick.txt−5 expected-failures/pandoc.txt−6 src/Ormolu/Diff.hs+10 src/Ormolu/Parser.hs+1−1 src/Ormolu/Parser/CommentStream.hs+79−35 src/Ormolu/Printer/Comments.hs+13−10
CHANGELOG.md+4
@@ -6,6 +6,10 @@
66* Fixed an idempotence issue with inline comments in tuples and parentheses.
77 [Issue 450](https://github.com/tweag/ormolu/issues/450).
88
9+* Fixed an idempotence issue when certain comments where picked up as
10+ “continuation” of a series of comments [Issue
11+ 449](https://github.com/tweag/ormolu/issues/449).
12+
913* Renamed the `--check-idempotency` flag to `--check-idempotence`.
1014 Apparently only the latter is correct.
1115
data/examples/declaration/data/with-comment-out.hsnew file+4
@@ -0,0 +1,4 @@
1+data A
2+ = B -- C
3+ | -- D
4+ E
data/examples/declaration/data/with-comment.hsnew file+5
@@ -0,0 +1,5 @@
1+data A =
2+ B -- C
3+
4+ -- D
5+ | E
default.nix−1
@@ -30,7 +30,6 @@ let
3030 expectedFailures = [
3131 "Agda"
3232 "aws"
33− "brick"
3433 "distributed-process"
3534 "esqueleto"
3635 "fay"
expected-failures/brick.txtdeleted−5
@@ -1,5 +0,0 @@
1−Formatting is not idempotent:
2− src/Brick/Widgets/List.hs<rendered>:573:3
3− before: "t at i\n in -- If t"
4− after: "t at i\n -"
5−Please, consider reporting the bug.
expected-failures/pandoc.txt−6
@@ -7,9 +7,3 @@ Formatting is not idempotent:
77 before: ".softbreak <$ endlin"
88 after: ".softbreak\n <$ en"
99Please, consider reporting the bug.
10−
11−Formatting is not idempotent:
12− src/Text/Pandoc/Writers/Docx.hs<rendered>:994:25
13− before: " -- w:p\n "
14− after: " -- w:p\n "
15−Please, consider reporting the bug.
src/Ormolu/Diff.hs+10
@@ -15,6 +15,7 @@ import qualified Data.Text as T
1515import qualified FastString as GHC
1616import GHC
1717import Ormolu.Imports (sortImports)
18+import Ormolu.Parser.CommentStream
1819import Ormolu.Parser.Result
1920import Ormolu.Utils
2021
@@ -69,6 +70,7 @@ matchIgnoringSrcSpans = genericQuery
6970 gzipWithQ
7071 ( genericQuery
7172 `extQ` srcSpanEq
73+ `extQ` commentEq
7274 `extQ` sourceTextEq
7375 `extQ` hsDocStringEq
7476 `extQ` importDeclQualifiedStyleEq
@@ -79,6 +81,14 @@ matchIgnoringSrcSpans = genericQuery
7981 | otherwise = Different []
8082 srcSpanEq :: SrcSpan -> GenericQ Diff
8183 srcSpanEq _ _ = Same
84+ commentEq :: Comment -> GenericQ Diff
85+ commentEq (Comment _ x) d =
86+ case cast d :: Maybe Comment of
87+ Nothing -> Different []
88+ Just (Comment _ y) ->
89+ if x == y
90+ then Same
91+ else Different []
8292 sourceTextEq :: SourceText -> GenericQ Diff
8393 sourceTextEq _ _ = Same
8494 importDeclQualifiedStyleEq :: ImportDeclQualifiedStyle -> GenericQ Diff
src/Ormolu/Parser.hs+1−1
@@ -97,7 +97,7 @@ parseModule Config {..} path rawInput = liftIO $ do
9797 Just err -> Left err
9898 Nothing ->
9999 let (stackHeader, shebangs, pragmas, comments) =
100− mkCommentStream extraComments pstate
100+ mkCommentStream input extraComments pstate
101101 in Right
102102 ParseResult
103103 { prParsedSource = hsModule,
src/Ormolu/Parser/CommentStream.hs+79−35
@@ -4,11 +4,16 @@
44
55-- | Functions for working with comment stream.
66module Ormolu.Parser.CommentStream
7− ( CommentStream (..),
8− Comment (..),
7+ ( -- * Comment stream
8+ CommentStream (..),
99 mkCommentStream,
10− isMultilineComment,
1110 showCommentStream,
11+
12+ -- * Comment
13+ Comment (..),
14+ unComment,
15+ hasAtomsBefore,
16+ isMultilineComment,
1217 )
1318where
1419
@@ -25,20 +30,20 @@ import Ormolu.Parser.Shebang
2530import Ormolu.Utils (showOutputable)
2631import SrcLoc
2732
33+----------------------------------------------------------------------------
34+-- Comment stream
35+
2836-- | A stream of 'RealLocated' 'Comment's in ascending order with respect to
2937-- beginning of corresponding spans.
3038newtype CommentStream = CommentStream [RealLocated Comment]
3139 deriving (Eq, Data, Semigroup, Monoid)
3240
33−-- | A wrapper for a single comment. The 'NonEmpty' list inside contains
34−-- lines of multiline comment @{- … -}@ or just single item\/line otherwise.
35−newtype Comment = Comment (NonEmpty String)
36− deriving (Eq, Show, Data)
37−
3841-- | Create 'CommentStream' from 'GHC.PState'. The pragmas and shebangs are
3942-- removed from the 'CommentStream'. Shebangs are only extracted from the
4043-- comments that come from the first argument.
4144mkCommentStream ::
45+ -- | Original input
46+ String ->
4247 -- | Extra comments to include
4348 [Located String] ->
4449 -- | Parser state to use for comment extraction
@@ -49,14 +54,14 @@ mkCommentStream ::
4954 [([RealLocated Comment], Pragma)],
5055 CommentStream
5156 )
52−mkCommentStream extraComments pstate =
57+mkCommentStream input extraComments pstate =
5358 ( mstackHeader,
5459 shebangs,
5560 pragmas,
5661 CommentStream comments
5762 )
5863 where
59− (comments, pragmas) = extractPragmas rawComments1
64+ (comments, pragmas) = extractPragmas input rawComments1
6065 (rawComments1, mstackHeader) = extractStackHeader rawComments0
6166 rawComments0 =
6267 L.sortOn (realSrcSpanStart . getRealSrcSpan) . mapMaybe toRealSpan $
@@ -67,10 +72,6 @@ mkCommentStream extraComments pstate =
6772 (GHC.annotations_comments pstate)
6873 (shebangs, otherExtraComments) = extractShebangs extraComments
6974
70−-- | Is this comment multiline-style?
71−isMultilineComment :: Comment -> Bool
72−isMultilineComment (Comment (x :| _)) = "{-" `L.isPrefixOf` x
73−
7475-- | Pretty-print a 'CommentStream'.
7576showCommentStream :: CommentStream -> String
7677showCommentStream (CommentStream xs) =
@@ -80,28 +81,67 @@ showCommentStream (CommentStream xs) =
8081 showComment (GHC.L l str) = showOutputable l ++ " " ++ show str
8182
8283----------------------------------------------------------------------------
83−-- Helpers
84+-- Comment
85+
86+-- | A wrapper for a single comment. The 'Bool' indicates whether there were
87+-- atoms before beginning of the comment in the original input. The
88+-- 'NonEmpty' list inside contains lines of multiline comment @{- … -}@ or
89+-- just single item\/line otherwise.
90+data Comment = Comment Bool (NonEmpty String)
91+ deriving (Eq, Show, Data)
8492
8593-- | Normalize comment string. Sometimes one multi-line comment is turned
8694-- into several lines for subsequent outputting with correct indentation for
8795-- each line.
88−mkComment :: RealLocated String -> RealLocated Comment
89−mkComment (L l s) =
90− L l . Comment . fmap dropTrailing $
91− if "{-" `L.isPrefixOf` s
92− then case NE.nonEmpty (lines s) of
93− Nothing -> s :| []
94− Just (x :| xs) ->
95− let getIndent y =
96− if all isSpace y
97− then startIndent
98− else length (takeWhile isSpace y)
99− n = minimum (startIndent : fmap getIndent xs)
100− in x :| (drop n <$> xs)
101− else s :| []
96+mkComment ::
97+ -- | Lines of original input with their indices
98+ [(Int, String)] ->
99+ -- | Raw comment string
100+ RealLocated String ->
101+ -- | Remaining lines of original input and the constructed 'Comment'
102+ ([(Int, String)], RealLocated Comment)
103+mkComment ls (L l s) = (ls', comment)
102104 where
105+ comment =
106+ L l . Comment atomsBefore . fmap dropTrailing $
107+ if "{-" `L.isPrefixOf` s
108+ then case NE.nonEmpty (lines s) of
109+ Nothing -> s :| []
110+ Just (x :| xs) ->
111+ let getIndent y =
112+ if all isSpace y
113+ then startIndent
114+ else length (takeWhile isSpace y)
115+ n = minimum (startIndent : fmap getIndent xs)
116+ in x :| (drop n <$> xs)
117+ else s :| []
118+ (atomsBefore, ls') =
119+ case dropWhile ((< commentLine) . fst) ls of
120+ [] -> (False, [])
121+ ((_, i) : ls'') ->
122+ case take 2 (dropWhile isSpace i) of
123+ "--" -> (False, ls'')
124+ "{-" -> (False, ls'')
125+ _ -> (True, ls'')
103126 dropTrailing = L.dropWhileEnd isSpace
104127 startIndent = srcSpanStartCol l - 1
128+ commentLine = srcSpanStartLine l
129+
130+-- | Get a collection of lines from a 'Comment'.
131+unComment :: Comment -> NonEmpty String
132+unComment (Comment _ xs) = xs
133+
134+-- | Check whether the 'Comment' had some non-whitespace atoms in front of
135+-- it in the original input.
136+hasAtomsBefore :: Comment -> Bool
137+hasAtomsBefore (Comment atomsBefore _) = atomsBefore
138+
139+-- | Is this comment multiline-style?
140+isMultilineComment :: Comment -> Bool
141+isMultilineComment (Comment _ (x :| _)) = "{-" `L.isPrefixOf` x
142+
143+----------------------------------------------------------------------------
144+-- Helpers
105145
106146-- | Get a 'String' from 'GHC.AnnotationComment'.
107147unAnnotationComment :: GHC.AnnotationComment -> Maybe String
@@ -130,25 +170,29 @@ extractStackHeader ::
130170extractStackHeader = \case
131171 [] -> ([], Nothing)
132172 (x : xs) ->
133− let comment = mkComment x
173+ let comment = snd (mkComment [] x)
134174 in if isStackHeader (unRealSrcSpan comment)
135175 then (xs, Just comment)
136176 else (x : xs, Nothing)
137177 where
138− isStackHeader (Comment (x :| _)) =
178+ isStackHeader (Comment _ (x :| _)) =
139179 "stack" `L.isPrefixOf` dropWhile isSpace (drop 2 x)
140180
141181-- | Extract pragmas and their associated comments.
142182extractPragmas ::
183+ String ->
143184 [RealLocated String] ->
144185 ([RealLocated Comment], [([RealLocated Comment], Pragma)])
145−extractPragmas = go id id
186+extractPragmas input = go initialLs id id
146187 where
147− go csSoFar pragmasSoFar = \case
188+ initialLs = zip [1 ..] (lines input)
189+ go ls csSoFar pragmasSoFar = \case
148190 [] -> (csSoFar [], pragmasSoFar [])
149191 (x : xs) ->
150192 case parsePragma (unRealSrcSpan x) of
151− Nothing -> go (csSoFar . (mkComment x :)) pragmasSoFar xs
193+ Nothing ->
194+ let (ls', x') = mkComment ls x
195+ in go ls' (csSoFar . (x' :)) pragmasSoFar xs
152196 Just pragma ->
153197 let combined = (csSoFar [], pragma)
154− in go id (pragmasSoFar . (combined :)) xs
198+ in go ls id (pragmasSoFar . (combined :)) xs
src/Ormolu/Printer/Comments.hs+13−10
@@ -13,7 +13,6 @@ module Ormolu.Printer.Comments
1313where
1414
1515import Control.Monad
16−import Data.Coerce (coerce)
1716import qualified Data.List.NonEmpty as NE
1817import qualified Data.Text as T
1918import Ormolu.Parser.CommentStream
@@ -191,7 +190,7 @@ commentFollowsElt ::
191190 -- | Comment to test
192191 RealLocated Comment ->
193192 Bool
194−commentFollowsElt ref mnSpn meSpn mlastMark (L l _) =
193+commentFollowsElt ref mnSpn meSpn mlastMark (L l comment) =
195194 -- A comment follows a AST element if all 4 conditions are satisfied:
196195 goesAfter
197196 && logicallyFollows
@@ -232,12 +231,16 @@ commentFollowsElt ref mnSpn meSpn mlastMark (L l _) =
232231 >= abs (startColumn ref - startColumn l)
233232 )
234233 continuation =
235− case mlastMark of
236− Just (HaddockSpan _ spn) ->
237− srcSpanEndLine spn + 1 == srcSpanStartLine l
238− Just (CommentSpan spn) ->
239− srcSpanEndLine spn + 1 == srcSpanStartLine l
240− _ -> False
234+ -- A comment is a continuation when it doesn't have non-whitespace
235+ -- lexemes in front of it and goes right after the previous comment.
236+ not (hasAtomsBefore comment)
237+ && ( case mlastMark of
238+ Just (HaddockSpan _ spn) ->
239+ srcSpanEndLine spn + 1 == srcSpanStartLine l
240+ Just (CommentSpan spn) ->
241+ srcSpanEndLine spn + 1 == srcSpanStartLine l
242+ _ -> False
243+ )
241244 lastInEnclosing =
242245 case meSpn of
243246 -- When there is no enclosing element, return false
@@ -259,7 +262,7 @@ spitCommentNow spn comment = do
259262 . sequence_
260263 . NE.intersperse newline
261264 . fmap (txt . T.pack)
262− . coerce
265+ . unComment
263266 $ comment
264267 setSpanMark (CommentSpan spn)
265268
@@ -275,6 +278,6 @@ spitCommentPending position spn comment = do
275278 . sequence_
276279 . NE.toList
277280 . fmap (registerPendingCommentLine position . T.pack)
278− . coerce
281+ . unComment
279282 $ comment
280283 setSpanMark (CommentSpan spn)