From 20a09de8ffa27af11a0c19161991207923638736 Mon Sep 17 00:00:00 2001 From: Florian Verdonck Date: Sat, 27 Feb 2021 11:13:49 +0100 Subject: [PATCH] Create custom range for brackets in Elmish elements inside of full SynExpr range. Fixes #1347. (#1495) --- src/Fantomas.Tests/ElmishTests.fs | 199 ++++++++++++++++++++++++++++++ src/Fantomas/CodePrinter.fs | 61 ++++++--- src/Fantomas/Context.fs | 1 + src/Fantomas/SourceParser.fs | 10 +- src/Fantomas/TriviaHelpers.fs | 9 ++ 5 files changed, 259 insertions(+), 21 deletions(-) diff --git a/src/Fantomas.Tests/ElmishTests.fs b/src/Fantomas.Tests/ElmishTests.fs index de40f7f2fe..b5ce9455c1 100644 --- a/src/Fantomas.Tests/ElmishTests.fs +++ b/src/Fantomas.Tests/ElmishTests.fs @@ -1091,3 +1091,202 @@ Gen.frequency [ genSubSynPat ] // """ + +[] +let ``don't repeat comment in nested Elmish element, 1347`` () = + formatSourceString + false + """ +let html = + Html.div [ + prop.className "navbar-menu" + prop.children [ + Html.div [ + prop.className "navbar-start" + prop.children [ + Html.a [ + prop.className "navbar-item" + ] + (* + Html.a [ prop.className "navbar-item"; prop.href (baseUrl +/ "Files") ] [ + prop.text "Files" + ]*) + ] + ] + ] + ] +""" + config + |> prepend newline + |> should + equal + """ +let html = + Html.div [ prop.className "navbar-menu" + prop.children [ Html.div [ prop.className "navbar-start" + prop.children [ Html.a [ prop.className "navbar-item" ] + (* + Html.a [ prop.className "navbar-item"; prop.href (baseUrl +/ "Files") ] [ + prop.text "Files" + ]*) + ] ] ] ] +""" + +[] +let ``don't repeat comment in nested Elmish element, idempotent check`` () = + formatSourceString + false + """ +let html2 = + Html.div [ prop.className "navbar-menu" + prop.children [ Html.div [ prop.className "navbar-start" + prop.children [ Html.a [ prop.className "navbar-item" ] + (* + Html.a [ prop.className "navbar-item"; prop.href (baseUrl +/ "Files") ] [ + prop.text "Files" + ]*) + ] ] ] ] +""" + config + |> prepend newline + |> should + equal + """ +let html2 = + Html.div [ prop.className "navbar-menu" + prop.children [ Html.div [ prop.className "navbar-start" + prop.children [ Html.a [ prop.className "navbar-item" ] + (* + Html.a [ prop.className "navbar-item"; prop.href (baseUrl +/ "Files") ] [ + prop.text "Files" + ]*) + ] ] ] ] +""" + +[] +let ``don't repeat comment in nested Elmish element, single element mode`` () = + formatSourceString + false + """ +let html = + Html.div [ + prop.className "navbar-menu" + prop.children [ + Html.div [ + prop.className "navbar-start" + prop.children [ + Html.a [ + prop.className "navbar-item" + ] + (* + Html.a [ prop.className "navbar-item"; prop.href (baseUrl +/ "Files") ] [ + prop.text "Files" + ]*) + ] + ] + ] + ] +""" + { config with + SingleArgumentWebMode = true } + |> prepend newline + |> should + equal + """ +let html = + Html.div [ + prop.className "navbar-menu" + prop.children [ + Html.div [ + prop.className "navbar-start" + prop.children [ + Html.a [ prop.className "navbar-item" ] + (* + Html.a [ prop.className "navbar-item"; prop.href (baseUrl +/ "Files") ] [ + prop.text "Files" + ]*) + ] + ] + ] + ] +""" + +[] +let ``don't repeat comment in nested Elmish element, single element mode idempotent`` () = + formatSourceString + false + """ +let html = + Html.div [ + prop.className "navbar-menu" + prop.children [ + Html.div [ + prop.className "navbar-start" + prop.children [ + Html.a [ prop.className "navbar-item" ] + (* + Html.a [ prop.className "navbar-item"; prop.href (baseUrl +/ "Files") ] [ + prop.text "Files" + ]*) + ] + ] + ] + ] +""" + { config with + SingleArgumentWebMode = true } + |> prepend newline + |> should + equal + """ +let html = + Html.div [ + prop.className "navbar-menu" + prop.children [ + Html.div [ + prop.className "navbar-start" + prop.children [ + Html.a [ prop.className "navbar-item" ] + (* + Html.a [ prop.className "navbar-item"; prop.href (baseUrl +/ "Files") ] [ + prop.text "Files" + ]*) + ] + ] + ] + ] +""" + +[] +let ``don't repeat comment in nested Elmish element, short block comment`` () = + formatSourceString + false + """ +let html = + Html.div [ + prop.className "navbar-menu" + prop.children [ + Html.div [ + prop.className "navbar-start" + prop.children [ + Html.a [ + prop.className "navbar-item" + ] + (* meh *) + ] + ] + ] + ] +""" + config + |> prepend newline + |> should + equal + """ +let html = + Html.div [ prop.className "navbar-menu" + prop.children [ Html.div [ prop.className "navbar-start" + prop.children [ Html.a [ prop.className "navbar-item" ] + (* meh *) + ] ] ] ] +""" diff --git a/src/Fantomas/CodePrinter.fs b/src/Fantomas/CodePrinter.fs index 16b6a4d23e..e747a9377e 100644 --- a/src/Fantomas/CodePrinter.fs +++ b/src/Fantomas/CodePrinter.fs @@ -1037,18 +1037,32 @@ and genExpr astContext synExpr ctx = let expr = match synExpr with - | ElmishReactWithoutChildren (identifier, isArray, children) when (not ctx.Config.DisableElmishSyntax) -> - fun ctx -> + | ElmishReactWithoutChildren (identifier, isArray, children, childrenRange) when + (not ctx.Config.DisableElmishSyntax) -> + fun (ctx: Context) -> + let tokenSize = if isArray then 2 else 1 + + let openingTokenRange, openTokenType = + ctx.MkRangeWith + (childrenRange.Start.Line, childrenRange.Start.Column) + (childrenRange.Start.Line, (childrenRange.Start.Column + tokenSize)), + (if isArray then LBRACK_BAR else LBRACK) + + let closingTokenRange, closingTokenType = + ctx.MkRangeWith + (childrenRange.End.Line, (childrenRange.End.Column - tokenSize)) + (childrenRange.End.Line, childrenRange.End.Column), + (if isArray then BAR_RBRACK else RBRACK) + let shortExpression = let noChildren = - ifElse isArray sepOpenAFixed sepOpenLFixed - +> ifElse isArray sepCloseAFixed sepCloseLFixed + tokN openingTokenRange openTokenType (ifElse isArray sepOpenAFixed sepOpenLFixed) + +> tokN closingTokenRange closingTokenType (ifElse isArray sepCloseAFixed sepCloseLFixed) let genChildren = - ifElse isArray sepOpenA sepOpenL + tokN openingTokenRange openTokenType (ifElse isArray sepOpenA sepOpenL) +> col sepSemi children (genExpr astContext) - +> enterNodeTokenByName synExpr.Range (if isArray then BAR_RBRACK else RBRACK) - +> ifElse isArray sepCloseA sepCloseL + +> tokN closingTokenRange closingTokenType (ifElse isArray sepCloseA sepCloseL) !-identifier +> sepSpace @@ -1057,27 +1071,42 @@ and genExpr astContext synExpr ctx = let elmishExpression = !-identifier +> sepSpace - +> ifElse isArray sepOpenA sepOpenL + +> tokN openingTokenRange openTokenType (ifElse isArray sepOpenA sepOpenL) +> atCurrentColumn ( col sepNln children (genExpr astContext) - +> enterNodeTokenByName synExpr.Range (if isArray then BAR_RBRACK else RBRACK) + +> onlyIf + (TriviaHelpers.``has content before that matches`` + (fun tn -> RangeHelpers.rangeEq tn.Range closingTokenRange) + (function + | Comment (BlockComment _) -> true + | _ -> false) + (Map.tryFindOrEmptyList closingTokenType ctx.TriviaTokenNodes)) + sepNln + +> tokN closingTokenRange closingTokenType (ifElse isArray sepCloseA sepCloseL) ) - +> ifElse isArray sepCloseA sepCloseL - +> leaveNodeTokenByName synExpr.Range (if isArray then BAR_RBRACK else RBRACK) let felizExpression = + let hasBlockCommentBeforeClosingToken = + TriviaHelpers.``has content before that matches`` + (fun tn -> RangeHelpers.rangeEq tn.Range closingTokenRange) + (function + | Comment (BlockComment _) -> true + | _ -> false) + (Map.tryFindOrEmptyList closingTokenType ctx.TriviaTokenNodes) + atCurrentColumn ( !-identifier +> sepSpace - +> ifElse isArray sepOpenAFixed sepOpenLFixed + +> tokN openingTokenRange openTokenType (ifElse isArray sepOpenAFixed sepOpenLFixed) +> indent +> sepNln +> col sepNln children (genExpr astContext) - +> unindent - +> sepNln - +> enterNodeTokenByName synExpr.Range (if isArray then BAR_RBRACK else RBRACK) + +> onlyIf hasBlockCommentBeforeClosingToken (sepNln +> unindent) + +> enterNodeTokenByName closingTokenRange closingTokenType + +> onlyIfNot hasBlockCommentBeforeClosingToken unindent + +> sepNlnUnlessLastEventIsNewline +> ifElse isArray sepCloseAFixed sepCloseLFixed - +> leaveNodeTokenByName synExpr.Range (if isArray then BAR_RBRACK else RBRACK) + +> leaveNodeTokenByName closingTokenRange closingTokenType ) let multilineExpression = diff --git a/src/Fantomas/Context.fs b/src/Fantomas/Context.fs index 76d6270899..df95c5f7e4 100644 --- a/src/Fantomas/Context.fs +++ b/src/Fantomas/Context.fs @@ -351,6 +351,7 @@ let internal lastWriteEventIsNewline ctx = (function | RestoreIndent _ | RestoreAtColumn _ + | UnIndentBy _ | Write "" -> true | _ -> false) |> Seq.tryHead diff --git a/src/Fantomas/SourceParser.fs b/src/Fantomas/SourceParser.fs index 2bf94ff6e6..88466c2e8e 100644 --- a/src/Fantomas/SourceParser.fs +++ b/src/Fantomas/SourceParser.fs @@ -1700,11 +1700,11 @@ let isFunctionBinding (p: SynPat) = let (|ElmishReactWithoutChildren|_|) e = match e with - | App (OptVar (ident, _, _), [ ArrayOrList (isArray, children, _) ]) - | App (OptVar (ident, _, _), [ ArrayOrListOfSeqExpr (isArray, CompExpr (_, Sequentials children)) ]) -> - Some(ident, isArray, children) - | App (OptVar (ident, _, _), [ ArrayOrListOfSeqExpr (isArray, CompExpr (_, singleChild)) ]) -> - Some(ident, isArray, [ singleChild ]) + | App (OptVar (ident, _, _), [ (ArrayOrList (isArray, children, _) as aolEx) ]) + | App (OptVar (ident, _, _), [ (ArrayOrListOfSeqExpr (isArray, CompExpr (_, Sequentials children)) as aolEx) ]) -> + Some(ident, isArray, children, aolEx.Range) + | App (OptVar (ident, _, _), [ (ArrayOrListOfSeqExpr (isArray, CompExpr (_, singleChild)) as aolEx) ]) -> + Some(ident, isArray, [ singleChild ], aolEx.Range) | _ -> None let (|ElmishReactWithChildren|_|) e = diff --git a/src/Fantomas/TriviaHelpers.fs b/src/Fantomas/TriviaHelpers.fs index fd662d569a..55b64aef50 100644 --- a/src/Fantomas/TriviaHelpers.fs +++ b/src/Fantomas/TriviaHelpers.fs @@ -18,6 +18,15 @@ module internal TriviaHelpers = |> Option.map (fun t -> t.ContentAfter |> List.exists contentAfter) |> Option.defaultValue false + let ``has content before that matches`` + (findTrivia: TriviaNode -> bool) + (contentBefore: TriviaContent -> bool) + (trivia: TriviaNode list) + = + List.tryFind findTrivia trivia + |> Option.map (fun t -> t.ContentBefore |> List.exists contentBefore) + |> Option.defaultValue false + let ``has single line comment before`` (triviaNode: TriviaNode) = triviaNode.ContentBefore |> List.exists