From be054f65491ce9d9bfb92ece501d1202914ff4a5 Mon Sep 17 00:00:00 2001
From: Tomasz Heimowski <tomek.heimowski@gmail.com>
Date: Tue, 3 Dec 2019 16:40:46 +0100
Subject: [PATCH] Consider Trivia Content before multiline members - fixes #569
 (#584)

* 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
---
 src/Fantomas.Tests/ClassTests.fs | 52 +++++++++++++++++++++++++++++++-
 src/Fantomas/CodePrinter.fs      | 38 ++++++++++-------------
 2 files changed, 67 insertions(+), 23 deletions(-)

diff --git a/src/Fantomas.Tests/ClassTests.fs b/src/Fantomas.Tests/ClassTests.fs
index a8534dc651..f0ee4e3d02 100644
--- a/src/Fantomas.Tests/ClassTests.fs
+++ b/src/Fantomas.Tests/ClassTests.fs
@@ -382,4 +382,54 @@ type Exception with
 
 type Exception with
     member inline __.FirstLine = __.Message.Split([| Environment.NewLine |], StringSplitOptions.RemoveEmptyEntries).[0]
-"""
\ No newline at end of file
+"""
+
+[<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
diff --git a/src/Fantomas/CodePrinter.fs b/src/Fantomas/CodePrinter.fs
index 5cbca81cf7..4abbbf802a 100644
--- a/src/Fantomas/CodePrinter.fs
+++ b/src/Fantomas/CodePrinter.fs
@@ -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
@@ -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 =