From 2fdf4767fa42b8bac065187f69f748d3805ec01f Mon Sep 17 00:00:00 2001 From: nojaf Date: Wed, 22 Sep 2021 21:08:14 +0200 Subject: [PATCH] Indent multiline record field expressions from the opening brace. --- src/Fantomas.Tests/CommentTests.fs | 47 +++++ src/Fantomas.Tests/CompilerDirectivesTests.fs | 8 +- .../NumberOfItemsRecordTests.fs | 32 +-- src/Fantomas.Tests/OperatorTests.fs | 8 +- src/Fantomas.Tests/RecordTests.fs | 183 +++++++++++++----- src/Fantomas/CodePrinter.fs | 111 +++++++---- src/Fantomas/Context.fs | 19 +- 7 files changed, 301 insertions(+), 107 deletions(-) diff --git a/src/Fantomas.Tests/CommentTests.fs b/src/Fantomas.Tests/CommentTests.fs index 80a4088833..bf1d54e482 100644 --- a/src/Fantomas.Tests/CommentTests.fs +++ b/src/Fantomas.Tests/CommentTests.fs @@ -289,6 +289,53 @@ let a = B = 7 } """ +[] +let ``comment alignment above record field`` () = + formatSourceString + false + """ +let a = + { c = 4 + // foo + // bar + B = 7 } +""" + config + |> prepend newline + |> should + equal + """ +let a = + { c = 4 + // foo + // bar + B = 7 } +""" + +[] +let ``comment alignment above record field, fsharp_space_around_delimiter = false`` () = + formatSourceString + false + """ +let a = + { c = 4 + // foo + // bar + B = 7 } +""" + { config with + SpaceAroundDelimiter = false } + |> prepend newline + |> should + equal + """ +let a = + {c = 4 + // foo + // bar + B = 7} +""" + [] let ``shouldn't break on one-line comment`` () = formatSourceString diff --git a/src/Fantomas.Tests/CompilerDirectivesTests.fs b/src/Fantomas.Tests/CompilerDirectivesTests.fs index 965f290d94..e0d126a17d 100644 --- a/src/Fantomas.Tests/CompilerDirectivesTests.fs +++ b/src/Fantomas.Tests/CompilerDirectivesTests.fs @@ -1911,7 +1911,7 @@ let config = #if WATCH #else - "https://fsprojects.github.io/fantomas/" + "https://fsprojects.github.io/fantomas/" #endif } """ @@ -1944,7 +1944,7 @@ let config = theme_variant = Some "red" root_url = #if WATCH - "http://localhost:8080/" + "http://localhost:8080/" #else #endif @@ -1979,9 +1979,9 @@ let config = theme_variant = Some "red" root_url = #if WATCH - "http://localhost:8080/" + "http://localhost:8080/" #else - "https://fsprojects.github.io/fantomas/" + "https://fsprojects.github.io/fantomas/" #endif } """ diff --git a/src/Fantomas.Tests/NumberOfItemsRecordTests.fs b/src/Fantomas.Tests/NumberOfItemsRecordTests.fs index 8850c0a008..46f455e8f8 100644 --- a/src/Fantomas.Tests/NumberOfItemsRecordTests.fs +++ b/src/Fantomas.Tests/NumberOfItemsRecordTests.fs @@ -70,8 +70,8 @@ let myRecord = Progress = "foo" Bar = { Zeta = "bar" } Address = - { Street = "Bakerstreet" - ZipCode = "9000" } + { Street = "Bakerstreet" + ZipCode = "9000" } Number = 42 } """ @@ -220,8 +220,8 @@ let ``anonymous record with multiple field update`` () = """ let a = {| foo with - Level = 7 - Square = 9 |} + Level = 7 + Square = 9 |} """ [] @@ -272,12 +272,12 @@ let anonRecord = """ let anonRecord = {| A = - {| A1 = "string" - A2LongerIdentifier = "foo" |} + {| A1 = "string" + A2LongerIdentifier = "foo" |} B = {| B1 = 7 |} C = - { C1 = "foo" - C2LongerIdentifier = "bar" } + { C1 = "foo" + C2LongerIdentifier = "bar" } D = { D1 = "bar" } |} """ @@ -797,9 +797,9 @@ let config = theme_variant = Some "red" root_url = #if WATCH - "http://localhost:8080/" + "http://localhost:8080/" #else - "https://fsprojects.github.io/fantomas/" + "https://fsprojects.github.io/fantomas/" #endif } """ @@ -1036,9 +1036,9 @@ let s = let r' = {| r with - a = x - b = y - z = c |} + a = x + b = y + z = c |} let s' = {| s with AReallyLongExpressionThatIsMuchLongerThan50Characters = 1 |} @@ -1054,9 +1054,9 @@ g s {| AReallyLongExpressionThatIsMuchLongerThan50Characters = 1 |} f r' {| r with - a = x - b = y - z = c |} + a = x + b = y + z = c |} g s' {| s with AReallyLongExpressionThatIsMuchLongerThan50Characters = 1 |} """ diff --git a/src/Fantomas.Tests/OperatorTests.fs b/src/Fantomas.Tests/OperatorTests.fs index 600c6ffe43..d4db9ca4b0 100644 --- a/src/Fantomas.Tests/OperatorTests.fs +++ b/src/Fantomas.Tests/OperatorTests.fs @@ -766,11 +766,11 @@ Fooey " let r = {| Foo = - a - && // && b - c + a + && // && b + c Bar = - \"\"\" + \"\"\" Fooey \"\"\" |} " diff --git a/src/Fantomas.Tests/RecordTests.fs b/src/Fantomas.Tests/RecordTests.fs index 572153aa56..1e3449c361 100644 --- a/src/Fantomas.Tests/RecordTests.fs +++ b/src/Fantomas.Tests/RecordTests.fs @@ -409,7 +409,7 @@ let r = && // && b c Bar = - \"\"\" + \"\"\" Fooey \"\"\" |} " @@ -533,22 +533,22 @@ type Database = .defaults( { Version = CurrentVersion Questions = - [| { Id = 0 - AuthorId = 1 - Title = \"What is the average wing speed of an unladen swallow?\" - Description = - \"\"\" + [| { Id = 0 + AuthorId = 1 + Title = \"What is the average wing speed of an unladen swallow?\" + Description = + \"\"\" Hello, yesterday I saw a flight of swallows and was wondering what their **average wing speed** is? If you know the answer please share it. \"\"\" - Answers = - [| { Id = 0 - CreatedAt = DateTime.Parse \"2017-09-14T19:57:33.103Z\" - AuthorId = 0 - Score = 2 - Content = - \"\"\" + Answers = + [| { Id = 0 + CreatedAt = DateTime.Parse \"2017-09-14T19:57:33.103Z\" + AuthorId = 0 + Score = 2 + Content = + \"\"\" > What do you mean, an African or European Swallow? > > Monty Python’s: The Holy Grail @@ -557,12 +557,12 @@ Ok I must admit, I use google to search the question and found a post explaining I thought you were asking it seriously, well done. x\"\"\" } - { Id = 1 - CreatedAt = DateTime.Parse \"2017-09-14T20:07:27.103Z\" - AuthorId = 2 - Score = 1 - Content = - \"\"\" + { Id = 1 + CreatedAt = DateTime.Parse \"2017-09-14T20:07:27.103Z\" + AuthorId = 2 + Score = 1 + Content = + \"\"\" Maxime, I believe you found [this blog post](http://www.saratoga.com/how-should-i-know/2013/07/what-is-the-average-air-speed-velocity-of-a-laden-swallow/). @@ -571,35 +571,35 @@ And so Robin, the conclusion of the post is: > In the end, it’s concluded that the airspeed velocity of a (European) unladen swallow is about 24 miles per hour or 11 meters per second. \"\"\" } |] - CreatedAt = DateTime.Parse \"2017-09-14T17:44:28.103Z\" } - { Id = 1 - AuthorId = 0 - Title = \"Why did you create Fable?\" - Description = - \"\"\" + CreatedAt = DateTime.Parse \"2017-09-14T17:44:28.103Z\" } + { Id = 1 + AuthorId = 0 + Title = \"Why did you create Fable?\" + Description = + \"\"\" Hello Alfonso, I wanted to know why you created Fable. Did you always plan to use F#? Or were you thinking in others languages? \"\"\" - Answers = [||] - CreatedAt = DateTime.Parse \"2017-09-12T09:27:28.103Z\" } |] + Answers = [||] + CreatedAt = DateTime.Parse \"2017-09-12T09:27:28.103Z\" } |] Users = - [| { Id = 0 - Firstname = \"Maxime\" - Surname = \"Mangel\" - Avatar = \"maxime_mangel.png\" } - { Id = 1 - Firstname = \"Robin\" - Surname = \"Munn\" - Avatar = \"robin_munn.png\" } - { Id = 2 - Firstname = \"Alfonso\" - Surname = \"Garciacaro\" - Avatar = \"alfonso_garciacaro.png\" } - { Id = 3 - Firstname = \"Guest\" - Surname = \"\" - Avatar = \"guest.png\" } |] } + [| { Id = 0 + Firstname = \"Maxime\" + Surname = \"Mangel\" + Avatar = \"maxime_mangel.png\" } + { Id = 1 + Firstname = \"Robin\" + Surname = \"Munn\" + Avatar = \"robin_munn.png\" } + { Id = 2 + Firstname = \"Alfonso\" + Surname = \"Garciacaro\" + Avatar = \"alfonso_garciacaro.png\" } + { Id = 3 + Firstname = \"Guest\" + Surname = \"\" + Avatar = \"guest.png\" } |] } ) .write () @@ -1715,8 +1715,8 @@ let ``anonymous record with multiline field, indent 2`` () = equal """ {| Foo = - // meh - someValue |} + // meh + someValue |} """ [] @@ -1728,7 +1728,7 @@ let ``anonymous record with multiline field, indent 3`` () = // meh someValue |} """ - { config with IndentSize = 2 } + { config with IndentSize = 3 } |> prepend newline |> should equal @@ -1736,4 +1736,93 @@ let ``anonymous record with multiline field, indent 3`` () = {| Foo = // meh someValue |} -""" \ No newline at end of file +""" + +[] +let ``anonymous record with multiline field, indent 5`` () = + formatSourceString + false + """ +{| Foo = + // meh + someValue |} +""" + { config with IndentSize = 5 } + |> prepend newline + |> should + equal + """ +{| Foo = + // meh + someValue |} +""" + +[] +let ``a foo`` () = + formatSourceString + false + """ +{| Foo = + someValue + // + a |} +""" + { config with IndentSize = 3 } + |> prepend newline + |> should + equal + """ +{| Foo = + someValue + // + a |} +""" + +[] +let ``long record field assigment`` () = + formatSourceString + false + """ +{ A = + // one indent starting from { + someFunctionCall + arg1 + arg2 + B = + // one indent starting from label B + someFunctionCall + arg1 + arg2 } +""" + config + |> prepend newline + |> should + equal + """ +{ A = + // one indent starting from { + someFunctionCall arg1 arg2 + B = + // one indent starting from label B + someFunctionCall arg1 arg2 } +""" + +[] +let ``anonymous update record, indent_size 3`` () = + formatSourceString + false + """ +{| f with Foo = + // meh + someValue |} +""" + { config with IndentSize = 3 } + |> prepend newline + |> should + equal + """ +{| f with + Foo = + // meh + someValue |} +""" diff --git a/src/Fantomas/CodePrinter.fs b/src/Fantomas/CodePrinter.fs index 20725ee2d2..80f568a7fb 100644 --- a/src/Fantomas/CodePrinter.fs +++ b/src/Fantomas/CodePrinter.fs @@ -1371,7 +1371,7 @@ and genExpr astContext synExpr ctx = let longExpression = ifAlignBrackets (genMultilineAnonRecordAlignBrackets isStruct fields copyInfo astContext) - (genMultilineAnonRecord isStruct fields copyInfo astContext) + (genMultilineAnonRecord isStruct fields copyInfo synExpr.Range astContext) fun (ctx: Context) -> let size = getRecordSize ctx fields @@ -2971,24 +2971,40 @@ and genMultilineRecordInstance | None -> match eo with | None -> - tokN synExpr.Range LBRACE sepOpenS - +> atCurrentColumn ( - sepNlnWhenWriteBeforeNewlineNotEmpty sepNone // comment after curly brace - +> fieldsExpr - ) - +> tokN - synExpr.Range - RBRACE - (sepNlnWhenWriteBeforeNewlineNotEmpty sepNone // comment after last record field - +> (fun ctx -> - // Edge case scenario to make sure that the closing brace is not before the opening one - // See unit test "multiline string before closing brace" - let delta = expressionStartColumn - ctx.Column - - if delta > 0 then - ((rep delta (!- " ")) +> sepCloseSFixed) ctx - else - ifElseCtx lastWriteEventIsNewline sepCloseSFixed sepCloseS ctx)) + fun (ctx: Context) -> + // position after `{ ` or `{` + let targetColumn = + ctx.Column + + (if ctx.Config.SpaceAroundDelimiter then + 2 + else + 1) + + atCurrentColumn + (tokN synExpr.Range LBRACE sepOpenS + +> sepNlnWhenWriteBeforeNewlineNotEmpty sepNone // comment after curly brace + +> col + sepSemiNln + xs + (fun e -> + // Add spaces to ensure the record field (incl trivia) starts at the right column. + addFixedSpaces targetColumn + // Lock the start of the record field, however keep potential indentations in relation to the opening curly brace + +> atCurrentColumn (genRecordFieldName astContext e)) + +> tokN + synExpr.Range + RBRACE + (sepNlnWhenWriteBeforeNewlineNotEmpty sepNone // comment after last record field + +> (fun ctx -> + // Edge case scenario to make sure that the closing brace is not before the opening one + // See unit test "multiline string before closing brace" + let delta = expressionStartColumn - ctx.Column + + if delta > 0 then + ((rep delta (!- " ")) +> sepCloseSFixed) ctx + else + ifElseCtx lastWriteEventIsNewline sepCloseSFixed sepCloseS ctx))) + ctx | Some e -> tokN synExpr.Range LBRACE sepOpenS +> genExpr astContext e @@ -3053,25 +3069,52 @@ and genMultilineRecordInstanceAlignBrackets +> sepCloseSFixed) |> atCurrentColumnIndent -and genMultilineAnonRecord (isStruct: bool) fields copyInfo astContext = +and genMultilineAnonRecord (isStruct: bool) fields copyInfo (range: Range) (astContext: ASTContext) = let recordExpr = - let fieldsExpr = - col sepSemiNln fields (genAnonRecordFieldName astContext) - match copyInfo with | Some e -> - genExpr astContext e - +> (!- " with" - +> indent - +> sepNln - +> fieldsExpr - +> unindent) - | None -> fieldsExpr + (tokN range LBRACK_BAR sepOpenAnonRecd) + +> atCurrentColumn ( + genExpr astContext e + +> (!- " with" + +> indent + +> sepNln + +> col sepSemiNln fields (genAnonRecordFieldName astContext) + +> unindent) + ) + +> (tokN range BAR_RBRACK sepCloseAnonRecd) + | None -> + fun ctx -> + // position after `{| ` or `{|` + let targetColumn = + ctx.Column + + (if ctx.Config.SpaceAroundDelimiter then + 3 + else + 2) + + atCurrentColumn + ((tokN range LBRACK_BAR sepOpenAnonRecd) + +> col + sepSemiNln + fields + (fun (AnonRecordFieldName (s, e)) -> + let expr = + if ctx.Config.IndentSize < 3 then + sepSpaceOrDoubleIndentAndNlnIfExpressionExceedsPageWidth (genExpr astContext e) + else + sepSpaceOrIndentAndNlnIfExpressionExceedsPageWidth (genExpr astContext e) + + // Add enough spaces to start at the right column but indent from the opening curly brace. + // Use a double indent when using a small indent size to avoid offset warnings. + addFixedSpaces targetColumn + +> !-s + +> sepEq + +> expr) + +> (tokN range BAR_RBRACK sepCloseAnonRecd)) + ctx - onlyIf isStruct !- "struct " - +> sepOpenAnonRecd - +> atCurrentColumn recordExpr - +> sepCloseAnonRecd + onlyIf isStruct !- "struct " +> recordExpr and genMultilineAnonRecordAlignBrackets (isStruct: bool) fields copyInfo astContext = let fieldsExpr = diff --git a/src/Fantomas/Context.fs b/src/Fantomas/Context.fs index 180225f640..cae67aac20 100644 --- a/src/Fantomas/Context.fs +++ b/src/Fantomas/Context.fs @@ -674,6 +674,12 @@ let internal sepSpace (ctx: Context) = | None -> ctx | _ -> (!- " ") ctx +// add actual spaces until the target column is reached, regardless of previous content +// use with care +let internal addFixedSpaces (targetColumn: int) (ctx: Context) : Context = + let delta = targetColumn - ctx.Column + onlyIf (delta > 0) (rep delta (!- " ")) ctx + let internal sepNln = !+ "" // Use a different WriteLine event to indicate that the newline was introduces due to trivia @@ -994,6 +1000,15 @@ let internal sepSpaceOrIndentAndNlnIfExpressionExceedsPageWidth expr (ctx: Conte expr ctx +let internal sepSpaceOrDoubleIndentAndNlnIfExpressionExceedsPageWidth expr (ctx: Context) = + expressionExceedsPageWidth + sepSpace + sepNone // before and after for short expressions + (indent +> indent +> sepNln) + (unindent +> unindent) // before and after for long expressions + expr + ctx + let internal sepSpaceWhenOrIndentAndNlnIfExpressionExceedsPageWidth (addSpace: Context -> bool) expr (ctx: Context) = expressionExceedsPageWidth (ifElseCtx addSpace sepSpace sepNone) @@ -1139,13 +1154,13 @@ let internal ifAlignBrackets f g = ifElseCtx (fun ctx -> ctx.Config.MultilineBlockBracketsOnSameColumn) f g let internal printTriviaContent (c: TriviaContent) (ctx: Context) = - let currentLastLine = lastWriteEventOnLastLine ctx + let currentLastLine = ctx.WriterModel.Lines |> List.tryHead // Some items like #if or Newline should be printed on a newline // It is hard to always get this right in CodePrinter, so we detect it based on the current code. let addNewline = currentLastLine - |> Option.map (fun line -> line.Length > 0) + |> Option.map (fun line -> line.Trim().Length > 0) |> Option.defaultValue false let addSpace =