Improve suggestions for inexhaustive case expression error

CHANGELOG.md

@@ -197,6 +197,36 @@
 
   ([Surya Rose](https://github.com/gearsdatapacks))
 
+  - The compiler now provides improved suggestions in the error for an
+    inexhaustive case expression. The following code:
+
+    ```gleam
+    let a = True
+    case a {}
+    ```
+
+    Now produces this error:
+
+    ```
+    error: Inexhaustive patterns
+      ┌─ /src/file.gleam:3:3
+      │
+    3 │   case a {}
+      │   ^^^^^^^^^
+
+    This case expression does not have a pattern for all possible values. If it
+    is run on one of the values without a pattern then it will crash.
+
+    The missing patterns are:
+
+        False
+        True
+    ```
+
+    Whereas before, it would suggest `_` as the only missing pattern.
+
+  ([Surya Rose](https://github.com/GearsDatapacks))
+
 ### Formatter
 
 ### Language Server

compiler-core/src/exhaustiveness.rs

@@ -240,6 +240,7 @@ pub struct Compiler<'a> {
     diagnostics: Diagnostics,
     patterns: Arena<Pattern>,
     environment: &'a Environment<'a>,
+    subject_variables: Vec<Variable>,
 }
 
 impl<'a> Compiler<'a> {
@@ -248,16 +249,23 @@ impl<'a> Compiler<'a> {
             environment,
             patterns,
             variable_id: 0,
+            subject_variables: Vec::new(),
             diagnostics: Diagnostics {
                 missing: false,
                 reachable: Vec::new(),
             },
         }
     }
 
+    pub fn subject_variable(&mut self, type_: Arc<Type>) -> Variable {
+        let variable = self.new_variable(type_);
+        self.subject_variables.push(variable.clone());
+        variable
+    }
+
     pub fn compile(mut self, rows: Vec<Row>) -> Match {
         Match {
-            tree: self.compile_rows(rows),
+            tree: self.check_empty_rows(rows),
             diagnostics: self.diagnostics,
         }
     }
@@ -289,6 +297,33 @@ impl<'a> Compiler<'a> {
         }
     }
 
+    fn check_empty_rows(&mut self, rows: Vec<Row>) -> Decision {
+        // If there are no rows, we get an immediate decision failure,
+        // which gives a pretty unhelpful error message. Instead, we
+        // run one pass of compiling to ensure we don't just suggest `_`
+        if rows.is_empty() {
+            // Even though we run a compile pass, an empty case expression is always
+            // invalid, so we make sure to report that here
+            self.diagnostics.missing = true;
+            let tree = self.compile_rows_for_variable(
+                self.subject_variables
+                    .first()
+                    .expect("Must have at least one subject")
+                    .clone(),
+                Vec::new(),
+            );
+            match tree {
+                // If there are no known constructors, we simply return failure since that
+                // better describes the state than an empty switch, and allows us to report
+                // `_` as the missing pattern.
+                Decision::Switch(_, cases, _) if cases.is_empty() => Decision::Failure,
+                _ => tree,
+            }
+        } else {
+            self.compile_rows(rows)
+        }
+    }
+
     fn compile_rows(&mut self, rows: Vec<Row>) -> Decision {
         if rows.is_empty() {
             self.diagnostics.missing = true;
@@ -314,7 +349,28 @@ impl<'a> Compiler<'a> {
             };
         }
 
-        match self.branch_mode(&rows) {
+        // Get the most referred to variable across all rows
+        let mut counts = HashMap::new();
+        for row in rows.iter() {
+            for col in &row.columns {
+                *counts.entry(col.variable.id).or_insert(0_usize) += 1
+            }
+        }
+
+        let variable = rows
+            .first()
+            .expect("Must have at least one row, otherwise we don't reach this point")
+            .columns
+            .iter()
+            .map(|col| col.variable.clone())
+            .max_by_key(|var| counts.get(&var.id).copied().unwrap_or(0))
+            .expect("The first row must have at least one column");
+
+        self.compile_rows_for_variable(variable, rows)
+    }
+
+    fn compile_rows_for_variable(&mut self, variable: Variable, rows: Vec<Row>) -> Decision {
+        match self.branch_mode(variable) {
             BranchMode::Infinite { variable } => {
                 let (cases, fallback) = self.compile_infinite_cases(rows, variable.clone());
                 Decision::Switch(variable, cases, Some(fallback))
@@ -694,31 +750,9 @@ impl<'a> Compiler<'a> {
         }
     }
 
-    /// Given a row, returns the kind of branch that is referred to the
-    /// most across all rows.
-    ///
-    /// # Panics
-    ///
-    /// Panics if there or no rows, or if the first row has no columns.
-    ///
-    fn branch_mode(&self, all_rows: &[Row]) -> BranchMode {
-        let mut counts = HashMap::new();
-
-        for row in all_rows {
-            for col in &row.columns {
-                *counts.entry(col.variable.id).or_insert(0_usize) += 1
-            }
-        }
-
-        let variable = all_rows
-            .first()
-            .expect("Must have at least one row")
-            .columns
-            .iter()
-            .map(|col| col.variable.clone())
-            .max_by_key(|var| counts.get(&var.id).copied().unwrap_or(0))
-            .expect("The first row must have at least one column");
-
+    /// Given a variable, returns the kind of branch associated with
+    /// that variable.
+    fn branch_mode(&self, variable: Variable) -> BranchMode {
         match collapse_links(variable.type_.clone()).as_ref() {
             Type::Fn { .. } | Type::Var { .. } => BranchMode::Infinite { variable },
 
@@ -763,7 +797,7 @@ impl<'a> Compiler<'a> {
     ///
     /// In a real compiler you'd have to ensure these variables don't conflict
     /// with other variables.
-    pub fn new_variable(&mut self, type_: Arc<Type>) -> Variable {
+    fn new_variable(&mut self, type_: Arc<Type>) -> Variable {
         let var = Variable {
             id: self.variable_id,
             type_,

compiler-core/src/type_/expression.rs

@@ -3332,7 +3332,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
         let mut compiler = Compiler::new(self.environment, Arena::new());
         let mut arena = PatternArena::new();
 
-        let subject_variable = compiler.new_variable(subject.clone());
+        let subject_variable = compiler.subject_variable(subject.clone());
 
         let mut rows = Vec::with_capacity(1);
 
@@ -3371,7 +3371,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
 
         let subject_variables = subject_types
             .iter()
-            .map(|t| compiler.new_variable(t.clone()))
+            .map(|t| compiler.subject_variable(t.clone()))
             .collect_vec();
 
         let mut rows = Vec::with_capacity(clauses.iter().map(Clause::pattern_count).sum::<usize>());

compiler-core/src/type_/tests/exhaustiveness.rs

@@ -1,4 +1,6 @@
-use crate::{assert_module_error, assert_no_warnings, assert_warning, assert_with_module_error};
+use crate::{
+    assert_error, assert_module_error, assert_no_warnings, assert_warning, assert_with_module_error,
+};
 
 #[test]
 fn whatever() {
@@ -1129,3 +1131,68 @@ pub fn main() {
 "
     );
 }
+
+// The following few tests all verify that the compiler provides useful errors
+// when there are no case arms, instead of just suggesting `_` as it did previously.
+#[test]
+fn empty_case_of_bool() {
+    assert_error!(
+        "
+let b = True
+case b {}
+"
+    );
+}
+
+#[test]
+fn empty_case_of_custom_type() {
+    assert_module_error!(
+        "
+type Wibble { Wibble Wobble Wubble }
+pub fn main() {
+  let wibble = Wobble
+  case wibble {}
+}
+"
+    );
+}
+
+#[test]
+fn empty_case_of_list() {
+    assert_error!(
+        "
+let list = []
+case list {}
+"
+    );
+}
+
+#[test]
+fn empty_case_of_int() {
+    assert_error!(
+        "
+let num = 24
+case num {}
+"
+    );
+}
+
+#[test]
+fn empty_case_of_float() {
+    assert_error!(
+        "
+let age = 10.6
+case age {}
+"
+    );
+}
+
+#[test]
+fn empty_case_of_string() {
+    assert_error!(
+        r#"
+let name = "John Doe"
+case name {}
+"#
+    );
+}

compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__exhaustiveness__empty_case_of_bool.snap

@@ -0,0 +1,17 @@
+---
+source: compiler-core/src/type_/tests/exhaustiveness.rs
+expression: "\nlet b = True\ncase b {}\n"
+---
+error: Inexhaustive patterns
+  ┌─ /src/one/two.gleam:3:1
+  │
+3 │ case b {}
+  │ ^^^^^^^^^
+
+This case expression does not have a pattern for all possible values. If it
+is run on one of the values without a pattern then it will crash.
+
+The missing patterns are:
+
+    False
+    True

compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__exhaustiveness__empty_case_of_custom_type.snap

@@ -0,0 +1,18 @@
+---
+source: compiler-core/src/type_/tests/exhaustiveness.rs
+expression: "\ntype Wibble { Wibble Wobble Wubble }\npub fn main() {\n  let wibble = Wobble\n  case wibble {}\n}\n"
+---
+error: Inexhaustive patterns
+  ┌─ /src/one/two.gleam:5:3
+  │
+5 │   case wibble {}
+  │   ^^^^^^^^^^^^^^
+
+This case expression does not have a pattern for all possible values. If it
+is run on one of the values without a pattern then it will crash.
+
+The missing patterns are:
+
+    Wibble
+    Wobble
+    Wubble

compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__exhaustiveness__empty_case_of_float.snap

@@ -0,0 +1,16 @@
+---
+source: compiler-core/src/type_/tests/exhaustiveness.rs
+expression: "\nlet age = 10.6\ncase age {}\n"
+---
+error: Inexhaustive patterns
+  ┌─ /src/one/two.gleam:3:1
+  │
+3 │ case age {}
+  │ ^^^^^^^^^^^
+
+This case expression does not have a pattern for all possible values. If it
+is run on one of the values without a pattern then it will crash.
+
+The missing patterns are:
+
+    _

compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__exhaustiveness__empty_case_of_int.snap

@@ -0,0 +1,16 @@
+---
+source: compiler-core/src/type_/tests/exhaustiveness.rs
+expression: "\nlet num = 24\ncase num {}\n"
+---
+error: Inexhaustive patterns
+  ┌─ /src/one/two.gleam:3:1
+  │
+3 │ case num {}
+  │ ^^^^^^^^^^^
+
+This case expression does not have a pattern for all possible values. If it
+is run on one of the values without a pattern then it will crash.
+
+The missing patterns are:
+
+    _

compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__exhaustiveness__empty_case_of_list.snap

@@ -0,0 +1,17 @@
+---
+source: compiler-core/src/type_/tests/exhaustiveness.rs
+expression: "\nlet list = []\ncase list {}\n"
+---
+error: Inexhaustive patterns
+  ┌─ /src/one/two.gleam:3:1
+  │
+3 │ case list {}
+  │ ^^^^^^^^^^^^
+
+This case expression does not have a pattern for all possible values. If it
+is run on one of the values without a pattern then it will crash.
+
+The missing patterns are:
+
+    []
+    [_, ..]

compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__exhaustiveness__empty_case_of_string.snap

@@ -0,0 +1,16 @@
+---
+source: compiler-core/src/type_/tests/exhaustiveness.rs
+expression: "\nlet name = \"John Doe\"\ncase name {}\n"
+---
+error: Inexhaustive patterns
+  ┌─ /src/one/two.gleam:3:1
+  │
+3 │ case name {}
+  │ ^^^^^^^^^^^^
+
+This case expression does not have a pattern for all possible values. If it
+is run on one of the values without a pattern then it will crash.
+
+The missing patterns are:
+
+    _