Merge pull request #294 from rtfeldman/more-type-errors

More type errors

compiler/can/src/expr.rs

@@ -688,7 +688,7 @@ fn canonicalize_when_branch<'a>(
     env: &mut Env<'a>,
     var_store: &VarStore,
     scope: &mut Scope,
-    region: Region,
+    _region: Region,
     branch: &'a ast::WhenBranch<'a>,
     output: &mut Output,
 ) -> (WhenBranch, References) {
@@ -721,7 +721,7 @@ fn canonicalize_when_branch<'a>(
         None => None,
         Some(loc_expr) => {
             let (can_guard, guard_branch_output) =
-                canonicalize_expr(env, var_store, &mut scope, region, &loc_expr.value);
+                canonicalize_expr(env, var_store, &mut scope, loc_expr.region, &loc_expr.value);
 
             branch_output.union(guard_branch_output);
             Some(can_guard)

compiler/constrain/src/expr.rs

@@ -181,9 +181,10 @@ pub fn constrain_expr(
                 region,
             );
 
-            cons.push(con);
+            // ensure constraints are solved in this order, gives better errors
-            cons.push(fields_con);
+            cons.insert(0, fields_con);
-            cons.push(record_con);
+            cons.insert(1, con);
+            cons.insert(2, record_con);
 
             exists(vars, And(cons))
         }
@@ -202,9 +203,12 @@ pub fn constrain_expr(
                 let list_elem_type = Type::Variable(*elem_var);
                 let mut constraints = Vec::with_capacity(1 + loc_elems.len());
 
-                for loc_elem in loc_elems {
+                for (index, loc_elem) in loc_elems.iter().enumerate() {
-                    let elem_expected =
+                    let elem_expected = ForReason(
-                        ForReason(Reason::ElemInList, list_elem_type.clone(), region);
+                        Reason::ElemInList { index },
+                        list_elem_type.clone(),
+                        loc_elem.region,
+                    );
                     let constraint =
                         constrain_expr(env, loc_elem.region, &loc_elem.value, elem_expected);
 
@@ -539,7 +543,11 @@ pub fn constrain_expr(
                                 cond_type.clone(),
                                 region,
                             ),
-                            ForReason(Reason::WhenBranch { index }, branch_type.clone(), region),
+                            ForReason(
+                                Reason::WhenBranch { index },
+                                branch_type.clone(),
+                                when_branch.value.region,
+                            ),
                         );
 
                         branch_cons.push(branch_con);

compiler/constrain/src/uniq.rs

@@ -596,9 +596,12 @@ pub fn constrain_expr(
                 let entry_type = Type::Variable(*elem_var);
                 let mut constraints = Vec::with_capacity(1 + loc_elems.len());
 
-                for loc_elem in loc_elems.iter() {
+                for (index, loc_elem) in loc_elems.iter().enumerate() {
-                    let elem_expected =
+                    let elem_expected = Expected::ForReason(
-                        Expected::ForReason(Reason::ElemInList, entry_type.clone(), region);
+                        Reason::ElemInList { index },
+                        entry_type.clone(),
+                        region,
+                    );
                     let constraint = constrain_expr(
                         env,
                         var_store,

compiler/reporting/src/report.rs

@@ -12,6 +12,7 @@ use ven_pretty::{BoxAllocator, DocAllocator, DocBuilder, Render, RenderAnnotated
 
 /// A textual report.
 pub struct Report {
+    pub title: String,
     pub filename: PathBuf,
     pub text: ReportText,
 }
@@ -121,6 +122,7 @@ pub fn can_problem(filename: PathBuf, problem: Problem) -> Report {
     };
 
     Report {
+        title: "SYNTAX PROBLEM".to_string(),
         filename,
         text: Concat(texts),
     }
@@ -300,11 +302,11 @@ where
             Url => {
                 self.write_str("<")?;
             }
-            GlobalTag | PrivateTag | RecordField | Keyword => {
+            GlobalTag | PrivateTag | Keyword => {
                 self.write_str("`")?;
             }
             CodeBlock | PlainText | LineNumber | Error | GutterBar | TypeVariable | Alias
-            | Module | Structure | Symbol | BinOp => {}
+            | RecordField | Module | Structure | Symbol | BinOp => {}
         }
         self.style_stack.push(*annotation);
         Ok(())
@@ -322,11 +324,11 @@ where
                 Url => {
                     self.write_str(">")?;
                 }
-                GlobalTag | PrivateTag | RecordField | Keyword => {
+                GlobalTag | PrivateTag | Keyword => {
                     self.write_str("`")?;
                 }
                 CodeBlock | PlainText | LineNumber | Error | GutterBar | TypeVariable | Alias
-                | Module | Structure | Symbol | BinOp => {}
+                | RecordField | Module | Structure | Symbol | BinOp => {}
             },
         }
         Ok(())

compiler/reporting/src/type_error.rs

@@ -74,6 +74,7 @@ fn report_mismatch(
     ];
 
     Report {
+        title: "TYPE MISMATCH".to_string(),
         filename,
         text: Concat(lines),
     }
@@ -104,6 +105,7 @@ fn report_bad_type(
     ];
 
     Report {
+        title: "TYPE MISMATCH".to_string(),
         filename,
         text: Concat(lines),
     }
@@ -119,8 +121,8 @@ fn to_expr_report(
     use ReportText::*;
 
     match expected {
-        Expected::NoExpectation(_expected_type) => todo!(),
+        Expected::NoExpectation(expected_type) => todo!("hit no expectation with type {:?}", expected_type),
-        Expected::FromAnnotation(_name, _arity, _sub_context, _expected_type) => todo!(),
+        Expected::FromAnnotation(_name, _arity, _sub_context, _expected_type) => todo!("hit from annotation {:?} {:?}",_sub_context, _expected_type ),
         Expected::ForReason(reason, expected_type, region) => match reason {
             Reason::IfCondition => {
                 let problem = Concat(vec![
@@ -159,6 +161,36 @@ fn to_expr_report(
                     // they don't know. ("Wait, what's truthiness?")
                 )
             }
+            Reason::WhenGuard => {
+                let problem = Concat(vec![
+                    plain_text("This "),
+                    keyword_text("if"),
+                    plain_text(" guard condition needs to be a "),
+                    ReportText::Type(Content::Alias(Symbol::BOOL_BOOL, vec![], Variable::BOOL)),
+                    plain_text("."),
+                ]);
+                report_bad_type(
+                    filename,
+                    &category,
+                    found,
+                    expected_type,
+                    region,
+                    Some(expr_region),
+                    problem,
+                    plain_text("Right now it’s"),
+                    Concat(vec![
+                        plain_text("but I need every "),
+                        keyword_text("if"),
+                        plain_text(" guard condition to evaluate to a "),
+                        ReportText::Type(Content::Alias(Symbol::BOOL_BOOL, vec![], Variable::BOOL)),
+                        plain_text("—either "),
+                        global_tag_text("True"),
+                        plain_text(" or "),
+                        global_tag_text("False"),
+                        plain_text("."),
+                    ]),
+                )
+            }
             Reason::IfBranch {
                 index,
                 total_branches,
@@ -211,11 +243,96 @@ fn to_expr_report(
                         )),
                         plain_text(&format!("The {} branch is", ith)),
                         plain_text("but all the previous branches have the type"),
-                        plain_text("instead."),
+                        Concat(vec![
+                            plain_text("instead. I need all branches in an "),
+                            keyword_text("if"),
+                            plain_text(" to have the same type!"),
+                        ]),
                     )
                 }
             },
-            _ => todo!(),
+            Reason::WhenBranch { index } => {
+                // NOTE: is 0-based
+
+                let ith = int_to_ordinal(index + 1);
+
+                report_mismatch(
+                    filename,
+                    &category,
+                    found,
+                    expected_type,
+                    region,
+                    Some(expr_region),
+                    Concat(vec![
+                        plain_text(&format!("The {} branch of this ", ith)),
+                        keyword_text("when"),
+                        plain_text(" does not match all the previous branches"),
+                    ]),
+                    plain_text(&format!("The {} branch is", ith)),
+                    plain_text("but all the previous branches have type"),
+                    Concat(vec![
+                        plain_text("instead. I need all branches of a "),
+                        keyword_text("when"),
+                        plain_text(" to have the same type!"),
+                    ]),
+                )
+            }
+            Reason::ElemInList { index } => {
+                // NOTE: is 0-based
+
+                let ith = int_to_ordinal(index + 1);
+
+                report_mismatch(
+                    filename,
+                    &category,
+                    found,
+                    expected_type,
+                    region,
+                    Some(expr_region),
+                    plain_text(&format!(
+                        "The {} element of this list does not match all the previous elements",
+                        ith
+                    )),
+                    plain_text(&format!("The {} element is", ith)),
+                    plain_text("but all the previous elements in the list have type"),
+                    plain_text("instead. I need all elements of a list to have the same type!"),
+                )
+            }
+            Reason::RecordUpdateValue(field) => report_mismatch(
+                filename,
+                &category,
+                found,
+                expected_type,
+                region,
+                Some(expr_region),
+                Concat(vec![
+                    plain_text("I cannot update the "),
+                    record_field_text(field.as_str()),
+                    plain_text(" field like this"),
+                ]),
+                Concat(vec![
+                    plain_text("You are trying to update "),
+                    record_field_text(field.as_str()),
+                    plain_text(" to be"),
+                ]),
+                plain_text("But it should be"),
+                plain_text("instead. Record update syntax does not allow you to change the type of fields. You can achieve that with record literal syntax."),
+            ),
+            other => {
+                //    AnonymousFnArg { arg_index: u8 },
+                //    NamedFnArg(String /* function name */, u8 /* arg index */),
+                //    AnonymousFnCall { arity: u8 },
+                //    NamedFnCall(String /* function name */, u8 /* arity */),
+                //    BinOpArg(BinOp, ArgSide),
+                //    BinOpRet(BinOp),
+                //    FloatLiteral,
+                //    IntLiteral,
+                //    NumLiteral,
+                //    InterpolatedStringVar,
+                //    RecordUpdateValue(Lowercase),
+                //    RecordUpdateKeys(Symbol, SendMap<Lowercase, Type>),
+                todo!("I don't have a message yet for reason {:?}", other)
+            }
         },
     }
 }
@@ -300,10 +417,10 @@ fn add_category(this_is: ReportText, category: &Category) -> ReportText {
             plain_text("expression produces"),
         ]),
 
-        List => Concat(vec![this_is, plain_text("a list of type")]),
+        List => Concat(vec![this_is, plain_text(" a list of type")]),
-        Num => Concat(vec![this_is, plain_text("a number of type")]),
+        Num => Concat(vec![this_is, plain_text(" a number of type")]),
-        Int => Concat(vec![this_is, plain_text("an integer of type")]),
+        Int => Concat(vec![this_is, plain_text(" an integer of type")]),
-        Float => Concat(vec![this_is, plain_text("a float of type")]),
+        Float => Concat(vec![this_is, plain_text(" a float of type")]),
         Str => Concat(vec![this_is, plain_text(" a string of type")]),
 
         Lambda => Concat(vec![this_is, plain_text("an anonymous function of type")]),
@@ -319,7 +436,7 @@ fn add_category(this_is: ReportText, category: &Category) -> ReportText {
             plain_text(" private tag application produces"),
         ]),
 
-        Record => Concat(vec![this_is, plain_text("a record of type")]),
+        Record => Concat(vec![this_is, plain_text(" a record of type")]),
 
         Accessor(field) => Concat(vec![
             plain_text("This "),
@@ -378,6 +495,7 @@ fn to_circular_report(
     ];
 
     Report {
+        title: "TYPE MISMATCH".to_string(),
         filename,
         text: Concat(lines),
     }

compiler/reporting/tests/test_reporting.rs

@@ -37,6 +37,7 @@ mod test_reporting {
     // use roc_problem::can;
     fn to_simple_report(text: ReportText) -> Report {
         Report {
+            title: "SYNTAX PROBLEM".to_string(),
             text: text,
             filename: filename_from_string(r"\code\proj\Main.roc"),
         }
@@ -781,6 +782,33 @@ mod test_reporting {
         )
     }
 
+    #[test]
+    fn when_if_guard() {
+        report_problem_as(
+            indoc!(
+                r#"
+                when 1 is
+                    2 if 1 -> 0x0
+                    _ -> 0x1
+                "#
+            ),
+            indoc!(
+                r#"
+                This `if` guard condition needs to be a Bool.
+
+                2 ┆      2 if 1 -> 0x0
+                  ┆           ^
+
+                Right now it’s a number of type
+
+                    Num a
+
+                but I need every `if` guard condition to evaluate to a Bool—either `True` or `False`.
+                "#
+            ),
+        )
+    }
+
     #[test]
     fn if_2_branch_mismatch() {
         report_problem_as(
@@ -840,6 +868,160 @@ mod test_reporting {
     //     )
     // }
 
+    #[test]
+    fn when_branch_mismatch() {
+        report_problem_as(
+            indoc!(
+                r#"
+                when 1 is
+                    2 -> "foo"
+                    3 -> {}
+                "#
+            ),
+            indoc!(
+                r#"
+                The 2nd branch of this `when` does not match all the previous branches
+
+                3 ┆      3 -> {}
+                  ┆           ^^
+
+                The 2nd branch is a record of type
+
+                    {}
+
+                but all the previous branches have type
+
+                    Str
+
+                instead. I need all branches of a `when` to have the same type!
+                "#
+            ),
+        )
+    }
+
+    #[test]
+    fn elem_in_list() {
+        report_problem_as(
+            indoc!(
+                r#"
+                [ 1, 3, "foo" ]
+                "#
+            ),
+            indoc!(
+                r#"
+                The 3rd element of this list does not match all the previous elements
+
+                1 ┆  [ 1, 3, "foo" ]
+                  ┆          ^^^^^
+
+                The 3rd element is a string of type
+
+                    Str
+
+                but all the previous elements in the list have type
+
+                    Num a
+
+                instead. I need all elements of a list to have the same type!
+                "#
+            ),
+        )
+    }
+
+    #[test]
+    fn record_update_value() {
+        report_problem_as(
+            indoc!(
+                r#"
+                x : { foo : {} }
+                x = { foo: {} }
+
+                { x & foo: "bar" }
+                "#
+            ),
+            indoc!(
+                r#"
+                I cannot update the .foo field like this
+
+                4 ┆  { x & foo: "bar" }
+                  ┆  ^^^^^^^^^^^^^^^^^^
+
+                You are trying to update .foo to be a string of type
+
+                    Str
+
+                But it should be
+
+                    {}
+
+                instead. Record update syntax does not allow you to change the type of fields. You can achieve that with record literal syntax.
+                "#
+            ),
+        )
+    }
+
+    // needs a bit more infrastructure re. diffing records
+    //    #[test]
+    //    fn record_update_keys() {
+    //        report_problem_as(
+    //            indoc!(
+    //                r#"
+    //                x : { foo : {} }
+    //                x = { foo: {} }
+    //
+    //                { x & baz: "bar" }
+    //                "#
+    //            ),
+    //            indoc!(
+    //                r#"
+    //                The `x` record does not have a `baz` field
+    //
+    //                4 ┆  { x & baz: "bar" }
+    //                  ┆        ^^^
+    //
+    //                This is usually a typo. Here are the `x` fields that are most similar
+    //
+    //                    { foo : {}
+    //                    }
+    //
+    //                So maybe `baz` should be `foo`?
+    //                "#
+    //            ),
+    //        )
+    //    }
+
+    //    #[test]
+    //    fn num_literal() {
+    //        report_problem_as(
+    //            indoc!(
+    //                r#"
+    //                x : Str
+    //                x = 4
+    //
+    //                x
+    //                "#
+    //            ),
+    //            indoc!(
+    //                r#"
+    //                Something is off with the body of the `x` definition
+    //
+    //                4 ┆  x = 4
+    //                  ┆      ^
+    //
+    //                The body is a number of type
+    //
+    //                    Num a
+    //
+    //                But the type annotation on `x` says that it should be
+    //
+    //                    Str
+    //
+    //                instead.
+    //                "#
+    //            ),
+    //        )
+    //    }
+
     #[test]
     fn circular_type() {
         report_problem_as(

compiler/types/src/types.rs

@@ -628,7 +628,7 @@ pub enum Reason {
     WhenGuard,
     IfCondition,
     IfBranch { index: usize, total_branches: usize },
-    ElemInList,
+    ElemInList { index: usize },
     RecordUpdateValue(Lowercase),
     RecordUpdateKeys(Symbol, SendMap<Lowercase, Type>),
 }