Put attributes underneath each other if they exceeds the max line len…

…gth. (#979) * Put attributes underneath each other if they exceeds the max line length. Fixes #629. * Update existing test.
fsprojects · Aug 10, 2020 · 6d8f084 · 6d8f084
1 parent 7ae29ad
commit 6d8f084
Show file tree

Hide file tree

Showing 3 changed files with 135 additions and 4 deletions.
diff --git a/src/Fantomas.Tests/AttributeTests.fs b/src/Fantomas.Tests/AttributeTests.fs
@@ -451,3 +451,126 @@ type Commenter =
 
       DisplayName: string }
 """
+
+[<Test>]
+let ``assembly attributes remain on own line, 629`` () =
+    formatSourceString false """
+namespace AltCover.Visualizer
+
+open System
+open System.Reflection
+open System.Runtime.InteropServices
+
+[<assembly:CLSCompliant(true)>]
+[<assembly:ComVisible(false)>]
+[<assembly:AssemblyTitle("AltCover.Visualizer")>]
+[<assembly:AssemblyDescription("Coverage and static analysis visualizer for NCover (possibly extended) and OpenCover")>]
+[<assembly:System.Resources.NeutralResourcesLanguageAttribute("en-GB")>]
+()
+"""  config
+    |> prepend newline
+    |> should equal """
+namespace AltCover.Visualizer
+
+open System
+open System.Reflection
+open System.Runtime.InteropServices
+
+[<assembly:CLSCompliant(true)>]
+[<assembly:ComVisible(false)>]
+[<assembly:AssemblyTitle("AltCover.Visualizer")>]
+[<assembly:AssemblyDescription("Coverage and static analysis visualizer for NCover (possibly extended) and OpenCover")>]
+[<assembly:System.Resources.NeutralResourcesLanguageAttribute("en-GB")>]
+()
+"""
+
+[<Test>]
+let ``multiple attributes inside SynAttributes that exceeds max line length, 629`` () =
+    formatSourceString false """
+//[<ApiExplorerSettings(IgnoreApi = true)>]
+[<Route("api/v1/admin/import")>]
+type RoleAdminImportController(akkaService: AkkaService) =
+  inherit Controller()
+
+  [<HttpGet("jobs/all");
+    ProducesResponseType(typeof<bool>, 200);
+    ProducesResponseType(404);
+    Authorize(AuthorizationScopePolicies.Read)>]
+  member _.ListJobs(): Task<UserCmdResponseMsg> =
+    task {
+      return!
+        akkaService.ImporterSystem.ApiMaster <? ApiMasterMsg.GetAllJobsCmd
+    }
+
+  [<HttpPost("jobs/create");
+    DisableRequestSizeLimit;
+    RequestFormLimits(MultipartBodyLengthLimit = 509715200L);
+    ProducesResponseType(typeof<RoleChangeSummaryDto list>, 200);
+    ProducesResponseType(404);
+    Authorize(AuthorizationScopePolicies.Write)>]
+  member _.StartJob(file: IFormFile, [<FromQuery>] args: ImporterJobArgs) =
+    let importer = akkaService.ImporterSystem
+
+    ActionResult.ofAsyncResult <| asyncResult {
+      let! state =
+        (LowerCaseString.create args.State, file)
+        |> pipeObjectThroughValidation [ (fst, [stateIsValid]); (snd, [(fun s -> Ok s)]) ]
+
+      let! filePath = FormFile.downloadAsTemp file
+
+      let job =
+        { JobType = EsriBoundaryImport
+          FileToImport = filePath
+          State = state
+          DryRun = args.DryRun }
+
+      importer.ApiMaster <! StartImportCmd job
+      return Ok job
+    }
+"""  config
+    |> prepend newline
+    |> should equal """
+//[<ApiExplorerSettings(IgnoreApi = true)>]
+[<Route("api/v1/admin/import")>]
+type RoleAdminImportController(akkaService: AkkaService) =
+    inherit Controller()
+
+    [<HttpGet("jobs/all");
+      ProducesResponseType(typeof<bool>, 200);
+      ProducesResponseType(404);
+      Authorize(AuthorizationScopePolicies.Read)>]
+    member _.ListJobs(): Task<UserCmdResponseMsg> =
+        task {
+            return! akkaService.ImporterSystem.ApiMaster
+                    <? ApiMasterMsg.GetAllJobsCmd
+        }
+
+    [<HttpPost("jobs/create");
+      DisableRequestSizeLimit;
+      RequestFormLimits(MultipartBodyLengthLimit = 509715200L);
+      ProducesResponseType(typeof<RoleChangeSummaryDto list>, 200);
+      ProducesResponseType(404);
+      Authorize(AuthorizationScopePolicies.Write)>]
+    member _.StartJob(file: IFormFile, [<FromQuery>] args: ImporterJobArgs) =
+        let importer = akkaService.ImporterSystem
+
+        ActionResult.ofAsyncResult
+        <| asyncResult {
+            let! state =
+                (LowerCaseString.create args.State, file)
+                |> pipeObjectThroughValidation [ (fst, [ stateIsValid ])
+                                                 (snd, [ (fun s -> Ok s) ]) ]
+
+            let! filePath = FormFile.downloadAsTemp file
+
+            let job =
+                { JobType = EsriBoundaryImport
+                  FileToImport = filePath
+                  State = state
+                  DryRun = args.DryRun }
+
+            importer.ApiMaster <! StartImportCmd job
+
+            return Ok job
+           }
+"""
diff --git a/src/Fantomas.Tests/FunctionDefinitionTests.fs b/src/Fantomas.Tests/FunctionDefinitionTests.fs
@@ -278,9 +278,9 @@ open System.Runtime.InteropServices
 open Accessibility
 
 [<DllImport("oleacc.dll")>]
-extern int AccessibleChildren(IAccessible paccContainer, int iChildStart, int cChildren, [<Out; MarshalAs(UnmanagedType.LPArray,
-                                                                                                          SizeParamIndex =
-                                                                                                              4s)>] System.Object [] rgvarChildren, int* pcObtained)
+extern int AccessibleChildren(IAccessible paccContainer, int iChildStart, int cChildren, [<Out;
+                                                                                           MarshalAs(UnmanagedType.LPArray,
+                                                                                                     SizeParamIndex = 4s)>] System.Object [] rgvarChildren, int* pcObtained)
 """
 
 [<Test>]

diff --git a/src/Fantomas/CodePrinter.fs b/src/Fantomas/CodePrinter.fs
@@ -218,6 +218,8 @@ and genModuleDeclList astContext e =
         let newlineAfterMultiline ctx =
             let expr =
                 match List.tryHead rest with
+                | Some (SynModuleDecl.DoExpr(_, SynExpr.Const(SynConst.Unit,_),rm)) when (match m with SynModuleDecl.Attributes(a,_) -> List.length a > 1) ->
+                    sepNlnConsideringTriviaContentBeforeWithAttributes rm attrs
                 | Some rm ->
                   let attrs = getRangesFromAttributesFromModuleDeclaration rm
                   sepNln +> sepNlnConsideringTriviaContentBeforeWithAttributes rm.Range attrs
@@ -414,7 +416,13 @@ and genAttributesCore astContext (ats: SynAttribute seq) =
                 if SourceTransformer.hasParenthesis e then id else sepSpace
             opt sepColonFixed target (!-) -- s +> argSpacing +> genExpr astContext e
         |> genTrivia attr.Range
-    ifElse (Seq.isEmpty ats) sepNone (!- "[<" +> atCurrentColumn (col sepSemi ats (genAttributeExpr astContext)) -- ">]")
+
+    let shortExpression = !- "[<" +> atCurrentColumn (col sepSemi ats (genAttributeExpr astContext)) -- ">]"
+    let longExpression = !- "[<" +> atCurrentColumn (col (sepSemi +> sepNln) ats (genAttributeExpr astContext)) -- ">]"
+
+    ifElse (Seq.isEmpty ats)
+        sepNone
+        (expressionFitsOnRestOfLine shortExpression longExpression)
 
 and genOnelinerAttributes astContext ats =
     let ats = List.collect (fun a -> a.Attributes) ats