Skip to content

Commit effba71

Browse files
nrcnrc
authored
Merge pull request #2679 from topecongiro/multi-lined-binary
Put operands on its own line when each fits in a single line
2 parents 195489d + 1f738ea commit effba71

20 files changed

+231
-60
lines changed

‎Configurations.md

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,12 @@ fn main() {
6464

6565
```rust
6666
fn main() {
67-
if lorem_ipsum && dolor_sit && amet_consectetur && lorem_sit && dolor_consectetur && amet_ipsum
67+
if lorem_ipsum
68+
&& dolor_sit
69+
&& amet_consectetur
70+
&& lorem_sit
71+
&& dolor_consectetur
72+
&& amet_ipsum
6873
&& lorem_consectetur
6974
{
7075
// ...
@@ -76,7 +81,12 @@ fn main() {
7681

7782
```rust
7883
fn main() {
79-
if lorem_ipsum && dolor_sit && amet_consectetur && lorem_sit && dolor_consectetur && amet_ipsum
84+
if lorem_ipsum
85+
&& dolor_sit
86+
&& amet_consectetur
87+
&& lorem_sit
88+
&& dolor_consectetur
89+
&& amet_ipsum
8090
&& lorem_consectetur
8191
{
8292
// ...
@@ -342,7 +352,8 @@ fn main() {
342352
let or = foofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoo
343353
|| barbarbarbarbarbarbarbarbarbarbarbarbarbarbarbar;
344354

345-
let sum = 123456789012345678901234567890 + 123456789012345678901234567890
355+
let sum = 123456789012345678901234567890
356+
+ 123456789012345678901234567890
346357
+ 123456789012345678901234567890;
347358

348359
let range = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
@@ -357,7 +368,8 @@ fn main() {
357368
let or = foofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoo ||
358369
barbarbarbarbarbarbarbarbarbarbarbarbarbarbarbar;
359370

360-
let sum = 123456789012345678901234567890 + 123456789012345678901234567890 +
371+
let sum = 123456789012345678901234567890 +
372+
123456789012345678901234567890 +
361373
123456789012345678901234567890;
362374

363375
let range = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa..

‎src/bin/main.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ fn main() {
3636

3737
let exit_code = match execute(&opts) {
3838
Ok((write_mode, summary)) => {
39-
if summary.has_operational_errors() || summary.has_parsing_errors()
39+
if summary.has_operational_errors()
40+
|| summary.has_parsing_errors()
4041
|| (summary.has_diff && write_mode == WriteMode::Check)
4142
{
4243
1

‎src/chains.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,8 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) -
265265
nested_shape.indent.to_string_with_newline(context.config)
266266
};
267267

268-
let first_connector = if is_small_parent || fits_single_line
268+
let first_connector = if is_small_parent
269+
|| fits_single_line
269270
|| last_line_extendable(&parent_rewrite)
270271
|| context.config.indent_style() == IndentStyle::Visual
271272
{
@@ -275,7 +276,8 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) -
275276
};
276277

277278
let result = if is_small_parent && rewrites.len() > 1 {
278-
let second_connector = if fits_single_line || rewrites[1] == "?"
279+
let second_connector = if fits_single_line
280+
|| rewrites[1] == "?"
279281
|| last_line_extendable(&rewrites[0])
280282
|| context.config.indent_style() == IndentStyle::Visual
281283
{

‎src/closures.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,10 @@ fn get_inner_expr<'a>(
109109

110110
// Figure out if a block is necessary.
111111
fn needs_block(block: &ast::Block, prefix: &str, context: &RewriteContext) -> bool {
112-
is_unsafe_block(block) || block.stmts.len() > 1
113-
|| block_contains_comment(block, context.codemap) || prefix.contains('\n')
112+
is_unsafe_block(block)
113+
|| block.stmts.len() > 1
114+
|| block_contains_comment(block, context.codemap)
115+
|| prefix.contains('\n')
114116
}
115117

116118
// Rewrite closure with a single expression wrapping its body with block.

‎src/comment.rs

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -579,7 +579,9 @@ fn light_rewrite_comment(
579579
/// Does not trim all whitespace. If a single space is trimmed from the left of the string,
580580
/// this function returns true.
581581
fn left_trim_comment_line<'a>(line: &'a str, style: &CommentStyle) -> (&'a str, bool) {
582-
if line.starts_with("//! ") || line.starts_with("/// ") || line.starts_with("/*! ")
582+
if line.starts_with("//! ")
583+
|| line.starts_with("/// ")
584+
|| line.starts_with("/*! ")
583585
|| line.starts_with("/** ")
584586
{
585587
(&line[4..], true)
@@ -589,13 +591,18 @@ fn left_trim_comment_line<'a>(line: &'a str, style: &CommentStyle) -> (&'a str,
589591
} else {
590592
(&line[opener.trim_right().len()..], false)
591593
}
592-
} else if line.starts_with("/* ") || line.starts_with("// ") || line.starts_with("//!")
593-
|| line.starts_with("///") || line.starts_with("** ")
594+
} else if line.starts_with("/* ")
595+
|| line.starts_with("// ")
596+
|| line.starts_with("//!")
597+
|| line.starts_with("///")
598+
|| line.starts_with("** ")
594599
|| line.starts_with("/*!")
595600
|| (line.starts_with("/**") && !line.starts_with("/**/"))
596601
{
597602
(&line[3..], line.chars().nth(2).unwrap() == ' ')
598-
} else if line.starts_with("/*") || line.starts_with("* ") || line.starts_with("//")
603+
} else if line.starts_with("/*")
604+
|| line.starts_with("* ")
605+
|| line.starts_with("//")
599606
|| line.starts_with("**")
600607
{
601608
(&line[2..], line.chars().nth(1).unwrap() == ' ')

‎src/config/summary.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,9 @@ impl Summary {
9090
}
9191

9292
pub fn has_no_errors(&self) -> bool {
93-
!(self.has_operational_errors || self.has_parsing_errors || self.has_formatting_errors
93+
!(self.has_operational_errors
94+
|| self.has_parsing_errors
95+
|| self.has_formatting_errors
9496
|| self.has_diff)
9597
}
9698

‎src/expr.rs

Lines changed: 104 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -86,16 +86,18 @@ pub fn format_expr(
8686
rewrite_call(context, &callee_str, args, inner_span, shape)
8787
}
8888
ast::ExprKind::Paren(ref subexpr) => rewrite_paren(context, subexpr, shape, expr.span),
89-
ast::ExprKind::Binary(ref op, ref lhs, ref rhs) => {
89+
ast::ExprKind::Binary(op, ref lhs, ref rhs) => {
9090
// FIXME: format comments between operands and operator
91-
rewrite_pair(
92-
&**lhs,
93-
&**rhs,
94-
PairParts::new("", &format!(" {} ", context.snippet(op.span)), ""),
95-
context,
96-
shape,
97-
context.config.binop_separator(),
98-
)
91+
rewrite_simple_binaries(context, expr, shape, op).or_else(|| {
92+
rewrite_pair(
93+
&**lhs,
94+
&**rhs,
95+
PairParts::new("", &format!(" {} ", context.snippet(op.span)), ""),
96+
context,
97+
shape,
98+
context.config.binop_separator(),
99+
)
100+
})
99101
}
100102
ast::ExprKind::Unary(ref op, ref subexpr) => rewrite_unary_op(context, op, subexpr, shape),
101103
ast::ExprKind::Struct(ref path, ref fields, ref base) => rewrite_struct_lit(
@@ -352,6 +354,80 @@ pub fn format_expr(
352354
})
353355
}
354356

357+
/// Collect operands that appears in the given binary operator in the opposite order.
358+
/// e.g. `collect_binary_items(e, ||)` for `a && b || c || d` returns `[d, c, a && b]`.
359+
fn collect_binary_items<'a>(mut expr: &'a ast::Expr, binop: ast::BinOp) -> Vec<&'a ast::Expr> {
360+
let mut result = vec![];
361+
let mut prev_lhs = None;
362+
loop {
363+
match expr.node {
364+
ast::ExprKind::Binary(inner_binop, ref lhs, ref rhs)
365+
if inner_binop.node == binop.node =>
366+
{
367+
result.push(&**rhs);
368+
expr = lhs;
369+
prev_lhs = Some(lhs);
370+
}
371+
_ => {
372+
if let Some(lhs) = prev_lhs {
373+
result.push(lhs);
374+
}
375+
break;
376+
}
377+
}
378+
}
379+
result
380+
}
381+
382+
/// Rewrites a binary expression whose operands fits within a single line.
383+
fn rewrite_simple_binaries(
384+
context: &RewriteContext,
385+
expr: &ast::Expr,
386+
shape: Shape,
387+
op: ast::BinOp,
388+
) -> Option<String> {
389+
let op_str = context.snippet(op.span);
390+
391+
// 2 = spaces around a binary operator.
392+
let sep_overhead = op_str.len() + 2;
393+
let nested_overhead = sep_overhead - 1;
394+
395+
let nested_shape = (match context.config.indent_style() {
396+
IndentStyle::Visual => shape.visual_indent(0),
397+
IndentStyle::Block => shape.block_indent(context.config.tab_spaces()),
398+
}).with_max_width(context.config);
399+
let nested_shape = match context.config.binop_separator() {
400+
SeparatorPlace::Back => nested_shape.sub_width(nested_overhead)?,
401+
SeparatorPlace::Front => nested_shape.offset_left(nested_overhead)?,
402+
};
403+
404+
let opt_rewrites: Option<Vec<_>> = collect_binary_items(expr, op)
405+
.iter()
406+
.rev()
407+
.map(|e| e.rewrite(context, nested_shape))
408+
.collect();
409+
if let Some(rewrites) = opt_rewrites {
410+
if rewrites.iter().all(|e| ::utils::is_single_line(e)) {
411+
let total_width = rewrites.iter().map(|s| s.len()).sum::<usize>()
412+
+ sep_overhead * (rewrites.len() - 1);
413+
414+
let sep_str = if total_width <= shape.width {
415+
format!(" {} ", op_str)
416+
} else {
417+
let indent_str = nested_shape.indent.to_string_with_newline(context.config);
418+
match context.config.binop_separator() {
419+
SeparatorPlace::Back => format!(" {}{}", op_str.trim_right(), indent_str),
420+
SeparatorPlace::Front => format!("{}{} ", indent_str, op_str.trim_left()),
421+
}
422+
};
423+
424+
return wrap_str(rewrites.join(&sep_str), context.config.max_width(), shape);
425+
}
426+
}
427+
428+
None
429+
}
430+
355431
#[derive(new, Clone, Copy)]
356432
pub struct PairParts<'a> {
357433
prefix: &'a str,
@@ -398,8 +474,10 @@ where
398474
.map(|first_line| first_line.ends_with('{'))
399475
.unwrap_or(false);
400476
if !rhs_result.contains('\n') || allow_same_line {
401-
let one_line_width = last_line_width(&lhs_result) + pp.infix.len()
402-
+ first_line_width(rhs_result) + pp.suffix.len();
477+
let one_line_width = last_line_width(&lhs_result)
478+
+ pp.infix.len()
479+
+ first_line_width(rhs_result)
480+
+ pp.suffix.len();
403481
if one_line_width <= shape.width {
404482
return Some(format!(
405483
"{}{}{}{}",
@@ -482,7 +560,9 @@ fn rewrite_empty_block(
482560
let user_str = user_str.trim();
483561
if user_str.starts_with('{') && user_str.ends_with('}') {
484562
let comment_str = user_str[1..user_str.len() - 1].trim();
485-
if block.stmts.is_empty() && !comment_str.contains('\n') && !comment_str.starts_with("//")
563+
if block.stmts.is_empty()
564+
&& !comment_str.contains('\n')
565+
&& !comment_str.starts_with("//")
486566
&& comment_str.len() + 4 <= shape.width
487567
{
488568
return Some(format!("{}{{ {} }}", prefix, comment_str));
@@ -1165,8 +1245,10 @@ pub fn is_simple_block(
11651245
attrs: Option<&[ast::Attribute]>,
11661246
codemap: &CodeMap,
11671247
) -> bool {
1168-
(block.stmts.len() == 1 && stmt_is_expr(&block.stmts[0])
1169-
&& !block_contains_comment(block, codemap) && attrs.map_or(true, |a| a.is_empty()))
1248+
(block.stmts.len() == 1
1249+
&& stmt_is_expr(&block.stmts[0])
1250+
&& !block_contains_comment(block, codemap)
1251+
&& attrs.map_or(true, |a| a.is_empty()))
11701252
}
11711253

11721254
/// Checks whether a block contains at most one statement or expression, and no
@@ -1176,7 +1258,8 @@ pub fn is_simple_block_stmt(
11761258
attrs: Option<&[ast::Attribute]>,
11771259
codemap: &CodeMap,
11781260
) -> bool {
1179-
block.stmts.len() <= 1 && !block_contains_comment(block, codemap)
1261+
block.stmts.len() <= 1
1262+
&& !block_contains_comment(block, codemap)
11801263
&& attrs.map_or(true, |a| a.is_empty())
11811264
}
11821265

@@ -1187,7 +1270,8 @@ pub fn is_empty_block(
11871270
attrs: Option<&[ast::Attribute]>,
11881271
codemap: &CodeMap,
11891272
) -> bool {
1190-
block.stmts.is_empty() && !block_contains_comment(block, codemap)
1273+
block.stmts.is_empty()
1274+
&& !block_contains_comment(block, codemap)
11911275
&& attrs.map_or(true, |a| inner_attributes(a).is_empty())
11921276
}
11931277

@@ -1702,7 +1786,8 @@ pub fn wrap_struct_field(
17021786
one_line_width: usize,
17031787
) -> String {
17041788
if context.config.indent_style() == IndentStyle::Block
1705-
&& (fields_str.contains('\n') || !context.config.struct_lit_single_line()
1789+
&& (fields_str.contains('\n')
1790+
|| !context.config.struct_lit_single_line()
17061791
|| fields_str.len() > one_line_width)
17071792
{
17081793
format!(
@@ -2028,7 +2113,8 @@ fn choose_rhs<R: Rewrite>(
20282113
}
20292114

20302115
pub fn prefer_next_line(orig_rhs: &str, next_line_rhs: &str, rhs_tactics: RhsTactics) -> bool {
2031-
rhs_tactics == RhsTactics::ForceNextLine || !next_line_rhs.contains('\n')
2116+
rhs_tactics == RhsTactics::ForceNextLine
2117+
|| !next_line_rhs.contains('\n')
20322118
|| count_newlines(orig_rhs) > count_newlines(next_line_rhs) + 1
20332119
}
20342120

‎src/imports.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -489,7 +489,9 @@ impl UseTree {
489489
}
490490

491491
fn share_prefix(&self, other: &UseTree) -> bool {
492-
if self.path.is_empty() || other.path.is_empty() || self.attrs.is_some()
492+
if self.path.is_empty()
493+
|| other.path.is_empty()
494+
|| self.attrs.is_some()
493495
|| !self.same_visibility(other)
494496
{
495497
false

0 commit comments

Comments
 (0)