fsprojects / fantomas

FSharp source code formatter
https://fsprojects.github.io/fantomas
Other
770 stars 194 forks source link

Refactor test helpers #2992

Closed josh-degraw closed 10 months ago

josh-degraw commented 10 months ago

A first attempt / POC to simplify test code by reducing duplication and improving readability.

There are now two helper functions:

let formatSignatureString = formatFSharpString true
let formatSourceString = formatFSharpString false

Since most code used formatSourceString false, I kept that name the same and just changed it to pass false to the implementation.

josh-degraw commented 10 months ago

Two remaining suggestions I'd make for test refactoring would be:

Now we could pipe config into formatSourceString "", which I personally feel like would make things a lot more readable by eliminating the currently necessary hanging indent when you need to pass config after the source string. Consider:

Long example collapsed ```fsharp [] let ``multiline downcast expression, `` () = formatSourceString """ longMethodName longArgument longArgument2 :?> List """ { config with MaxLineLength = 30 SpaceBeforeUppercaseInvocation = true SpaceBeforeColon = true SpaceBeforeSemicolon = true AlignFunctionSignatureToIndentation = true AlternativeLongMemberDefinitions = true } |> prepend newline |> should equal """ longMethodName longArgument longArgument2 :?> List ``` would change to: ```fsharp [] let ``multiline downcast expression, `` () = { config with MaxLineLength = 30 SpaceBeforeUppercaseInvocation = true SpaceBeforeColon = true SpaceBeforeSemicolon = true AlignFunctionSignatureToIndentation = true AlternativeLongMemberDefinitions = true } |> formatSourceString """ longMethodName longArgument longArgument2 :?> List """ |> prepend newline |> should equal """ longMethodName longArgument longArgument2 :?> List ```

The second suggestion I'd make is to modify how prepend string is currently necessary almost everywhere. For example, even for one-line code tests such as this, we often need to add a newline, either explicitly or via prepend newline:

[<Test>]
let ``should keep parens around active patterns`` () =
    formatSourceString
        """let (|Boolean|_|) = Boolean.parse
    """
        config
    |> should
        equal
        """let (|Boolean|_|) = Boolean.parse
"""

We could add a helper like this to eliminate the need to call prepend newline or add an explicit newline in most places:

let shouldEqualTrimmed (y: string) (s: string) =
    let x = s.Replace("\r\n", "\n").Trim()
    y.Trim()
    |> should equal x