elm / compiler

Compiler for Elm, a functional language for reliable webapps.
https://elm-lang.org/
BSD 3-Clause "New" or "Revised" License
7.51k stars 656 forks source link

Types are incorrect in error message #1062

Closed zgotsch closed 8 years ago

zgotsch commented 8 years ago

I am working through the Elm architecture tutorial and I was getting an unhelpful unification error message from the compiler. When reporting the expected types and the encountered type, they did not match what I expected from the code.

Here is the code which is causing the error:

type Action
  = Reset
  | AddCounter
  | RemoveCounter
  | Update CounterId Counter.Action

type alias CounterId = Int

type alias Model =
  { counters : List (CounterId, Counter.Model)
  , nextId : CounterId
  }

update : Action -> Model -> Model
update action model =
  case action of
    Update id act -> { model |
      counters <- (List.map
        (\(i, model) ->
          if | i == id -> Counter.update act model
             | otherwise -> model)
        model.counters)
      }

The lambda is returning a Counter.Model when it should be returning a (CounterId, Counter.Model), but the error message says:

-- TYPE MISMATCH ------------------------------------------- ././CounterList.elm

The 2nd branch of this `case` results in an unexpected type of value.

37|   case action of
38|     Reset -> init 0 3
39|     -- AddCounter -> { counters = (model.nextId, Counter.init 0)
40|     --               , nextId = model.nextId + 1
41|     --               }
42|     -- RemoveCounter -> case model.counters of
43|     --   hd::tl -> { counters = tl
44|     --             , nextId = model.nextId - 1
45|     --             }
46|     --   [] -> model
47|>    Update id act -> { model |
48|>      counters <- (List.map
49|>        (\(i, model) ->
50|>          if | i == id -> Counter.update act model
51|>             | otherwise -> model)
52|>        model.counters)
53|>      }

All branches should match so that no matter which one we take, we always get
back the same type of value.

As I infer the type of values flowing through your program, I see a conflict
between these two types:

    (Int, Int)

    Int

I don't understand why it is saying (Int, Int) and Int in the error message, instead of (CounterId, Counter.Model) and Counter.Model. Fixing the type of the lambda causes these messages to disappear.

P.S. There is also an error for the function not matching the signature which gives the same types:

-- TYPE MISMATCH ------------------------------------------- ././CounterList.elm

The type annotation for `update` does not match its definition.

35| update : Action -> Model -> Model
             ^^^^^^^^^^^^^^^^^^^^^^^^
As I infer the type of values flowing through your program, I see a conflict
between these two types:

    (Int, Int)

    Int
evancz commented 8 years ago

So when we show an error message, we need to "realias" things. Since CounterId is just an alias for Int in your code, type inference will treat it just like an Int. This means that at some point we have to figure out if any particular Int should be displayed to the user as CounterId or whatever else.

This also means that you can realias too aggressively. If we display all ints as a CounterId we would have the inverse of this issue (and have had it in other releases!) which I think is actually way worse. So this feature has to be implemented very carefully to get a good experience in all cases.

Point is, I think it's plausible to call this a bug, but given how we are organizing our repos, I think it'd make the most sense to retarget this to this repo which is all about improving error messages in Elm. The plan is to gather a bunch of reports such that I can go through in one big batch and fix all the interrelated stuff. Do you mind closing this here and retargeting to that repo?

zgotsch commented 8 years ago

Will do! Thanks, Evan.

zgotsch commented 8 years ago

Moved to elm-lang/error-message-catalog.

evancz commented 8 years ago

Cool, thank you :)