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

Line comments after null value used as argument in function call are removed #1676

Closed
3 tasks
rehael opened this issue Apr 23, 2021 · 3 comments · Fixed by #1688
Closed
3 tasks

Line comments after null value used as argument in function call are removed #1676

rehael opened this issue Apr 23, 2021 · 3 comments · Fixed by #1688

Comments

@rehael
Copy link
Contributor

rehael commented Apr 23, 2021

Issue created from fantomas-online

Code

let v = f null // comment

let lostComment =
    f // comment
        null // comment
        v // comment
        null // comment
        42 // comment
        null // comment
    // comment

Result

let v = f null

let lostComment =
    f // comment
        null
        v // comment
        null
        42 // comment
        null
// comment

Problem description

Comments after null are removed – should be left as is, as it is for other argument types. Looks like null is handled specially.

I was trying to find how it is handled in Fantomas' code, but clearly my fsharp-fu is not enough to grasp it, so probably can't help directly with making a PR/fix. :(

Extra information

  • The formatted result breaks by code.
  • The formatted result gives compiler warnings.
  • I or my company would be willing to help fix this.

Options

Fantomas Master at 04/17/2021 11:23:36 - 26674f9

Default Fantomas configuration

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

@nojaf
Copy link
Contributor

nojaf commented Apr 23, 2021

Hello, thank you for reporting this issue.
Comments are not part of the F# Abstract Syntax tree.
To overcome this limitation, Fantomas uses a concept called "trivia".
In short, trivia is the detection of missing information in the F# tokens.
After finding those trivia, we try to assign them to existing nodes of the tree.
See https://www.youtube.com/watch?v=FCrkUqCCzgI&ab_channel=nojaf for more information.

In this specific scenario, the comments are found and assigned to a SynExpr_Null node.
You can see this in the "trivia" tab of the online tool.

image

image

The problem you are facing is that the trivia is not printed in the CodePrinter.

|> (match synExpr with
| SynExpr.App _ -> genTriviaFor SynExpr_App synExpr.Range
| SynExpr.AnonRecd _ -> genTriviaFor SynExpr_AnonRecd synExpr.Range
| SynExpr.Record _ -> genTriviaFor SynExpr_Record synExpr.Range
| SynExpr.Ident _ -> genTriviaFor SynExpr_Ident synExpr.Range
| SynExpr.IfThenElse _ -> genTriviaFor SynExpr_IfThenElse synExpr.Range
| SynExpr.Lambda _ -> genTriviaFor SynExpr_Lambda synExpr.Range
| SynExpr.ForEach _ -> genTriviaFor SynExpr_ForEach synExpr.Range
| SynExpr.For _ -> genTriviaFor SynExpr_For synExpr.Range
| SynExpr.Match _ -> genTriviaFor SynExpr_Match synExpr.Range
| SynExpr.MatchBang _ -> genTriviaFor SynExpr_MatchBang synExpr.Range
| SynExpr.YieldOrReturn _ -> genTriviaFor SynExpr_YieldOrReturn synExpr.Range
| SynExpr.YieldOrReturnFrom _ -> genTriviaFor SynExpr_YieldOrReturnFrom synExpr.Range
| SynExpr.TryFinally _ -> genTriviaFor SynExpr_TryFinally synExpr.Range
| SynExpr.LongIdentSet _ -> genTriviaFor SynExpr_LongIdentSet synExpr.Range
| SynExpr.ArrayOrList _ -> genTriviaFor SynExpr_ArrayOrList synExpr.Range
| SynExpr.ArrayOrListOfSeqExpr _ -> genTriviaFor SynExpr_ArrayOrListOfSeqExpr synExpr.Range
| SynExpr.Paren _ -> genTriviaFor SynExpr_Paren synExpr.Range
| SynExpr.InterpolatedString _ -> genTriviaFor SynExpr_InterpolatedString synExpr.Range
| SynExpr.Tuple _ -> genTriviaFor SynExpr_Tuple synExpr.Range
| SynExpr.DoBang _ -> genTriviaFor SynExpr_DoBang synExpr.Range
| SynExpr.TryWith _ -> genTriviaFor SynExpr_TryWith synExpr.Range
| SynExpr.New _ -> genTriviaFor SynExpr_New synExpr.Range
| SynExpr.Assert _ -> genTriviaFor SynExpr_Assert synExpr.Range
| SynExpr.While _ -> genTriviaFor SynExpr_While synExpr.Range
| SynExpr.MatchLambda _ -> genTriviaFor SynExpr_MatchLambda synExpr.Range
| SynExpr.LongIdent _ -> genTriviaFor SynExpr_LongIdent synExpr.Range
| SynExpr.DotGet _ -> genTriviaFor SynExpr_DotGet synExpr.Range
| SynExpr.Upcast _ -> genTriviaFor SynExpr_Upcast synExpr.Range
| SynExpr.Downcast _ -> genTriviaFor SynExpr_Downcast synExpr.Range
| SynExpr.DotIndexedGet _ -> genTriviaFor SynExpr_DotIndexedGet synExpr.Range
| SynExpr.DotIndexedSet _ -> genTriviaFor SynExpr_DotIndexedSet synExpr.Range
| SynExpr.ObjExpr _ -> genTriviaFor SynExpr_ObjExpr synExpr.Range
| SynExpr.JoinIn _ -> genTriviaFor SynExpr_JoinIn synExpr.Range
| SynExpr.Do _ -> genTriviaFor SynExpr_Do synExpr.Range
| SynExpr.TypeApp _ -> genTriviaFor SynExpr_TypeApp synExpr.Range
| SynExpr.Lazy _ -> genTriviaFor SynExpr_Lazy synExpr.Range
| _ -> id)

SynExpr_Null should be added there.

Are you interested in submitting a PR for this?
If so, please read our Contribution Guidelines carefully.

@rehael
Copy link
Contributor Author

rehael commented Apr 23, 2021

I will try understanding this and working on PR – in case of any problems I will ask here.

BTW: Similar issue with comment removal in pattern match:

Before:

match v with
| x
| y // comment
| z -> 42
| _ -> 0

After:

match v with
| x
| y
| z -> 42
| _ -> 0

@nojaf
Copy link
Contributor

nojaf commented Apr 23, 2021

That would be the same category of problems yet there would not be one fix that would solve both problems.
So, I encourage you to open a new issue for each case you encounter (as mentioned here).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants