Fix multiline handling

fixtures/small/multiline_aryptn_actual.rb

@@ -0,0 +1,8 @@
+case []
+in [first,
+  *,
+  last]
+  puts "bees!!!"
+else
+  "no bees"
+end

fixtures/small/multiline_aryptn_expected.rb

@@ -0,0 +1,10 @@
+case []
+in [
+    first,
+    *,
+    last
+  ]
+  puts("bees!!!")
+else
+  "no bees"
+end

fixtures/small/multiline_fndptn_actual.rb

@@ -0,0 +1,7 @@
+case []
+in *, "bees are present",
+  *; then
+  puts "bees!!!"
+else
+  "no bees"
+end

fixtures/small/multiline_fndptn_expected.rb

@@ -0,0 +1,10 @@
+case []
+in [
+    *,
+    "bees are present",
+    *
+  ]
+  puts("bees!!!")
+else
+  "no bees"
+end

librubyfmt/rubyfmt_lib.rb

@@ -391,8 +391,28 @@ def on_break(arg)
     [:break, arg, start_end_for_keyword('break')]
   end
 
-  def on_var_field(*args)
-    with_lineno { super }
+  def on_var_field(ident)
+    line = if ident
+      # ident.last = [line, col]
+      ident.last.first
+    else
+      # When ident is nil, this represents a "*" pattern, which
+      # is lexed as an op. *However*, this can also happen during
+      # an "implicit" splat, e.g. `in String,` (with a trailing comma),
+      # in which case there's not a last op_location either, so at that
+      # point we give up because `lineno` would be incorrect
+      #
+      # Important to note: in this case, we must `shift` and not `pop`,
+      # because in the case of fndptn, this isn't run until after both
+      # splats have been lexed, so their op locations will be in the reverse order
+      @op_locations.shift
+    end
+    start_end = if line
+      [[line, line]]
+    else
+      [nil]
+    end
+    super + start_end
   end
 
   def on_tlambda(*args)

librubyfmt/src/format.rs

@@ -3411,62 +3411,69 @@ fn format_aryptn(ps: &mut dyn ConcreteParserState, aryptn: Aryptn) {
     let Aryptn(_, maybe_collection_name, maybe_pre_star_list, maybe_star, maybe_post_star_list) =
         aryptn;
     if let Some(collection_name) = maybe_collection_name {
-        format_var_ref(ps, collection_name.clone());
+        format_var_ref(ps, collection_name);
     }
-    ps.breakable_of(
-        BreakableDelims::for_array(),
-        Box::new(|ps| {
-            let mut vals = Vec::new();
-            if let Some(pre_star_list) = maybe_pre_star_list {
-                vals.append(
-                    &mut pre_star_list
-                        .into_iter()
-                        .map(|item| item.into_expression())
-                        .collect::<Vec<_>>(),
-                );
-            }
-            if let Some(star) = maybe_star {
-                vals.push(pattern_splat_as_expr(star.clone()));
-            }
-            if let Some(post_star_list) = maybe_post_star_list {
-                vals.append(
-                    &mut post_star_list
-                        .into_iter()
-                        .map(|item| item.into_expression())
-                        .collect::<Vec<_>>(),
-                );
-            }
-            format_list_like_thing_items(ps, vals, None, false);
-        }),
-    );
+    ps.new_block(Box::new(|ps| {
+        ps.breakable_of(
+            BreakableDelims::for_array(),
+            Box::new(|ps| {
+                let mut vals = Vec::new();
+                if let Some(pre_star_list) = maybe_pre_star_list {
+                    vals.append(
+                        &mut pre_star_list
+                            .into_iter()
+                            .map(|item| item.into_expression())
+                            .collect::<Vec<_>>(),
+                    );
+                }
+                if let Some(star) = maybe_star {
+                    vals.push(pattern_splat_as_expr(star));
+                }
+                if let Some(post_star_list) = maybe_post_star_list {
+                    vals.append(
+                        &mut post_star_list
+                            .into_iter()
+                            .map(|item| item.into_expression())
+                            .collect::<Vec<_>>(),
+                    );
+                }
+                format_list_like_thing_items(ps, vals, None, false);
+            }),
+        );
+    }));
 }
 
 fn format_fndptn(ps: &mut dyn ConcreteParserState, fndptn: Fndptn) {
     let Fndptn(_, maybe_collection_name, pre_splat, values, post_splat) = fndptn;
     if let Some(collection_name) = maybe_collection_name {
         format_var_ref(ps, collection_name);
     }
-    ps.breakable_of(
-        BreakableDelims::for_array(),
-        Box::new(|ps| {
-            let mut vals = values
-                .into_iter()
-                .map(|item| item.into_expression())
-                .collect::<Vec<_>>();
-            vals.insert(0, pattern_splat_as_expr(pre_splat));
-            vals.push(pattern_splat_as_expr(post_splat));
-
-            format_list_like_thing_items(ps, vals, None, false);
-        }),
-    );
+    ps.new_block(Box::new(|ps| {
+        ps.breakable_of(
+            BreakableDelims::for_array(),
+            Box::new(|ps| {
+                let mut vals = values
+                    .into_iter()
+                    .map(|item| item.into_expression())
+                    .collect::<Vec<_>>();
+                vals.insert(0, pattern_splat_as_expr(pre_splat));
+                vals.push(pattern_splat_as_expr(post_splat));
+
+                format_list_like_thing_items(ps, vals, None, false);
+            }),
+        );
+    }));
 }
 
 fn pattern_splat_as_expr(var_field: VarField) -> Expression {
     let mut ident = "*".to_string();
     if let Some(name) = var_field.1 {
         ident.push_str(name.to_local_string().as_str());
     }
-    Expression::Ident(Ident::new(ident, LineCol(var_field.2.start_line(), 0)))
+    Expression::Ident(Ident::new(
+        ident,
+        LineCol(var_field.2.map(|se| se.start_line()).unwrap_or(0), 0),
+    ))
 }
 
 pub fn format_retry(ps: &mut dyn ConcreteParserState, r: Retry) {

librubyfmt/src/ripper_tree_types.rs

@@ -271,12 +271,11 @@ impl Expression {
                 .as_ref()
                 .and_then(|exprs| exprs.first().and_then(|expr| expr.start_line()))
                 .or_else(|| {
-                    Some(
-                        star.as_ref()
-                            .expect("If first exprs list is empty, there must be a * pattern")
-                            .2
-                            .start_line(),
-                    )
+                    star.as_ref()
+                        .expect("If first exprs list is empty, there must be a * pattern")
+                        .2
+                        .as_ref()
+                        .map(|se| se.start_line())
                 }),
             // Pick the first of either expression, since these can be e.g. `foo..bar` or `foo..` or `..bar`
             Expression::Dot2(Dot2(_, maybe_first_expr, maybe_second_expr))
@@ -328,7 +327,9 @@ pub enum MLhsInner {
 impl MLhsInner {
     pub fn start_line(&self) -> Option<u64> {
         match self {
-            MLhsInner::VarField(VarField(.., start_end)) => Some(start_end.start_line()),
+            MLhsInner::VarField(VarField(.., start_end)) => {
+                start_end.as_ref().map(|se| se.start_line())
+            }
             MLhsInner::Field(Field(_, expr, ..)) => expr.start_line(),
             MLhsInner::RestParam(RestParam(.., rest_param_assignable)) => rest_param_assignable
                 .as_ref()
@@ -611,7 +612,9 @@ impl RestParamAssignable {
         match self {
             RestParamAssignable::ArefField(ArefField(.., linecol))
             | RestParamAssignable::Ident(Ident(.., linecol)) => Some(linecol.0),
-            RestParamAssignable::VarField(VarField(.., start_end)) => Some(start_end.start_line()),
+            RestParamAssignable::VarField(VarField(.., start_end)) => {
+                start_end.as_ref().map(|se| se.start_line())
+            }
         }
     }
 }
@@ -632,7 +635,9 @@ pub enum Assignable {
 impl Assignable {
     pub fn start_line(&self) -> Option<u64> {
         match self {
-            Assignable::VarField(VarField(.., start_end)) => Some(start_end.start_line()),
+            Assignable::VarField(VarField(.., start_end)) => {
+                start_end.as_ref().map(|se| se.start_line())
+            }
             Assignable::RestParam(RestParam(.., rest_param_assignable)) => rest_param_assignable
                 .as_ref()
                 .and_then(|rpa| rpa.start_line()),
@@ -671,7 +676,11 @@ pub struct ConstPathField(pub const_path_field_tag, pub Box<Expression>, pub Con
 
 def_tag!(var_field_tag, "var_field");
 #[derive(Deserialize, Debug, Clone)]
-pub struct VarField(pub var_field_tag, pub Option<VarRefType>, pub StartEnd);
+pub struct VarField(
+    pub var_field_tag,
+    pub Option<VarRefType>,
+    pub Option<StartEnd>, // In rare cases, this can be nil, see `on_var_field` in rubyfmt_lib
+);
 
 def_tag!(field_tag, "field");
 #[derive(Deserialize, Debug, Clone)]
@@ -2397,7 +2406,7 @@ impl ExpressionOrVarField {
         match self {
             ExpressionOrVarField::Expression(expr) => expr,
             ExpressionOrVarField::VarField(var_field) => {
-                let start_line = var_field.2.start_line();
+                let start_line = var_field.2.map(|se| se.start_line()).unwrap_or(0);
                 Expression::Ident(Ident::new(
                     var_field
                         .1
@@ -2412,7 +2421,9 @@ impl ExpressionOrVarField {
     pub fn start_line(&self) -> Option<u64> {
         match self {
             ExpressionOrVarField::Expression(expr) => expr.start_line(),
-            ExpressionOrVarField::VarField(var_field) => Some(var_field.2.start_line()),
+            ExpressionOrVarField::VarField(var_field) => {
+                var_field.2.as_ref().map(|se| se.start_line())
+            }
         }
     }
 }