Skip to content

Commit

Permalink
Fix multiline handling
Browse files Browse the repository at this point in the history
  • Loading branch information
reese authored Jan 8, 2024
1 parent 403a0b2 commit e1e2692
Show file tree
Hide file tree
Showing 7 changed files with 127 additions and 54 deletions.
8 changes: 8 additions & 0 deletions fixtures/small/multiline_aryptn_actual.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
case []
in [first,
*,
last]
puts "bees!!!"
else
"no bees"
end
10 changes: 10 additions & 0 deletions fixtures/small/multiline_aryptn_expected.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
case []
in [
first,
*,
last
]
puts("bees!!!")
else
"no bees"
end
7 changes: 7 additions & 0 deletions fixtures/small/multiline_fndptn_actual.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
case []
in *, "bees are present",
*; then
puts "bees!!!"
else
"no bees"
end
10 changes: 10 additions & 0 deletions fixtures/small/multiline_fndptn_expected.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
case []
in [
*,
"bees are present",
*
]
puts("bees!!!")
else
"no bees"
end
24 changes: 22 additions & 2 deletions librubyfmt/rubyfmt_lib.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
87 changes: 47 additions & 40 deletions librubyfmt/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3413,60 +3413,67 @@ fn format_aryptn(ps: &mut dyn ConcreteParserState, aryptn: Aryptn) {
if let Some(collection_name) = maybe_collection_name {
format_var_ref(ps, collection_name.clone());
}
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.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);
}),
);
}));
}

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) {
Expand Down
35 changes: 23 additions & 12 deletions librubyfmt/src/ripper_tree_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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())
}
}
}
}
Expand All @@ -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()),
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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
Expand All @@ -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())
}
}
}
}
Expand Down

0 comments on commit e1e2692

Please sign in to comment.