List of functions misaligned and breaks (Elmish)

cgravill commented 2 years ago

Code

module Comps

open Fable.React.Props
open Fulma

let settingsMenu settings updateSettings =

    // Style 1
    let props =
        Radio.Input.Props[Checked false
                          OnChange(fun _ -> settings |> updateSettings)]

    // Style 2
    let props =
        Radio.Input.Props[
            Checked false
            OnChange(fun _ -> settings |> updateSettings)]

    props

Result

module Comps

open Fable.React.Props
open Fulma

let settingsMenu settings updateSettings =

    // Style 1
    let props =
        Radio.Input.Props[Checked false
        OnChange(fun _ -> settings |> updateSettings)]

    // Style 2
    let props =
        Radio.Input.Props[Checked false
        OnChange(fun _ -> settings |> updateSettings)]

    props

Problem description

This is an extraction of some involved Elmish code.

Fantomas.FormatConfig+FormatException: Formatted content is not valid F# code

The online tool clarifies the issue:

(11,8) (11, 53)Error597parse
Successive arguments should be separated by spaces or tupled, and arguments involving function or method applications should be parenthesized

(16,8) (16, 53)Error597parse
Successive arguments should be separated by spaces or tupled, and arguments involving function or method applications should be parenthesized

It seems as if the top level bind is being prioritised over the list constructions.

The issue seems to have started somewhere between 4.6.0 and 4.7.x

Extra information

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

Options

Fantomas master branch at 2022-03-15T07:45:16Z - f10e2392ff7207059345cf75223ef9431c36821d

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 commented 2 years ago

Hey Colin, another wonderful find from the 4.7 release 😅. I believe the source is being mistaken for a new index syntax expression. Adding a space before [ seems to help. I should look into more detail but at first glance, what happens is making sense to me.

cgravill commented 2 years ago

We've got some fancily formatted code so we're good at finding some edge cases! It's also why I like Fantomas so much, so our code becomes and stays more uniform. :-)

This one took a while to narrow down but the good news is that it's the last class of issues for us, and after other fixes, we now format cleanly with 4.7.3

nojaf commented 2 years ago

Hey, after taking another look I have some more insights. This is a bug in terms that the Sequential inside the IndexWithoutDotExpr should be at the same column to remain valid.

https://github.com/fsprojects/fantomas/blob/3e8086fcf3e0e64e7ac7a323280a8b3170dccb36/src/Fantomas/CodePrinter.fs#L1762-L1766

The helper function atCurrentColumnIndent should be used when the expr is a Sequential.

That being said, the expression is correctly identified as an index (without dot) expression. This is not what you want when you are using Elmish inspired code. So use spaces to get there.

Are you interested in submitting a PR for this one?

cgravill commented 2 years ago

I can give that go. I've already actioned the workaround in our codebase but it would nice if code in these styles could be reformatted safely.

fsprojects / fantomas