diff --git a/compiler/can/src/expr.rs b/compiler/can/src/expr.rs index c69eff20f0..81a9e4d747 100644 --- a/compiler/can/src/expr.rs +++ b/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) diff --git a/compiler/constrain/src/expr.rs b/compiler/constrain/src/expr.rs index cbce2021ce..79a8e6c3be 100644 --- a/compiler/constrain/src/expr.rs +++ b/compiler/constrain/src/expr.rs @@ -181,9 +181,10 @@ pub fn constrain_expr( region, ); - cons.push(con); - cons.push(fields_con); - cons.push(record_con); + // ensure constraints are solved in this order, gives better errors + cons.insert(0, fields_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 { - let elem_expected = - ForReason(Reason::ElemInList, list_elem_type.clone(), region); + for (index, loc_elem) in loc_elems.iter().enumerate() { + let elem_expected = ForReason( + 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); diff --git a/compiler/constrain/src/uniq.rs b/compiler/constrain/src/uniq.rs index 38bdf328ab..c91d9e8673 100644 --- a/compiler/constrain/src/uniq.rs +++ b/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() { - let elem_expected = - Expected::ForReason(Reason::ElemInList, entry_type.clone(), region); + for (index, loc_elem) in loc_elems.iter().enumerate() { + let elem_expected = Expected::ForReason( + Reason::ElemInList { index }, + entry_type.clone(), + region, + ); let constraint = constrain_expr( env, var_store, diff --git a/compiler/reporting/src/report.rs b/compiler/reporting/src/report.rs index 7f38c7edf3..da7a1ffcb5 100644 --- a/compiler/reporting/src/report.rs +++ b/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(()) diff --git a/compiler/reporting/src/type_error.rs b/compiler/reporting/src/type_error.rs index 8675e6e9a5..fc1e30fa87 100644 --- a/compiler/reporting/src/type_error.rs +++ b/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::FromAnnotation(_name, _arity, _sub_context, _expected_type) => todo!(), + Expected::NoExpectation(expected_type) => todo!("hit no expectation with type {:?}", expected_type), + 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), + 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")]), - Num => Concat(vec![this_is, plain_text("a number of type")]), - Int => Concat(vec![this_is, plain_text("an integer of type")]), - Float => Concat(vec![this_is, plain_text("a float of type")]), + List => Concat(vec![this_is, plain_text(" a list of type")]), + Num => Concat(vec![this_is, plain_text(" a number of type")]), + Int => Concat(vec![this_is, plain_text(" an integer 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), } diff --git a/compiler/reporting/tests/test_reporting.rs b/compiler/reporting/tests/test_reporting.rs index ba785aa362..8095efaa52 100644 --- a/compiler/reporting/tests/test_reporting.rs +++ b/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( diff --git a/compiler/types/src/types.rs b/compiler/types/src/types.rs index 3ee5bad08c..3c6a892d94 100644 --- a/compiler/types/src/types.rs +++ b/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), }