Skip to content

Commit

Permalink
Print trivia before with keyword. Fixes #1503. (#1537)
Browse files Browse the repository at this point in the history
  • Loading branch information
nojaf committed Mar 25, 2021
1 parent a4257ea commit 360c1f8
Show file tree
Hide file tree
Showing 5 changed files with 170 additions and 22 deletions.
62 changes: 62 additions & 0 deletions src/Fantomas.Tests/NewlineBetweenTypeDefinitionAndMembersTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -348,3 +348,65 @@ type andSeq<'t> =
match this with
| AndSeq xs -> xs.GetEnumerator() :> _
"""

[<Test>]
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"
"""

[<Test>]
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"
"""
64 changes: 64 additions & 0 deletions src/Fantomas.Tests/TypeDeclarationTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -2463,3 +2463,67 @@ type public Foo() =
Blablablabla = moreStuff
)
"""

[<Test>]
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"
"""

[<Test>]
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
"""
32 changes: 16 additions & 16 deletions src/Fantomas/CodePrinter.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -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) =
Expand Down Expand Up @@ -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 }
Expand Down Expand Up @@ -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 }
Expand Down Expand Up @@ -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 }
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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 }
Expand Down Expand Up @@ -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 }
Expand All @@ -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 =
Expand All @@ -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)) =
Expand All @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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'

Expand All @@ -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

Expand Down
32 changes: 27 additions & 5 deletions src/Fantomas/Context.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/Fantomas/TriviaHelpers.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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 ->
Expand Down

0 comments on commit 360c1f8

Please sign in to comment.