-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Example of unfriendly closure error messages #6
base: master
Are you sure you want to change the base?
Conversation
Can you get this down to a more minimal example? As it is, it requires elm-linear-algebra. Is it possible to reproduce the character of the mistake without any extra packages? Ideally the code would be a small let that still demonstrates the issue. |
I messed around with this a bit today, and it seems like the trouble is that the conflict arises when you have the type annotation. So the whole expression type checks properly, but the value it gives out does not have the type listed in the annotation. So it is true that the large expression is the root of the error. Does that make sense? What behavior would you expect given these facts? |
I also refactored the code to how I would probably write such a thing: hacky2 : Dict.Dict String ParsedSvgData -> List Country
hacky2 countryShapes =
List.map makeCountry (Dict.toList countryShapes)
makeCountry ( id, record ) =
{ code = id
, name = record.title
, lines = makeLines record.polygons
}
makeLines polygons =
case polygons of
first :: rest ->
let
vectors = List.map makeVector first
in
List.map2 helper vectors rest ++ makeLines rest
[] ->
[]
makeVector ( x, y ) =
( ( x / scale ), ( y / scale ) )
helper v1 v2 =
( { a1 = v1 }, { a2 = v2 } ) And it seems like it is reporting the error message in an entirely appropriate way. Breaking the function up in this way also means that it is much easier to do unit testing on the relevant functions. And add type annotations. It may be possible to try to push the information from the type annotation down into the expression, but I am not totally sure. That could help with this case maybe... |
@rtfeldman has shared another example in this category, which is "the type annotation does not match the defined value." I have a potential improvement that would spit out this error message for you:
@jadski, how does this look? |
@evancz - thanks for this research and feedback. My example was designed to show-off a non-trivial (not necessarily optimally written) closure error message and the resulting helplessness I felt. Your code definitely looks easier to manage, and no doubt represents better practice. However I would feel irked if I was forced to adopt your style because the other generates cryptic error messages. Unless I've missed something, your revised error message example doesn't really simplify diagnosis of the offending clause. The problem was that the error message did not report with precision which clause in the let was at fault (although the parser presumably knows this information) - I would hope to see something more like this (where makeVector is defined in the let of the hacky2 function) -- TYPE MISMATCH ------------------------------------------------------ hats.elm The type annotation for 'makeVector' does not match its definition. 66| makeVector ( x, y ) = ^^^^^^ To be more specific, type inference is leading to a conflict between this type: Math.Vector2.Vec2 and this type: (Float, Float) I know you've improved error messages since this discussion, so I'll check these out. |
Is this solved? |
As requested from https://groups.google.com/forum/#!topic/elm-discuss/3WPnKhN_Dqo