Improved collapsing nested cases with list tails

Author: matiascr

compiler-core/src/ast.rs

@@ -2661,6 +2661,10 @@ impl SrcSpan {
             end: self.end.max(with.end),
         }
     }
+
+    pub fn len(&self) -> usize {
+        (self.end - self.start) as usize
+    }
 }
 
 #[derive(Debug, PartialEq, Eq, Clone)]
@@ -3165,7 +3169,7 @@ pub enum BoundVariableName {
     ListTail {
         name: EcoString,
         /// The location of the whole tail, from the `..` prefix until the end of the variable.
-        tail_prefix_location: SrcSpan,
+        tail_location: SrcSpan,
     },
     /// Any other variable name.
     Regular { name: EcoString },
@@ -3412,7 +3416,7 @@ impl TypedPattern {
                     variables.push(BoundVariable {
                         name: BoundVariableName::ListTail {
                             name,
-                            tail_prefix_location: tail.location,
+                            tail_location: tail.location,
                         },
                         location,
                         type_: type_.clone(),

compiler-core/src/language_server/code_action.rs

@@ -28,7 +28,6 @@ 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::{
@@ -4801,8 +4800,7 @@ impl<'a, IO> PatternMatchOnValue<'a, IO> {
         };
 
         let variable_start = (variable_location.start - clause_location.start) as usize;
-        let variable_end =
-            variable_start + (variable_location.end - variable_location.start) as usize;
+        let variable_end = variable_start + variable_location.len();
 
         let clause_code = code_at(self.module, clause_location);
         let patterns = patterns
@@ -8273,34 +8271,43 @@ impl<'a> CollapseNestedCase<'a> {
                     name: BoundVariableName::Regular { .. } | BoundVariableName::ListTail { .. },
                     ..
                 } => {
-                    // If the variable is a regular variable we'll have to replace
-                    // it entirely with the new pattern taking its place.
+                    let trimmed_contents = new_content.trim();
+
+                    let pattern_is_literal_list =
+                        trimmed_contents.starts_with("[") && trimmed_contents.ends_with("]");
+                    let pattern_is_discard = trimmed_contents == "_";
+
+                    let span_to_replace = match (
+                        &matched_variable.name,
+                        // We verify whether the pattern is compatible with the list prefix `..`.
+                        // For example, `..var` is valid syntax, but `..[]` and `.._` are not.
+                        pattern_is_literal_list || pattern_is_discard,
+                    ) {
+                        // We normally replace the selected variable with the pattern.
+                        (BoundVariableName::Regular { .. }, _) => matched_variable_span,
+
+                        // If the selected pattern is not a list, we also replace it normally.
+                        (BoundVariableName::ListTail { .. }, false) => matched_variable_span,
+                        // If the pattern is a list to also remove the list tail prefix.
+                        (BoundVariableName::ListTail { tail_location, .. }, true) => {
+                            // When it's a list literal, we remove the surrounding brackets.
+                            let len = trimmed_contents.len();
+                            if let Some(slice) = new_content.trim().get(1..(len - 1)) {
+                                new_content = slice.to_string()
+                            };
 
-                    let variable_span_to_use = if let BoundVariableName::ListTail {
-                        tail_prefix_location: prefix_and_variable_span,
-                        ..
-                    } = matched_variable.name
-                    {
-                        // 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
+                            *tail_location
+                        }
+
+                        (BoundVariableName::ShorthandLabel { .. }, _) => unreachable!(),
                     };
 
-                    let variable_start_in_pattern =
-                        (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 start_of_pattern =
+                        (span_to_replace.start - matched_pattern_span.start) as usize;
+                    let pattern_length = span_to_replace.len();
 
-                    let variable_end_in_pattern = variable_start_in_pattern + variable_length;
-                    let replaced_range = variable_start_in_pattern..variable_end_in_pattern;
+                    let end_of_pattern = start_of_pattern + pattern_length;
+                    let replaced_range = start_of_pattern..end_of_pattern;
 
                     new_pattern.replace_range(replaced_range, &new_content);
                 }

compiler-core/src/language_server/engine.rs

@@ -1469,7 +1469,7 @@ fn code_action_unused_values(
     }
 
     // Sort spans by start position, with longer spans coming first
-    unused_values.sort_by_key(|span| (span.start, -(span.end as i64 - span.start as i64)));
+    unused_values.sort_by_key(|span| (span.start, -(span.len() as i64)));
 
     let mut processed_lsp_range = Vec::new();
 

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

@@ -9981,7 +9981,10 @@ fn collapse_nested_case_combines_list_with_tail() {
         [[1]] -> 1
         [_, [3, 4]] -> 3
         [_, _, [5, 6, 7], _] -> 5
-        _ -> -1
+        tail_list -> {
+            echo tail_list
+            -1
+        }
       }
     _ -> -1
   }

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

@@ -1,6 +1,6 @@
 ---
 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}"
+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        tail_list -> {\n            echo tail_list\n            -1\n        }\n      }\n    _ -> -1\n  }\n}"
 ---
 ----- BEFORE ACTION
 pub fn main(elems: List(List(Int))) {
@@ -13,7 +13,10 @@ pub fn main(elems: List(List(Int))) {
         [[1]] -> 1
         [_, [3, 4]] -> 3
         [_, _, [5, 6, 7], _] -> 5
-        _ -> -1
+        tail_list -> {
+            echo tail_list
+            -1
+        }
       }
     _ -> -1
   }
@@ -28,7 +31,10 @@ pub fn main(elems: List(List(Int))) {
     [_, [1]] -> 1
     [_, _, [3, 4]] -> 3
     [_, _, _, [5, 6, 7], _] -> 5
-    [_, _] -> -1
+    [_, ..tail_list] -> {
+            echo tail_list
+            -1
+        }
     _ -> -1
   }
 }