improved error messages for function definitions

compiler/constrain/src/expr.rs

@@ -307,7 +307,7 @@ pub fn constrain_expr(
         }
         Var(symbol) => Lookup(*symbol, expected, region),
         Closure(fn_var, _symbol, _recursive, args, boxed) => {
-            let (loc_body_expr, ret_var) = &**boxed;
+            let (loc_body_expr, ret_var) = boxed.as_ref();
             let mut state = PatternState {
                 headers: SendMap::default(),
                 vars: Vec::with_capacity(args.len()),
@@ -1001,13 +1001,18 @@ fn constrain_def(env: &Env, def: &Def, body_con: Constraint) -> Constraint {
                 &mut pattern_state.headers,
             );
 
+            let env = &Env {
+                home: env.home,
+                rigids: ftv,
+            };
+
             let annotation_expected = FromAnnotation(
                 def.loc_pattern.clone(),
                 arity,
                 AnnotationSource::TypedBody {
                     region: annotation.region,
                 },
-                signature,
+                signature.clone(),
             );
 
             pattern_state.constraints.push(Eq(
@@ -1017,15 +1022,118 @@ fn constrain_def(env: &Env, def: &Def, body_con: Constraint) -> Constraint {
                 annotation.region,
             ));
 
-            constrain_expr(
+            // when a def is annotated, and it's body is a closure, treat this
-                &Env {
+            // as a named function (in elm terms) for error messages.
-                    home: env.home,
+            //
-                    rigids: ftv,
+            // This means we get errors like "the first argument of `f` is weird"
+            // instead of the more generic "something is wrong with the body of `f`"
+            match (&def.loc_expr.value, &signature) {
+                (
+                    Closure(fn_var, _symbol, _recursive, args, boxed),
+                    Type::Function(arg_types, _),
+                ) if true => {
+                    let expected = annotation_expected;
+                    let region = def.loc_expr.region;
+
+                    let (loc_body_expr, ret_var) = boxed.as_ref();
+                    let mut state = PatternState {
+                        headers: SendMap::default(),
+                        vars: Vec::with_capacity(args.len()),
+                        constraints: Vec::with_capacity(1),
+                    };
+                    let mut vars = Vec::with_capacity(state.vars.capacity() + 1);
+                    let mut pattern_types = Vec::with_capacity(state.vars.capacity());
+                    let ret_var = *ret_var;
+                    let ret_type = Type::Variable(ret_var);
+
+                    vars.push(ret_var);
+
+                    let it = args.iter().zip(arg_types.iter()).enumerate();
+                    for (index, ((pattern_var, loc_pattern), loc_ann)) in it {
+                        {
+                            // ensure type matches the one in the annotation
+                            let opt_label =
+                                if let Pattern::Identifier(label) = def.loc_pattern.value {
+                                    Some(label)
+                                } else {
+                                    None
+                                };
+                            let pattern_type: &Type = loc_ann;
+
+                            let pattern_expected = PExpected::ForReason(
+                                PReason::TypedArg {
+                                    index: Index::zero_based(index),
+                                    opt_name: opt_label,
                                 },
+                                pattern_type.clone(),
+                                loc_pattern.region,
+                            );
+
+                            constrain_pattern(
+                                env,
+                                &loc_pattern.value,
+                                loc_pattern.region,
+                                pattern_expected,
+                                &mut state,
+                            );
+                        }
+
+                        {
+                            // record the correct type in pattern_var
+                            let pattern_type = Type::Variable(*pattern_var);
+                            pattern_types.push(pattern_type.clone());
+
+                            state.vars.push(*pattern_var);
+                            state.constraints.push(Constraint::Eq(
+                                pattern_type.clone(),
+                                Expected::NoExpectation(loc_ann.clone()),
+                                Category::Storage,
+                                loc_pattern.region,
+                            ));
+
+                            vars.push(*pattern_var);
+                        }
+                    }
+
+                    let fn_type = Type::Function(pattern_types, Box::new(ret_type.clone()));
+                    let body_type = NoExpectation(ret_type);
+                    let ret_constraint =
+                        constrain_expr(env, loc_body_expr.region, &loc_body_expr.value, body_type);
+
+                    vars.push(*fn_var);
+                    let defs_constraint = And(state.constraints);
+
+                    exists(
+                        vars,
+                        And(vec![
+                            Let(Box::new(LetConstraint {
+                                rigid_vars: Vec::new(),
+                                flex_vars: state.vars,
+                                def_types: state.headers,
+                                def_aliases: SendMap::default(),
+                                defs_constraint,
+                                ret_constraint,
+                            })),
+                            // "the closure's type is equal to expected type"
+                            Eq(fn_type.clone(), expected, Category::Lambda, region),
+                            // "fn_var is equal to the closure's type" - fn_var is used in code gen
+                            Eq(
+                                Type::Variable(*fn_var),
+                                NoExpectation(fn_type),
+                                Category::Storage,
+                                region,
+                            ),
+                        ]),
+                    )
+                }
+
+                _ => constrain_expr(
+                    &env,
                     def.loc_expr.region,
                     &def.loc_expr.value,
                     annotation_expected,
-            )
+                ),
+            }
         }
         None => constrain_expr(
             env,

compiler/reporting/src/error/type.rs

@@ -953,6 +953,45 @@ fn to_pattern_report<'b>(
         }
 
         PExpected::ForReason(reason, expected_type, region) => match reason {
+            PReason::TypedArg { opt_name, index } => {
+                let name = match opt_name {
+                    Some(n) => alloc.symbol_unqualified(n),
+                    None => alloc.text(" this definition "),
+                };
+                let doc = alloc.stack(vec![
+                    alloc
+                        .text("The ")
+                        .append(alloc.text(index.ordinal()))
+                        .append(alloc.text(" argument to "))
+                        .append(name.clone())
+                        .append(alloc.text(" is weird:")),
+                    alloc.region(region),
+                    pattern_type_comparision(
+                        alloc,
+                        found,
+                        expected_type,
+                        add_pattern_category(
+                            alloc,
+                            alloc.text("The argument is a pattern that matches"),
+                            &category,
+                        ),
+                        alloc.concat(vec![
+                            alloc.text("But the annotation on "),
+                            name,
+                            alloc.text(" says the "),
+                            alloc.text(index.ordinal()),
+                            alloc.text(" argument should be:"),
+                        ]),
+                        vec![],
+                    ),
+                ]);
+
+                Report {
+                    filename,
+                    title: "TYPE MISMATCH".to_string(),
+                    doc,
+                }
+            }
             PReason::WhenMatch { index } => {
                 if index == Index::FIRST {
                     let doc = alloc.stack(vec![
@@ -1295,10 +1334,17 @@ pub fn to_doc<'b>(
                 alloc,
                 fields
                     .into_iter()
-                    .map(|(k, v)| {
+                    .map(|(k, value)| {
                         (
                             alloc.string(k.as_str().to_string()),
-                            to_doc(alloc, Parens::Unnecessary, v.into_inner()),
+                            match value {
+                                RecordField::Optional(v) => {
+                                    RecordField::Optional(to_doc(alloc, Parens::Unnecessary, v))
+                                }
+                                RecordField::Required(v) => {
+                                    RecordField::Required(to_doc(alloc, Parens::Unnecessary, v))
+                                }
+                            },
                         )
                     })
                     .collect(),
@@ -1594,12 +1640,18 @@ fn diff_record<'b>(
                 left: (
                     field.clone(),
                     alloc.string(field.as_str().to_string()),
-                    diff.left,
+                    match t1 {
+                        RecordField::Optional(_) => RecordField::Optional(diff.left),
+                        RecordField::Required(_) => RecordField::Required(diff.left),
+                    },
                 ),
                 right: (
                     field.clone(),
                     alloc.string(field.as_str().to_string()),
-                    diff.right,
+                    match t2 {
+                        RecordField::Optional(_) => RecordField::Optional(diff.right),
+                        RecordField::Required(_) => RecordField::Required(diff.right),
+                    },
                 ),
                 status: diff.status,
             }
@@ -1608,7 +1660,7 @@ fn diff_record<'b>(
         (
             field.clone(),
             alloc.string(field.as_str().to_string()),
-            to_doc(alloc, Parens::Unnecessary, tipe.clone().into_inner()),
+            tipe.map(|t| to_doc(alloc, Parens::Unnecessary, t.clone())),
         )
     };
     let shared_keys = fields1
@@ -1661,7 +1713,8 @@ fn diff_record<'b>(
 
     let ext_diff = ext_to_diff(alloc, ext1, ext2);
 
-    let mut fields_diff: Diff<Vec<(Lowercase, RocDocBuilder<'b>, RocDocBuilder<'b>)>> = Diff {
+    let mut fields_diff: Diff<Vec<(Lowercase, RocDocBuilder<'b>, RecordField<RocDocBuilder<'b>>)>> =
+        Diff {
             left: vec![],
             right: vec![],
             status: Status::Similar,
@@ -1938,7 +1991,7 @@ mod report_text {
 
     pub fn record<'b>(
         alloc: &'b RocDocAllocator<'b>,
-        entries: Vec<(RocDocBuilder<'b>, RocDocBuilder<'b>)>,
+        entries: Vec<(RocDocBuilder<'b>, RecordField<RocDocBuilder<'b>>)>,
         opt_ext: Option<RocDocBuilder<'b>>,
     ) -> RocDocBuilder<'b> {
         let ext_doc = if let Some(t) = opt_ext {
@@ -1951,8 +2004,15 @@ mod report_text {
             alloc.text("{}").append(ext_doc)
         } else {
             let entry_to_doc =
-                |(field_name, field_type): (RocDocBuilder<'b>, RocDocBuilder<'b>)| {
+                |(field_name, field_type): (RocDocBuilder<'b>, RecordField<RocDocBuilder<'b>>)| {
-                    field_name.append(alloc.text(" : ")).append(field_type)
+                    match field_type {
+                        RecordField::Required(field) => {
+                            field_name.append(alloc.text(" : ")).append(field)
+                        }
+                        RecordField::Optional(field) => {
+                            field_name.append(alloc.text(" ? ")).append(field)
+                        }
+                    }
                 };
 
             let starts =

compiler/reporting/tests/test_reporting.rs

@@ -1630,6 +1630,21 @@ mod test_reporting {
 
     #[test]
     fn bad_double_rigid() {
+        // this previously reported the message below, not sure which is better
+        //
+        //                Something is off with the body of the `f` definition:
+        //
+        //                1 ┆  f : a, b -> a
+        //                2 ┆  f = \x, y -> if True then x else y
+        //                  ┆      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+        //
+        //                The body is an anonymous function of type:
+        //
+        //                    a, a -> a
+        //
+        //                But the type annotation on `f` says it should be:
+        //
+        //                    a, b -> a
         report_problem_as(
             indoc!(
                 r#"
@@ -1643,21 +1658,22 @@ mod test_reporting {
                 r#"
                 -- TYPE MISMATCH ---------------------------------------------------------------
 
-                Something is off with the body of the `f` definition:
+                This `if` has an `else` branch with a different type from its `then` branch:
 
-                1 ┆  f : a, b -> a
                 2 ┆  f = \x, y -> if True then x else y
-                  ┆      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+                  ┆                                   ^
 
-                The body is an anonymous function of type:
+                This `y` value is a:
 
-                    a, a -> a
+                    b
 
-                But the type annotation on `f` says it should be:
+                but the `then` branch has the type:
 
-                    a, b -> a
+                    a
 
-                Hint: Your type annotation uses `a` and `b` as separate type variables.
+                I need all branches in an `if` to have the same type!
+
+                Hint: Your type annotation uses `b` and `a` as separate type variables.
                 Your code seems to be saying they are the same though. Maybe they
                 should be the same your type annotation? Maybe your code uses them in
                 a weird way?
@@ -1958,27 +1974,17 @@ mod test_reporting {
                 r#"
                 -- TYPE MISMATCH ---------------------------------------------------------------
 
-                Something is off with the body of the `f` definition:
+                The `r` record does not have a `.foo` field:
 
-                1 ┆   f : { fo: Int }ext -> Int
+                3 ┆      r2 = { r & foo: r.fo }
-                2 ┆>  f = \r ->
+                  ┆                 ^^^^^^^^^
-                3 ┆>      r2 = { r & foo: r.fo }
-                4 ┆>
-                5 ┆>      r2.fo
 
-                The body is an anonymous function of type:
+                This is usually a typo. Here are the `r` fields that are most similar:
 
-                    { fo : Int, foo : Int }a -> Int
+                    { fo : Int
+                    }ext
 
-                But the type annotation on `f` says it should be:
+                So maybe `.foo` should be `.fo`?
-
-                    { fo : Int }ext -> Int
-
-                Hint: Seems like a record field typo. Maybe `foo` should be `fo`?
-
-                Hint: Can more type annotations be added? Type annotations always help
-                me give more specific messages, and I think they could help a lot in
-                this case
                 "#
             ),
         )
@@ -3492,7 +3498,7 @@ mod test_reporting {
             indoc!(
                 r#"
                 f : { x : Int, y ? Int } -> Int
-                f = \{ x, y ? "foo" } -> x + y
+                f = \{ x, y ? "foo" } -> (\g, _ -> g) x y
 
                 f
                 "#
@@ -3501,18 +3507,18 @@ mod test_reporting {
                 r#"
                 -- TYPE MISMATCH ---------------------------------------------------------------
 
-                The 2nd argument to `add` is not what I expect:
+                The 1st argument to `f` is weird:
 
-                2 ┆  f = \{ x, y ? "foo" } -> x + y
+                2 ┆  f = \{ x, y ? "foo" } -> (\g, _ -> g) x y
-                  ┆                               ^
+                  ┆       ^^^^^^^^^^^^^^^^
 
-                This `y` value is a:
+                The argument is a pattern that matches record values of type:
 
-                    Str
+                    { x : Int, y ? Str }
 
-                But `add` needs the 2nd argument to be:
+                But the annotation on `f` says the 1st argument should be:
 
-                    Num a
+                    { x : Int, y ? Int }
                 "#
             ),
         )
@@ -3538,21 +3544,18 @@ mod test_reporting {
                 r#"
                 -- TYPE MISMATCH ---------------------------------------------------------------
 
-                Something is off with the body of the `f` definition:
+                The 1st pattern in this `when` is causing a mismatch:
 
-                1 ┆   f : { x : Int, y : Int } -> Int
+                4 ┆              { x, y : "foo" } -> x + 0
-                2 ┆>  f = \r ->
+                  ┆              ^^^^^^^^^^^^^^^^
-                3 ┆>          when r is
-                4 ┆>              { x, y : "foo" } -> x + 0
-                5 ┆>              _ -> 0
 
-                The body is an anonymous function of type:
+                The first pattern is trying to match record values of type:
 
-                    { x : Num Integer, y : Str } -> Num Integer
+                    { x : Int, y : Str }
 
-                But the type annotation on `f` says it should be:
+                But the expression between `when` and `is` has the type:
 
-                    { x : Int, y : Int } -> Int
+                    { x : Int, y : Int }
                 "#
             ),
         )
@@ -3579,21 +3582,18 @@ mod test_reporting {
                 r#"
                 -- TYPE MISMATCH ---------------------------------------------------------------
 
-                Something is off with the body of the `f` definition:
+                The 1st pattern in this `when` is causing a mismatch:
 
-                1 ┆   f : { x : Int, y ? Int } -> Int
+                4 ┆              { x, y ? "foo" } -> (\g, _ -> g) x y
-                2 ┆>  f = \r ->
+                  ┆              ^^^^^^^^^^^^^^^^
-                3 ┆>          when r is
-                4 ┆>              { x, y ? "foo" } -> (\g, _ -> g) x y
-                5 ┆>              _ -> 0
 
-                The body is an anonymous function of type:
+                The first pattern is trying to match record values of type:
 
-                    { x : Num Integer, y : Str } -> Num Integer
+                    { x : Int, y ? Str }
 
-                But the type annotation on `f` says it should be:
+                But the expression between `when` and `is` has the type:
 
-                    { x : Int, y : Int } -> Int
+                    { x : Int, y ? Int }
                 "#
             ),
         )

compiler/types/src/types.rs

@@ -41,6 +41,17 @@ impl<T> RecordField<T> {
             Required(t) => t,
         }
     }
+
+    pub fn map<F, U>(&self, mut f: F) -> RecordField<U>
+    where
+        F: FnMut(&T) -> U,
+    {
+        use RecordField::*;
+        match self {
+            Optional(t) => Optional(f(t)),
+            Required(t) => Required(f(t)),
+        }
+    }
 }
 
 impl RecordField<Type> {
@@ -820,8 +831,17 @@ pub struct RecordStructure {
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum PReason {
-    WhenMatch { index: Index },
+    TypedArg {
-    TagArg { tag_name: TagName, index: Index },
+        opt_name: Option<Symbol>,
+        index: Index,
+    },
+    WhenMatch {
+        index: Index,
+    },
+    TagArg {
+        tag_name: TagName,
+        index: Index,
+    },
     PatternGuard,
 }