on master help sign in
Merge branch 'master' into pr/1129
commit3c71ce84dac17ffe7cdbcfc1123b45acbdb282a0 authorEvan Czaplicki <info@elm-lang.org> dateTue 27 Oct 2015, 20:07 (11 years ago) parent5382c9d30c4b9b
6 files changed+395−36
hints/missing-patterns.md+142 hints/recursive-alias.md+161 hints/type-annotations.md+66 src/Reporting/Error/Canonicalize.hs+6−5 src/Reporting/Error/Type.hs+15−26 src/Type/Constrain/Expression.hs+5−5
hints/missing-patterns.mdnew file+142
@@ -0,0 +1,142 @@
1+# Hints for Missing Patterns
2+
3+Elm checks to make sure that all possible inputs to a function or `case` are handled. This gives us the guarantee that no Elm code is every going to crash because data had an unexpected shape.
4+
5+There are a couple techniques for making this work for you in every scenario.
6+
7+
8+## The danger of wildcard patterns
9+
10+A common scenario is that you want to add a tag to a union type that is used in a bunch of places. For example, maybe you are working with some widgets:
11+
12+```elm
13+type Widget
14+ = ScatterPlot ...
15+ | Table ...
16+
17+
18+isGraph : Widget -> Bool
19+isGraph widget =
20+ case widget of
21+ ScatterPlot _ ->
22+ True
23+
24+ _ ->
25+ False
26+```
27+
28+Notice the wildcard pattern in `isGraph`. This will hurt us! When you add `BarChart` as a new kind of `Widget` you are introducing a bug that the compiler cannot catch. A `BarChart` is a graph, but the code deciding that is just lumping a bunch of stuff together.
29+
30+It is better to explicitly list all possible patterns, like this:
31+
32+```elm
33+isGraph : Widget -> Bool
34+isGraph widget =
35+ case widget of
36+ ScatterPlot _ ->
37+ True
38+
39+ Table _ ->
40+ False
41+```
42+
43+So now when you want to add `BarChart`, the compiler will say "hey, what should `isGraph` do when it sees a bar chart?" This is a tiny bit of extra work, but it is very worth it!
44+
45+
46+## I want to go fast!
47+
48+Okay, so we are still editing that `Widget` code. We are not using any dangerous wildcards anymore, but there are 20 or 30 functions all pattern matching on `Widget` and doing something.
49+
50+```elm
51+type Widget
52+ = ScatterPlot ...
53+ | Table ...
54+
55+
56+isGraph : Widget -> Bool
57+isGraph widget =
58+ case widget of
59+ ScatterPlot _ ->
60+ True
61+
62+ Table _ ->
63+ False
64+
65+
66+-- and maybe a bunch of other things
67+```
68+
69+So now you add a new tag `BarChart` to `Widget`. This will trigger errors in all the functions that pattern match on widgets. In a big project, maybe this is a ton of functions.
70+
71+To get things running quickly, it can be helpful to use [`Debug.crash`](http://package.elm-lang.org/packages/elm-lang/core/latest/Debug#crash) temporarily. So you might update the code to this:
72+
73+```elm
74+type Widget
75+ = ScatterPlot ...
76+ | Table ...
77+ | BarChart ...
78+
79+
80+isGraph : Widget -> Bool
81+isGraph widget =
82+ case widget of
83+ ScatterPlot _ ->
84+ True
85+
86+ Table _ ->
87+ False
88+
89+ BarChart _ ->
90+ Debug.crash "TODO"
91+
92+
93+-- and maybe a bunch of other things
94+```
95+
96+Rather than figure out the exact details, just put in `Debug.crash "TODO"` for now. The Elm compiler is actually aware of `Debug.crash` so when it sees it in a `case` like this, it will augment the crash message with a bunch of helpful information. It will tell you:
97+
98+ 1. The name of the module that contains the offending code.
99+ 2. The line numbers of the `case` containing the crash.
100+ 3. The particular value that led to this crashing branch.
101+
102+From that information you have a pretty good idea of what went wrong and can go fix it.
103+
104+I tend to use `TODO` as the message when my goal is to go quick because it makes it easy to go and find all remaining todos in my code before a release.
105+
106+> And in 99.999% of cases, a `Debug.crash` should never make it to a release. It is nice for debugging (like it says in the name) but it is not really a good idea to use it in other ways. When you hit a `Debug.crash` the program really does crash. It is just done working.
107+
108+
109+## A list that definitely is not empty
110+
111+This can come up from time to time, but Elm **will not** let you write code like this:
112+
113+```elm
114+last : List a -> a
115+last list =
116+ case list of
117+ [x] ->
118+ x
119+
120+ _ :: rest ->
121+ last rest
122+```
123+
124+This is no good. It does not handle the empty list. The recommended definition would be:
125+
126+```elm
127+last : List a -> Maybe a
128+last list =
129+ case list of
130+ [] ->
131+ Nothing
132+
133+ [x] ->
134+ Just x
135+
136+ _ :: rest ->
137+ last rest
138+```
139+
140+This is nice because it lets users know that there might be a failure, so they can recover from it however they want.
141+
142+**Note:** You can define helpers like [`(?)`](https://github.com/elm-lang/core/issues/216) or [`unsafe`](https://github.com/elm-lang/core/issues/215) to make it easier to work with `Maybe` values a bunch. These helpers are being considered for inclusion in core, but it's not hard to put them in your own code and see if they make things nicer for you.
hints/recursive-alias.mdnew file+161
@@ -0,0 +1,161 @@
1+# Hints for Recursive Type Aliases
2+
3+At the root of this issue is the distinction between a `type` and a `type alias`.
4+
5+
6+## What is a type alias?
7+
8+When you create a type alias, you are just creating a shorthand to refer to an existing type. So when you say the following:
9+
10+```elm
11+type alias Time = Float
12+
13+type alias Degree = Float
14+
15+type alias Weight = Float
16+```
17+
18+You have not created any *new* types, you just made some alternate names for `Float`. You can write down things like this and it'll work fine:
19+
20+```elm
21+add : Time -> Degree -> Weight
22+add time degree =
23+ time + degree
24+```
25+
26+This is kind of a weird way to use type aliases though. The typical usage would be for records, where you do not want to write out the whole thing every time. Stuff like this:
27+
28+```elm
29+type alias Person =
30+ { name : String
31+ , age : Int
32+ , height : Float
33+ }
34+```
35+
36+It is much easier to write down `Person` in a type, and then it will just expand out to the underlying type when the compiler checks the program.
37+
38+
39+## Recursive type aliases?
40+
41+Okay, so lets say you have some type that may contain itself. In Elm, a common example of this is a comment that might have subcomments:
42+
43+```elm
44+type alias Comment =
45+ { message : String
46+ , upvotes : Int
47+ , downvotes : Int
48+ , responses : List Comment
49+ }
50+```
51+
52+Now remember that type *aliases* are just alternate names for the real type. So to make `Comment` into a concrete type, the compiler would start expanding it out.
53+
54+```elm
55+ { message : String
56+ , upvotes : Int
57+ , downvotes : Int
58+ , responses :
59+ List
60+ { message : String
61+ , upvotes : Int
62+ , downvotes : Int
63+ , responses :
64+ List
65+ { message : String
66+ , upvotes : Int
67+ , downvotes : Int
68+ , responses : List ...
69+ }
70+ }
71+ }
72+```
73+
74+The compiler cannot deal with values like this. It would just keep expanding forever.
75+
76+
77+## Recursive types!
78+
79+In cases where you want a recursive type, you need to actually create a brand new type. This is what the `type` keyword is for. A simple example of this can be seen when defining a linked list:
80+
81+```elm
82+type List
83+ = Empty
84+ | Node Int List
85+```
86+
87+No matter what, the type of `Node n xs` is going to be `List`. There is no expansion to be done. This means you can represent recursive structures with types do not explode into infinity.
88+
89+So let's return to wanting to represent a `Comment` that may have responses. There are a couple ways to do this:
90+
91+
92+### Obvious, but kind of annoying
93+
94+```elm
95+type Comment =
96+ Comment
97+ { message : String
98+ , upvotes : Int
99+ , downvotes : Int
100+ , responses : List Comment
101+ }
102+```
103+
104+Now lets say you want to register an upvote on a comment:
105+
106+```elm
107+upvote : Comment -> Comment
108+upvote (Comment comment) =
109+ Comment { comment | upvotes = 1 + comment.upvotes }
110+```
111+
112+It is kind of annoying that we now have to unwrap and wrap the record to do anything with it.
113+
114+
115+### Less obvious, but nicer
116+
117+```elm
118+type alias Comment =
119+ { message : String
120+ , upvotes : Int
121+ , downvotes : Int
122+ , responses : Responses
123+ }
124+
125+type Responses = Responses (List Comment)
126+```
127+
128+In this world, we introduce the `Responses` type to capture the recursion, but `Comment` is still an alias for a record. This means the `upvote` function looks nice again:
129+
130+```elm
131+upvote : Comment -> Comment
132+upvote comment =
133+ { comment | upvotes = 1 + comment.upvotes }
134+```
135+
136+So rather than having to unwrap a `Comment` to do *anything* to it, you only have to do some unwrapping in the cases where you are doing something recusive. In practice, this means you will do less unwrapping which is nice.
137+
138+
139+## Mutually recursive type aliases
140+
141+It is also possible to build type aliases that are *mutually* recursive. That might be something like this:
142+
143+```elm
144+type alias Comment =
145+ { message : String
146+ , upvotes : Int
147+ , downvotes : Int
148+ , responses : Responses
149+ }
150+
151+type alias Responses =
152+ { sortBy : SortBy
153+ , responses : List Comment
154+ }
155+
156+type SortBy = Time | Score | MostResponses
157+```
158+
159+When you try to expand `Comment` you have to expand `Responses` which needs to expand `Comment` which needs to expand `Responses`, etc.
160+
161+So this is just a fancy case of a self-recusive type alias. The solution is the same. Somewhere in that cycle, you need to define an actual `type` to end the infinite expansion.
hints/type-annotations.mdnew file+66
@@ -0,0 +1,66 @@
1+# Hints for Type Annotation Problems
2+
3+At the root of this kind of issue is always the fact that a type annotation in your code does not match the corresponding definition. Now that may mean that the type annotation is "wrong" or it may mean that the definition is "wrong". The compiler cannot figure out your intent, only that there is some mismatch.
4+
5+This document is going to outline the various things that can go wrong and show some examples.
6+
7+
8+## Annotation vs. Definition
9+
10+The most common issue is with user-defined type variables that are too general. So lets say you have defined a function like this:
11+
12+```elm
13+addPair : (a, a) -> a
14+addPair (x, y) =
15+ x + y
16+```
17+
18+The issue is that the type annotation is saying "I will accept a tuple containing literally *anything*" but the definition is using `(+)` which requires things to be numbers. So the compiler is going to infer that the true type of the definition is this:
19+
20+```elm
21+addPair : (number, number) -> number
22+```
23+
24+So you will probably see an error saying "I cannot match `a` with `number`" which is essentially saying, you are trying to provide a type annotation that is **too general**. You are saying `addPair` accepts anything, but in fact, it can only handle numbers.
25+
26+In cases like this, you want to go with whatever the compiler inferred. It is good at figuring this kind of stuff out ;)
27+
28+
29+## Annotation vs. Itself
30+
31+It is also possible to have a type annotation that clashes with itself. This is probably more rare, but someone will run into it eventually. Let's use another version of `addPair` with problems:
32+
33+```elm
34+addPair : (Int, Int) -> number
35+addPair (x, y) =
36+ x + y
37+```
38+
39+In this case the annotation says we should get a `number` out, but because we were specific about the inputs being `Int`, the output should also be an `Int`.
40+
41+
42+## Annotation vs. Internal Annotation
43+
44+A quite tricky case is when an outer type annotation clashes with an inner type annotation. Here is an example of this:
45+
46+```elm
47+filter : (a -> Bool) -> List a -> List a
48+filter isOkay list =
49+ let
50+ keepIfOkay : a -> Maybe a
51+ keepIfOkay x =
52+ if isOkay x then Just x else Nothing
53+ in
54+ List.filterMap keepIfOkay list
55+```
56+
57+This case is very unfortunate because all the type annotations are correct, but there is a detail of how type inference works right now that **user-defined type variables are not shared between annotations**. This can lead to probably the worst type error messages we have because the problem here is that `a` in the outer annotation does not equal `a` in the inner annotation.
58+
59+For now the best route is to leave off the inner annotation. It is unfortunate, and hopefully we will be able to do a nicer thing in future releases.
60+
61+
62+
63+
64+
65+
66+
src/Reporting/Error/Canonicalize.hs+6−5
@@ -182,8 +182,9 @@ toReport localizer err =
182182 , indent 4 $
183183 RenderType.decl localizer name tvars [(name, [unsafePromote tipe])]
184184 , text $
185− "This creates a brand new type that cannot be expanded. Read more about this at:\n"
186− ++ Help.hintLink "infinite-alias"
185+ "This is kind of a subtle distinction. I suggested the naive fix, but you can\n"
186+ ++ "often do something a bit nicer. So I would recommend reading more at:\n"
187+ ++ Help.hintLink "recursive-alias"
187188 ]
188189 )
189190
@@ -194,11 +195,11 @@ toReport localizer err =
194195 "This type alias is part of a mutually recursive set of type aliases."
195196 ( Help.stack
196197 [ text "The following type aliases are mutually recursive:"
197− , Help.drawCycle (map (\(_, name, _, _) -> name) aliases)
198+ , indent 4 (Help.drawCycle (map (\(_, name, _, _) -> name) aliases))
198199 , Help.reflowParagraph $
199200 "You need to convert at least one `type alias` into a `type`. This is a kind of\
200− \ subtle distinction, so definitely read how to fix it and have great code:"
201− ++ Help.hintLink "infinite-alias"
201+ \ subtle distinction, so definitely read up on this before you make a fix: "
202+ ++ Help.hintLink "recursive-alias"
202203 ]
203204 )
204205
src/Reporting/Error/Type.hs+15−26
@@ -508,41 +508,30 @@ reasonToString reason =
508508 go "Only strings and lists are both comparable and appendable."
509509
510510 BadVar (Just (Rigid _)) (Just (Rigid _)) ->
511− Just $ Help.stack $
512− [ hintDoc <> text "This usually happens when a few factors come together."
513− , indent 4 $ vcat $
514− [ text "1. You have a generic function with a type annotation."
515− , text "2. In its definition, there is a `let` with generic type annotations."
516− ]
517− , Help.reflowParagraph
518− "The issue is that these type variables are not actually shared between\
519− \ outer and inner type annotations right now. You can usually fix this by\
520− \ commenting out the inner type annotations."
521− ]
522−
523− BadVar (Just (Rigid (Just name))) _ ->
524− go $
525− "Looks like the type annotation is too generic. Type variable\
526− \ `" ++ name ++ "` is saying *many* kinds of value can be given, but the\
527− \ actual definition needs a more specific type. I am not sure whether the\
528− \ type annotation or the definition is \"right\" though, you may have to\
529− \ make the annotation more specific or make the definition more generic."
511+ go doubleRigidError
530512
531513 BadVar (Just (Rigid _)) _ ->
532− go genericRigidError
514+ go singleRigidError
533515
534516 BadVar _ (Just (Rigid _)) ->
535− go genericRigidError
517+ go singleRigidError
536518
537519 BadVar _ _ ->
538520 Nothing
539521
540522
541−genericRigidError :: String
542−genericRigidError =
543− "There is some problem with a rigid type variable. It is very likely that a\
544− \ type annotation is too generic. One type is saying *many* kinds of value\
545− \ can be given, but the other needs a more specific type."
523+singleRigidError :: String
524+singleRigidError =
525+ "A type annotation is too generic. You can probably just switch to the\
526+ \ type I inferred. These issues can be subtle though, so read more about it. "
527+ ++ Help.hintLink "type-annotations"
528+
529+
530+doubleRigidError :: String
531+doubleRigidError =
532+ "A type annotation is clashing with itself or with a sub-annotation.\
533+ \ This can be particularly tricky, so read more about it. "
534+ ++ Help.hintLink "type-annotations"
546535
547536
548537hintDoc :: Doc
src/Type/Constrain/Expression.hs+5−5
@@ -89,7 +89,7 @@ constrain env annotatedExpr@(A.A region expression) tipe =
8989 (CLet [monoscheme (Fragment.typeEnv fragment)]
9090 (Fragment.typeConstraint fragment /\ bodyCon)
9191 )
92− return $ con /\ CEqual Error.Lambda region tipe (argType ==> resType)
92+ return $ con /\ CEqual Error.Lambda region (argType ==> resType) tipe
9393
9494 E.App _ _ ->
9595 let
@@ -125,7 +125,7 @@ constrain env annotatedExpr@(A.A region expression) tipe =
125125
126126 newVars <- mapM (\_ -> mkVar Nothing) fields
127127 let newFields = Map.fromList (zip (map fst fields) (map VarN newVars))
128− let cNew = CEqual Error.Record region tipe (record newFields t)
128+ let cNew = CEqual Error.Record region (record newFields t) tipe
129129
130130 cs <- Monad.zipWithM (constrain env) (map snd fields) (map VarN newVars)
131131
@@ -140,7 +140,7 @@ constrain env annotatedExpr@(A.A region expression) tipe =
140140 (map VarN vars)
141141 let fields' = Map.fromList (zip (map fst fields) (map VarN vars))
142142 let recordType = record fields' (TermN EmptyRecord1)
143− return (ex vars (CAnd (fieldCons ++ [CEqual Error.Record region tipe recordType])))
143+ return (ex vars (CAnd (fieldCons ++ [CEqual Error.Record region recordType tipe])))
144144
145145 E.Let defs body ->
146146 do bodyCon <- constrain env body tipe
@@ -281,7 +281,7 @@ constrainBinop env region op leftExpr@(A.A leftRegion _) rightExpr@(A.A rightReg
281281 , CInstance region (V.toString op) opType
282282 , CEqual (Error.BinopLeft op leftRegion) region (VarN leftVar') (VarN leftVar)
283283 , CEqual (Error.BinopRight op rightRegion) region (VarN rightVar') (VarN rightVar)
284− , CEqual (Error.Binop op) region tipe (VarN answerVar)
284+ , CEqual (Error.Binop op) region (VarN answerVar) tipe
285285 ]
286286
287287
@@ -307,7 +307,7 @@ constrainList env region exprs tipe =
307307 return ( (var, region'), con )
308308
309309 varToCon var =
310− CEqual Error.List region tipe (Env.getType env "List" <| VarN var)
310+ CEqual Error.List region (Env.getType env "List" <| VarN var) tipe
311311
312312
313313-- CONSTRAIN IF EXPRESSIONS