Fantomas is unable to format valid F# (.net 6.0) program

[KarolBajkowski]

Hi.

I ran into some issue while formatting one of my programs. Below are details:

Issue created from fantomas-online

Code

try
    try
        ()
#if FOO
        ()
#endif
    with
    | _ -> ()
with 
| _ -> ()

Error

System.Exception: Fantomas is trying to format the input multiple times due to the detect of multiple defines.
There is a problem with merging all the code back together.
[FOO] has 4 fragments
[] has 5 fragments
Please raise an issue at https://fsprojects.github.io/fantomas-tools/#/fantomas/preview.
   at Microsoft.FSharp.Core.PrintfModule.PrintFormatToStringThenFail@1439.Invoke(String message) in D:\a\_work\1\s\src\fsharp\FSharp.Core\printf.fs:line 1439
   at Fantomas.CodeFormatterImpl.format@416-3.Invoke(FSharpList`1 _arg2) in /app/.deps/fantomas/src/Fantomas/CodeFormatterImpl.fs:line 438
   at Microsoft.FSharp.Control.AsyncPrimitives.CallThenInvokeNoHijackCheck[a,b](AsyncActivation`1 ctxt, b result1, FSharpFunc`2 userCode) in D:\a\_work\1\s\src\fsharp\FSharp.Core\async.fs:line 464
   at Fantomas.Async.map@173-2.Invoke(AsyncActivation`1 ctxt) in /app/.deps/fantomas/src/Fantomas/Utils.fs:line 173
   at Microsoft.FSharp.Control.Trampoline.Execute(FSharpFunc`2 firstAction) in D:\a\_work\1\s\src\fsharp\FSharp.Core\async.fs:line 104
--- End of stack trace from previous location ---
   at FantomasOnline.Server.Shared.Http.formatResponse@95-6.Invoke(Unit unitVar0) in /app/src/server/FantomasOnline.Shared/Http.fs:line 95
   at Ply.TplPrimitives.AwaitableContinuation`3.Invoke(Unit r)

Problem description

Fantomas is unable to format valid F# (.net 6.0) program. I was able to reduce the case to the pasted code. I tried to reduce it even more but I was not able to find smaller code.

Extra information

The used program compile just fine with F# compiler so I think the issue is in Fantomas tool.

Options

Fantomas 4.6 branch at 11/10/2021 15:04:59 - 8d864c3

Default Fantomas configuration

[nojaf]

Hello Karol, thank you for reporting this issue.
I'm afraid this has nothing to do with .NET 6, there are a few bugs in Fantomas, especially when your code contains #if FOO kinds of things.

The short version, without a lot of context, is that Fantomas will format your code snippet twice (once with FOO and once without) and then merge both results together. In that last merge phase, things went wrong.
I've made a video about this in the past to explain it a bit better.

If you format the code only using the FOO directive you get:

try
    try
        ()
#if FOO
        ()
#endif
    with
    | _ -> ()
#endif
with
| _ -> ()

That second #endif shouldn't be there and that is causing the merge problem.

Are you interested in submitting a fix for this?

[KarolBajkowski]

It would be cool if I could fix that bug! :) But I'm not sure If I'll be able to, since it's a totally new source base for me and in some places, it's quite complicated. But I want to try. It would be a great opportunity to learn something new!

I started looking into that bug and I found a couple of questions about fantomas source code and design (I found one case in which I cannot fully understand why AST was generated in such a way). I'm not sure if this thread is the right place for asking for help/discussion? Or is there some separate 'dev' channel or some other way where I can ask questions?

EDIT:
Ok, I missed contributing.md document where everything is described :D I'll start from that and I'll start getting knowledge based on what was already published/created 👍

[nojaf]

Wonderful, I really recommend the YouTube series and feel free to poke me on our gitter channel.

[nojaf]

One other thing I believe is relevant here is:

fantomas/src/Fantomas/CodePrinter.fs

Line 975 in 8d864c3

let kw tokenName f = tokN synExpr.Range tokenName f

used in

fantomas/src/Fantomas/CodePrinter.fs

Lines 2026 to 2038 in 8d864c3

	| TryWith (e, cs) ->
	atCurrentColumn (
	kw TRY !- "try "
	+> indent
	+> sepNln
	+> genExpr astContext e
	+> unindent
	+> kw WITH !+~ "with"
	+> indentOnWith
	+> sepNln
	+> col sepNln cs (genClause astContext true)
	+> unindentOnWith
	)

kw WITH !+~ "with" will basically look for the WITH keyword in the entire synExpr range.

A better range would be from the end of the tryExpr till the start of the withCases.

And as the problem is a trivia one, you can replace the #if with a code comment instead. This will also be slightly easier to tackle.

try
    try
        ()
// #if FOO
        ()
// #endif
    with
    | _ -> ()
with 
| _ -> ()

Example.

A similar thing was done in https://github.com/fsprojects/fantomas/pull/1914/files.
In #1914 a custom range was created that is closer to the actual token instead of the entire expression.

I hope this helps.

[KarolBajkowski]

Thank you! That helps a lot!

Yes, something weird is happening with trivia (comments in that case) in combination with nested try with. Probably an even simpler case:

try
    try
        2222222
    // xxxxx
    with
    | _ -> 333333
with 
| _ -> 4444444

Example

// xxxxx comment is duplicated

But what is surprising me the most is why on typed AST we get Const (1, type Microsoft.FSharp.Core.int), val matchValue, node when there are no 1 constants...

For now, I don't have an answer for that but definitely, I'll investigate it.

[nojaf]

Hmm, I don't really know what is happening there. Fantomas does not use the Typed AST, so who knows.
The comment being duplicated is because the comment is found by the outer try/with as well.
The range of looking for the with should be more focused.

[nojaf]

In the future, I actually envision adding the range of the with keyword to the untyped tree.
Then, we don't need to guess between what range the keyword is placed but can pinpoint it directly.

[KarolBajkowski]

@nojaf I created draft PR for that here: #1971 Any help/comments/suggestions are welcome to finalize it! :)