jfmengels / elm-review-simplify

Provides elm-review rules to simplify your Elm code
https://package.elm-lang.org/packages/jfmengels/elm-review-simplify/latest/
BSD 3-Clause "New" or "Revised" License
19 stars 9 forks source link

Simplify wrappings and/or matching for case expressions #99

Open jfmengels opened 1 year ago

jfmengels commented 1 year ago

What the simplification should do:

Remove unnecessary branches when a value evaluated by a case expression is wrapped/equal by a constructor.

What problems does it solve:

This would remove unused code.

Example of things the rule would report:

Simplify

value =
  case Ok (fn a) of
    Ok b ->
      doSomething b
    _ -> ... -- other branches

to

value =
  case fn a of
    b ->
      doSomething b
    -- no other branches

Same for things like wrapping in Just and other things, and even matching on plain values

a =
  case Nothing of
    Nothing ->
      doSomething ()
    Just a -> ...

to

a =
  doSomething ()

I think that similarly, if we have a wildcard branch, and a pattern obviously doesn't match, then we can remove the branch:

value =
  case Constructor (fn a) of
    Constructor (B b) ->
      doSomething b
    OtherConstructor b -> -- unnecessary
    _ -> -- wildcard

to

value =
  case Constructor (fn a) of
    Constructor (B b) ->
      doSomething b
    _ -> -- wildcard

Example of things the rule would not report:

To stay correct, we should be pretty sure that we're not removing necessary branches. So we'd need to make sure that we only remove branches where the outer pattern is obviously wrong/correct (depending on what we want to do), and no inner pattern matches are there.

For instance: if we have a branch Constructor a ( b, and, otherVariables ) -> and the expression is wrapped in Constructor, we can apply a simplification. But if the branch has nested constructor matches like Constructor a ( B b, _, _ ) -> or has list destructurings Constructor (a :: []) -> then we shouldn't do anything.

Creating a different issue

The problem with the first example is that we end up with a case expression that is only assigning a variable. I'm thinking that this becomes an opportunity to transform that variable to a let variable, which might be a non-controversial.

value =
  case fn a of
    b ->
      doSomething b
    -- no other branches
-->
value =
  let b = fn a
  in
  doSomething b

I am looking for:

Advice on the question above, and someone to implement it if they're up for it 🙂