From 360c1f87a8835fb7a1befc66edb6495de603c4c9 Mon Sep 17 00:00:00 2001 From: Florian Verdonck Date: Thu, 25 Mar 2021 17:56:06 +0100 Subject: [PATCH] Print trivia before with keyword. Fixes #1503. (#1537) --- ...ineBetweenTypeDefinitionAndMembersTests.fs | 62 ++++++++++++++++++ src/Fantomas.Tests/TypeDeclarationTests.fs | 64 +++++++++++++++++++ src/Fantomas/CodePrinter.fs | 32 +++++----- src/Fantomas/Context.fs | 32 ++++++++-- src/Fantomas/TriviaHelpers.fs | 2 +- 5 files changed, 170 insertions(+), 22 deletions(-) diff --git a/src/Fantomas.Tests/NewlineBetweenTypeDefinitionAndMembersTests.fs b/src/Fantomas.Tests/NewlineBetweenTypeDefinitionAndMembersTests.fs index 55bb2de52e..6b77bd479e 100644 --- a/src/Fantomas.Tests/NewlineBetweenTypeDefinitionAndMembersTests.fs +++ b/src/Fantomas.Tests/NewlineBetweenTypeDefinitionAndMembersTests.fs @@ -348,3 +348,65 @@ type andSeq<'t> = match this with | AndSeq xs -> xs.GetEnumerator() :> _ """ + +[] +let ``blank line before with keyword should be preserved`` () = + formatSourceString + false + """ +type A = + | B of int + | C + + with + member this.GetB = + match this with + | B x -> x + | _ -> failwith "shouldn't happen" +""" + config + |> prepend newline + |> should + equal + """ +type A = + | B of int + | C + + member this.GetB = + match this with + | B x -> x + | _ -> failwith "shouldn't happen" +""" + +[] +let ``blank line before and after with keyword should be preserved`` () = + formatSourceString + false + """ +type A = + | B of int + | C + + with + + member this.GetB = + match this with + | B x -> x + | _ -> failwith "shouldn't happen" +""" + config + |> prepend newline + |> should + equal + """ +type A = + | B of int + | C + + + member this.GetB = + match this with + | B x -> x + | _ -> failwith "shouldn't happen" +""" diff --git a/src/Fantomas.Tests/TypeDeclarationTests.fs b/src/Fantomas.Tests/TypeDeclarationTests.fs index 53ecc8b812..701ee231ac 100644 --- a/src/Fantomas.Tests/TypeDeclarationTests.fs +++ b/src/Fantomas.Tests/TypeDeclarationTests.fs @@ -2463,3 +2463,67 @@ type public Foo() = Blablablabla = moreStuff ) """ + +[] +let ``blank line before with keyword should be preserved`` () = + formatSourceString + false + """ +type A = + | B of int + | C + + with + member this.GetB = + match this with + | B x -> x + | _ -> failwith "shouldn't happen" +""" + config + |> prepend newline + |> should + equal + """ +type A = + | B of int + | C + + member this.GetB = + match this with + | B x -> x + | _ -> failwith "shouldn't happen" +""" + +[] +let ``member inside compiler define using with keyword, 1503`` () = + formatSourceString + false + """ +type A = + | B of int + | C + +#if DEBUG + with + member this.GetB = + match this with + | B x -> x + | _ -> failwith "shouldn't happen" +#endif +""" + { config with IndentSize = 2 } + |> prepend newline + |> should + equal + """ +type A = + | B of int + | C + +#if DEBUG + member this.GetB = + match this with + | B x -> x + | _ -> failwith "shouldn't happen" +#endif +""" diff --git a/src/Fantomas/CodePrinter.fs b/src/Fantomas/CodePrinter.fs index 9c4c5e1104..7b6e452961 100644 --- a/src/Fantomas/CodePrinter.fs +++ b/src/Fantomas/CodePrinter.fs @@ -3322,7 +3322,7 @@ and genAlternativeAppWithParenthesis app astContext = | Choice1Of2 t -> genAlternativeAppWithTupledArgument t astContext | Choice2Of2 s -> genAlternativeAppWithSingleParenthesisArgument s astContext -and sepNlnBetweenTypeAndMembers (ms: SynMemberDefn list) = +and sepNlnBetweenTypeAndMembers (tdr: SynTypeDefnRepr) (ms: SynMemberDefn list) = match List.tryHead ms with | Some m -> let range, mainNodeType = @@ -3339,7 +3339,7 @@ and sepNlnBetweenTypeAndMembers (ms: SynMemberDefn list) = | SynMemberDefn.NestedType (_, _, r) -> r, SynMemberDefn_NestedType | SynMemberDefn.AutoProperty (_, _, _, _, _, _, _, _, _, _, r) -> r, SynMemberDefn_AutoProperty - sepNlnTypeAndMembers (Some range) mainNodeType + sepNlnTypeAndMembers tdr.Range.End range mainNodeType | None -> sepNone and genTypeDefn astContext (TypeDef (ats, px, ao, tds, tcs, tdr, ms, s, preferPostfix) as node) = @@ -3368,7 +3368,7 @@ and genTypeDefn astContext (TypeDef (ats, px, ao, tds, tcs, tdr, ms, s, preferPo { astContext with HasVerticalBar = true }) +> onlyIf (List.isNotEmpty ms) sepNln - +> sepNlnBetweenTypeAndMembers ms + +> sepNlnBetweenTypeAndMembers tdr ms +> genMemberDefnList { astContext with InterfaceRange = None } @@ -3423,7 +3423,7 @@ and genTypeDefn astContext (TypeDef (ats, px, ao, tds, tcs, tdr, ms, s, preferPo +> sepEq +> unionCases +> onlyIf (List.isNotEmpty ms) sepNln - +> sepNlnBetweenTypeAndMembers ms + +> sepNlnBetweenTypeAndMembers tdr ms +> genMemberDefnList { astContext with InterfaceRange = None } @@ -3485,7 +3485,7 @@ and genTypeDefn astContext (TypeDef (ats, px, ao, tds, tcs, tdr, ms, s, preferPo (indent ++ "with" +> indent +> sepNln - +> sepNlnBetweenTypeAndMembers ms + +> sepNlnBetweenTypeAndMembers tdr ms +> genMemberDefnList { astContext with InterfaceRange = None } @@ -3528,7 +3528,7 @@ and genTypeDefn astContext (TypeDef (ats, px, ao, tds, tcs, tdr, ms, s, preferPo +> genTypeDefKind tdk +> indent +> onlyIf (List.isNotEmpty others) sepNln - +> sepNlnBetweenTypeAndMembers ms + +> sepNlnBetweenTypeAndMembers tdr ms +> genMemberDefnList astContext others +> unindent ++ "end" @@ -3561,7 +3561,7 @@ and genTypeDefn astContext (TypeDef (ats, px, ao, tds, tcs, tdr, ms, s, preferPo +> indent // Remember that we use MemberDefn of parent node +> sepNln - +> sepNlnBetweenTypeAndMembers ms + +> sepNlnBetweenTypeAndMembers tdr ms +> genMemberDefnList { astContext with InterfaceRange = None } @@ -3637,7 +3637,7 @@ and genMultilineSimpleRecordTypeDefn tdr ms ao' fs astContext = +> leaveNodeTokenByName tdr.Range RBRACE +> optSingle (fun _ -> unindent) ao' +> onlyIf (List.isNotEmpty ms) sepNln - +> sepNlnBetweenTypeAndMembers ms + +> sepNlnBetweenTypeAndMembers tdr ms +> genMemberDefnList { astContext with InterfaceRange = None } @@ -3659,13 +3659,13 @@ and genMultilineSimpleRecordTypeDefnAlignBrackets tdr ms ao' fs astContext = +> sepCloseSFixed +> optSingle (fun _ -> unindent) ao' +> onlyIf (List.isNotEmpty ms) sepNln - +> sepNlnBetweenTypeAndMembers ms + +> sepNlnBetweenTypeAndMembers tdr ms +> genMemberDefnList { astContext with InterfaceRange = None } ms -and sepNlnBetweenSigTypeAndMembers (ms: SynMemberSig list) = +and sepNlnBetweenSigTypeAndMembers (synTypeDefnRepr: SynTypeDefnSigRepr) (ms: SynMemberSig list) : Context -> Context = match List.tryHead ms with | Some m -> let range, mainNodeType = @@ -3676,7 +3676,7 @@ and sepNlnBetweenSigTypeAndMembers (ms: SynMemberSig list) = | SynMemberSig.NestedType (_, r) -> r, SynMemberSig_NestedType | SynMemberSig.ValField (_, r) -> r, SynMemberSig_ValField - sepNlnTypeAndMembers (Some range) mainNodeType + sepNlnTypeAndMembers synTypeDefnRepr.Range.End range mainNodeType | None -> sepNone and genSigTypeDefn astContext (SigTypeDef (ats, px, ao, tds, tcs, tdr, ms, s, preferPostfix)) = @@ -3701,7 +3701,7 @@ and genSigTypeDefn astContext (SigTypeDef (ats, px, ao, tds, tcs, tdr, ms, s, pr (genEnumCase { astContext with HasVerticalBar = true }) - +> sepNlnBetweenSigTypeAndMembers ms + +> sepNlnBetweenSigTypeAndMembers tdr ms +> colPre sepNln sepNln ms (genMemberSig astContext) // Add newline after un-indent to be spacing-correct +> unindent @@ -3741,7 +3741,7 @@ and genSigTypeDefn astContext (SigTypeDef (ats, px, ao, tds, tcs, tdr, ms, s, pr typeName +> sepEq +> unionCases - +> sepNlnBetweenSigTypeAndMembers ms + +> sepNlnBetweenSigTypeAndMembers tdr ms +> colPre sepNln sepNln ms (genMemberSig astContext) +> unindent @@ -3788,7 +3788,7 @@ and genSigTypeDefn astContext (SigTypeDef (ats, px, ao, tds, tcs, tdr, ms, s, pr !- " with" +> indent +> sepNln - +> sepNlnBetweenSigTypeAndMembers ms + +> sepNlnBetweenSigTypeAndMembers tdr ms +> col sepNln ms (genMemberSig astContext) +> unindent @@ -3853,7 +3853,7 @@ and genSigSimpleRecord tdr ms ao' fs astContext = +> col sepSemiNln fs (genField astContext "") ) +> sepCloseS - +> sepNlnBetweenSigTypeAndMembers ms + +> sepNlnBetweenSigTypeAndMembers tdr ms +> colPre sepNln sepNln ms (genMemberSig astContext) +> optSingle (fun _ -> unindent) ao' @@ -3871,7 +3871,7 @@ and genSigSimpleRecordAlignBrackets tdr ms ao' fs astContext = +> unindent +> sepNln +> sepCloseSFixed - +> sepNlnBetweenSigTypeAndMembers ms + +> sepNlnBetweenSigTypeAndMembers tdr ms +> colPre sepNln sepNln ms (genMemberSig astContext) +> onlyIf (Option.isSome ao') unindent diff --git a/src/Fantomas/Context.fs b/src/Fantomas/Context.fs index ce1383aa5c..d56b9e4aba 100644 --- a/src/Fantomas/Context.fs +++ b/src/Fantomas/Context.fs @@ -1388,11 +1388,33 @@ let internal sepNlnForEmptyNamespace (namespaceRange: Range) ctx = || hasPrintableContent node.ContentAfter -> ctx | _ -> sepNln ctx -let internal sepNlnTypeAndMembers (firstMemberRange: Range option) (mainNodeType: FsAstType) ctx = - match firstMemberRange with - | Some range when (ctx.Config.NewlineBetweenTypeDefinitionAndMembers) -> - sepNlnConsideringTriviaContentBeforeForMainNode mainNodeType range ctx - | _ -> ctx +let internal sepNlnTypeAndMembers + (lastPositionBeforeMembers: Pos) + (firstMemberRange: Range) + (mainNodeType: FsAstType) + (ctx: Context) + : Context = + let triviaNodeOfWithKeyword : TriviaNode option = + let r = + ctx.MkRange lastPositionBeforeMembers firstMemberRange.Start + + Map.tryFindOrEmptyList WITH ctx.TriviaTokenNodes + |> TriviaHelpers.``keyword token inside range`` r + |> List.choose + (fun (_, tn) -> + if List.isNotEmpty tn.ContentBefore then + Some tn + else + None) + |> List.tryHead + + match triviaNodeOfWithKeyword with + | Some tn -> printContentBefore tn ctx + | None -> + if ctx.Config.NewlineBetweenTypeDefinitionAndMembers then + sepNlnConsideringTriviaContentBeforeForMainNode mainNodeType firstMemberRange ctx + else + ctx let internal sepNlnWhenWriteBeforeNewlineNotEmpty fallback (ctx: Context) = if String.isNotNullOrEmpty ctx.WriterModel.WriteBeforeNewline then diff --git a/src/Fantomas/TriviaHelpers.fs b/src/Fantomas/TriviaHelpers.fs index e0bb06ac57..acf9d863a3 100644 --- a/src/Fantomas/TriviaHelpers.fs +++ b/src/Fantomas/TriviaHelpers.fs @@ -48,7 +48,7 @@ module internal TriviaHelpers = |> Option.map contentAfterEnd) |> Option.defaultValue false - let ``keyword token inside range`` range (trivia: TriviaNode list) = + let ``keyword token inside range`` (range: Range) (trivia: TriviaNode list) : (Token * TriviaNode) list = trivia |> List.choose (fun t ->