Quickfix for adding >> changing to much

[Krzysztof-Cieslak]

Reported by @ovatsus in https://github.com/ionide/ionide-vscode-fsharp/issues/738

Ionide-fsharp 3.17.3

If I have some code like this:

let array = [| "A"; "B"; "C"|]

let foo (s: string) = s

let bar = 
    array
    |> Array.map (sprintf "something %s")
    |> Array.map foo

There's a quick fix suggestion that turns it into this:

let bar = 
    Array.map ((sprintf "something %s") >> foo) array

However I think the correct fix should be to this to keep the existing style

let bar = 
    array
    |> Array.map ((sprintf "something %s") >> foo)

[robertlyson]

Wanted to have a look into this issue, but seems I can't find a way to put a fix in place. Any suggestions where to look first? I Started with a test case, but doesn't even seem to reformat function as in the above example:

    [<Test>]
    member this.``Quickfix for composition reimplementing function is to replace pipeline with function composition.``() = 
        let source = """
module Program

let foo (s: string) = s

let bar = 
    [| "A"; "B"; "C"|]
    |> Array.map (sprintf "something %s")
    |> Array.map foo
"""

        let expected = """
module Program

let foo (s: string) = s

let bar = 
    [| "A"; "B"; "C"|]
    |> Array.map ((sprintf "something %s") >> foo)
"""

        this.Parse source
        Assert.AreEqual(expected, this.ApplyQuickFix source)

throws

TestFunctionReimplementationRules+TestFunctionReimplementationRules.Quickfix for composition reimplementing function is to replace pipeline with function composition.

  Expected string length 127 but was 139. Strings differ at index 94.
  Expected: "...  |> Array.map ((sprintf "something %s") >> foo)\n"
  But was:  "...  |> Array.map (sprintf "something %s")\n    |> Array.map foo\n"
  ------------------------------^

  at TestFunctionReimplementationRules+TestFunctionReimplementationRules.Quickfix for composition reimplementing function is to replace pipeline with function composition. () [0x00017] in ..

looks like it doesn't apply fix at all and returns

let bar = 
    [| "A"; "B"; "C"|]
    |> Array.map (sprintf "something %s")
    |> Array.map foo

[jgardella]

Hi @robertlyson, happy to hear you're interested in contributing.

I was also not able to reproduce this by adding a test to the TestFunctionReimplementationRules test module. I think the reason is that this fix is actually a result of one of the default hints. I think this one: Array.map f (Array.map g x) ===> Array.map (g >> f) x.

You can add this test to the TestHintMatcher.fs file to reproduce:

    [<Test>]
    member this.``Quickfix for composition reimplementing function is to replace pipeline with function composition.``() = 
        let source = """
let bar = 
    [| "A"; "B"; "C"|]
    |> Array.map (sprintf "something %s")
    |> Array.map foo
"""

        let expected = """
let bar = 
    [| "A"; "B"; "C"|]
    |> Array.map ((sprintf "something %s") >> foo)
"""

        this.Parse(source, generateHintConfig ["Array.map f (Array.map g x) ===> Array.map (g >> f) x"])
        let fix = this.ApplyQuickFix source
        Assert.AreEqual(expected, fix)

Let me know if you have any more questions!