Skip to content

Add implementation of match arm alignment #1704

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 12 commits into from
42 changes: 42 additions & 0 deletions Configurations.md
Original file line number Diff line number Diff line change
Expand Up @@ -965,6 +965,48 @@ struct Dolor<T>
}
```


## `match_arm_align_threshold`

Maximum number of spaces between `match` statement patterns for the arms to be aligned with each other.

- **Default value**: `0`
- **Possible values**: any positive integer

**Note:** A value greater than or equal to `max_width` results in match arm alignment being applied regardless of the spacing between match arm patterns. A value of `0` will never align match arms.

#### Difference in width shorter than `match_arm_align_threshold`:
```rust
match lorem {
Lorem::Ipsum => {
lorem();
ipsum();
}
Lorem::DolorSitAmetConsecteturAdipiscingElitSedDo => (),
Lorem::Eiusmod => {
lorem();
ipsum();
}
}
```

#### Difference in width longer than `match_arm_align_threshold`:
```rust
match lorem {
Lorem::Ipsum => {
lorem();
ipsum();
}
Lorem::DolorSitAmetConsecteturAdipiscingElitSedDo => (),
Lorem::Eiusmod => {
lorem();
ipsum();
}
}
```

See also: [`indent_match_arms`](#indent_match_arms), [`wrap_match_arms`](#wrap_match_arms).

## `match_block_trailing_comma`

Put a trailing comma after a block based match arm (non-block arms are not affected)
Expand Down
5 changes: 3 additions & 2 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ configuration_option_enum! { TypeDensity:
Wide,
}


impl Density {
pub fn to_list_tactic(self) -> ListTactic {
match self {
Expand Down Expand Up @@ -598,7 +597,9 @@ create_config! {
"What Write Mode to use when none is supplied: Replace, Overwrite, Display, Diff, Coverage";
condense_wildcard_suffixes: bool, false, "Replace strings of _ wildcards by a single .. in \
tuple patterns";
combine_control_expr: bool, true, "Combine control expressions with funciton calls."
combine_control_expr: bool, true, "Combine control expressions with funciton calls.";
match_arm_align_threshold: usize, 0, "Maximum difference in width between match arms for \
them to be aligned. 0 means never align."
}

#[cfg(test)]
Expand Down
133 changes: 93 additions & 40 deletions src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,8 @@ fn format_expr(
ast::ExprKind::Loop(..) |
ast::ExprKind::While(..) |
ast::ExprKind::WhileLet(..) => {
to_control_flow(expr, expr_type).and_then(|control_flow| {
control_flow.rewrite(context, shape)
})
to_control_flow(expr, expr_type)
.and_then(|control_flow| control_flow.rewrite(context, shape))
}
ast::ExprKind::Block(ref block) => block.rewrite(context, shape),
ast::ExprKind::Match(ref cond, ref arms) => {
Expand Down Expand Up @@ -939,16 +938,12 @@ fn to_control_flow<'a>(expr: &'a ast::Expr, expr_type: ExprType) -> Option<Contr
ast::ExprKind::ForLoop(ref pat, ref cond, ref block, label) => {
Some(ControlFlow::new_for(pat, cond, block, label, expr.span))
}
ast::ExprKind::Loop(ref block, label) => Some(
ControlFlow::new_loop(block, label, expr.span),
),
ast::ExprKind::While(ref cond, ref block, label) => Some(ControlFlow::new_while(
None,
cond,
block,
label,
expr.span,
)),
ast::ExprKind::Loop(ref block, label) => {
Some(ControlFlow::new_loop(block, label, expr.span))
}
ast::ExprKind::While(ref cond, ref block, label) => {
Some(ControlFlow::new_while(None, cond, block, label, expr.span))
}
ast::ExprKind::WhileLet(ref pat, ref cond, ref block, label) => {
Some(ControlFlow::new_while(
Some(pat),
Expand Down Expand Up @@ -1209,23 +1204,20 @@ impl<'a> ControlFlow<'a> {
};

Some((
format!(
"{}{}{}{}{}",
label_string,
self.keyword,
between_kwd_cond_comment.as_ref().map_or(
if pat_expr_string.is_empty() ||
pat_expr_string.starts_with('\n')
{
""
} else {
" "
},
|s| &**s,
),
pat_expr_string,
after_cond_comment.as_ref().map_or(block_sep, |s| &**s)
),
format!("{}{}{}{}{}",
label_string,
self.keyword,
between_kwd_cond_comment
.as_ref()
.map_or(if pat_expr_string.is_empty() ||
pat_expr_string.starts_with('\n') {
""
} else {
" "
},
|s| &**s),
pat_expr_string,
after_cond_comment.as_ref().map_or(block_sep, |s| &**s)),
used_width,
))
}
Expand Down Expand Up @@ -1479,20 +1471,65 @@ fn rewrite_match(
};
let mut result = format!("match {}{}{{", cond_str, block_sep);

let arm_shape = if context.config.indent_match_arms() {
shape.block_indent(context.config.tab_spaces())
} else {
shape.block_indent(0)
};
let mut min_pat_pos = 0;
let mut max_pat_pos = 0;
let mut should_align_arms = true;

if context.config.match_arm_align_threshold() == 0 {
should_align_arms = false;
}

if let Some(arm) = arms.first() {
min_pat_pos = arm_pat_end_pos_relative(arm);
max_pat_pos = min_pat_pos;
}

let arm_indent_str = arm_shape.indent.to_string(context.config);
for arm in arms.iter() {
let pat_pos = arm_pat_end_pos_relative(arm);

if max_pat_pos - min_pat_pos > context.config.match_arm_align_threshold() {
should_align_arms = false;
}

if pat_pos > max_pat_pos {
max_pat_pos = pat_pos;
} else if pat_pos < min_pat_pos {
min_pat_pos = pat_pos;
}
}

if context.config.match_arm_align_threshold() >= context.config.max_width() {
should_align_arms = true;
}

let open_brace_pos = context.codemap.span_after(
mk_sp(cond.span.hi, arm_start_pos(&arms[0])),
"{",
);

let mut arm_shape = if context.config.indent_match_arms() {
shape.block_indent(context.config.tab_spaces())
} else {
shape.block_indent(0)
};

for (i, arm) in arms.iter().enumerate() {
let alignment = if should_align_arms {
max_pat_pos - arm_pat_end_pos_relative(arm)
} else {
0
};

arm_shape = if context.config.indent_match_arms() {
shape.block_indent(context.config.tab_spaces())
} else {
shape.block_indent(0)
};

arm_shape.alignment = alignment;

let arm_indent_str = arm_shape.indent.to_string(context.config);

// Make sure we get the stuff between arms.
let missed_str = if i == 0 {
context.snippet(mk_sp(open_brace_pos, arm_start_pos(arm)))
Expand All @@ -1519,14 +1556,16 @@ fn rewrite_match(
result.push_str(arm_comma(context.config, &arm.body));
}
}
let last_arm_indent_str = arm_shape.indent.to_string(context.config);

// BytePos(1) = closing match brace.
let last_span = mk_sp(arm_end_pos(&arms[arms.len() - 1]), span.hi - BytePos(1));
let last_comment = context.snippet(last_span);
let comment = try_opt!(rewrite_match_arm_comment(
context,
&last_comment,
arm_shape,
&arm_indent_str,
&last_arm_indent_str,
));
result.push_str(&comment);
result.push('\n');
Expand All @@ -1552,6 +1591,11 @@ fn arm_end_pos(arm: &ast::Arm) -> BytePos {
arm.body.span.hi
}

fn arm_pat_end_pos_relative(arm: &ast::Arm) -> usize {
let &ast::Arm { ref pats, .. } = arm;
(pats[pats.len() - 1].span.hi - pats[0].span.lo).0 as usize
}

fn arm_comma(config: &Config, body: &ast::Expr) -> &'static str {
if config.match_block_trailing_comma() {
","
Expand Down Expand Up @@ -1653,6 +1697,11 @@ impl Rewrite for ast::Arm {
let alt_block_sep = String::from("\n") +
&shape.indent.block_only().to_string(context.config);

let mut alignment_str = String::with_capacity(shape.alignment);
for _ in 0..shape.alignment {
alignment_str.push(' ');
}

let pat_width = extra_offset(&pats_str, shape);
// Let's try and get the arm body on the same line as the condition.
// 4 = ` => `.len()
Expand Down Expand Up @@ -1681,9 +1730,10 @@ impl Rewrite for ast::Arm {
};

return Some(format!(
"{}{} =>{}{}{}",
"{}{} {}=>{}{}{}",
attr_str.trim_left(),
pats_str,
alignment_str,
block_sep,
body_str,
comma
Expand Down Expand Up @@ -1723,9 +1773,10 @@ impl Rewrite for ast::Arm {

if context.config.wrap_match_arms() {
Some(format!(
"{}{} =>{}{}{}\n{}{}",
"{}{} {}=>{}{}{}\n{}{}",
attr_str.trim_left(),
pats_str,
alignment_str,
block_sep,
indent_str,
next_line_body,
Expand All @@ -1734,9 +1785,10 @@ impl Rewrite for ast::Arm {
))
} else {
Some(format!(
"{}{} =>{}{}{}{}",
"{}{} {}=>{}{}{}{}",
attr_str.trim_left(),
pats_str,
alignment_str,
block_sep,
indent_str,
next_line_body,
Expand Down Expand Up @@ -2205,6 +2257,7 @@ fn last_arg_shape(
width: try_opt!(max_width.checked_sub(overhead)),
indent: arg_indent,
offset: 0,
alignment: 0,
})
}

Expand Down
2 changes: 2 additions & 0 deletions src/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,7 @@ impl<'a> FmtVisitor<'a> {
width: context.config.max_width(),
indent: self.block_indent,
offset: self.block_indent.alignment,
alignment: 0,
};
let missing_comment = rewrite_missing_comment_on_field(
&context,
Expand Down Expand Up @@ -1572,6 +1573,7 @@ pub fn rewrite_static(
width: context.config.max_width(),
indent: offset,
offset: offset.alignment,
alignment: 0,
},
)
})
Expand Down
9 changes: 9 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,8 @@ pub struct Shape {
// Indentation + any already emitted text on the first line of the current
// statement.
pub offset: usize,
// Alignment to other structures
pub alignment: usize,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than add this to Shape, can you pass this as an extra arg to the relevant functions?

}

impl Shape {
Expand All @@ -245,6 +247,7 @@ impl Shape {
width: width,
indent: indent,
offset: indent.alignment,
alignment: 0,
}
}

Expand All @@ -253,6 +256,7 @@ impl Shape {
width: config.max_width().checked_sub(indent.width()).unwrap_or(0),
indent: indent,
offset: indent.alignment,
alignment: 0,
}
}

Expand All @@ -271,6 +275,7 @@ impl Shape {
width: width,
indent: indent,
offset: offset,
alignment: 0,
}
}

Expand All @@ -280,6 +285,7 @@ impl Shape {
width: self.width,
indent: Indent::new(self.indent.block_indent, alignment),
offset: alignment,
alignment: 0,
}
}

Expand All @@ -289,12 +295,14 @@ impl Shape {
width: self.width,
indent: Indent::new(self.indent.block_indent + extra_width, 0),
offset: 0,
alignment: 0,
}
} else {
Shape {
width: self.width,
indent: self.indent + extra_width,
offset: self.indent.alignment + extra_width,
alignment: 0,
}
}
}
Expand Down Expand Up @@ -329,6 +337,7 @@ impl Shape {
width: try_opt!(self.width.checked_sub(width)),
indent: self.indent + width,
offset: self.offset + width,
alignment: 0,
})
}

Expand Down
16 changes: 16 additions & 0 deletions tests/source/configs-match_arm_align_threshold-above.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// rustfmt-match_arm_align_threshold: 10
// Align match arms

fn main() {
match lorem {
Lorem::Ipsum => {
lorem();
ipsum();
}
Lorem::DolorSitAmetConsecteturAdipiscingElitSedDo => (),
Lorem::Eiusmod => {
lorem();
ipsum();
}
}
}
Loading