Fantomas shouldn't remove parens when using the dynamic operator (?)

[alfonsogarciacaro]

I just applied Fantomas to a project with great results, thanks a lot for your work!

I found however a tricky issue that took me a while to find out. I had a piece of code like this:

let memoEquals (x:'a) (y:'a) =
  (true, JS.Object.keys(x)) ||> Seq.fold (fun eq k ->
    eq && (isFunction x?(k) || eqByRef x?(k) y?(k)))

After applying Fantomas, it became:

let memoEquals (x: 'a) (y: 'a) =
    (true, JS.Object.keys(x))
    ||> Seq.fold(fun eq k -> eq && (isFunction x?k || eqByRef x?k y?k))

Note that parens are removed from x?k and y?k. This is changing the meaning because in the first version the generated code is x[k] while in the second is x.k. Ideally Fantomas should distinguish when the dynamic operator is being applied and keep parens in that case.

[alfonsogarciacaro]

Apparently there was another issue involving the dynamic operator #39, so hopefully this is doable 👍 Please let me know if you need other examples.

Note that the different semantics depending on the parens is not a Fable-specific thing but it's due to how the dynamic operator works in F# (it interprets the next ident as a string unless you use parents).

[nojaf]

Thanks for reporting this.
I believe an easy reproduce is the following.

AST shows a sign of the dynamic operator so I believe this should be doable.

App
          (NonAtomic,false,
           App
             (NonAtomic,true,Ident op_Dynamic,Ident x,
              tmp.fsx (1,19--1,21) IsSynthetic=false),
           App
             (NonAtomic,false,
              App
                (NonAtomic,true,Ident op_Addition,Ident k,
                 tmp.fsx (1,22--1,25) IsSynthetic=false),
              Const (Int32 1,tmp.fsx (1,26--1,27) IsSynthetic=false),

[jindraivanek]

Parens are removed from AST, but in case x?k, k is string const.

See x?(k) vs x?k.

[jindraivanek]

We probably should always do x?(k) except when k is Const String.

[nojaf]

Hi @alfonsogarciacaro, we have an early access nuget feed since a couple of days.
See readme for instructions.

Would you mind testing it out and check if your problem is solved? It be great if someone else tries the nightly feed, just to be sure it works for other people (beside @jindraivanek and myself) as well. Then I would feel confident to announce it.

[alfonsogarciacaro]

Thanks for sending the instructions to install the nightly, I was a bit busy today, but I'll check as soon as possible 👍

[alfonsogarciacaro]

I've tried the nightly from MyGet and can confirm the issue is fixed, thanks a lot! 👏 👏