Consider wider default format for record declarations if any XML doc comments are present on fields

CHANGELOG.md

@@ -17,6 +17,7 @@
 
 * Update FCS to More cleanup, e063dd2a7005faf953619ab4f232d1e3606c7ed2
 * Rename `fsharp_ragnarok` to `fsharp_experimental_stroupstrup_style` and `fsharp_keep_indent_in_branch` to `fsharp_experimental_keep_indent_in_branch`. [#2251](https://github.com/fsprojects/fantomas/pull/2251)
+* Consider wider default format for record declarations if any XML doc comments are present on fields. [#1879](https://github.com/fsprojects/fantomas/issues/1879)
 
 ## [5.0.0-alpha-005] - 2022-05-07
 

src/Fantomas.Core.Tests/CommentTests.fs

@@ -298,12 +298,14 @@ type IlxGenIntraAssemblyInfo =
         """
 /// Non-local information related to internals of code generation within an assembly
 type IlxGenIntraAssemblyInfo =
-    { /// A table recording the generated name of the static backing fields for each mutable top level value where
-      /// we may need to take the address of that value, e.g. static mutable module-bound values which are structs. These are
-      /// only accessible intra-assembly. Across assemblies, taking the address of static mutable module-bound values is not permitted.
-      /// The key to the table is the method ref for the property getter for the value, which is a stable name for the Val's
-      /// that come from both the signature and the implementation.
-      StaticFieldInfo: Dictionary<ILMethodRef, ILFieldSpec> }
+    {
+        /// A table recording the generated name of the static backing fields for each mutable top level value where
+        /// we may need to take the address of that value, e.g. static mutable module-bound values which are structs. These are
+        /// only accessible intra-assembly. Across assemblies, taking the address of static mutable module-bound values is not permitted.
+        /// The key to the table is the method ref for the property getter for the value, which is a stable name for the Val's
+        /// that come from both the signature and the implementation.
+        StaticFieldInfo: Dictionary<ILMethodRef, ILFieldSpec>
+    }
 """
 
 [<Test>]
@@ -448,26 +450,28 @@ type IlxGenOptions =
         """
 [<NoEquality; NoComparison>]
 type IlxGenOptions =
-    { fragName: string
-      generateFilterBlocks: bool
-      workAroundReflectionEmitBugs: bool
-      emitConstantArraysUsingStaticDataBlobs: bool
-      // If this is set, then the last module becomes the "main" module and its toplevel bindings are executed at startup
-      mainMethodInfo: Tast.Attribs option
-      localOptimizationsAreOn: bool
-      generateDebugSymbols: bool
-      testFlagEmitFeeFeeAs100001: bool
-      ilxBackend: IlxGenBackend
-      /// Indicates the code is being generated in FSI.EXE and is executed immediately after code generation
-      /// This includes all interactively compiled code, including #load, definitions, and expressions
-      isInteractive: bool
-      // Indicates the code generated is an interactive 'it' expression. We generate a setter to allow clearing of the underlying
-      // storage, even though 'it' is not logically mutable
-      isInteractiveItExpr: bool
-      // Indicates System.SerializableAttribute is available in the target framework
-      netFxHasSerializableAttribute: bool
-      /// Whenever possible, use callvirt instead of call
-      alwaysCallVirt: bool }
+    {
+        fragName: string
+        generateFilterBlocks: bool
+        workAroundReflectionEmitBugs: bool
+        emitConstantArraysUsingStaticDataBlobs: bool
+        // If this is set, then the last module becomes the "main" module and its toplevel bindings are executed at startup
+        mainMethodInfo: Tast.Attribs option
+        localOptimizationsAreOn: bool
+        generateDebugSymbols: bool
+        testFlagEmitFeeFeeAs100001: bool
+        ilxBackend: IlxGenBackend
+        /// Indicates the code is being generated in FSI.EXE and is executed immediately after code generation
+        /// This includes all interactively compiled code, including #load, definitions, and expressions
+        isInteractive: bool
+        // Indicates the code generated is an interactive 'it' expression. We generate a setter to allow clearing of the underlying
+        // storage, even though 'it' is not logically mutable
+        isInteractiveItExpr: bool
+        // Indicates System.SerializableAttribute is available in the target framework
+        netFxHasSerializableAttribute: bool
+        /// Whenever possible, use callvirt instead of call
+        alwaysCallVirt: bool
+    }
 """
 
 [<Test>]

src/Fantomas.Core.Tests/Fantomas.Core.Tests.fsproj

@@ -113,6 +113,7 @@
     <Compile Include="Stroupstrup\FunctionApplicationSingleListTests.fs" />
     <Compile Include="Stroupstrup\FunctionApplicationDualListTests.fs" />
     <Compile Include="IdentTests.fs" />
+    <Compile Include="RecordDeclarationsWithXMLDocTests.fs" />
   </ItemGroup>
   <ItemGroup>
     <ProjectReference Include="..\Fantomas.Extras\Fantomas.Extras.fsproj" />

src/Fantomas.Core.Tests/NumberOfItemsRecordTests.fs

@@ -440,14 +440,16 @@ module RecordSignature
 
 /// Represents simple XML elements.
 type Element =
-    { /// The attribute collection.
-      Attributes: IDictionary<Name, string>
+    {
+        /// The attribute collection.
+        Attributes: IDictionary<Name, string>
 
-      /// The children collection.
-      Children: seq<INode>
+        /// The children collection.
+        Children: seq<INode>
 
-      /// The qualified name.
-      Name: Name }
+        /// The qualified name.
+        Name: Name
+    }
 """
 
 [<Test>]

src/Fantomas.Core.Tests/RecordDeclarationsWithXMLDocTests.fs

@@ -0,0 +1,195 @@
+module Fantomas.Core.Tests.RecordDeclarationsWithXMLDocTests
+
+open NUnit.Framework
+open FsUnit
+open Fantomas.Core.Tests.TestHelper
+
+[<Test>]
+let ``each record field has xml comment`` () =
+    formatSourceString
+        false
+        """
+/// Represents additional information for SynExpr.TryWith
+[<NoEquality; NoComparison>]
+type SynExprTryWithTrivia =
+    { /// The syntax range of the `try` keyword.
+      TryKeyword: range
+
+      /// The syntax range from the beginning of the `try` keyword till the end of the `with` keyword.
+      TryToWithRange: range
+
+      /// The syntax range of the `with` keyword
+      WithKeyword: range
+
+      /// The syntax range from the beginning of the `with` keyword till the end of the TryWith expression.
+      WithToEndRange: range }
+"""
+        config
+    |> prepend newline
+    |> should
+        equal
+        """
+/// Represents additional information for SynExpr.TryWith
+[<NoEquality; NoComparison>]
+type SynExprTryWithTrivia =
+    {
+        /// The syntax range of the `try` keyword.
+        TryKeyword: range
+
+        /// The syntax range from the beginning of the `try` keyword till the end of the `with` keyword.
+        TryToWithRange: range
+
+        /// The syntax range of the `with` keyword
+        WithKeyword: range
+
+        /// The syntax range from the beginning of the `with` keyword till the end of the TryWith expression.
+        WithToEndRange: range
+    }
+"""
+
+[<Test>]
+let ``each record field has xml comment in signature file`` () =
+    formatSourceString
+        true
+        """
+/// Represents additional information for SynExpr.TryWith
+[<NoEquality; NoComparison>]
+type SynExprTryWithTrivia =
+    { /// The syntax range of the `try` keyword.
+      TryKeyword: range
+
+      /// The syntax range from the beginning of the `try` keyword till the end of the `with` keyword.
+      TryToWithRange: range
+
+      /// The syntax range of the `with` keyword
+      WithKeyword: range
+
+      /// The syntax range from the beginning of the `with` keyword till the end of the TryWith expression.
+      WithToEndRange: range }
+"""
+        config
+    |> prepend newline
+    |> should
+        equal
+        """
+/// Represents additional information for SynExpr.TryWith
+[<NoEquality; NoComparison>]
+type SynExprTryWithTrivia =
+    {
+        /// The syntax range of the `try` keyword.
+        TryKeyword: range
+
+        /// The syntax range from the beginning of the `try` keyword till the end of the `with` keyword.
+        TryToWithRange: range
+
+        /// The syntax range of the `with` keyword
+        WithKeyword: range
+
+        /// The syntax range from the beginning of the `with` keyword till the end of the TryWith expression.
+        WithToEndRange: range
+    }
+"""
+
+[<Test>]
+let ``a single record field has xml doc comment`` () =
+    formatSourceString
+        false
+        """
+type SynExprTryWithTrivia =
+    { TryKeyword: range
+      /// The syntax range from the beginning of the `try` keyword till the end of the `with` keyword.
+      TryToWithRange: range
+      WithKeyword: range
+      WithToEndRange: range }
+"""
+        config
+    |> prepend newline
+    |> should
+        equal
+        """
+type SynExprTryWithTrivia =
+    {
+        TryKeyword: range
+        /// The syntax range from the beginning of the `try` keyword till the end of the `with` keyword.
+        TryToWithRange: range
+        WithKeyword: range
+        WithToEndRange: range
+    }
+"""
+
+[<Test>]
+let ``a single record field has xml doc comment in signature file`` () =
+    formatSourceString
+        true
+        """
+type SynExprTryWithTrivia =
+    { TryKeyword: range
+      /// The syntax range from the beginning of the `try` keyword till the end of the `with` keyword.
+      TryToWithRange: range
+      WithKeyword: range
+      WithToEndRange: range }
+"""
+        config
+    |> prepend newline
+    |> should
+        equal
+        """
+type SynExprTryWithTrivia =
+    {
+        TryKeyword: range
+        /// The syntax range from the beginning of the `try` keyword till the end of the `with` keyword.
+        TryToWithRange: range
+        WithKeyword: range
+        WithToEndRange: range
+    }
+"""
+
+[<Test>]
+let ``no xml docs, should be cramped style`` () =
+    formatSourceString
+        false
+        """
+type SynExprTryWithTrivia =
+    {
+        TryKeyword: range
+        TryToWithRange: range
+        WithKeyword: range
+        WithToEndRange: range
+    }
+"""
+        config
+    |> prepend newline
+    |> should
+        equal
+        """
+type SynExprTryWithTrivia =
+    { TryKeyword: range
+      TryToWithRange: range
+      WithKeyword: range
+      WithToEndRange: range }
+"""
+
+[<Test>]
+let ``no xml docs, should be cramped style in signature file`` () =
+    formatSourceString
+        true
+        """
+type SynExprTryWithTrivia =
+    {
+        TryKeyword: range
+        TryToWithRange: range
+        WithKeyword: range
+        WithToEndRange: range
+    }
+"""
+        config
+    |> prepend newline
+    |> should
+        equal
+        """
+type SynExprTryWithTrivia =
+    { TryKeyword: range
+      TryToWithRange: range
+      WithKeyword: range
+      WithToEndRange: range }
+"""

src/Fantomas.Core.Tests/RecordTests.fs

@@ -74,14 +74,16 @@ module RecordSignature
 
 /// Represents simple XML elements.
 type Element =
-    { /// The attribute collection.
-      Attributes: IDictionary<Name, string>
+    {
+        /// The attribute collection.
+        Attributes: IDictionary<Name, string>
 
-      /// The children collection.
-      Children: seq<INode>
+        /// The children collection.
+        Children: seq<INode>
 
-      /// The qualified name.
-      Name: Name }
+        /// The qualified name.
+        Name: Name
+    }
 
     interface INode
 
@@ -825,14 +827,16 @@ module RecordSignature
 
 /// Represents simple XML elements.
 type Element =
-    { /// The attribute collection.
-      Attributes: IDictionary<Name, string>
+    {
+        /// The attribute collection.
+        Attributes: IDictionary<Name, string>
 
-      /// The children collection.
-      Children: seq<INode>
+        /// The children collection.
+        Children: seq<INode>
 
-      /// The qualified name.
-      Name: Name }
+        /// The qualified name.
+        Name: Name
+    }
 """
 
 [<Test>]

src/Fantomas.Core/CodePrinter.fs

@@ -3593,17 +3593,20 @@ and genTypeDefn
             +> col sepSemi fs (genField astContext "")
             +> genTriviaFor SynTypeDefnSimpleRepr_Record_ClosingBrace closingBrace sepCloseS
 
-        let multilineExpression =
-            ifAlignBrackets
-                (genMultilineSimpleRecordTypeDefnAlignBrackets
+        let multilineExpression (ctx: Context) =
+            if ctx.Config.MultilineBlockBracketsOnSameColumn
+               || (List.exists (fun (SynField (xmlDoc = xmlDoc)) -> not xmlDoc.IsEmpty) fs) then
+                genMultilineSimpleRecordTypeDefnAlignBrackets
                     astContext
                     openingBrace
                     withKeyword
                     ms
                     ao'
                     fs
-                    closingBrace)
-                (genMultilineSimpleRecordTypeDefn astContext openingBrace withKeyword ms ao' fs closingBrace)
+                    closingBrace
+                    ctx
+            else
+                genMultilineSimpleRecordTypeDefn astContext openingBrace withKeyword ms ao' fs closingBrace ctx
 
         let bodyExpr size ctx =
             if (List.isEmpty ms) then
@@ -3898,14 +3901,16 @@ and genSigTypeDefn
         let smallExpression =
             sepSpace
             +> optSingle (fun ao -> genAccess ao +> sepSpace) ao'
-            +> genTriviaFor SynExpr_Record_OpeningBrace openingBrace sepOpenS
+            +> genTriviaFor SynTypeDefnSimpleRepr_Record_OpeningBrace openingBrace sepOpenS
             +> col sepSemi fs (genField astContext "")
-            +> genTriviaFor SynExpr_Record_ClosingBrace closingBrace sepCloseS
+            +> genTriviaFor SynTypeDefnSimpleRepr_Record_ClosingBrace closingBrace sepCloseS
 
-        let multilineExpression =
-            ifAlignBrackets
-                (genSigSimpleRecordAlignBrackets astContext openingBrace withKeyword ms ao' fs closingBrace)
-                (genSigSimpleRecord astContext openingBrace withKeyword ms ao' fs closingBrace)
+        let multilineExpression (ctx: Context) =
+            if ctx.Config.MultilineBlockBracketsOnSameColumn
+               || (List.exists (fun (SynField (xmlDoc = xmlDoc)) -> not xmlDoc.IsEmpty) fs) then
+                genSigSimpleRecordAlignBrackets astContext openingBrace withKeyword ms ao' fs closingBrace ctx
+            else
+                genSigSimpleRecord astContext openingBrace withKeyword ms ao' fs closingBrace ctx
 
         let bodyExpr size ctx =
             if (List.isEmpty ms) then