Improve non-exhaustive typechecking errors

We will replace this with proper exhaustiveness checking when we have
that in the typechecking phase, but for now this should do.

compiler/constrain/src/expr.rs

@@ -708,7 +708,9 @@ pub fn constrain_expr(
 
             // After solving the condition variable with what's expected from the branch patterns,
             // check it against the condition expression.
-            // This is basically exhaustiveness checking, but doesn't check for redundancy.
+            // TODO: when we have exhaustiveness checking during the typechecking phase, perform
+            // exhaustiveness checking when this expectation fails. That will produce better error
+            // messages.
             let cond_constraint = constrain_expr(
                 constraints,
                 env,

reporting/src/error/type.rs

@@ -1151,34 +1151,50 @@ fn to_expr_report<'b>(
                 )
             }
 
-            Reason::WhenBranches => report_mismatch(
+            Reason::WhenBranches => {
-                alloc,
+                let snippet = alloc.region_with_subregion(
-                lines,
+                    lines.convert_region(region),
-                filename,
+                    lines.convert_region(expr_region),
-                &category,
+                );
-                found,
+
-                expected_type,
+                let this_is = alloc.concat([
-                // TODO: these should be flipped but `report_mismatch` takes the wrong arguments to
-                // `region_with_subregion`
-                expr_region,
-                Some(region),
-                alloc.concat([
-                    alloc.reflow("The branches of this "),
-                    alloc.keyword("when"),
-                    alloc.reflow(" expression don't match the condition:"),
-                ]),
-                alloc.concat([
                     alloc.reflow("The "),
                     alloc.keyword("when"),
                     alloc.reflow(" condition is"),
-                ]),
+                ]);
-                alloc.reflow("But the branch patterns have type:"),
+
-                Some(alloc.concat([
+                let wanted = alloc.reflow("But the branch patterns have type:");
+                let details = Some(alloc.concat([
                     alloc.reflow("The branches must be cases of the "),
                     alloc.keyword("when"),
                     alloc.reflow(" condition's type!"),
-                ])),
+                ]));
-            ),
+
+                let lines = [
+                    alloc.concat([
+                        alloc.reflow("The branches of this "),
+                        alloc.keyword("when"),
+                        alloc.reflow(" expression don't match the condition:"),
+                    ]),
+                    snippet,
+                    type_comparison(
+                        alloc,
+                        found,
+                        expected_type,
+                        ExpectationContext::WhenCondition,
+                        add_category(alloc, this_is, &category),
+                        wanted,
+                        details,
+                    ),
+                ];
+
+                Report {
+                    title: "TYPE MISMATCH".to_string(),
+                    filename,
+                    doc: alloc.stack(lines),
+                    severity: Severity::RuntimeError,
+                }
+            }
 
             Reason::LowLevelOpArg { op, arg_index } => {
                 panic!(
@@ -1253,7 +1269,10 @@ fn count_arguments(tipe: &ErrorType) -> usize {
 enum ExpectationContext<'a> {
     /// An expected type was discovered from a type annotation. Corresponds to
     /// [`Expected::FromAnnotation`](Expected::FromAnnotation).
-    Annotation { on: RocDocBuilder<'a> },
+    Annotation {
+        on: RocDocBuilder<'a>,
+    },
+    WhenCondition,
     /// When we don't know the context, or it's not relevant.
     Arbitrary,
 }
@@ -1262,6 +1281,7 @@ impl<'a> std::fmt::Debug for ExpectationContext<'a> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
             ExpectationContext::Annotation { .. } => f.write_str("Annotation"),
+            ExpectationContext::WhenCondition => f.write_str("WhenCondition"),
             ExpectationContext::Arbitrary => f.write_str("Arbitrary"),
         }
     }
@@ -2679,22 +2699,24 @@ fn diff_tag_union<'b>(
     let all_fields_shared = left.peek().is_none() && right.peek().is_none();
 
     let status = match (ext_has_fixed_fields(&ext1), ext_has_fixed_fields(&ext2)) {
-        (true, true) => match left.peek() {
+        (true, true) => match (left.peek(), right.peek()) {
-            Some((f, _, _, _)) => Status::Different(vec![Problem::TagTypo(
+            (Some((f, _, _, _)), Some(_)) => Status::Different(vec![Problem::TagTypo(
                 f.clone(),
                 fields2.keys().cloned().collect(),
             )]),
-            None => {
+            (Some(_), None) => {
-                if right.peek().is_none() {
+                let status =
-                    Status::Similar
+                    Status::Different(vec![Problem::TagsMissing(left.map(|v| v.0).collect())]);
-                } else {
+                left = left_keys.iter().map(to_unknown_docs).peekable();
-                    let result =
+                status
-                        Status::Different(vec![Problem::TagsMissing(right.map(|v| v.0).collect())]);
-                    // we just used the values in `right`.  in
-                    right = right_keys.iter().map(to_unknown_docs).peekable();
-                    result
-                }
             }
+            (None, Some(_)) => {
+                let status =
+                    Status::Different(vec![Problem::TagsMissing(right.map(|v| v.0).collect())]);
+                right = right_keys.iter().map(to_unknown_docs).peekable();
+                status
+            }
+            (None, None) => Status::Similar,
         },
         (false, true) => match left.peek() {
             Some((f, _, _, _)) => Status::Different(vec![Problem::TagTypo(
@@ -3294,6 +3316,33 @@ fn type_problem_to_pretty<'b>(
             alloc.reflow("."),
         ])),
 
+        (TagsMissing(missing), ExpectationContext::WhenCondition) => match missing.split_last() {
+            None => alloc.nil(),
+            Some(split) => {
+                let missing_tags = match split {
+                    (f1, []) => alloc.tag_name(f1.clone()).append(alloc.reflow(" tag.")),
+                    (last, init) => alloc
+                        .intersperse(init.iter().map(|v| alloc.tag_name(v.clone())), ", ")
+                        .append(alloc.reflow(" and "))
+                        .append(alloc.tag_name(last.clone()))
+                        .append(alloc.reflow(" tags.")),
+                };
+
+                let tip1 = alloc
+                    .tip()
+                    .append(alloc.reflow("Looks like the branches are missing coverage of the "))
+                    .append(missing_tags);
+
+                let tip2 = alloc
+                    .tip()
+                    .append(alloc.reflow("Maybe you need to add a catch-all branch, like "))
+                    .append(alloc.keyword("_"))
+                    .append(alloc.reflow("?"));
+
+                alloc.stack([tip1, tip2])
+            }
+        },
+
         (TagsMissing(missing), _) => match missing.split_last() {
             None => alloc.nil(),
             Some((f1, [])) => {

reporting/tests/test_reporting.rs

@@ -2740,11 +2740,10 @@ mod test_reporting {
 
                     [ Left a ]
 
-                Tip: Seems like a tag typo. Maybe `Right` should be `Left`?
+                Tip: Looks like a closed tag union does not have the `Right` tag.
 
-                Tip: Can more type annotations be added? Type annotations always help
+                Tip: Closed tag unions can't grow, because that might change the size
-                me give more specific messages, and I think they could help a lot in
+                in memory. Can you use an open tag union?
-                this case
                 "#
             ),
         )
@@ -2790,7 +2789,6 @@ mod test_reporting {
                     Red -> 3
                 "#
             ),
-            // TODO(2903): improve tag typo quality
             indoc!(
                 r#"
                 ── TYPE MISMATCH ───────────────────────────────────────── /code/proj/Main.roc ─
@@ -2810,11 +2808,9 @@ mod test_reporting {
 
                 The branches must be cases of the `when` condition's type!
 
-                Tip: Seems like a tag typo. Maybe `Green` should be `Red`?
+                Tip: Looks like the branches are missing coverage of the `Green` tag.
 
-                Tip: Can more type annotations be added? Type annotations always help
+                Tip: Maybe you need to add a catch-all branch, like `_`?
-                me give more specific messages, and I think they could help a lot in
-                this case
                 "#
             ),
         )
@@ -2833,7 +2829,6 @@ mod test_reporting {
                     Green -> 1
                 "#
             ),
-            // TODO(2903): improve tag typo quality
             indoc!(
                 r#"
                 ── TYPE MISMATCH ───────────────────────────────────────── /code/proj/Main.roc ─
@@ -2854,11 +2849,9 @@ mod test_reporting {
 
                 The branches must be cases of the `when` condition's type!
 
-                Tip: Seems like a tag typo. Maybe `Blue` should be `Red`?
+                Tip: Looks like the branches are missing coverage of the `Blue` tag.
 
-                Tip: Can more type annotations be added? Type annotations always help
+                Tip: Maybe you need to add a catch-all branch, like `_`?
-                me give more specific messages, and I think they could help a lot in
-                this case
                 "#
             ),
         )
@@ -2877,7 +2870,6 @@ mod test_reporting {
                     NotAsked -> 3
                 "#
             ),
-            // TODO(2903): improve tag typo quality
             indoc!(
                 r#"
                 ── TYPE MISMATCH ───────────────────────────────────────── /code/proj/Main.roc ─
@@ -2897,11 +2889,10 @@ mod test_reporting {
 
                 The branches must be cases of the `when` condition's type!
 
-                Tip: Seems like a tag typo. Maybe `Success` should be `NotAsked`?
+                Tip: Looks like the branches are missing coverage of the
+                `Success`, `Failure` and `Loading` tags.
 
-                Tip: Can more type annotations be added? Type annotations always help
+                Tip: Maybe you need to add a catch-all branch, like `_`?
-                me give more specific messages, and I think they could help a lot in
-                this case
                 "#
             ),
         )
@@ -8714,7 +8705,6 @@ I need all branches in an `if` to have the same type!
                     $F B -> ""
                 "#
             ),
-            // TODO(2903): improve tag typo quality
             indoc!(
                 r#"
                 ── TYPE MISMATCH ───────────────────────────────────────── /code/proj/Main.roc ─
@@ -8735,11 +8725,9 @@ I need all branches in an `if` to have the same type!
 
                 The branches must be cases of the `when` condition's type!
 
-                Tip: Seems like a tag typo. Maybe `C` should be `A`?
+                Tip: Looks like the branches are missing coverage of the `C` tag.
 
-                Tip: Can more type annotations be added? Type annotations always help
+                Tip: Maybe you need to add a catch-all branch, like `_`?
-                me give more specific messages, and I think they could help a lot in
-                this case
                 "#
             ),
         )