jfmengels/elm-review-simplify

Result.toMaybe,Maybe.map,Maybe.withDefault -> Result.map,Result.withDefault

Open

#287 opened on Feb 9, 2024

View on GitHub
 (0 comments) (4 reactions) (0 assignees)Elm (10 forks)github user discovery
enhancementhelp wanted

Repository metrics

Stars
 (22 stars)
PR merge metrics
 (PR metrics pending)

Description

|> Result.toMaybe
|> Maybe.map foo
|> Maybe.withDefault default

⬇️

|> Result.map foo
|> Result.withDefault default

Slack thread: https://elmlang.slack.com/archives/C010RT4D1PT/p1707473993252109 (The main result is that we should focus on the single Maybe.map first, and only handle sequences of multiple Maybe.maps as a nice-to-have later.)

What the rule should do: Remove an unneeded conversion from Result to Maybe if all we do with the Maybe later is to map+withDefault it.

Now that I write this, there is also a simpler variant without the maps:

|> Result.toMaybe
|> Maybe.withDefault default

⬇️

|> Result.withDefault default

So in its full generality, the rule should handle 0+ Maybe.maps in the same way.

What problems does it solve: Needless conversion that could be skipped

Example of things the rule would report:

0 Maybe.maps:

|> Result.toMaybe
|> Maybe.withDefault default

⬇️

|> Result.withDefault default

1 Maybe.map:

|> Result.toMaybe
|> Maybe.map foo
|> Maybe.withDefault default

⬇️

|> Result.map foo
|> Result.withDefault default

(Nice to have) 2+ Maybe.maps:

|> Result.toMaybe
|> Maybe.map foo
|> Maybe.map bar
|> Maybe.withDefault default

⬇️

|> Result.map foo
|> Result.map bar
|> Result.withDefault default

Should this be part of the Simplify rule or should it be a new rule?

Should be part of Simplify. We intentionally don't have an opinion on fusing maps together in this simplification, so it should be non-controversial.

I am looking for:

  • Someone to implement it for me

Contributor guide