Improve assigning trivia after attributes (#951)

* Improve assigning trivia after attributes. Fixes #949

* Add stricter check whether there attributes that might have trivia content after.

* Remove unused opens.

* Fixed fsharplint issue

* Corrected parent range in RecordType Field.

src/Fantomas.CoreGlobalTool.Tests/TestHelpers.fs

@@ -4,7 +4,6 @@ open System
 open System.Diagnostics
 open System.IO
 open System.Text
-open Fantomas
 
 type TemporaryFileCodeSample internal (codeSnippet: string, ?hasByteOrderMark: bool, ?fileName: string) =
     let hasByteOrderMark = defaultArg hasByteOrderMark false

src/Fantomas.Tests/AttributeTests.fs

@@ -346,3 +346,105 @@ let ``should preserve multiple return type attributes`` () =
     formatSourceString false """let f x : [<return: AttributeOne;AttributeTwo;AttributeThree("foo")>] int = x""" config
     |> should equal """let f x: [<return:AttributeOne; AttributeTwo; AttributeThree("foo")>] int = x
 """
+
+[<Test>]
+let ``attribute, new line, let binding`` () =
+    formatSourceString false """
+    [<Foo>]
+
+let bar = 7
+"""  config
+    |> prepend newline
+    |> should equal """
+[<Foo>]
+
+let bar = 7
+"""
+
+[<Test>]
+let ``attribute, new line, type declaration`` () =
+    formatSourceString false """
+[<Foo>]
+
+type Bar = Bar of string
+"""  config
+    |> prepend newline
+    |> should equal """
+[<Foo>]
+
+type Bar = Bar of string
+"""
+
+[<Test>]
+let ``attribute, new line, attribute, newline, let binding`` () =
+    formatSourceString false """
+[<Foo>]
+
+[<Meh>]
+
+let bar = 7
+"""  config
+    |> prepend newline
+    |> should equal """
+[<Foo>]
+
+[<Meh>]
+
+let bar = 7
+"""
+
+[<Test>]
+let ``attribute, new line, attribute, line comment, type declaration`` () =
+    formatSourceString false """
+[<Foo>]
+
+[<Meh>]
+// foo
+type Text = string
+"""  config
+    |> prepend newline
+    |> should equal """
+[<Foo>]
+
+[<Meh>]
+// foo
+type Text = string
+"""
+
+[<Test>]
+let ``attribute, hash directive, attribute, hash directive, type declaration`` () =
+    formatSourceString false """
+[<Foo>]
+#if FOO
+[<Meh>]
+#endif
+type Text = string
+"""  config
+    |> prepend newline
+    |> should equal """
+[<Foo>]
+#if FOO
+[<Meh>]
+#endif
+type Text = string
+"""
+
+[<Test>]
+let ``attribute, line comment, attribute, new line, record definition field`` () =
+    formatSourceString false """
+type Commenter =
+    { [<JsonProperty("display_name")>]
+      // foo
+      [<Bar>]
+      
+      DisplayName: string }
+"""  config
+    |> prepend newline
+    |> should equal """
+type Commenter =
+    { [<JsonProperty("display_name")>]
+      // foo
+      [<Bar>]
+
+      DisplayName: string }
+"""

src/Fantomas.Tests/CheckTests.fs

@@ -2,7 +2,6 @@ module Fantomas.Tests.CheckTests
 
 open NUnit.Framework
 open FsUnit
-open Fantomas.FormatConfig
 open Fantomas.FakeHelpers
 open Fantomas.Tests.TestHelper
 

src/Fantomas.Tests/QueueTests.fs

@@ -6,7 +6,7 @@ open FsUnit
 open FsCheck
 
 module Queue =
-    let ofLists xss = (Queue.empty, xss) ||> List.fold (fun q xs -> Queue.append q xs) 
+    let ofLists xss = (Queue.empty, xss) ||> List.fold Queue.append 
 
 [<Test>]
 let ``Queue.append``() =

src/Fantomas.Tests/TypeDeclarationTests.fs

@@ -1413,3 +1413,43 @@ type OuterType =
     abstract Apply<'r> : InnerType<'r>
         -> 'r when 'r : comparison
 """
+
+[<Test>]
+let ``attribute on type and abstract member followed by type, 949`` () =
+    formatSourceString false """
+[<AllowNullLiteral>]
+type TimelineOptionsGroupCallbackFunction =
+    [<Emit "$0($1...)">]
+    abstract Invoke: group:TimelineGroup * callback:(TimelineGroup option -> unit) -> unit
+
+type TimelineOptionsGroupEditableType = U2<bool, TimelineGroupEditableOption>
+"""  config
+    |> prepend newline
+    |> should equal """
+[<AllowNullLiteral>]
+type TimelineOptionsGroupCallbackFunction =
+    [<Emit "$0($1...)">]
+    abstract Invoke: group:TimelineGroup * callback:(TimelineGroup option -> unit) -> unit
+
+type TimelineOptionsGroupEditableType = U2<bool, TimelineGroupEditableOption>
+"""
+
+[<Test>]
+let ``attribute on type and abstract member followed by let binding`` () =
+    formatSourceString false """
+[<AllowNullLiteral>]
+type TimelineOptionsGroupCallbackFunction =
+    [<Emit "$0($1...)">]
+    abstract Invoke: group:TimelineGroup * callback:(TimelineGroup option -> unit) -> unit
+
+let myBinding a = 7
+"""  config
+    |> prepend newline
+    |> should equal """
+[<AllowNullLiteral>]
+type TimelineOptionsGroupCallbackFunction =
+    [<Emit "$0($1...)">]
+    abstract Invoke: group:TimelineGroup * callback:(TimelineGroup option -> unit) -> unit
+
+let myBinding a = 7
+"""

src/Fantomas/AstTransformer.fs

@@ -69,7 +69,7 @@ module private Ast =
              FsAstNode = modOrNs
              Childs =
                  [yield! (if isModule = SynModuleOrNamespaceKind.DeclaredNamespace then collectIdents longIdent else [])
-                  yield! attrs |> List.map (visitSynAttributeList modOrNs)
+                  yield! (visitSynAttributeLists range attrs)
                   yield! (decls |> List.map visitSynModuleDecl)]}
 
     and visitSynModuleDecl(ast: SynModuleDecl) : Node =
@@ -125,7 +125,7 @@ module private Ast =
              Range = r range
              Properties = p []
              FsAstNode = ast
-             Childs = attrs |> List.map (visitSynAttributeList ast)}
+             Childs = visitSynAttributeLists range attrs }
         | SynModuleDecl.HashDirective(hash,range) ->
             {Type = "SynModuleDecl.HashDirective"
              Range = r range
@@ -810,7 +810,7 @@ module private Ast =
                     if access.IsSome then yield "access" ==> (access.Value |> visitSynAccess)]
              FsAstNode = mbrDef
              Childs =
-                 [yield! attrs |> List.map (visitSynAttributeList mbrDef)
+                 [yield! (visitSynAttributeLists range attrs)
                   yield visitSynSimplePats ctorArgs]}
         | SynMemberDefn.ImplicitInherit(inheritType,inheritArgs,inheritAlias,range) ->
             {Type = "SynMemberDefn.ImplicitInherit"
@@ -871,7 +871,7 @@ module private Ast =
                     if getSetRange.IsSome then yield "getSetRange" ==> (getSetRange.Value |> r)]
              FsAstNode = mbrDef
              Childs =
-                 [yield! attrs |> List.map (visitSynAttributeList mbrDef)
+                 [yield! (visitSynAttributeLists range attrs)
                   if typeOpt.IsSome then yield visitSynType typeOpt.Value
                   yield visitSynExpr synExpr]}
 
@@ -902,7 +902,7 @@ module private Ast =
              FsAstNode = sp
              Childs =
                  [yield visitSynSimplePat simplePat
-                  yield! attrs |> List.map (visitSynAttributeList sp)]}
+                  yield! (visitSynAttributeLists range attrs)]}
 
     and visitSynSimplePats(sp: SynSimplePats): Node =
         match sp with
@@ -933,7 +933,7 @@ module private Ast =
                     if access.IsSome then yield "access" ==> (access.Value |> visitSynAccess)]
              FsAstNode = binding
              Childs =
-                 [yield! attrs |> List.map (visitSynAttributeList binding)
+                 [yield! (visitSynAttributeLists range attrs)
                   yield visitSynValData valData
                   yield visitSynPat headPat
                   if returnInfo.IsSome then yield visitSynBindingReturnInfo returnInfo.Value
@@ -960,7 +960,7 @@ module private Ast =
                     if access.IsSome then yield "access" ==> (access.Value |> visitSynAccess)]
              FsAstNode = svs
              Childs =
-                 [yield! attrs |> List.map (visitSynAttributeList svs)
+                 [yield! (visitSynAttributeLists range attrs)
                   yield visitSynValTyparDecls explicitValDecls
                   yield visitSynType synType
                   yield visitSynValInfo arity
@@ -983,7 +983,7 @@ module private Ast =
              Properties = p []
              FsAstNode = std
              Childs =
-                 [yield! attrs |> List.map (visitSynAttributeList std)
+                 [yield! (visitSynAttributeLists typar.Range attrs)
                   yield visitSynTypar typar]}
 
     and visitSynTypar(typar: SynTypar): Node =
@@ -1012,7 +1012,7 @@ module private Ast =
              FsAstNode = returnInfo
              Childs =
                  [yield visitSynType typeName
-                  yield! (attrs |> List.map (visitSynAttributeList returnInfo))]}
+                  yield! (visitSynAttributeLists range attrs)]}
 
     and visitSynPat(sp: SynPat): Node =
         match sp with
@@ -1052,7 +1052,7 @@ module private Ast =
              FsAstNode = sp
              Childs =
                  [yield visitSynPat synPat
-                  yield! attrs |> List.map (visitSynAttributeList sp)]}
+                  yield! (visitSynAttributeLists range attrs)]}
         | SynPat.Or(synPat,synPat2,range) ->
             {Type = "SynPat.Or"
              Range = r range
@@ -1177,7 +1177,7 @@ module private Ast =
                     if access.IsSome then yield "access" ==> (access.Value |> visitSynAccess)]
              FsAstNode = sci
              Childs =
-                 [yield! (attribs |> List.map (visitSynAttributeList sci))
+                 [yield! (visitSynAttributeLists range attribs)
                   yield! (typeParams |> List.map(visitSynTyparDecl))]}
 
     and visitSynTypeDefnRepr(stdr: SynTypeDefnRepr): Node =
@@ -1346,7 +1346,7 @@ module private Ast =
                     if access.IsSome then yield "access" ==> (access.Value |> visitSynAccess)]
              FsAstNode = sedr
              Childs =
-                 [yield! attrs |> List.map (visitSynAttributeList sedr)
+                 [yield! (visitSynAttributeLists range attrs)
                   yield visitSynUnionCase unionCase]}
 
     and visitSynAttribute(attr: SynAttribute): Node =
@@ -1360,10 +1360,22 @@ module private Ast =
          FsAstNode = attr
          Childs = [visitSynExpr attr.ArgExpr]}
 
-    and visitSynAttributeList (parent:obj) (attrs: SynAttributeList): Node =
+    and visitSynAttributeLists (parentRange:range) (attrs: SynAttributeList list) : Node list =
+        match attrs with
+        | [h] -> visitSynAttributeList parentRange h |> List.singleton
+        | _::tail ->
+            let aRanges =
+                tail
+                |> List.map (fun a -> a.Range)
+                |> fun r -> r @ [parentRange]
+            List.zip attrs aRanges
+            |> List.map (fun (a,r) -> visitSynAttributeList r a)
+        | [] -> []
+
+    and visitSynAttributeList (parentRange:range) (attrs: SynAttributeList): Node =
         {Type = "SynAttributeList"
          Range = r attrs.Range
-         Properties = p ["parent", parent]
+         Properties = p ["linesBetweenParent", box (parentRange.StartLine - attrs.Range.EndLine - 1)]
          FsAstNode = attrs
          Childs = attrs.Attributes |> List.map visitSynAttribute
         }
@@ -1379,7 +1391,7 @@ module private Ast =
              FsAstNode = uc
              Childs =
                  [yield visitSynUnionCaseType uct
-                  yield! attrs |> List.map (visitSynAttributeList uc)]}
+                  yield! (visitSynAttributeLists range attrs)]}
 
     and visitSynUnionCaseType(uct: SynUnionCaseType) =
         match uct with
@@ -1405,11 +1417,15 @@ module private Ast =
              Range = r range
              Properties = p []
              FsAstNode = sec
-             Childs = [yield! attrs |> List.map (visitSynAttributeList sec); yield visitIdent ident]}
+             Childs = [yield! (visitSynAttributeLists range attrs)
+                       yield visitIdent ident]}
 
     and visitSynField(sfield: SynField): Node =
         match sfield with
         | Field(attrs,isStatic,ident,typ,_,_,access,range) ->
+            let parentRange =
+                Option.map (fun (i:Ident) -> i.idRange) ident |> Option.defaultValue range
+
             {Type = "Field"
              Range = r range
              Properties =
@@ -1418,7 +1434,7 @@ module private Ast =
                     if access.IsSome then yield "access" ==> (access.Value |> visitSynAccess)]
              FsAstNode = sfield
              Childs =
-                 [yield! attrs |> List.map (visitSynAttributeList sfield)
+                 [yield! (visitSynAttributeLists parentRange attrs)
                   yield visitSynType typ]}
 
     and visitSynType(st: SynType) =
@@ -1560,13 +1576,18 @@ module private Ast =
     and visitSynArgInfo(sai: SynArgInfo) =
         match sai with
         | SynArgInfo(attrs,optional,ident) ->
+            let parentRange =
+                ident
+                |> Option.map (fun i -> i.idRange)
+                |> Option.defaultValue range.Zero
+
             {Type = "SynArgInfo"
              Range = noRange
              Properties =
                  p [if ident.IsSome then yield "ident" ==> i ident.Value
                     yield "optional" ==> optional]
              FsAstNode = sai
-             Childs = [yield! attrs |> List.map (visitSynAttributeList sai)]}
+             Childs = [yield! (visitSynAttributeLists parentRange attrs)]}
 
     and visitSynAccess(a: SynAccess) =
         match a with
@@ -1613,7 +1634,7 @@ module private Ast =
              FsAstNode = modOrNs
              Childs =
                  [yield! (if isModule = SynModuleOrNamespaceKind.DeclaredNamespace then visitLongIdent longIdent else [])
-                  yield! attrs |> List.map (visitSynAttributeList modOrNs)
+                  yield! (visitSynAttributeLists range attrs)
                   yield! (decls |> List.map visitSynModuleSigDecl)]}
 
     and visitSynModuleSigDecl(ast: SynModuleSigDecl) : Node =

src/Fantomas/CodePrinter.fs

@@ -427,7 +427,7 @@ and genAttributes astContext (ats: SynAttributes) =
             let dontAddNewline =
                 TriviaHelpers.``has content after that ends with``
                     (fun t -> t.Range = a.Range)
-                    (function | Directive(_) | Newline -> true | _ -> false)
+                    (function | Directive(_) | Newline | Comment(LineCommentOnSingleLine(_)) -> true | _ -> false)
                     ctx.Trivia
             let chain =
                 acc +>

src/Fantomas/Queue.fs

@@ -61,7 +61,7 @@ type Queue<'T> (data : list<'T[]>, length : int) =
         let rec exists xs =
             match xs with
             | (arr: _[]) :: tl ->
-                while r = false && i < arr.Length do
+                while not r && i < arr.Length do
                     if f arr.[i] then r <- true
                     i <- i + 1
                 i <- 0