avh4 / elm-format

elm-format formats Elm source code according to a standard set of rules based on the official Elm Style Guide
BSD 3-Clause "New" or "Revised" License
1.31k stars 145 forks source link

Remove unnecessary parens in pipelines (possibly: in general) #710

Open lydell opened 3 years ago

lydell commented 3 years ago

Concrete use case

Here’s a thing I see a lot in the code base at work:

selectHighlightedItem selectedMsg viewModel model =
    (itemIndexPairAt model.highlightedIndex viewModel.itemIndexPairs
        |> Maybe.map Tuple.second
    )
        |> Task.succeed
        |> Task.perform selectedMsg

That would be nicer without those parens:

selectHighlightedItem selectedMsg viewModel model =
    itemIndexPairAt model.highlightedIndex viewModel.itemIndexPairs
        |> Maybe.map Tuple.second
        |> Task.succeed
        |> Task.perform selectedMsg

The parens were left behind during refactoring. elm-format often removes unnecessary parens, so one is used not having to think about it. For example, List.map (String.toLower) words is turned into List.map String.toLower words.

Technical note

As far as I have understood, elm-format currently does not know about operator precedence and that’s why this isn’t already implemented. In Elm 0.18 you could define your own operators, but since Elm 0.19 you can’t. There’s just the ones from Basics, Parser and Url. So my proposal is to hard code the precedence of each known operator, and either only enable this formatting in Elm 0.19 code or do something clever in Elm 0.18 (or older) code when there are custom operators.

The bigger picture

Unnecessary parens in pipelines is the only case I can think of off the top of my head that I remember encountering in real code.

But it might be worth thinking about generalizing the concept. For example, maybe elm-format wants to control all parentheses – it would print, and not print, them as it sees it being best, regardless of what construct they’re used in. I would personally like that – I would never be on the lookout for unnecessary parens again. But there’s also the risk of people disagreeing on which parens are making readability worse/better: https://github.com/prettier/prettier/issues/187. That Prettier issue is pretty long and contains lots of bad output from Prettier that has been improved over a long period of time. There still isn’t full agreement.

It could also be worth thinking about if knowing operator precedence unlocks other formatting possibilities.

Possibly related issues:

lydell commented 3 years ago

I thought of some more use cases:

Whenever I make a function call, I tend to first write parens (), then the function name (String.slice) and then the argument(s) (String.slice start end input), because it’s so easy to forget parens and end up 7 parameters to the enclosing call instead of 4. In case I was in a position where the parens are unnecessary, it’s nice when elm-format automatically removes them. But it currently can’t remove them in all cases, such as in "foo" ++ (String.toUpper "bar" ++ "baz").

The last example is also possible to end up with if you have "foo" ++ and someFunc (String.toUpper "bar" ++ "baz") and then cut (String.toUpper "bar" ++ "baz") to "foo" ++: "foo" ++ (String.toUpper "bar" ++ "baz"). That happened to me today.