Repository: gleam-lang/gleam (rust)

Issue:
Improve suggestions for inexhaustive case expression error
<!--
Gleam is post-v1, so breaking changes are not possible.

Any new addition to the language itself needs to enable something that is not possible in Gleam today, and solve existing pain-points.

For anything other than trivial and uncontroversial additions please make a GitHub discussion before making an issue. https://github.com/gleam-lang/gleam/discussions/categories/ideas-suggestions
-->

Currently, for this code:
```gleam
case True {}
```

The compiler produces this error:

```
error: Inexhaustive patterns
  ┌─ FILE
  │  
5 │ ╭   case True {
6 │ │     
7 │ │   }
  │ ╰───^

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:

    _
```

This is not super helpful; it would be much better to suggest `True` and `False` as missing patterns instead. Also, #2957 cannot be implemented in a particularly useful manner until this is improved. (See [here](https://github.com/gleam-lang/gleam/pull/3537#discussion_r1723230130))

Edit: There was some confusion about the above code. The matching on the literal value `True`, has nothing to do with this. It's merely to demonstrate this error. 

Pull request:
Improve suggestions for inexhaustive case expression error
Closes #3544.
Instead of just returning `Failure` when there are no rows, it runs exhaustiveness checking on the first subject of the case expression to give better suggestions to improve it. 
In future we should maybe check other subjects too, to further improve this. Although this could produce a big decision tree, and we don't do that at the moment for other patterns so I left it.
For example, at the moment, for this code:
```gleam
case True, False {}
```
It will suggest `True, _` and `False, _`. (Actually it won't, because of #2426, but that's a separate issue)

However, if we check both subjects, it will suggest all four possible combinations.
However, consider this possibility:
```
type Wibble { A B C D }
case A, B, C, D {}
```
This would produce `4^4 = 256` possible options, which is a bit much. So I've left it with just the one for now.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 55e982da7..99ad3d284 100644
--- a/CHANGELOG.md
+++ b/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
diff --git a/compiler-core/src/exhaustiveness.rs b/compiler-core/src/exhaustiveness.rs
index 3cd18baa4..142b09ca8 100644
--- a/compiler-core/src/exhaustiveness.rs
+++ b/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,6 +249,7 @@ impl<'a> Compiler<'a> {
             environment,
             patterns,
             variable_id: 0,
+            subject_variables: Vec::new(),
             diagnostics: Diagnostics {
                 missing: false,
                 reachable: Vec::new(),
@@ -255,9 +257,15 @@ impl<'a> Compiler<'a> {
         }
     }
 
+    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_,
diff --git a/compiler-core/src/type_/expression.rs b/compiler-core/src/type_/expression.rs
index 645d67340..cd1054bf7 100644
--- a/compiler-core/src/type_/expression.rs
+++ b/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>());


Test patch:
diff --git a/compiler-core/src/type_/tests/exhaustiveness.rs b/compiler-core/src/type_/tests/exhaustiveness.rs
index 793c652a4..827306129 100644
--- a/compiler-core/src/type_/tests/exhaustiveness.rs
+++ b/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 {}
+"#
+    );
+}
diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__exhaustiveness__empty_case_of_bool.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__exhaustiveness__empty_case_of_bool.snap
new file mode 100644
index 000000000..15183ed2b
--- /dev/null
+++ b/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
diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__exhaustiveness__empty_case_of_custom_type.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__exhaustiveness__empty_case_of_custom_type.snap
new file mode 100644
index 000000000..a71f265c3
--- /dev/null
+++ b/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
diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__exhaustiveness__empty_case_of_float.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__exhaustiveness__empty_case_of_float.snap
new file mode 100644
index 000000000..a213b0c78
--- /dev/null
+++ b/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:
+
+    _
diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__exhaustiveness__empty_case_of_int.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__exhaustiveness__empty_case_of_int.snap
new file mode 100644
index 000000000..73978c53a
--- /dev/null
+++ b/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:
+
+    _
diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__exhaustiveness__empty_case_of_list.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__exhaustiveness__empty_case_of_list.snap
new file mode 100644
index 000000000..ef7a5c139
--- /dev/null
+++ b/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:
+
+    []
+    [_, ..]
diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__exhaustiveness__empty_case_of_string.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__exhaustiveness__empty_case_of_string.snap
new file mode 100644
index 000000000..2e7aad5b2
--- /dev/null
+++ b/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:
+
+    _