"Use |> instead of >>" suggestion leading to code that doesn't type check

[jhrcek]

Hello @jfmengels I really like elm-review, I use it in most of elm projects I'm working on an it's greatly helping me to keep my code clean.

Today I got my first incorrect suggestion, so I thought I would report it :smile:

I'm too lazy to come up with smallest reproducer, so here are 2 screenshots that will hopefully be sufficient to reproduce the issue. If it helps you, I can create more self contained reproducer, but might take a few day before I get to it.

Type error after applying the fix:

[lue-bird]

Are you on the latest version? I can't seem to reproduce it.

[jhrcek]

I think I'm on latest version of everything:

details

~/Devel/github.com/jhrcek/relation-explorer> elm-review --version
2.10.3
~/Devel/github.com/jhrcek/relation-explorer> cat review/elm.json 
{
    "type": "application",
    "source-directories": [
        "src"
    ],
    "elm-version": "0.19.1",
    "dependencies": {
        "direct": {
            "elm/core": "1.0.5",
            "jfmengels/elm-review": "2.13.1",
            "jfmengels/elm-review-common": "1.3.3",
            "jfmengels/elm-review-performance": "1.0.2",
            "jfmengels/elm-review-simplify": "2.1.3",
            "jfmengels/elm-review-unused": "1.2.0",
            "stil4m/elm-syntax": "7.3.2",
            "truqu/elm-review-noleftpizza": "2.0.1"
        },
        "indirect": {
            "elm/bytes": "1.0.8",
            "elm/html": "1.0.0",
            "elm/json": "1.1.3",
            "elm/parser": "1.1.0",
            "elm/project-metadata-utils": "1.0.2",
            "elm/random": "1.0.0",
            "elm/time": "1.0.0",
            "elm/virtual-dom": "1.0.3",
            "elm-explorations/test": "2.1.2",
            "miniBill/elm-unicode": "1.0.3",
            "pzp1997/assoc-list": "1.0.0",
            "rtfeldman/elm-hex": "1.0.0",
            "stil4m/structured-writer": "1.0.3"
        }
    },
    "test-dependencies": {
        "direct": {
            "elm-explorations/test": "2.1.2"
        },
        "indirect": {}
    }
}
  

I stripped down my project to just a reproducer for this issue. Please check jan/elm-review branch in this repo https://github.com/jhrcek/relation-explorer/tree/jan/elm-review and do the following to reproduce in top level project directory:

npm install
npx elm-review

[lue-bird]

Strange, I followed the exact steps you provided but the fix it suggests is different from what you pasted and doesn't produce a compiler error.

[jhrcek]

Sorry, it seems I misunderstood the suggestion given by elm-review. It turns out when I apply the fix via elm-review --fix it does the right thing and the code compiles / works. I misinterpreted it as "just replace >> with |> in place without any other changes" . So I think you can close this issue (unless you'd like to e.g. change the suggestion so it's clearer that it's not just about replacing >> with |> wherever it appears (as opposed to e.g. moving it out of parentheses etc.)

[jfmengels]

Hi @jhrcek :wave:

This rule is really meant to be used in fix mode (or rather, it will be much better/easier that way), especially since all reports are autofixable. So you're going to have a better time in fix mode than trying to apply fixes manually.

Do you by any chance have a suggestion on how to improve the error message in a way that you would not have confused?

[jhrcek]

Makes sense. I thought hard about it, but couldn't come up with a way to make the description more universally understandable (given the variety of syntactic contexts it can appear in). Let's close the issue then.