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.

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

Idempotency problem when having a comment in an anonymous record and Stroustrup formatting #2566

Closed
2 of 4 tasks
abelbraaksma opened this issue Oct 8, 2022 · 2 comments
Closed
2 of 4 tasks
Labels
bug (soundness) good first issue Long hanging fruit: easy issue to get your feet wet!

Comments

@abelbraaksma
Copy link
Member

abelbraaksma commented Oct 8, 2022

Issue created from fantomas-online

Original code

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

Formatted code

let x = {|
    Y = 42
    // test

|}

Reformatted code

let x = {|
    Y = 42
    // test


|}

Problem description

Fantomas was not able to produce the same code after reformatting the result:

  • Having a comment on the last line of an anonymous record leads to an extra empty line being added each time you format
  • This is a relatively common scenario, i.e. using a comment temporarily to disable a certain field

Workaround

Just don't do this in code.

Extra information

  • The formatted result breaks my code.
  • The formatted result gives compiler warnings.
  • I or my company would be willing to help fix this.
  • Breaks Husky integration with precommit hook, the last line "walks down" on each format

Options

Fantomas main branch at 2022-10-08T06:02:14Z - b0eea04

    { config with
                MultilineBlockBracketsOnSameColumn = true
                ExperimentalStroustrupStyle = true }

Related example

A minor bug happens when Stroustrup is off. Note the comment being outdented and the closing |} bracket one space too far. Not sure it's related, see example without Stroustrup

Without Stroustrup example, before formatting

let x = {|
   Y = 42
   Z = "string"
   Foo = "Bar"
   // test
|}

Without Stroustrup after formatting

let x =
    {| Y = 42
       Z = "string"
       Foo = "Bar"
    // test
     |}

Did you know that you can ignore files when formatting from fantomas-tool or the FAKE targets by using a .fantomasignore file? YES 😃

@abelbraaksma abelbraaksma changed the title Idempotency problem when having a comment in an anonymous record Idempotency problem when having a comment in an anonymous record and Stroustrup formatting Oct 8, 2022
@nojaf
Copy link
Contributor

nojaf commented Oct 9, 2022

Hello, thanks for reporting this problem.
The root problem is with fsharp_multiline_block_brackets_on_same_column=true.

SynExpr_AnonRecd_Field was chosen as the trivia anchor (content after). In case of a normal record the closing brace would be picked (content before).

The braces for anonymous records are not picked up as nodes.

Regular record:

| SynExpr.Record (_, _, recordFields, StartEndRange 1 (openingBrace, range, closingBrace)) ->

Anon record:

| SynExpr.AnonRecd (_, _, recordFields, range) ->

The StartEndRange active pattern captures additional ranges for the tokens. (As the range starts and ends with these included).

In CodePrinter, trivia attached to the closing brace (in regular record instance) is taking into account the leading trivia:

+> ifElseCtx lastWriteEventIsNewline sepNone sepNln

A similar thing could be achieved for anon records.

Are you interested in submitting a PR?

PS: As for the other problems, this is a known problem and won't be solved by fixing the idempotency issue.

@abelbraaksma
Copy link
Member Author

@dawedawe thanks for tackling this! You guys are too fast for me 😆.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug (soundness) good first issue Long hanging fruit: easy issue to get your feet wet!
Projects
None yet
Development

No branches or pull requests

2 participants