From 3f703fd33d8412fb870bd06fa14ffba087d33b3c Mon Sep 17 00:00:00 2001 From: topecongiro Date: Wed, 3 May 2017 11:21:31 +0900 Subject: [PATCH 1/6] Use block indent when visual indent failed inside closure block --- src/expr.rs | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/src/expr.rs b/src/expr.rs index 25d9a1ed397..75a7ff21abe 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -570,6 +570,19 @@ fn rewrite_closure(capture: ast::CaptureBy, rewrite.map(|rw| format!("{} {}", prefix, rw)) } + fn no_weird_visual_indent(block_str: &str, context: &RewriteContext) -> bool { + let mut prev_indent_width = 0; + for line in block_str.lines() { + let cur_indent_width = line.find(|c: char| !c.is_whitespace()).unwrap_or(0); + if prev_indent_width > cur_indent_width + context.config.tab_spaces && + line.find('}').unwrap_or(0) != cur_indent_width { + return false; + } + prev_indent_width = cur_indent_width; + } + true + } + fn rewrite_closure_block(block: &ast::Block, prefix: String, context: &RewriteContext, @@ -577,22 +590,27 @@ fn rewrite_closure(capture: ast::CaptureBy, -> Option { // Start with visual indent, then fall back to block indent if the // closure is large. - let rewrite = try_opt!(block.rewrite(&context, shape)); - - let block_threshold = context.config.closure_block_indent_threshold; - if block_threshold < 0 || rewrite.matches('\n').count() <= block_threshold as usize { - if let Some(rewrite) = wrap_str(rewrite, context.config.max_width, shape) { - return Some(format!("{} {}", prefix, rewrite)); + if let Some(block_str) = block.rewrite(&context, shape) { + let block_threshold = context.config.closure_block_indent_threshold; + if (block_threshold < 0 || + block_str.matches('\n').count() <= block_threshold as usize) && + no_weird_visual_indent(&block_str, context) { + if let Some(block_str) = block_str.rewrite(context, shape) { + return Some(format!("{} {}", prefix, block_str)); + } } } // The body of the closure is big enough to be block indented, that // means we must re-format. - let block_shape = shape.block(); - let rewrite = try_opt!(block.rewrite(&context, block_shape)); + let block_shape = Shape { + width: context.config.max_width - shape.block().indent.width(), + ..shape.block() + }; + let block_str = try_opt!(block.rewrite(&context, block_shape)); Some(format!("{} {}", prefix, - try_opt!(wrap_str(rewrite, block_shape.width, block_shape)))) + try_opt!(block_str.rewrite(context, block_shape)))) } } From 2c1d896f6078521aedd9e153ad6940bb2917cf2f Mon Sep 17 00:00:00 2001 From: topecongiro Date: Wed, 3 May 2017 11:22:36 +0900 Subject: [PATCH 2/6] Split a long chain with a single child --- src/chains.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/chains.rs b/src/chains.rs index 8e176fce724..8cb726bd4b4 100644 --- a/src/chains.rs +++ b/src/chains.rs @@ -176,9 +176,15 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) - .fold(0, |a, b| a + first_line_width(b)) + parent_rewrite.len(); let one_line_len = rewrites.iter().fold(0, |a, r| a + r.len()) + parent_rewrite.len(); - let veto_single_line = if one_line_len > context.config.chain_one_line_max - 1 && - rewrites.len() > 1 { - true + let veto_single_line = if one_line_len > context.config.chain_one_line_max - 1 { + if rewrites.len() > 1 { + true + } else if rewrites.len() == 1 { + let one_line_len = parent_rewrite.len() + first_line_width(&rewrites[0]); + one_line_len > shape.width + } else { + false + } } else if context.config.take_source_hints && subexpr_list.len() > 1 { // Look at the source code. Unless all chain elements start on the same // line, we won't consider putting them on a single line either. From e91d498b8f771ed9ddc1abbedf4d261883d4d505 Mon Sep 17 00:00:00 2001 From: topecongiro Date: Wed, 3 May 2017 11:24:08 +0900 Subject: [PATCH 3/6] Keep a chain with length chain_one_line_max in a single line --- src/chains.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/chains.rs b/src/chains.rs index 8cb726bd4b4..3d148d19a55 100644 --- a/src/chains.rs +++ b/src/chains.rs @@ -176,7 +176,7 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) - .fold(0, |a, b| a + first_line_width(b)) + parent_rewrite.len(); let one_line_len = rewrites.iter().fold(0, |a, r| a + r.len()) + parent_rewrite.len(); - let veto_single_line = if one_line_len > context.config.chain_one_line_max - 1 { + let veto_single_line = if one_line_len > context.config.chain_one_line_max { if rewrites.len() > 1 { true } else if rewrites.len() == 1 { From f5da9d779fd0e72c97823eb808e13d44ebad86fb Mon Sep 17 00:00:00 2001 From: topecongiro Date: Wed, 3 May 2017 11:26:25 +0900 Subject: [PATCH 4/6] Format source codes --- src/file_lines.rs | 12 ++++-------- src/items.rs | 14 ++++---------- src/string.rs | 5 +---- src/types.rs | 4 ++-- 4 files changed, 11 insertions(+), 24 deletions(-) diff --git a/src/file_lines.rs b/src/file_lines.rs index f11a0aaf6e2..b52d790ed37 100644 --- a/src/file_lines.rs +++ b/src/file_lines.rs @@ -123,10 +123,8 @@ impl FileLines { Some(ref map) => map, }; - match canonicalize_path_string(range.file_name()).and_then(|canonical| { - map.get_vec(&canonical) - .ok_or(()) - }) { + match canonicalize_path_string(range.file_name()) + .and_then(|canonical| map.get_vec(&canonical).ok_or(())) { Ok(ranges) => ranges.iter().any(|r| r.contains(Range::from(range))), Err(_) => false, } @@ -140,10 +138,8 @@ impl FileLines { Some(ref map) => map, }; - match canonicalize_path_string(range.file_name()).and_then(|canonical| { - map.get_vec(&canonical) - .ok_or(()) - }) { + match canonicalize_path_string(range.file_name()) + .and_then(|canonical| map.get_vec(&canonical).ok_or(())) { Ok(ranges) => ranges.iter().any(|r| r.intersects(Range::from(range))), Err(_) => false, } diff --git a/src/items.rs b/src/items.rs index 8ea97b177e8..0b2ca5a52cb 100644 --- a/src/items.rs +++ b/src/items.rs @@ -278,8 +278,7 @@ impl<'a> FmtVisitor<'a> { result.push(' '); } - self.single_line_fn(&result, block) - .or_else(|| Some(result)) + self.single_line_fn(&result, block).or_else(|| Some(result)) } pub fn rewrite_required_fn(&mut self, @@ -912,9 +911,7 @@ fn format_struct_struct(context: &RewriteContext, let snippet = context.snippet(mk_sp(body_lo, span.hi - BytePos(1))); if snippet.trim().is_empty() { // `struct S {}` - } else if snippet - .trim_right_matches(&[' ', '\t'][..]) - .ends_with('\n') { + } else if snippet.trim_right_matches(&[' ', '\t'][..]).ends_with('\n') { // fix indent result.push_str(&snippet.trim_right()); result.push('\n'); @@ -1030,9 +1027,7 @@ fn format_tuple_struct(context: &RewriteContext, let snippet = context.snippet(mk_sp(body_lo, context.codemap.span_before(span, ")"))); if snippet.is_empty() { // `struct S ()` - } else if snippet - .trim_right_matches(&[' ', '\t'][..]) - .ends_with('\n') { + } else if snippet.trim_right_matches(&[' ', '\t'][..]).ends_with('\n') { result.push_str(&snippet.trim_right()); result.push('\n'); result.push_str(&offset.to_string(context.config)); @@ -1229,8 +1224,7 @@ impl Rewrite for ast::StructField { let type_offset = shape.indent.block_indent(context.config); let rewrite_type_in_next_line = || { let budget = try_opt!(context.config.max_width.checked_sub(type_offset.width())); - self.ty - .rewrite(context, Shape::legacy(budget, type_offset)) + self.ty.rewrite(context, Shape::legacy(budget, type_offset)) }; let last_line_width = last_line_width(&result) + type_annotation_spacing.1.len(); diff --git a/src/string.rs b/src/string.rs index e1e0681ba56..37a569ce67d 100644 --- a/src/string.rs +++ b/src/string.rs @@ -51,10 +51,7 @@ pub fn rewrite_string<'a>(orig: &str, fmt: &StringFormat<'a>) -> Option let ender_length = fmt.line_end.len(); // If we cannot put at least a single character per line, the rewrite won't // succeed. - let max_chars = try_opt!(shape - .width - .checked_sub(fmt.opener.len() + ender_length + 1)) + - 1; + let max_chars = try_opt!(shape.width.checked_sub(fmt.opener.len() + ender_length + 1)) + 1; // Snip a line at a time from `orig` until it is used up. Push the snippet // onto result. diff --git a/src/types.rs b/src/types.rs index ca98401db14..e2a95c66832 100644 --- a/src/types.rs +++ b/src/types.rs @@ -704,9 +704,9 @@ fn rewrite_bare_fn(bare_fn: &ast::BareFnTy, .lifetimes .iter() .map(|l| { - l.rewrite(context, + l.rewrite(context, Shape::legacy(try_opt!(shape.width.checked_sub(6)), shape.indent + 4)) - }) + }) .intersperse(Some(", ".to_string())) .collect::>())); result.push_str("> "); From 6d14ac84a40a4eae077a691de4baae885f97ea1c Mon Sep 17 00:00:00 2001 From: topecongiro Date: Wed, 3 May 2017 11:26:31 +0900 Subject: [PATCH 5/6] Update tests --- tests/source/chains.rs | 2 ++ tests/source/closure.rs | 15 +++++++++++++++ tests/source/configs-chain_indent-block.rs | 2 +- tests/source/configs-chain_indent-visual.rs | 2 +- tests/target/chains.rs | 3 +++ tests/target/closure.rs | 13 +++++++++++++ tests/target/configs-chain_indent-block.rs | 2 +- tests/target/configs-chain_indent-visual.rs | 2 +- 8 files changed, 37 insertions(+), 4 deletions(-) diff --git a/tests/source/chains.rs b/tests/source/chains.rs index 5987195e031..20d320ccde8 100644 --- a/tests/source/chains.rs +++ b/tests/source/chains.rs @@ -13,6 +13,8 @@ fn main() { bbbbbbbbbbbbbbbbbbb.ccccccccccccccccccccccccccccccccccccc.ddddddddddddddddddddddddddd.eeeeeeee(); + let f = fooooooooooooooooooooooooooooooooooooooooooooooooooo.baaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaar; + // Test case where first chain element isn't a path, but is shorter than // the size of a tab. x() diff --git a/tests/source/closure.rs b/tests/source/closure.rs index c9968a996f4..f6e8c093082 100644 --- a/tests/source/closure.rs +++ b/tests/source/closure.rs @@ -89,3 +89,18 @@ fn foo() { }; }); } + +fn issue1405() { + open_raw_fd(fd, b'r') + .and_then(|file| Capture::new_raw(None, |_, err| unsafe { + raw::pcap_fopen_offline(file, err) + })); +} + +fn issue1466() { + let vertex_buffer = frame.scope(|ctx| { + let buffer = + ctx.create_host_visible_buffer::>(&vertices); + ctx.create_device_local_buffer(buffer) + }); +} diff --git a/tests/source/configs-chain_indent-block.rs b/tests/source/configs-chain_indent-block.rs index 264e96625fe..d77709422b2 100644 --- a/tests/source/configs-chain_indent-block.rs +++ b/tests/source/configs-chain_indent-block.rs @@ -2,5 +2,5 @@ // Chain indent fn main() { - let lorem = ipsum.dolor().sit().amet().consectetur().adipiscing().elit(); + let lorem = ipsum.dolor().sit().amet().consectetur().adipiscing().elite(); } diff --git a/tests/source/configs-chain_indent-visual.rs b/tests/source/configs-chain_indent-visual.rs index 3c85400daa4..67714d32045 100644 --- a/tests/source/configs-chain_indent-visual.rs +++ b/tests/source/configs-chain_indent-visual.rs @@ -2,5 +2,5 @@ // Chain indent fn main() { - let lorem = ipsum.dolor().sit().amet().consectetur().adipiscing().elit(); + let lorem = ipsum.dolor().sit().amet().consectetur().adipiscing().elite(); } diff --git a/tests/target/chains.rs b/tests/target/chains.rs index 2b14ec147d4..b8e47607b83 100644 --- a/tests/target/chains.rs +++ b/tests/target/chains.rs @@ -13,6 +13,9 @@ fn main() { .ddddddddddddddddddddddddddd .eeeeeeee(); + let f = fooooooooooooooooooooooooooooooooooooooooooooooooooo + .baaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaar; + // Test case where first chain element isn't a path, but is shorter than // the size of a tab. x().y(|| match cond() { diff --git a/tests/target/closure.rs b/tests/target/closure.rs index c9edec799e2..f6d9a855a33 100644 --- a/tests/target/closure.rs +++ b/tests/target/closure.rs @@ -106,3 +106,16 @@ fn foo() { }; }); } + +fn issue1405() { + open_raw_fd(fd, b'r').and_then(|file| { + Capture::new_raw(None, |_, err| unsafe { raw::pcap_fopen_offline(file, err) }) + }); +} + +fn issue1466() { + let vertex_buffer = frame.scope(|ctx| { + let buffer = ctx.create_host_visible_buffer::>(&vertices); + ctx.create_device_local_buffer(buffer) + }); +} diff --git a/tests/target/configs-chain_indent-block.rs b/tests/target/configs-chain_indent-block.rs index 89357e55801..b172e293b72 100644 --- a/tests/target/configs-chain_indent-block.rs +++ b/tests/target/configs-chain_indent-block.rs @@ -8,5 +8,5 @@ fn main() { .amet() .consectetur() .adipiscing() - .elit(); + .elite(); } diff --git a/tests/target/configs-chain_indent-visual.rs b/tests/target/configs-chain_indent-visual.rs index 158ad432ffa..ef7dac93f7f 100644 --- a/tests/target/configs-chain_indent-visual.rs +++ b/tests/target/configs-chain_indent-visual.rs @@ -7,5 +7,5 @@ fn main() { .amet() .consectetur() .adipiscing() - .elit(); + .elite(); } From 58d957be3fd559787b7f489ee5f1089ce9a87f30 Mon Sep 17 00:00:00 2001 From: topecongiro Date: Thu, 4 May 2017 00:21:51 +0900 Subject: [PATCH 6/6] Check format failures explicitly in visit_block --- src/expr.rs | 24 ++++++------------------ src/visitor.rs | 5 +++++ 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/src/expr.rs b/src/expr.rs index 75a7ff21abe..5876409a9fa 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -570,19 +570,6 @@ fn rewrite_closure(capture: ast::CaptureBy, rewrite.map(|rw| format!("{} {}", prefix, rw)) } - fn no_weird_visual_indent(block_str: &str, context: &RewriteContext) -> bool { - let mut prev_indent_width = 0; - for line in block_str.lines() { - let cur_indent_width = line.find(|c: char| !c.is_whitespace()).unwrap_or(0); - if prev_indent_width > cur_indent_width + context.config.tab_spaces && - line.find('}').unwrap_or(0) != cur_indent_width { - return false; - } - prev_indent_width = cur_indent_width; - } - true - } - fn rewrite_closure_block(block: &ast::Block, prefix: String, context: &RewriteContext, @@ -592,9 +579,7 @@ fn rewrite_closure(capture: ast::CaptureBy, // closure is large. if let Some(block_str) = block.rewrite(&context, shape) { let block_threshold = context.config.closure_block_indent_threshold; - if (block_threshold < 0 || - block_str.matches('\n').count() <= block_threshold as usize) && - no_weird_visual_indent(&block_str, context) { + if block_threshold < 0 || block_str.matches('\n').count() <= block_threshold as usize { if let Some(block_str) = block_str.rewrite(context, shape) { return Some(format!("{} {}", prefix, block_str)); } @@ -697,8 +682,11 @@ impl Rewrite for ast::Block { }; visitor.visit_block(self); - - Some(format!("{}{}", prefix, visitor.buffer)) + if visitor.failed { + None + } else { + Some(format!("{}{}", prefix, visitor.buffer)) + } } } diff --git a/src/visitor.rs b/src/visitor.rs index 86fb1bce9d9..71013d085d5 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -39,6 +39,7 @@ pub struct FmtVisitor<'a> { // FIXME: use an RAII util or closure for indenting pub block_indent: Indent, pub config: &'a Config, + pub failed: bool, } impl<'a> FmtVisitor<'a> { @@ -65,6 +66,9 @@ impl<'a> FmtVisitor<'a> { Shape::legacy(self.config.max_width - self.block_indent.width(), self.block_indent)); + if rewrite.is_none() { + self.failed = true; + } self.push_rewrite(stmt.span, rewrite); } ast::StmtKind::Mac(ref mac) => { @@ -457,6 +461,7 @@ impl<'a> FmtVisitor<'a> { alignment: 0, }, config: config, + failed: false, } }