From 9b3a78f0fd288cfb5bdbb60139091b5b8666d56d Mon Sep 17 00:00:00 2001 From: Brandon Chinn Date: Sat, 20 Jan 2024 15:14:00 -0800 Subject: [PATCH] Improve comment placement in if-then-else blocks --- CHANGELOG.md | 2 + .../if-single-line-functions-do-out.hs | 5 ++ .../function/if-single-line-functions-do.hs | 5 ++ .../function/if-single-line-functions-out.hs | 4 ++ .../function/if-single-line-functions.hs | 4 ++ .../if-with-comment-above-branches-out.hs | 7 +++ ...t.hs => if-with-comment-above-branches.hs} | 0 .../if-with-comment-before-do-blocks-out.hs | 10 ++++ .../if-with-comment-before-do-blocks.hs | 10 ++++ .../if-with-comment-in-branches-out.hs | 14 ++++++ ...-comment-in-branches-with-functions-out.hs | 12 +++++ ...with-comment-in-branches-with-functions.hs | 12 +++++ .../function/if-with-comment-in-branches.hs | 14 ++++++ .../if-with-comment-next-to-keyword-out.hs | 10 ++++ .../if-with-comment-next-to-keyword.hs | 10 ++++ .../value/function/if-with-comment-out.hs | 8 --- src/Ormolu/Printer/Meat/Declaration/Value.hs | 50 +++++++++++++++---- src/Ormolu/Utils.hs | 3 ++ 18 files changed, 161 insertions(+), 19 deletions(-) create mode 100644 data/examples/declaration/value/function/if-single-line-functions-do-out.hs create mode 100644 data/examples/declaration/value/function/if-single-line-functions-do.hs create mode 100644 data/examples/declaration/value/function/if-single-line-functions-out.hs create mode 100644 data/examples/declaration/value/function/if-single-line-functions.hs create mode 100644 data/examples/declaration/value/function/if-with-comment-above-branches-out.hs rename data/examples/declaration/value/function/{if-with-comment.hs => if-with-comment-above-branches.hs} (100%) create mode 100644 data/examples/declaration/value/function/if-with-comment-before-do-blocks-out.hs create mode 100644 data/examples/declaration/value/function/if-with-comment-before-do-blocks.hs create mode 100644 data/examples/declaration/value/function/if-with-comment-in-branches-out.hs create mode 100644 data/examples/declaration/value/function/if-with-comment-in-branches-with-functions-out.hs create mode 100644 data/examples/declaration/value/function/if-with-comment-in-branches-with-functions.hs create mode 100644 data/examples/declaration/value/function/if-with-comment-in-branches.hs create mode 100644 data/examples/declaration/value/function/if-with-comment-next-to-keyword-out.hs create mode 100644 data/examples/declaration/value/function/if-with-comment-next-to-keyword.hs delete mode 100644 data/examples/declaration/value/function/if-with-comment-out.hs diff --git a/CHANGELOG.md b/CHANGELOG.md index 314368ded..9529e6b96 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ * Don't error when the `JavaScriptFFI` language pragma is present. [Issue 1087](https://github.com/tweag/ormolu/issues/1087). +* Improve comment placement in if-then-else blocks. [Issue + 998](https://github.com/tweag/ormolu/issues/998). ## Ormolu 0.7.3.0 diff --git a/data/examples/declaration/value/function/if-single-line-functions-do-out.hs b/data/examples/declaration/value/function/if-single-line-functions-do-out.hs new file mode 100644 index 000000000..e150a4a5b --- /dev/null +++ b/data/examples/declaration/value/function/if-single-line-functions-do-out.hs @@ -0,0 +1,5 @@ +foo x = do + y <- quux + if x > 2 + then bar x + else baz y diff --git a/data/examples/declaration/value/function/if-single-line-functions-do.hs b/data/examples/declaration/value/function/if-single-line-functions-do.hs new file mode 100644 index 000000000..e150a4a5b --- /dev/null +++ b/data/examples/declaration/value/function/if-single-line-functions-do.hs @@ -0,0 +1,5 @@ +foo x = do + y <- quux + if x > 2 + then bar x + else baz y diff --git a/data/examples/declaration/value/function/if-single-line-functions-out.hs b/data/examples/declaration/value/function/if-single-line-functions-out.hs new file mode 100644 index 000000000..c5d83a623 --- /dev/null +++ b/data/examples/declaration/value/function/if-single-line-functions-out.hs @@ -0,0 +1,4 @@ +foo x = + if x > 2 + then bar x + else baz x diff --git a/data/examples/declaration/value/function/if-single-line-functions.hs b/data/examples/declaration/value/function/if-single-line-functions.hs new file mode 100644 index 000000000..c5d83a623 --- /dev/null +++ b/data/examples/declaration/value/function/if-single-line-functions.hs @@ -0,0 +1,4 @@ +foo x = + if x > 2 + then bar x + else baz x diff --git a/data/examples/declaration/value/function/if-with-comment-above-branches-out.hs b/data/examples/declaration/value/function/if-with-comment-above-branches-out.hs new file mode 100644 index 000000000..dfdfb3349 --- /dev/null +++ b/data/examples/declaration/value/function/if-with-comment-above-branches-out.hs @@ -0,0 +1,7 @@ +foo = + if undefined + -- then comment + then undefined + -- else comment + else do + undefined diff --git a/data/examples/declaration/value/function/if-with-comment.hs b/data/examples/declaration/value/function/if-with-comment-above-branches.hs similarity index 100% rename from data/examples/declaration/value/function/if-with-comment.hs rename to data/examples/declaration/value/function/if-with-comment-above-branches.hs diff --git a/data/examples/declaration/value/function/if-with-comment-before-do-blocks-out.hs b/data/examples/declaration/value/function/if-with-comment-before-do-blocks-out.hs new file mode 100644 index 000000000..e9abc389d --- /dev/null +++ b/data/examples/declaration/value/function/if-with-comment-before-do-blocks-out.hs @@ -0,0 +1,10 @@ +foo = + if something + -- then comment + then do + stuff + stuff + -- else comment + else do + stuff + stuff diff --git a/data/examples/declaration/value/function/if-with-comment-before-do-blocks.hs b/data/examples/declaration/value/function/if-with-comment-before-do-blocks.hs new file mode 100644 index 000000000..e9abc389d --- /dev/null +++ b/data/examples/declaration/value/function/if-with-comment-before-do-blocks.hs @@ -0,0 +1,10 @@ +foo = + if something + -- then comment + then do + stuff + stuff + -- else comment + else do + stuff + stuff diff --git a/data/examples/declaration/value/function/if-with-comment-in-branches-out.hs b/data/examples/declaration/value/function/if-with-comment-in-branches-out.hs new file mode 100644 index 000000000..08d790fda --- /dev/null +++ b/data/examples/declaration/value/function/if-with-comment-in-branches-out.hs @@ -0,0 +1,14 @@ +foo = + if x + then + -- comment 1 + -- comment 2 + case a of + Just b -> _ + Nothing -> _ + else + -- comment 3 + -- comment 4 + case a of + Just b -> _ + Nothing -> _ diff --git a/data/examples/declaration/value/function/if-with-comment-in-branches-with-functions-out.hs b/data/examples/declaration/value/function/if-with-comment-in-branches-with-functions-out.hs new file mode 100644 index 000000000..355143932 --- /dev/null +++ b/data/examples/declaration/value/function/if-with-comment-in-branches-with-functions-out.hs @@ -0,0 +1,12 @@ +foo = + if x + then + -- comment 1 + -- comment 2 + foo 1 2 3 + else + -- comment 3 + -- comment 4 + foo + "here" + "there" diff --git a/data/examples/declaration/value/function/if-with-comment-in-branches-with-functions.hs b/data/examples/declaration/value/function/if-with-comment-in-branches-with-functions.hs new file mode 100644 index 000000000..355143932 --- /dev/null +++ b/data/examples/declaration/value/function/if-with-comment-in-branches-with-functions.hs @@ -0,0 +1,12 @@ +foo = + if x + then + -- comment 1 + -- comment 2 + foo 1 2 3 + else + -- comment 3 + -- comment 4 + foo + "here" + "there" diff --git a/data/examples/declaration/value/function/if-with-comment-in-branches.hs b/data/examples/declaration/value/function/if-with-comment-in-branches.hs new file mode 100644 index 000000000..08d790fda --- /dev/null +++ b/data/examples/declaration/value/function/if-with-comment-in-branches.hs @@ -0,0 +1,14 @@ +foo = + if x + then + -- comment 1 + -- comment 2 + case a of + Just b -> _ + Nothing -> _ + else + -- comment 3 + -- comment 4 + case a of + Just b -> _ + Nothing -> _ diff --git a/data/examples/declaration/value/function/if-with-comment-next-to-keyword-out.hs b/data/examples/declaration/value/function/if-with-comment-next-to-keyword-out.hs new file mode 100644 index 000000000..54f495f7f --- /dev/null +++ b/data/examples/declaration/value/function/if-with-comment-next-to-keyword-out.hs @@ -0,0 +1,10 @@ +foo = + if x + then -- comment 1 + -- comment 2 + foo 1 2 3 + else -- comment 3 + -- comment 4 + foo + "here" + "there" diff --git a/data/examples/declaration/value/function/if-with-comment-next-to-keyword.hs b/data/examples/declaration/value/function/if-with-comment-next-to-keyword.hs new file mode 100644 index 000000000..16f60a936 --- /dev/null +++ b/data/examples/declaration/value/function/if-with-comment-next-to-keyword.hs @@ -0,0 +1,10 @@ +foo = + if x + then -- comment 1 + -- comment 2 + foo 1 2 3 + else -- comment 3 + -- comment 4 + foo + "here" + "there" diff --git a/data/examples/declaration/value/function/if-with-comment-out.hs b/data/examples/declaration/value/function/if-with-comment-out.hs deleted file mode 100644 index dd24e401c..000000000 --- a/data/examples/declaration/value/function/if-with-comment-out.hs +++ /dev/null @@ -1,8 +0,0 @@ -foo = - if undefined - then -- then comment - undefined - else -- else comment - - do - undefined diff --git a/src/Ormolu/Printer/Meat/Declaration/Value.hs b/src/Ormolu/Printer/Meat/Declaration/Value.hs index 70bb79067..fa34b5d05 100644 --- a/src/Ormolu/Printer/Meat/Declaration/Value.hs +++ b/src/Ormolu/Printer/Meat/Declaration/Value.hs @@ -363,8 +363,8 @@ p_hsCmd' isApp s = \case p_case isApp cmdPlacement p_hsCmd e mgroup HsCmdLamCase _ variant mgroup -> p_lamcase isApp variant cmdPlacement p_hsCmd mgroup - HsCmdIf _ _ if' then' else' -> - p_if cmdPlacement p_hsCmd if' then' else' + HsCmdIf anns _ if' then' else' -> + p_if cmdPlacement p_hsCmd anns if' then' else' HsCmdLet _ _ localBinds _ c -> p_let p_hsCmd localBinds c HsCmdDo _ es -> do @@ -731,8 +731,8 @@ p_hsExpr' isApp s = \case p_unboxedSum N tag arity (located e p_hsExpr) HsCase _ e mgroup -> p_case isApp exprPlacement p_hsExpr e mgroup - HsIf _ if' then' else' -> - p_if exprPlacement p_hsExpr if' then' else' + HsIf anns if' then' else' -> + p_if exprPlacement p_hsExpr anns if' then' else' HsMultiIf _ guards -> do txt "if" breakpoint @@ -971,6 +971,8 @@ p_if :: (body -> Placement) -> -- | Render (body -> R ()) -> + -- | Annotations + EpAnn AnnsIf -> -- | If LHsExpr GhcPs -> -- | Then @@ -978,21 +980,47 @@ p_if :: -- | Else LocatedA body -> R () -p_if placer render if' then' else' = do +p_if placer render epAnn if' then' else' = do txt "if" space located if' p_hsExpr breakpoint inci $ do - txt "then" + locatedToken thenSpan "then" space - located then' $ \x -> - placeHanging (placer x) (render x) + placeHangingLocated thenSpan then' breakpoint - txt "else" + locatedToken elseSpan "else" space - located else' $ \x -> - placeHanging (placer x) (render x) + placeHangingLocated elseSpan else' + where + (thenSpan, elseSpan, commentSpans) = + case epAnn of + EpAnn {anns = AnnsIf {aiThen, aiElse}, comments} -> + ( loc' $ epaLocationRealSrcSpan aiThen, + loc' $ epaLocationRealSrcSpan aiElse, + map (anchor . getLoc) $ + case comments of + EpaComments cs -> cs + EpaCommentsBalanced pre post -> pre <> post + ) + EpAnnNotUsed -> + (noSrcSpan, noSrcSpan, []) + + locatedToken tokenSpan token = + located (L tokenSpan ()) $ \_ -> txt token + + betweenSpans spanA spanB s = spanA < s && s < spanB + + placeHangingLocated tokenSpan bodyLoc@(L _ body) = do + let bodySpan = getLoc' bodyLoc + hasComments = fromMaybe False $ do + tokenRealSpan <- srcSpanToRealSrcSpan tokenSpan + bodyRealSpan <- srcSpanToRealSrcSpan bodySpan + pure $ any (betweenSpans tokenRealSpan bodyRealSpan) commentSpans + placement = if hasComments then Normal else placer body + switchLayout [tokenSpan, bodySpan] $ + placeHanging placement (located bodyLoc render) p_let :: -- | Render diff --git a/src/Ormolu/Utils.hs b/src/Ormolu/Utils.hs index acf1c5f7d..b4b946c85 100644 --- a/src/Ormolu/Utils.hs +++ b/src/Ormolu/Utils.hs @@ -147,6 +147,9 @@ class HasSrcSpan l where instance HasSrcSpan SrcSpan where loc' = id +instance HasSrcSpan RealSrcSpan where + loc' l = RealSrcSpan l Strict.Nothing + instance HasSrcSpan (SrcSpanAnn' ann) where loc' = locA