elm / error-message-catalog

A catalog of broken Elm programs / data to improve error messages
BSD 3-Clause "New" or "Revised" License
173 stars 17 forks source link

Compiler doesn't catch partially redundant case..of branch #190

Open ckoster22 opened 7 years ago

ckoster22 commented 7 years ago

I noticed this when pattern matching with Maybes. Here's a SSCCE.

import Html exposing (beginnerProgram, div, button, text)
import Html.Events exposing (onClick)

main =
  beginnerProgram { model = 0, view = view, update = update }

view model =
  div []
    [ button [ onClick Decrement ] [ text "-" ]
    , div [] [ text (toString model) ]
    , button [ onClick (Increment Nothing) ] [ text "+" ]
    , button [ onClick (Increment (Just 3)) ] [ text "Add 3" ]
    ]

type Msg = Increment (Maybe Int) | Decrement

update msg model =
  case msg of
    Increment (Just num) ->
      model + num

    Increment maybeNum ->
      handleModelUpdateWithMaybe model maybeNum

    Decrement ->
      model - 1

handleModelUpdateWithMaybe : Int -> Maybe Int -> Int
handleModelUpdateWithMaybe model maybeNum =
  model + (Maybe.withDefault 1 maybeNum)

The second branch could only match on Increment Nothing since the first Increment (Just num) branch takes care of the Just case.

I'd expect an error from the compiler, such as:

-- REDUNDANT PATTERN -----------------------------------------------------------

The following pattern is redundant.

28|     Increment maybeNum ->
        ^^^^^^^^^^^^^^^^^^

This branch could only match on

    Increment Nothing

But you're saying it could match on

    Increment (Maybe Int)

Other branches were found that make the above branch redundant.

25|    Increment (Just num) ->
                 ^^^^^^^^^^
ckoster22 commented 7 years ago

I just came across a situation where this compiler error wouldn't be helpful as described above. I had a relatively large union type and I had a case statement that looked something like this.

case myUnionType of
    option1 ->
        -- do something
    option2 ->
        -- do something else
    _ ->
        -- do something else for options 3 thru 10

In that situation, it'd be annoying for the compiler tell me the _ pattern is partially redundant.

I'm conflicted because I think the compiler error in the above post would be useful but the same compiler error in this post would be a nuisance.

I'll leave this issue open for now and see what others think.