Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Capture StartEndRange information in SynExpr.AnonRecd to fix #2566 #2591

Merged
merged 5 commits into from
Oct 24, 2022
Merged

Capture StartEndRange information in SynExpr.AnonRecd to fix #2566 #2591

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
721b129
Capture StartEndRange information in SynExpr.AnonRecd to fix #2566
dawedawe Oct 19, 2022
e599116
- fix StartEndRange size parameter
dawedawe Oct 20, 2022
3482caf
Merge branch 'main' into fix-2566
dawedawe Oct 20, 2022
7d4463d
Fix spacing for comments after opening brace
dawedawe Oct 21, 2022
4daf897
Merge branch 'main' into fix-2566
nojaf Oct 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Fixed
* Idempotency problem with `struct end`. [#2592](https://github.com/fsprojects/fantomas/issues/2592)
* 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

Expand Down
58 changes: 58 additions & 0 deletions src/Fantomas.Core.Tests/MultilineBlockBracketsOnSameColumnRecordTests.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1395,3 +1395,61 @@ let f
=
x
"""

[<Test>]
let ``comment in bracket ranges of anonymous type, 2566`` () =
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 =
{| // test1
Y = 42
Z = "string"
Foo = "Bar"
// test2
|}

let y =
{|
Y = 42
// test
|}

let z = {| Y = 42 |}

let a =
{| // test1
foo with
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it is the same as we have it for regular records.
But in the future, we might want to have something like

{| // foo
   bar with
    A = 1
    B = 2 |}

I don't feel strongly about this, so no need to address this in this PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should better be handled in a separate PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me!
I have no idea why the CI is failing, the NuGet lock file should not be throwing errors.
I think we need to apply the steps explained here.
I'll do that in a separate PR so you can rebase with main.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is also dotnet/fsharp#13678.
I hope it is ok.

Level = 7
Square = 9
// test2
|}
"""
57 changes: 57 additions & 0 deletions src/Fantomas.Core.Tests/RecordTests.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2179,3 +2179,60 @@ type ExprFolder<'State> =
-> Exp
-> 'State |}
"""

[<Test>]
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 =
{| // 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
|}
"""
7 changes: 5 additions & 2 deletions src/Fantomas.Core/AstTransformer.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 2 (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) ->
Expand Down
35 changes: 21 additions & 14 deletions src/Fantomas.Core/CodePrinter.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -915,18 +915,18 @@ 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
+> genTriviaFor SynExpr_AnonRecd_OpeningBrace openingBrace sepOpenAnonRecd
+> optSingle (fun e -> genExpr e +> !- " with ") copyInfo
+> col sepSemi fields genAnonRecordFieldName
+> sepCloseAnonRecd
+> genTriviaFor SynExpr_AnonRecd_ClosingBrace closingBrace sepCloseAnonRecd

let longExpression =
ifAlignBrackets
(genMultilineAnonRecordAlignBrackets isStruct fields copyInfo)
(genMultilineAnonRecord isStruct fields copyInfo)
(genMultilineAnonRecordAlignBrackets openingBrace isStruct fields copyInfo closingBrace)
(genMultilineAnonRecord openingBrace isStruct fields copyInfo closingBrace)

fun (ctx: Context) ->
let size = getRecordSize ctx fields
Expand Down Expand Up @@ -2363,23 +2363,25 @@ 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
+> sepNlnWhenWriteBeforeNewlineNotEmpty // comment after curly brace
+> 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
+> sepNlnWhenWriteBeforeNewlineNotEmpty // comment after curly brace
+> col sepNln fields (fun (AnonRecordFieldName (ident, eq, e, range)) ->
let expr =
if ctx.Config.IndentSize < 3 then
Expand All @@ -2394,12 +2396,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 =
and genMultilineAnonRecordAlignBrackets openingBrace (isStruct: bool) fields copyInfo closingBrace =
let fieldsExpr = col sepNln fields genAnonRecordFieldName

let copyExpr fieldsExpr e =
Expand All @@ -2414,12 +2416,17 @@ and genMultilineAnonRecordAlignBrackets (isStruct: bool) fields copyInfo =

let genAnonRecord =
match copyInfo with
| Some ci -> sepOpenAnonRecd +> copyExpr fieldsExpr ci +> sepNln +> sepCloseAnonRecdFixed
| Some ci ->
genTriviaFor SynExpr_AnonRecd_OpeningBrace openingBrace sepOpenAnonRecd
+> sepNlnWhenWriteBeforeNewlineNotEmpty // comment after curly brace
+> copyExpr fieldsExpr ci
+> sepNln
+> genTriviaFor SynExpr_AnonRecd_ClosingBrace closingBrace sepCloseAnonRecdFixed
| None ->
sepOpenAnonRecdFixed
genTriviaFor SynExpr_AnonRecd_OpeningBrace openingBrace sepOpenAnonRecd
+> indentSepNlnUnindent fieldsExpr
+> sepNln
+> sepCloseAnonRecdFixed
+> genTriviaFor SynExpr_AnonRecd_ClosingBrace closingBrace sepCloseAnonRecdFixed

ifElse isStruct !- "struct " sepNone +> genAnonRecord

Expand Down
3 changes: 2 additions & 1 deletion src/Fantomas.Core/SourceParser.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 2 (openingBrace, _, closingBrace)) ->
Some(openingBrace, isStruct, fields, Option.map fst copyInfo, closingBrace)
| _ -> None

let (|ObjExpr|_|) =
Expand Down
2 changes: 2 additions & 0 deletions src/Fantomas.Core/TriviaTypes.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down