From 721b129e882a3d2720c9efc90c2fbf203472eee2 Mon Sep 17 00:00:00 2001 From: dawe Date: Wed, 19 Oct 2022 18:39:02 +0200 Subject: [PATCH 1/3] Capture StartEndRange information in SynExpr.AnonRecd to fix #2566 --- CHANGELOG.md | 5 ++ ...ineBlockBracketsOnSameColumnRecordTests.fs | 57 +++++++++++++++++++ src/Fantomas.Core/AstTransformer.fs | 7 ++- src/Fantomas.Core/CodePrinter.fs | 16 ++++-- src/Fantomas.Core/SourceParser.fs | 3 +- src/Fantomas.Core/TriviaTypes.fs | 2 + 6 files changed, 81 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b54eed7c3..4f9a19ae6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## [Unreleased] + +### Fixed +* Idempotency problem when having a comment in an anonymous record and Stroustrup formatting. [#2566](https://github.com/fsprojects/fantomas/issues/2566) + ## [5.1.0-beta-001] - 2022-10-19 ### Fixed diff --git a/src/Fantomas.Core.Tests/MultilineBlockBracketsOnSameColumnRecordTests.fs b/src/Fantomas.Core.Tests/MultilineBlockBracketsOnSameColumnRecordTests.fs index c21b3d8ac8..4c400c385e 100644 --- a/src/Fantomas.Core.Tests/MultilineBlockBracketsOnSameColumnRecordTests.fs +++ b/src/Fantomas.Core.Tests/MultilineBlockBracketsOnSameColumnRecordTests.fs @@ -1395,3 +1395,60 @@ let f = x """ + +[] +let ``comment in last line of anonymous type, 2566`` () = + formatSourceString + false + """ +let x = {| + Y = 42 + Z = "string" + Foo = "Bar" + // test + |} + +let y = {| + Y = 42 + // test +|} + +let z = {| + Y = 42 +|} + +let a = {| + foo with + Level = 7 + Square = 9 + // test +|} +""" + config + |> prepend newline + |> should + equal + """ +let x = + {| + Y = 42 + Z = "string" + Foo = "Bar" + // test + |} + +let y = + {| + Y = 42 + // test + |} + +let z = {| Y = 42 |} + +let a = + {| foo with + Level = 7 + Square = 9 + // test + |} +""" diff --git a/src/Fantomas.Core/AstTransformer.fs b/src/Fantomas.Core/AstTransformer.fs index 30c9566314..566d606171 100644 --- a/src/Fantomas.Core/AstTransformer.fs +++ b/src/Fantomas.Core/AstTransformer.fs @@ -231,12 +231,15 @@ and visitSynExpr (synExpr: SynExpr) : TriviaNode list = yield mkNode SynExpr_Record_ClosingBrace closingBrace |]) |> List.singleton |> finalContinuation - | SynExpr.AnonRecd (_, _, recordFields, range) -> + | SynExpr.AnonRecd (_, _, recordFields, StartEndRange 1 (openingBrace, range, closingBrace)) -> mkSynExprNode SynExpr_AnonRecd synExpr range - (sortChildren [| yield! List.map visitAnonRecordField recordFields |]) + (sortChildren + [| yield mkNode SynExpr_AnonRecd_OpeningBrace openingBrace + yield! List.map visitAnonRecordField recordFields + yield mkNode SynExpr_AnonRecd_ClosingBrace closingBrace |]) |> List.singleton |> finalContinuation | SynExpr.New (_, typeName, expr, range) -> diff --git a/src/Fantomas.Core/CodePrinter.fs b/src/Fantomas.Core/CodePrinter.fs index b3bba00970..84a1defd98 100644 --- a/src/Fantomas.Core/CodePrinter.fs +++ b/src/Fantomas.Core/CodePrinter.fs @@ -915,17 +915,17 @@ and genExpr synExpr ctx = let size = getRecordSize ctx xs isSmallExpression size smallRecordExpr multilineRecordExpr ctx - | AnonRecord (isStruct, fields, copyInfo) -> + | AnonRecord (openingBrace, isStruct, fields, copyInfo, closingBrace) -> let smallExpression = onlyIf isStruct !- "struct " +> sepOpenAnonRecd +> optSingle (fun e -> genExpr e +> !- " with ") copyInfo +> col sepSemi fields genAnonRecordFieldName - +> sepCloseAnonRecd + +> genTriviaFor SynExpr_AnonRecd_ClosingBrace closingBrace (sepSpace +> sepCloseAnonRecdFixed) let longExpression = ifAlignBrackets - (genMultilineAnonRecordAlignBrackets isStruct fields copyInfo) + (genMultilineAnonRecordAlignBrackets openingBrace isStruct fields copyInfo closingBrace) (genMultilineAnonRecord isStruct fields copyInfo) fun (ctx: Context) -> @@ -2399,7 +2399,7 @@ and genMultilineAnonRecord (isStruct: bool) fields copyInfo = onlyIf isStruct !- "struct " +> recordExpr -and genMultilineAnonRecordAlignBrackets (isStruct: bool) fields copyInfo = +and genMultilineAnonRecordAlignBrackets _ (isStruct: bool) fields copyInfo closingBrace = let fieldsExpr = col sepNln fields genAnonRecordFieldName let copyExpr fieldsExpr e = @@ -2414,12 +2414,16 @@ and genMultilineAnonRecordAlignBrackets (isStruct: bool) fields copyInfo = let genAnonRecord = match copyInfo with - | Some ci -> sepOpenAnonRecd +> copyExpr fieldsExpr ci +> sepNln +> sepCloseAnonRecdFixed + | Some ci -> + sepOpenAnonRecd + +> copyExpr fieldsExpr ci + +> sepNln + +> genTriviaFor SynExpr_AnonRecd_ClosingBrace closingBrace sepCloseAnonRecdFixed | None -> sepOpenAnonRecdFixed +> indentSepNlnUnindent fieldsExpr +> sepNln - +> sepCloseAnonRecdFixed + +> genTriviaFor SynExpr_AnonRecd_ClosingBrace closingBrace sepCloseAnonRecdFixed ifElse isStruct !- "struct " sepNone +> genAnonRecord diff --git a/src/Fantomas.Core/SourceParser.fs b/src/Fantomas.Core/SourceParser.fs index fe27efb03d..1ad8e825f1 100644 --- a/src/Fantomas.Core/SourceParser.fs +++ b/src/Fantomas.Core/SourceParser.fs @@ -1071,7 +1071,8 @@ let (|Record|_|) = let (|AnonRecord|_|) = function - | SynExpr.AnonRecd (isStruct, copyInfo, fields, _) -> Some(isStruct, fields, Option.map fst copyInfo) + | SynExpr.AnonRecd (isStruct, copyInfo, fields, StartEndRange 1 (openingBrace, _, closingBrace)) -> + Some(openingBrace, isStruct, fields, Option.map fst copyInfo, closingBrace) | _ -> None let (|ObjExpr|_|) = diff --git a/src/Fantomas.Core/TriviaTypes.fs b/src/Fantomas.Core/TriviaTypes.fs index e69b3d2c03..00262a5934 100644 --- a/src/Fantomas.Core/TriviaTypes.fs +++ b/src/Fantomas.Core/TriviaTypes.fs @@ -75,6 +75,8 @@ type FsAstType = | SynExpr_Record_OpeningBrace | SynExpr_Record_ClosingBrace | SynExpr_AnonRecd + | SynExpr_AnonRecd_OpeningBrace + | SynExpr_AnonRecd_ClosingBrace | SynExpr_AnonRecd_Field | SynExpr_AnonRecd_Field_Equals | SynExpr_New From e599116b2f72cb7351f5aab2afc328df6b66aae3 Mon Sep 17 00:00:00 2001 From: dawe Date: Thu, 20 Oct 2022 22:13:32 +0200 Subject: [PATCH 2/3] - fix StartEndRange size parameter - pass openingBrace, closingBrace to genMultilineAnonRecord, too - call genTriviaFor for openingBrace, too - expand tests to cover opening braces --- ...ineBlockBracketsOnSameColumnRecordTests.fs | 18 +++--- src/Fantomas.Core.Tests/RecordTests.fs | 56 +++++++++++++++++++ src/Fantomas.Core/AstTransformer.fs | 2 +- src/Fantomas.Core/CodePrinter.fs | 22 ++++---- src/Fantomas.Core/SourceParser.fs | 2 +- 5 files changed, 78 insertions(+), 22 deletions(-) diff --git a/src/Fantomas.Core.Tests/MultilineBlockBracketsOnSameColumnRecordTests.fs b/src/Fantomas.Core.Tests/MultilineBlockBracketsOnSameColumnRecordTests.fs index 4c400c385e..ecc80086dd 100644 --- a/src/Fantomas.Core.Tests/MultilineBlockBracketsOnSameColumnRecordTests.fs +++ b/src/Fantomas.Core.Tests/MultilineBlockBracketsOnSameColumnRecordTests.fs @@ -1397,15 +1397,15 @@ let f """ [] -let ``comment in last line of anonymous type, 2566`` () = +let ``comment in bracket ranges of anonymous type, 2566`` () = formatSourceString false """ -let x = {| +let x = {| // test1 Y = 42 Z = "string" Foo = "Bar" - // test + // test2 |} let y = {| @@ -1417,11 +1417,11 @@ let z = {| Y = 42 |} -let a = {| +let a = {| // test1 foo with Level = 7 Square = 9 - // test + // test2 |} """ config @@ -1430,11 +1430,11 @@ let a = {| equal """ let x = - {| + {| // test1 Y = 42 Z = "string" Foo = "Bar" - // test + // test2 |} let y = @@ -1446,9 +1446,9 @@ let y = let z = {| Y = 42 |} let a = - {| foo with + {| foo with// test1 Level = 7 Square = 9 - // test + // test2 |} """ diff --git a/src/Fantomas.Core.Tests/RecordTests.fs b/src/Fantomas.Core.Tests/RecordTests.fs index 8063b1a096..ac0df86bc3 100644 --- a/src/Fantomas.Core.Tests/RecordTests.fs +++ b/src/Fantomas.Core.Tests/RecordTests.fs @@ -2179,3 +2179,59 @@ type ExprFolder<'State> = -> Exp -> 'State |} """ + +[] +let ``comment in bracket ranges of anonymous type`` () = + formatSourceString + false + """ +let x = {| // test1 + Y = 42 + Z = "string" + Foo = "Bar" + // test2 + |} + +let y = {| + Y = 42 + // test +|} + +let z = {| + Y = 42 +|} + +let a = {| // test1 + foo with + Level = 7 + Square = 9 + // test2 +|} +""" + config + |> prepend newline + |> should + equal + """ +let x = + {| Y = // test1 + 42 + Z = "string" + Foo = "Bar" + // test2 + |} + +let y = + {| Y = 42 + // test + |} + +let z = {| Y = 42 |} + +let a = + {| foo with// test1 + Level = 7 + Square = 9 + // test2 + |} +""" diff --git a/src/Fantomas.Core/AstTransformer.fs b/src/Fantomas.Core/AstTransformer.fs index 566d606171..14e97e3580 100644 --- a/src/Fantomas.Core/AstTransformer.fs +++ b/src/Fantomas.Core/AstTransformer.fs @@ -231,7 +231,7 @@ and visitSynExpr (synExpr: SynExpr) : TriviaNode list = yield mkNode SynExpr_Record_ClosingBrace closingBrace |]) |> List.singleton |> finalContinuation - | SynExpr.AnonRecd (_, _, recordFields, StartEndRange 1 (openingBrace, range, closingBrace)) -> + | SynExpr.AnonRecd (_, _, recordFields, StartEndRange 2 (openingBrace, range, closingBrace)) -> mkSynExprNode SynExpr_AnonRecd synExpr diff --git a/src/Fantomas.Core/CodePrinter.fs b/src/Fantomas.Core/CodePrinter.fs index 84a1defd98..279960f35e 100644 --- a/src/Fantomas.Core/CodePrinter.fs +++ b/src/Fantomas.Core/CodePrinter.fs @@ -918,15 +918,15 @@ and genExpr synExpr ctx = | AnonRecord (openingBrace, isStruct, fields, copyInfo, closingBrace) -> let smallExpression = onlyIf isStruct !- "struct " - +> sepOpenAnonRecd + +> genTriviaFor SynExpr_AnonRecd_OpeningBrace openingBrace sepOpenAnonRecd +> optSingle (fun e -> genExpr e +> !- " with ") copyInfo +> col sepSemi fields genAnonRecordFieldName - +> genTriviaFor SynExpr_AnonRecd_ClosingBrace closingBrace (sepSpace +> sepCloseAnonRecdFixed) + +> genTriviaFor SynExpr_AnonRecd_ClosingBrace closingBrace sepCloseAnonRecd let longExpression = ifAlignBrackets (genMultilineAnonRecordAlignBrackets openingBrace isStruct fields copyInfo closingBrace) - (genMultilineAnonRecord isStruct fields copyInfo) + (genMultilineAnonRecord openingBrace isStruct fields copyInfo closingBrace) fun (ctx: Context) -> let size = getRecordSize ctx fields @@ -2363,23 +2363,23 @@ and genInheritConstructor (inheritCtor: SynType * SynExpr) = +> expressionFitsOnRestOfLine (genExpr px) (genMultilineFunctionApplicationArguments px) | OtherInheritConstructor (t, e) -> genType t +> sepSpaceOrIndentAndNlnIfExpressionExceedsPageWidth (genExpr e) -and genMultilineAnonRecord (isStruct: bool) fields copyInfo = +and genMultilineAnonRecord openingBrace (isStruct: bool) fields copyInfo closingBrace = let recordExpr = match copyInfo with | Some e -> - sepOpenAnonRecd + genTriviaFor SynExpr_AnonRecd_OpeningBrace openingBrace sepOpenAnonRecd +> atCurrentColumn ( genExpr e +> (!- " with" +> indentSepNlnUnindent (col sepNln fields genAnonRecordFieldName)) ) - +> sepCloseAnonRecd + +> genTriviaFor SynExpr_AnonRecd_ClosingBrace closingBrace sepCloseAnonRecd | None -> fun ctx -> // position after `{| ` or `{|` let targetColumn = ctx.Column + (if ctx.Config.SpaceAroundDelimiter then 3 else 2) atCurrentColumn - (sepOpenAnonRecd + (genTriviaFor SynExpr_AnonRecd_OpeningBrace openingBrace sepOpenAnonRecdFixed +> col sepNln fields (fun (AnonRecordFieldName (ident, eq, e, range)) -> let expr = if ctx.Config.IndentSize < 3 then @@ -2394,12 +2394,12 @@ and genMultilineAnonRecord (isStruct: bool) fields copyInfo = +> genEq SynExpr_AnonRecd_Field_Equals eq +> expr +> leaveNodeFor SynExpr_AnonRecd_Field range) - +> sepCloseAnonRecd) + +> genTriviaFor SynExpr_AnonRecd_ClosingBrace closingBrace sepCloseAnonRecd) ctx onlyIf isStruct !- "struct " +> recordExpr -and genMultilineAnonRecordAlignBrackets _ (isStruct: bool) fields copyInfo closingBrace = +and genMultilineAnonRecordAlignBrackets openingBrace (isStruct: bool) fields copyInfo closingBrace = let fieldsExpr = col sepNln fields genAnonRecordFieldName let copyExpr fieldsExpr e = @@ -2415,12 +2415,12 @@ and genMultilineAnonRecordAlignBrackets _ (isStruct: bool) fields copyInfo closi let genAnonRecord = match copyInfo with | Some ci -> - sepOpenAnonRecd + genTriviaFor SynExpr_AnonRecd_OpeningBrace openingBrace sepOpenAnonRecd +> copyExpr fieldsExpr ci +> sepNln +> genTriviaFor SynExpr_AnonRecd_ClosingBrace closingBrace sepCloseAnonRecdFixed | None -> - sepOpenAnonRecdFixed + genTriviaFor SynExpr_AnonRecd_OpeningBrace openingBrace sepOpenAnonRecd +> indentSepNlnUnindent fieldsExpr +> sepNln +> genTriviaFor SynExpr_AnonRecd_ClosingBrace closingBrace sepCloseAnonRecdFixed diff --git a/src/Fantomas.Core/SourceParser.fs b/src/Fantomas.Core/SourceParser.fs index 1ad8e825f1..53890aabb9 100644 --- a/src/Fantomas.Core/SourceParser.fs +++ b/src/Fantomas.Core/SourceParser.fs @@ -1071,7 +1071,7 @@ let (|Record|_|) = let (|AnonRecord|_|) = function - | SynExpr.AnonRecd (isStruct, copyInfo, fields, StartEndRange 1 (openingBrace, _, closingBrace)) -> + | SynExpr.AnonRecd (isStruct, copyInfo, fields, StartEndRange 2 (openingBrace, _, closingBrace)) -> Some(openingBrace, isStruct, fields, Option.map fst copyInfo, closingBrace) | _ -> None From 7d4463d0fc1ed0245ed27cd51ae3b3f358d2bfe3 Mon Sep 17 00:00:00 2001 From: dawe Date: Fri, 21 Oct 2022 10:49:15 +0200 Subject: [PATCH 3/3] Fix spacing for comments after opening brace --- .../MultilineBlockBracketsOnSameColumnRecordTests.fs | 3 ++- src/Fantomas.Core.Tests/RecordTests.fs | 7 ++++--- src/Fantomas.Core/CodePrinter.fs | 3 +++ 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/Fantomas.Core.Tests/MultilineBlockBracketsOnSameColumnRecordTests.fs b/src/Fantomas.Core.Tests/MultilineBlockBracketsOnSameColumnRecordTests.fs index ecc80086dd..14d6eca02b 100644 --- a/src/Fantomas.Core.Tests/MultilineBlockBracketsOnSameColumnRecordTests.fs +++ b/src/Fantomas.Core.Tests/MultilineBlockBracketsOnSameColumnRecordTests.fs @@ -1446,7 +1446,8 @@ let y = let z = {| Y = 42 |} let a = - {| foo with// test1 + {| // test1 + foo with Level = 7 Square = 9 // test2 diff --git a/src/Fantomas.Core.Tests/RecordTests.fs b/src/Fantomas.Core.Tests/RecordTests.fs index ac0df86bc3..57ab6faedd 100644 --- a/src/Fantomas.Core.Tests/RecordTests.fs +++ b/src/Fantomas.Core.Tests/RecordTests.fs @@ -2214,8 +2214,8 @@ let a = {| // test1 equal """ let x = - {| Y = // test1 - 42 + {| // test1 + Y = 42 Z = "string" Foo = "Bar" // test2 @@ -2229,7 +2229,8 @@ let y = let z = {| Y = 42 |} let a = - {| foo with// test1 + {| // test1 + foo with Level = 7 Square = 9 // test2 diff --git a/src/Fantomas.Core/CodePrinter.fs b/src/Fantomas.Core/CodePrinter.fs index 0590453d09..0b4f0bebfd 100644 --- a/src/Fantomas.Core/CodePrinter.fs +++ b/src/Fantomas.Core/CodePrinter.fs @@ -2368,6 +2368,7 @@ and genMultilineAnonRecord openingBrace (isStruct: bool) fields copyInfo closing match copyInfo with | Some e -> genTriviaFor SynExpr_AnonRecd_OpeningBrace openingBrace sepOpenAnonRecd + +> sepNlnWhenWriteBeforeNewlineNotEmpty // comment after curly brace +> atCurrentColumn ( genExpr e +> (!- " with" +> indentSepNlnUnindent (col sepNln fields genAnonRecordFieldName)) @@ -2380,6 +2381,7 @@ and genMultilineAnonRecord openingBrace (isStruct: bool) fields copyInfo closing atCurrentColumn (genTriviaFor SynExpr_AnonRecd_OpeningBrace openingBrace sepOpenAnonRecdFixed + +> sepNlnWhenWriteBeforeNewlineNotEmpty // comment after curly brace +> col sepNln fields (fun (AnonRecordFieldName (ident, eq, e, range)) -> let expr = if ctx.Config.IndentSize < 3 then @@ -2416,6 +2418,7 @@ and genMultilineAnonRecordAlignBrackets openingBrace (isStruct: bool) fields cop match copyInfo with | Some ci -> genTriviaFor SynExpr_AnonRecd_OpeningBrace openingBrace sepOpenAnonRecd + +> sepNlnWhenWriteBeforeNewlineNotEmpty // comment after curly brace +> copyExpr fieldsExpr ci +> sepNln +> genTriviaFor SynExpr_AnonRecd_ClosingBrace closingBrace sepCloseAnonRecdFixed