Update with fixes to collapsing nested cases with lists

Author: matiascr

CHANGELOG.md

@@ -341,3 +341,7 @@
 - Fixed a bug where the compiler would generate invalid Erlang and TypeScript
   code for unused opaque types referencing private types.
   ([Surya Rose](https://github.com/GearsDatapacks))
+
+- Fixed a bug where the collapse nested case would produce invalid code on a
+  list tail pattern.
+  ([Matias Carlander](https://github.com/matiascr))

compiler-core/src/ast.rs

@@ -3155,22 +3155,30 @@ impl TypedPattern {
 pub enum BoundVariable {
     /// A record's labelled field introduced with the shorthand syntax.
     ShorthandLabel { name: EcoString, location: SrcSpan },
+    /// The variable as the tail of a list.
+    ListTail {
+        name: EcoString,
+        location: SrcSpan,
+        /// The location of the whole tail, from the `..` prefix until the end of the variable.
+        location_including_prefix: SrcSpan,
+    },
     /// Any other variable.
     Regular { name: EcoString, location: SrcSpan },
 }
 
 impl BoundVariable {
     pub fn name(&self) -> EcoString {
         match self {
-            BoundVariable::ShorthandLabel { name, .. } | BoundVariable::Regular { name, .. } => {
-                name.clone()
-            }
+            BoundVariable::ShorthandLabel { name, .. }
+            | BoundVariable::ListTail { name, .. }
+            | BoundVariable::Regular { name, .. } => name.clone(),
         }
     }
 
     pub fn location(&self) -> SrcSpan {
         match self {
             BoundVariable::ShorthandLabel { location, .. }
+            | BoundVariable::ListTail { location, .. }
             | BoundVariable::Regular { location, .. } => *location,
         }
     }
@@ -3389,9 +3397,15 @@ impl TypedPattern {
                 for element in elements {
                     element.collect_bound_variables(variables);
                 }
-                if let Some(tail) = tail {
-                    tail.pattern.collect_bound_variables(variables);
-                }
+                if let Some(tail) = tail
+                    && let Pattern::Variable { name, location, .. } = tail.pattern.to_owned()
+                {
+                    variables.push(BoundVariable::ListTail {
+                        name,
+                        location,
+                        location_including_prefix: tail.location,
+                    })
+                };
             }
             Pattern::Constructor { arguments, .. } => {
                 for argument in arguments {

compiler-core/src/language_server/code_action.rs

@@ -27,6 +27,7 @@ use ecow::{EcoString, eco_format};
 use im::HashMap;
 use itertools::Itertools;
 use lsp_types::{CodeAction, CodeActionKind, CodeActionParams, Position, Range, TextEdit, Url};
+use regex::Regex;
 use vec1::{Vec1, vec1};
 
 use super::{
@@ -8122,19 +8123,39 @@ impl<'a> CollapseNestedCase<'a> {
         let pattern_text: String = code_at(self.module, matched_pattern_span).into();
         let matched_variable_span = matched_variable.location();
 
-        let pattern_with_variable = |new_content: String| {
+        let pattern_with_variable = |mut new_content: String| {
             let mut new_pattern = pattern_text.clone();
 
             match matched_variable {
-                BoundVariable::Regular { .. } => {
+                BoundVariable::Regular { .. } | BoundVariable::ListTail { .. } => {
                     // If the variable is a regular variable we'll have to replace
                     // it entirely with the new pattern taking its place.
+
+                    let variable_span_to_use = if let BoundVariable::ListTail {
+                        location_including_prefix: prefix_and_variable_span,
+                        ..
+                    } = matched_variable
+                    {
+                        // We need to un-nest the list when replacing the tail
+                        let surrounding_brackets = Regex::new(r"(^\[|\]$)").expect("a valid regex");
+                        new_content = surrounding_brackets
+                            .replace_all(new_content.trim(), "")
+                            .to_string();
+
+                        // We need to also remove the prefix when replacing the variable
+                        // at the tail of the list
+                        prefix_and_variable_span
+                    } else {
+                        &matched_variable_span
+                    };
+
                     let variable_start_in_pattern =
-                        matched_variable_span.start - matched_pattern_span.start;
-                    let variable_length = matched_variable_span.end - matched_variable_span.start;
+                        (variable_span_to_use.start - matched_pattern_span.start) as usize;
+                    let variable_length =
+                        (variable_span_to_use.end - variable_span_to_use.start) as usize;
+
                     let variable_end_in_pattern = variable_start_in_pattern + variable_length;
-                    let replaced_range =
-                        variable_start_in_pattern as usize..variable_end_in_pattern as usize;
+                    let replaced_range = variable_start_in_pattern..variable_end_in_pattern;
 
                     new_pattern.replace_range(replaced_range, &new_content);
                 }

compiler-core/src/language_server/tests/action.rs

@@ -9922,6 +9922,49 @@ fn collapse_nested_case_combines_inner_and_outer_guards_and_adds_parentheses_whe
     );
 }
 
+#[test]
+fn collapse_nested_case_combines_list_with_unformatted_tail() {
+    assert_code_action!(
+        COLLAPSE_NESTED_CASE,
+        r#"pub fn main(elems: List(String)) {
+  case elems {
+    [first, .. rest_of_list] ->
+      case rest_of_list {
+        [] -> 0
+        ["first"] -> 1
+        [_, "second"] -> 2
+        [_, "second", "third", _] -> 4
+        _ -> -1
+      }
+    _ -> -1
+  }
+}"#,
+        find_position_of("rest").to_selection()
+    );
+}
+
+#[test]
+fn collapse_nested_case_combines_list_with_tail() {
+    assert_code_action!(
+        COLLAPSE_NESTED_CASE,
+        r#"pub fn main(elems: List(List(Int))) {
+  case elems {
+    [] -> 0
+    [_, ..tail] ->
+      case tail {
+        [] -> -1
+        [[1]] -> 1
+        [_, [3, 4]] -> 3
+        [_, _, [5, 6, 7], _] -> 5
+        _ -> -1
+      }
+    _ -> -1
+  }
+}"#,
+        find_position_of("tail").to_selection()
+    );
+}
+
 // https://github.com/gleam-lang/gleam/issues/3786
 #[test]
 fn type_variables_from_other_functions_do_not_change_annotations() {

compiler-core/src/language_server/tests/snapshots/gleam_core__language_server__tests__action__collapse_nested_case_combines_inner_and_lists.snap

@@ -0,0 +1,34 @@
+---
+source: compiler-core/src/language_server/tests/action.rs
+expression: "pub fn main(elems: List(List(Int))) {\n  case elems {\n    [] -> 0\n    [_, ..tail] ->\n      case tail {\n        [] -> -1\n        [[1]] -> 1\n        [_, [3, 4]] -> 3\n        [_, _, [5, 6, 7], _] -> 5\n        _ -> -1\n      }\n    _ -> -1\n  }\n}"
+---
+----- BEFORE ACTION
+pub fn main(elems: List(List(Int))) {
+  case elems {
+    [] -> 0
+    [_, ..tail] ->
+          ↑       
+      case tail {
+        [] -> -1
+        [[1]] -> 1
+        [_, [3, 4]] -> 3
+        [_, _, [5, 6, 7], _] -> 5
+        _ -> -1
+      }
+    _ -> -1
+  }
+}
+
+
+----- AFTER ACTION
+pub fn main(elems: List(List(Int))) {
+  case elems {
+    [] -> 0
+    [_, ] -> -1
+    [_, [1]] -> 1
+    [_, _, [3, 4]] -> 3
+    [_, _, _, [5, 6, 7], _] -> 5
+    [_, _] -> -1
+    _ -> -1
+  }
+}

compiler-core/src/language_server/tests/snapshots/gleam_core__language_server__tests__action__collapse_nested_case_combines_list_with_tail.snap

@@ -0,0 +1,32 @@
+---
+source: compiler-core/src/language_server/tests/action.rs
+expression: "pub fn main(elems: List(String)) {\n  case elems {\n    [first, .. rest_of_list] ->\n      case rest_of_list {\n        [] -> 0\n        [\"first\"] -> 1\n        [_, \"second\"] -> 2\n        [_, \"second\", \"third\", _] -> 4\n        _ -> -1\n      }\n    _ -> -1\n  }\n}"
+---
+----- BEFORE ACTION
+pub fn main(elems: List(String)) {
+  case elems {
+    [first, .. rest_of_list] ->
+               ↑               
+      case rest_of_list {
+        [] -> 0
+        ["first"] -> 1
+        [_, "second"] -> 2
+        [_, "second", "third", _] -> 4
+        _ -> -1
+      }
+    _ -> -1
+  }
+}
+
+
+----- AFTER ACTION
+pub fn main(elems: List(String)) {
+  case elems {
+    [first, ] -> 0
+    [first, "first"] -> 1
+    [first, _, "second"] -> 2
+    [first, _, "second", "third", _] -> 4
+    [first, _] -> -1
+    _ -> -1
+  }
+}