New Line after Computation Expression Name Breaks Code

[panmona]

Issue created from fantomas-online

Code

let testXYZ value =
    test "A very long and descriptive text about what this test is supposed to do and test for the test case" {
        let a, b = someModule.someFun value
        Expect.isLessThan a b "Expected a to be lower than b"
    }

Result

let testXYZ value = test
    "A very long and descriptive text about what this test is supposed to do and test for the test case" {
    let a, b = someModule.someFun value
    Expect.isLessThan a b "Expected a to be lower than b"
}

Problem description

There's a new line inserted after the "test" computation expression keyword, which breaks the code:

I would expect the code to be:

// ...
test "A very long and descriptive text about what this test is supposed to do and test for the test case" {
// ...

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.

Options

Fantomas main branch at 2023-04-05T14:35:42Z - 1492e67

    { config with
                MultilineBracketStyle = stroustrup
                NewlineBeforeMultilineComputationExpression = false }

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

[nojaf]

Hi there, this is really stretching the boundaries. If we go with your proposal, the next user will open an issue that max_line_length is no longer respected 🙃.

I need to give some thought to whether we really want to address this. It has "Bad code leads to badly formatted result" written all over it. Perhaps a more sensible thing to do here is not to allow CEs that are not composed out of a single ident.

For now, I would advise you to increase the max_line_length for that file.

[panmona]

I'd totally agree with you if this really only was "leads to badly formatted result" and I wouldn't say that a good name of a test is bad code. In our actual code the text is actually much shorter, as it is slightly nested and the function has additional parameters. It looks more like:

let tests =
    let precisionInitialTest value precision =
        test "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiu" {
            let myVal = doX()
            Expect.equal myVal (value / precision + 5) ""
        }
        
    precisionInitialTest 1
    precisionInitialTest 2
    precisionInitialTest 3

In this case fantomas in v6 fails with: Failed to format file: MyFile.fs : Formatting MyFile.fs leads to invalid F# code. Such a failure state is a state that in my opinion should definitely be avoided.

Increasing the max_line_length unfortunately has other unintended side effects in the file. And it then formats it as the following, not sure what the styleguide recommends for these cases:

let precisionInitialTest value precision = test "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiu" {

Not sure what the right way forward is here, but having Fantomas fail definitely seems like something we should avoid.

[majocha]

This makes me think if there is a way to make Fantomas maintain the offside rules in a general way.

[nojaf]

I wouldn't say that a good name of a test is bad code

The bad code here is the way your testing framework went with using a parameterized computation expression IMHO.

in my opinion should definitely be avoided.

You can very much avoid this by flipping fsharp_newline_before_multiline_computation_expression to true.

not sure what the styleguide recommends for these cases

If it says nothing, then ask.

[panmona]

Thanks. Flipping it to true will cause all other computation expressions (async, task etc.) to also be on a new line.
For now the pragmatic solution for this case seems to be to put the name into a separate binding and then use that binding as the param to the computation expression. (The testing framework is Expecto btw)

I do still think though that Fantomas should respect the max line length correctly and not fail.

[nojaf]

Feel free to take initiative in investigating this, I'm ok with having a sensible solution for this problem. As long as it respects existing rules, has style guide approval and is properly implemented.

I'm going to leave this as a discussion until further notice.