user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Improve formatting of series of binop expressions

Browse source 
This commit changes the handling of binops (and potentially other pairs), where
the expressions are logically a list, e.g., `a + b + c`. It makes the single
line vs multi-line approaches explicit and introduces a lowering step.

This improves formatting in a number of places, mostly improving consistency of
formatting with very short sub-expressions, but also some weird indentation.

Closes #2802
This commit is contained in:
Nick Cameron 2018-06-30 19:53:06 +12:00
parent b68fd9e6bf
commit a4cdb68925
2 changed files with 221 additions and 88 deletions
Download patch file Download diff file Expand all files Collapse all files

80
src/expr.rs
View file

@ -31,7 +31,7 @@ use lists::{
use macros::{rewrite_macro, MacroArg, MacroPosition};
use matches::rewrite_match;
use overflow;
use pairs::{rewrite_pair, PairParts};
use pairs::{rewrite_all_pairs, rewrite_pair, PairParts};
use patterns::{can_be_overflowed_pat, is_short_pattern, TuplePatField};
use rewrite::{Rewrite, RewriteContext};
use shape::{Indent, Shape};
@ -89,7 +89,7 @@ pub fn format_expr(
ast::ExprKind::Paren(ref subexpr) => rewrite_paren(context, subexpr, shape, expr.span),
ast::ExprKind::Binary(op, ref lhs, ref rhs) => {
// FIXME: format comments between operands and operator
rewrite_simple_binaries(context, expr, shape, op).or_else(|| {
rewrite_all_pairs(expr, shape, context).or_else(|| {
rewrite_pair(
&**lhs,
&**rhs,
@ -362,80 +362,6 @@ pub fn format_expr(
})
}
/// Collect operands that appears in the given binary operator in the opposite order.
/// e.g. `collect_binary_items(e, ||)` for `a && b || c || d` returns `[d, c, a && b]`.
fn collect_binary_items<'a>(mut expr: &'a ast::Expr, binop: ast::BinOp) -> Vec<&'a ast::Expr> {
let mut result = vec![];
let mut prev_lhs = None;
loop {
match expr.node {
ast::ExprKind::Binary(inner_binop, ref lhs, ref rhs)
if inner_binop.node == binop.node =>
{
result.push(&**rhs);
expr = lhs;
prev_lhs = Some(lhs);
}
_ => {
if let Some(lhs) = prev_lhs {
result.push(lhs);
}
break;
}
}
}
result
}
/// Rewrites a binary expression whose operands fits within a single line.
fn rewrite_simple_binaries(
context: &RewriteContext,
expr: &ast::Expr,
shape: Shape,
op: ast::BinOp,
) -> Option<String> {
let op_str = context.snippet(op.span);
// 2 = spaces around a binary operator.
let sep_overhead = op_str.len() + 2;
let nested_overhead = sep_overhead - 1;
let nested_shape = (match context.config.indent_style() {
IndentStyle::Visual => shape.visual_indent(0),
IndentStyle::Block => shape.block_indent(context.config.tab_spaces()),
}).with_max_width(context.config);
let nested_shape = match context.config.binop_separator() {
SeparatorPlace::Back => nested_shape.sub_width(nested_overhead)?,
SeparatorPlace::Front => nested_shape.offset_left(nested_overhead)?,
};
let opt_rewrites: Option<Vec<_>> = collect_binary_items(expr, op)
.iter()
.rev()
.map(|e| e.rewrite(context, nested_shape))
.collect();
if let Some(rewrites) = opt_rewrites {
if rewrites.iter().all(|e| ::utils::is_single_line(e)) {
let total_width = rewrites.iter().map(|s| s.len()).sum::<usize>()
+ sep_overhead * (rewrites.len() - 1);
let sep_str = if total_width <= shape.width {
format!(" {} ", op_str)
} else {
let indent_str = nested_shape.indent.to_string_with_newline(context.config);
match context.config.binop_separator() {
SeparatorPlace::Back => format!(" {}{}", op_str.trim_right(), indent_str),
SeparatorPlace::Front => format!("{}{} ", indent_str, op_str.trim_left()),
}
};
return wrap_str(rewrites.join(&sep_str), context.config.max_width(), shape);
}
}
None
}
pub fn rewrite_array<T: Rewrite + Spanned + ToExpr>(
name: &str,
exprs: &[&T],
@ -2004,7 +1930,7 @@ fn choose_rhs<R: Rewrite>(
}
(None, Some(ref new_rhs)) => Some(format!("{}{}", new_indent_str, new_rhs)),
(None, None) => None,
(Some(ref orig_rhs), _) => Some(format!(" {}", orig_rhs)),
(Some(orig_rhs), _) => Some(format!(" {}", orig_rhs)),
}
}
}