Elm-format: Remove unnecessary parens

Created on 2 Nov 2016  路  12Comments  路  Source: avh4/elm-format

HT @rtfeldman

  • "a" ++ (f x y) ++ "b" -> "a" ++ f x y ++ "b"
  • (f x) y -> f x y
  • (x) -> x
  • ([1, 2]) -> [ 1, 2 ]
  • 1 + ((x + 1)) -> 1 + (x + 1)

BUT NOT:

  • (x + 1) + (y - 1) -> x + 1 + y - 1 (for infix expressions, we want to allow unnecessary parens)

Most helpful comment

The following example I often see with beginners. Removing the params in this case would be educational:

if (x) then
    something
else
    somethingElse

Programmers that move from Java/Javascript to Elm tend to do this.

All 12 comments

Some more examples:

{ blah = (foo << bar)
, stuff =
    (if conditional then
        something
     else
        somethingElse
    )
, thing = (doSomething (func arg))
}

The following example I often see with beginners. Removing the params in this case would be educational:

if (x) then
    something
else
    somethingElse

Programmers that move from Java/Javascript to Elm tend to do this.

Similarly true for case (foo) of as well.

Implemented in b4c90109...e4579233

Be on the lookout for bugs this may have introduced!

I think I found a bug:

foo bool =
    "A"
        ++ case bool of
            True ->
                "true"

            False ->
                "false"

Is now formatted as:

foo bool =
    "A"
        ++ (case bool of
                True ->
                    "true"

                False ->
                    "false"
           )

But these parens are not required

The same holds for f <| case .... This gets rewritten as f <| (case ...). What applies to case, also seems to hold for let and if statements.

I've found another case where lambdas will get parens when these are not required:

foo = 
    1 |> \x -> x + 1

Is formatted as:

foo =
    1 |> (\x -> x + 1)

I have used elm-analyse to find these cases after formatting with a build on the current master (5534bf66cbf2844c43b8aea58a9c5c60b62dd80c).

Another bug which actually breaks code:

The following code:

foo maybeF =
    (maybeF |> Maybe.withDefault identity)
        1

is formatted as:

foo maybeF =
    maybeF |> Maybe.withDefault identity
        1

The broken case is fixed in 936c8a89.

For the if/case/let/lamba at the end of infix expressions, I left parens there because those four types of expressions do not have a clearly-delimited end, thus parens are required for them if they are in the middle of an infix expressions, so it seemed reasonable to use the parens when those are at the end as well for consistency (and reduces code diffs if you later add more terms to the infix expression):

f =
    1
        |> (\x y -> x + 1)
        |> (\x -> x 2)

(In the above code, parens are required with the first lambda, and optional with the second lambda.)

I agree with the consistency that this creates. However, I see in my codebase some code will become more vertical. The parens that are currently added to the if/let/case will be placed at the end of the expression on a newline. This with the normal two blank lines makes the files a bit longer. If this is a deliberate choice I accept this, but I think some people might find it annoying. Is it a good idea to add a refactoring strategy, such as extract function, to the style guide?

Handling of tests is fixed in 936c8a89...b108911e

Added something rough to the style guide: 85c78b1b...7a0a0153

Was this page helpful?
0 / 5 - 0 ratings

Related issues

rtfeldman picture rtfeldman  路  4Comments

ahstro picture ahstro  路  3Comments

yonigibbs picture yonigibbs  路  7Comments

ahstro picture ahstro  路  4Comments

conradwt picture conradwt  路  8Comments