Skip to content

Commit

Permalink
Consider Trivia Content before multiline members - fixes #569 (#584)
Browse files Browse the repository at this point in the history
* Consider Trivia Content before multiline members - fixes #569

The pattern matching above uses nested `sepMember` function
to consider Trivia Content. The difference between those
pattern matching cases is that latter one matches on when
there are trailing non-multiline members.
There's some code duplication between those two now, which
I'm happy to refactor - feedback welcome.

* Reuse MultilineMemberDefnL pattern match case

This makes all tests still happy, but I'm not sure if I didn't do
something stupid here. Good thing is it clears up the code a bit.

* Add unit test for original repro code in 569

Plus addressing review comments
  • Loading branch information
theimowski authored and nojaf committed Dec 3, 2019
1 parent 6413638 commit be054f6
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 23 deletions.
52 changes: 51 additions & 1 deletion src/Fantomas.Tests/ClassTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -382,4 +382,54 @@ type Exception with
type Exception with
member inline __.FirstLine = __.Message.Split([| Environment.NewLine |], StringSplitOptions.RemoveEmptyEntries).[0]
"""
"""

[<Test>]
let ``no extra new lines between interface members, 569``() =
let original = """namespace Quartz.Fsharp
module Logging =
open Quartz.Logging
open System
//todo: it seems that quartz doesn't use mapped and nested context,
//however, check if this is the best implementation for this interface
type private QuartzLoggerWrapper(f) =
interface ILogProvider with
member this.OpenMappedContext(_, _) =
{ new IDisposable with
member this.Dispose() = () }
member this.OpenNestedContext _ =
{ new IDisposable with
member this.Dispose() = () }
member this.GetLogger _name = new Logger(f)
let SetQuartzLoggingFunction f =
let loggerFunction level (func: Func<string>) exc parameters =
let wrappedFunction = Helpers.nullValuesToOptions (fun (x: Func<string>) -> (fun () -> x.Invoke())) func
let wrappedException = Helpers.nullValuesToOptions id exc
f level wrappedFunction wrappedException (parameters |> List.ofArray)
LogProvider.SetCurrentLogProvider(QuartzLoggerWrapper(loggerFunction))
let SetQuartzLogger l = LogProvider.SetCurrentLogProvider(l)
"""

formatSourceString false original config |> should equal original

[<Test>]
let ``no extra new lines between type members, 569``() =
let original = """type A() =
member this.MemberA =
if true then 0 else 1
member this.MemberB =
if true then 2 else 3
member this.MemberC = 0
"""

formatSourceString false original config |> should equal original
38 changes: 16 additions & 22 deletions src/Fantomas/CodePrinter.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1919,26 +1919,6 @@ and genMemberDefnList astContext node =
| [] -> col sepNln xs (genMemberDefn astContext) ctx
| _ -> (col sepNln xs (genMemberDefn astContext) +> rep 2 sepNln +> genMemberDefnList astContext ys) ctx

| MultilineMemberDefnL(xs, []) ->
let sepMember (m:Composite<SynMemberDefn, SynBinding>) =
match m with
| Pair(x1,_) ->
let attributes = getRangesFromAttributesFromSynBinding x1
sepNln +> sepNlnConsideringTriviaContentBeforeWithAttributes x1.RangeOfBindingSansRhs attributes
| Single x ->
let attributes = getRangesFromAttributesFromSynMemberDefinition x
sepNln +> sepNlnConsideringTriviaContentBeforeWithAttributes x.Range attributes

let firstTwoNln =
match List.tryHead xs with
| Some xsh -> sepMember xsh
| None -> rep 2 sepNln

firstTwoNln
+> colEx sepMember xs (function
| Pair(x1, x2) -> genPropertyWithGetSet astContext (x1, x2)
| Single x -> genMemberDefn astContext x)

| MultilineMemberDefnL(xs, ys) ->
let sepNlnFirstExpr =
match List.tryHead xs with
Expand All @@ -1950,11 +1930,25 @@ and genMemberDefnList astContext node =
sepNlnConsideringTriviaContentBeforeWithAttributes xsh.Range attributes
| _ -> sepNln

let sepMember (m:Composite<SynMemberDefn, SynBinding>) =
match m with
| Pair(x1,_) ->
let attributes = getRangesFromAttributesFromSynBinding x1
sepNln +> sepNlnConsideringTriviaContentBeforeWithAttributes x1.RangeOfBindingSansRhs attributes
| Single x ->
let attributes = getRangesFromAttributesFromSynMemberDefinition x
sepNln +> sepNlnConsideringTriviaContentBeforeWithAttributes x.Range attributes

let genYs =
match ys with
| [ ] -> sepNone
| _ -> sepNln +> genMemberDefnList astContext ys

sepNln +> sepNlnFirstExpr
+> col (rep 2 sepNln) xs (function
+> colEx sepMember xs (function
| Pair(x1, x2) -> genPropertyWithGetSet astContext (x1, x2)
| Single x -> genMemberDefn astContext x)
+> sepNln +> genMemberDefnList astContext ys
+> genYs

| OneLinerMemberDefnL(xs, ys) ->
let sepNlnFirstExpr =
Expand Down

0 comments on commit be054f6

Please sign in to comment.