diff --git a/src/chains.rs b/src/chains.rs index eb9771a2356a6..be38300619641 100644 --- a/src/chains.rs +++ b/src/chains.rs @@ -447,7 +447,7 @@ trait ChainFormatter { // Parent is the first item in the chain, e.g., `foo` in `foo.bar.baz()`. // Root is the parent plus any other chain items placed on the first line to // avoid an orphan. E.g., - // ``` + // ```ignore // foo.bar // .baz() // ``` @@ -509,7 +509,7 @@ impl<'a> ChainFormatterShared<'a> { // know whether 'overflowing' the last child make a better formatting: // // A chain with overflowing the last child: - // ``` + // ```ignore // parent.child1.child2.last_child( // a, // b, @@ -518,7 +518,7 @@ impl<'a> ChainFormatterShared<'a> { // ``` // // A chain without overflowing the last child (in vertical layout): - // ``` + // ```ignore // parent // .child1 // .child2 @@ -527,7 +527,7 @@ impl<'a> ChainFormatterShared<'a> { // // In particular, overflowing is effective when the last child is a method with a multi-lined // block-like argument (e.g. closure): - // ``` + // ```ignore // parent.child1.child2.last_child(|a, b, c| { // let x = foo(a, b, c); // let y = bar(a, b, c); diff --git a/src/comment.rs b/src/comment.rs index 561989a1a11bb..9dd7d68965598 100644 --- a/src/comment.rs +++ b/src/comment.rs @@ -488,70 +488,75 @@ impl ItemizedBlock { } } -fn rewrite_comment_inner( - orig: &str, - block_style: bool, - style: CommentStyle, - shape: Shape, - config: &Config, - is_doc_comment: bool, -) -> Option { - let (opener, closer, line_start) = if block_style { - CommentStyle::SingleBullet.to_str_tuplet() - } else { - comment_style(orig, config.normalize_comments()).to_str_tuplet() - }; +struct CommentRewrite<'a> { + result: String, + code_block_buffer: String, + is_prev_line_multi_line: bool, + code_block_attr: Option, + item_block_buffer: String, + item_block: Option, + comment_line_separator: String, + indent_str: String, + max_chars: usize, + fmt_indent: Indent, + fmt: StringFormat<'a>, - let max_chars = shape - .width - .checked_sub(closer.len() + opener.len()) - .unwrap_or(1); - let indent_str = shape.indent.to_string_with_newline(config); - let fmt_indent = shape.indent + (opener.len() - line_start.len()); - let mut fmt = StringFormat { - opener: "", - closer: "", - line_start, - line_end: "", - shape: Shape::legacy(max_chars, fmt_indent), - trim_end: true, - config, - }; + opener: String, + closer: String, + line_start: String, +} - let line_breaks = count_newlines(orig.trim_right()); - let lines = orig - .lines() - .enumerate() - .map(|(i, mut line)| { - line = trim_right_unless_two_whitespaces(line.trim_left(), is_doc_comment); - // Drop old closer. - if i == line_breaks && line.ends_with("*/") && !line.starts_with("//") { - line = line[..(line.len() - 2)].trim_right(); - } +impl<'a> CommentRewrite<'a> { + fn new( + orig: &'a str, + block_style: bool, + shape: Shape, + config: &'a Config, + ) -> CommentRewrite<'a> { + let (opener, closer, line_start) = if block_style { + CommentStyle::SingleBullet.to_str_tuplet() + } else { + comment_style(orig, config.normalize_comments()).to_str_tuplet() + }; - line - }) - .map(|s| left_trim_comment_line(s, &style)) - .map(|(line, has_leading_whitespace)| { - if orig.starts_with("/*") && line_breaks == 0 { - ( - line.trim_left(), - has_leading_whitespace || config.normalize_comments(), - ) - } else { - (line, has_leading_whitespace || config.normalize_comments()) - } - }); + let max_chars = shape + .width + .checked_sub(closer.len() + opener.len()) + .unwrap_or(1); + let indent_str = shape.indent.to_string_with_newline(config).to_string(); + let fmt_indent = shape.indent + (opener.len() - line_start.len()); + + let mut cr = CommentRewrite { + result: String::with_capacity(orig.len() * 2), + code_block_buffer: String::with_capacity(128), + is_prev_line_multi_line: false, + code_block_attr: None, + item_block_buffer: String::with_capacity(128), + item_block: None, + comment_line_separator: format!("{}{}", indent_str, line_start), + max_chars, + indent_str, + fmt_indent, + + fmt: StringFormat { + opener: "", + closer: "", + line_start, + line_end: "", + shape: Shape::legacy(max_chars, fmt_indent), + trim_end: true, + config, + }, - let mut result = String::with_capacity(orig.len() * 2); - result.push_str(opener); - let mut code_block_buffer = String::with_capacity(128); - let mut is_prev_line_multi_line = false; - let mut code_block_attr = None; - let mut item_block_buffer = String::with_capacity(128); - let mut item_block: Option = None; - let comment_line_separator = format!("{}{}", indent_str, line_start); - let join_block = |s: &str, sep: &str| { + opener: opener.to_owned(), + closer: closer.to_owned(), + line_start: line_start.to_owned(), + }; + cr.result.push_str(opener); + cr + } + + fn join_block(s: &str, sep: &str) -> String { let mut result = String::with_capacity(s.len() + 128); let mut iter = s.lines().peekable(); while let Some(line) = iter.next() { @@ -563,186 +568,252 @@ fn rewrite_comment_inner( }); } result - }; + } - for (i, (line, has_leading_whitespace)) in lines.enumerate() { + fn finish(mut self) -> String { + if !self.code_block_buffer.is_empty() { + // There is a code block that is not properly enclosed by backticks. + // We will leave them untouched. + self.result.push_str(&self.comment_line_separator); + self.result.push_str(&Self::join_block( + &trim_custom_comment_prefix(&self.code_block_buffer), + &self.comment_line_separator, + )); + } + + if !self.item_block_buffer.is_empty() { + // the last few lines are part of an itemized block + self.fmt.shape = Shape::legacy(self.max_chars, self.fmt_indent); + let mut ib = None; + ::std::mem::swap(&mut ib, &mut self.item_block); + let ib = ib.unwrap(); + let item_fmt = ib.create_string_format(&self.fmt); + self.result.push_str(&self.comment_line_separator); + self.result.push_str(&ib.opener); + match rewrite_string( + &self.item_block_buffer.replace("\n", " "), + &item_fmt, + self.max_chars.saturating_sub(ib.indent), + ) { + Some(s) => self.result.push_str(&Self::join_block( + &s, + &format!("{}{}", &self.comment_line_separator, ib.line_start), + )), + None => self.result.push_str(&Self::join_block( + &self.item_block_buffer, + &self.comment_line_separator, + )), + }; + } + + self.result.push_str(&self.closer); + if self.result.ends_with(&self.opener) && self.opener.ends_with(' ') { + // Trailing space. + self.result.pop(); + } + + self.result + } + + fn handle_line( + &mut self, + orig: &'a str, + i: usize, + line: &'a str, + has_leading_whitespace: bool, + ) -> bool { let is_last = i == count_newlines(orig); - if let Some(ref ib) = item_block { + if let Some(ref ib) = self.item_block { if ib.in_block(&line) { - item_block_buffer.push_str(&line); - item_block_buffer.push('\n'); - continue; + self.item_block_buffer.push_str(&line); + self.item_block_buffer.push('\n'); + return false; } - is_prev_line_multi_line = false; - fmt.shape = Shape::legacy(max_chars, fmt_indent); - let item_fmt = ib.create_string_format(&fmt); - result.push_str(&comment_line_separator); - result.push_str(&ib.opener); + self.is_prev_line_multi_line = false; + self.fmt.shape = Shape::legacy(self.max_chars, self.fmt_indent); + let item_fmt = ib.create_string_format(&self.fmt); + self.result.push_str(&self.comment_line_separator); + self.result.push_str(&ib.opener); match rewrite_string( - &item_block_buffer.replace("\n", " "), + &self.item_block_buffer.replace("\n", " "), &item_fmt, - max_chars.saturating_sub(ib.indent), + self.max_chars.saturating_sub(ib.indent), ) { - Some(s) => result.push_str(&join_block( + Some(s) => self.result.push_str(&Self::join_block( &s, - &format!("{}{}", &comment_line_separator, ib.line_start), + &format!("{}{}", &self.comment_line_separator, ib.line_start), + )), + None => self.result.push_str(&Self::join_block( + &self.item_block_buffer, + &self.comment_line_separator, )), - None => result.push_str(&join_block(&item_block_buffer, &comment_line_separator)), }; - item_block_buffer.clear(); - } else if let Some(ref attr) = code_block_attr { + self.item_block_buffer.clear(); + } else if self.code_block_attr.is_some() { if line.starts_with("```") { - let code_block = match attr { + let code_block = match self.code_block_attr.as_ref().unwrap() { CodeBlockAttribute::Ignore | CodeBlockAttribute::Text => { - trim_custom_comment_prefix(&code_block_buffer) + trim_custom_comment_prefix(&self.code_block_buffer) } - _ if code_block_buffer.is_empty() => String::new(), + _ if self.code_block_buffer.is_empty() => String::new(), _ => { - let mut config = config.clone(); + let mut config = self.fmt.config.clone(); config.set().format_doc_comments(false); - match ::format_code_block(&code_block_buffer, &config) { + match ::format_code_block(&self.code_block_buffer, &config) { Some(ref s) => trim_custom_comment_prefix(s), - None => trim_custom_comment_prefix(&code_block_buffer), + None => trim_custom_comment_prefix(&self.code_block_buffer), } } }; if !code_block.is_empty() { - result.push_str(&comment_line_separator); - result.push_str(&join_block(&code_block, &comment_line_separator)); + self.result.push_str(&self.comment_line_separator); + self.result + .push_str(&Self::join_block(&code_block, &self.comment_line_separator)); } - code_block_buffer.clear(); - result.push_str(&comment_line_separator); - result.push_str(line); - code_block_attr = None; + self.code_block_buffer.clear(); + self.result.push_str(&self.comment_line_separator); + self.result.push_str(line); + self.code_block_attr = None; } else { - code_block_buffer.push_str(&hide_sharp_behind_comment(line)); - code_block_buffer.push('\n'); + self.code_block_buffer + .push_str(&hide_sharp_behind_comment(line)); + self.code_block_buffer.push('\n'); } - continue; + return false; } - code_block_attr = None; - item_block = None; + self.code_block_attr = None; + self.item_block = None; if line.starts_with("```") { - code_block_attr = Some(CodeBlockAttribute::new(&line[3..])) - } else if config.wrap_comments() && ItemizedBlock::is_itemized_line(&line) { + self.code_block_attr = Some(CodeBlockAttribute::new(&line[3..])) + } else if self.fmt.config.wrap_comments() && ItemizedBlock::is_itemized_line(&line) { let ib = ItemizedBlock::new(&line); - item_block_buffer.push_str(&line[ib.indent..]); - item_block_buffer.push('\n'); - item_block = Some(ib); - continue; + self.item_block_buffer.push_str(&line[ib.indent..]); + self.item_block_buffer.push('\n'); + self.item_block = Some(ib); + return false; } - if result == opener { - let force_leading_whitespace = opener == "/* " && count_newlines(orig) == 0; - if !has_leading_whitespace && !force_leading_whitespace && result.ends_with(' ') { - result.pop(); + if self.result == self.opener { + let force_leading_whitespace = &self.opener == "/* " && count_newlines(orig) == 0; + if !has_leading_whitespace && !force_leading_whitespace && self.result.ends_with(' ') { + self.result.pop(); } if line.is_empty() { - continue; + return false; } - } else if is_prev_line_multi_line && !line.is_empty() { - result.push(' ') + } else if self.is_prev_line_multi_line && !line.is_empty() { + self.result.push(' ') } else if is_last && line.is_empty() { // trailing blank lines are unwanted - if !closer.is_empty() { - result.push_str(&indent_str); + if !self.closer.is_empty() { + self.result.push_str(&self.indent_str); } - break; + return true; } else { - result.push_str(&comment_line_separator); - if !has_leading_whitespace && result.ends_with(' ') { - result.pop(); + self.result.push_str(&self.comment_line_separator); + if !has_leading_whitespace && self.result.ends_with(' ') { + self.result.pop(); } } - if config.wrap_comments() && line.len() > fmt.shape.width && !has_url(line) { - match rewrite_string(line, &fmt, max_chars) { + if self.fmt.config.wrap_comments() && line.len() > self.fmt.shape.width && !has_url(line) { + match rewrite_string(line, &self.fmt, self.max_chars) { Some(ref s) => { - is_prev_line_multi_line = s.contains('\n'); - result.push_str(s); + self.is_prev_line_multi_line = s.contains('\n'); + self.result.push_str(s); } - None if is_prev_line_multi_line => { + None if self.is_prev_line_multi_line => { // We failed to put the current `line` next to the previous `line`. // Remove the trailing space, then start rewrite on the next line. - result.pop(); - result.push_str(&comment_line_separator); - fmt.shape = Shape::legacy(max_chars, fmt_indent); - match rewrite_string(line, &fmt, max_chars) { + self.result.pop(); + self.result.push_str(&self.comment_line_separator); + self.fmt.shape = Shape::legacy(self.max_chars, self.fmt_indent); + match rewrite_string(line, &self.fmt, self.max_chars) { Some(ref s) => { - is_prev_line_multi_line = s.contains('\n'); - result.push_str(s); + self.is_prev_line_multi_line = s.contains('\n'); + self.result.push_str(s); } None => { - is_prev_line_multi_line = false; - result.push_str(line); + self.is_prev_line_multi_line = false; + self.result.push_str(line); } } } None => { - is_prev_line_multi_line = false; - result.push_str(line); + self.is_prev_line_multi_line = false; + self.result.push_str(line); } } - fmt.shape = if is_prev_line_multi_line { + self.fmt.shape = if self.is_prev_line_multi_line { // 1 = " " - let offset = 1 + last_line_width(&result) - line_start.len(); + let offset = 1 + last_line_width(&self.result) - self.line_start.len(); Shape { - width: max_chars.saturating_sub(offset), - indent: fmt_indent, - offset: fmt.shape.offset + offset, + width: self.max_chars.saturating_sub(offset), + indent: self.fmt_indent, + offset: self.fmt.shape.offset + offset, } } else { - Shape::legacy(max_chars, fmt_indent) + Shape::legacy(self.max_chars, self.fmt_indent) }; } else { - if line.is_empty() && result.ends_with(' ') && !is_last { + if line.is_empty() && self.result.ends_with(' ') && !is_last { // Remove space if this is an empty comment or a doc comment. - result.pop(); + self.result.pop(); } - result.push_str(line); - fmt.shape = Shape::legacy(max_chars, fmt_indent); - is_prev_line_multi_line = false; + self.result.push_str(line); + self.fmt.shape = Shape::legacy(self.max_chars, self.fmt_indent); + self.is_prev_line_multi_line = false; } + + false } - if !code_block_buffer.is_empty() { - // There is a code block that is not properly enclosed by backticks. - // We will leave them untouched. - result.push_str(&comment_line_separator); - result.push_str(&join_block( - &trim_custom_comment_prefix(&code_block_buffer), - &comment_line_separator, - )); - } - if !item_block_buffer.is_empty() { - // the last few lines are part of an itemized block - let ib = item_block.unwrap(); - fmt.shape = Shape::legacy(max_chars, fmt_indent); - let item_fmt = ib.create_string_format(&fmt); - result.push_str(&comment_line_separator); - result.push_str(&ib.opener); - match rewrite_string( - &item_block_buffer.replace("\n", " "), - &item_fmt, - max_chars.saturating_sub(ib.indent), - ) { - Some(s) => result.push_str(&join_block( - &s, - &format!("{}{}", &comment_line_separator, ib.line_start), - )), - None => result.push_str(&join_block(&item_block_buffer, &comment_line_separator)), - }; - } +} - result.push_str(closer); - if result.ends_with(opener) && opener.ends_with(' ') { - // Trailing space. - result.pop(); +fn rewrite_comment_inner( + orig: &str, + block_style: bool, + style: CommentStyle, + shape: Shape, + config: &Config, + is_doc_comment: bool, +) -> Option { + let mut rewriter = CommentRewrite::new(orig, block_style, shape, config); + + let line_breaks = count_newlines(orig.trim_right()); + let lines = orig + .lines() + .enumerate() + .map(|(i, mut line)| { + line = trim_right_unless_two_whitespaces(line.trim_left(), is_doc_comment); + // Drop old closer. + if i == line_breaks && line.ends_with("*/") && !line.starts_with("//") { + line = line[..(line.len() - 2)].trim_right(); + } + + line + }) + .map(|s| left_trim_comment_line(s, &style)) + .map(|(line, has_leading_whitespace)| { + if orig.starts_with("/*") && line_breaks == 0 { + ( + line.trim_left(), + has_leading_whitespace || config.normalize_comments(), + ) + } else { + (line, has_leading_whitespace || config.normalize_comments()) + } + }); + + for (i, (line, has_leading_whitespace)) in lines.enumerate() { + if rewriter.handle_line(orig, i, line, has_leading_whitespace) { + break; + } } - Some(result) + Some(rewriter.finish()) } const RUSTFMT_CUSTOM_COMMENT_PREFIX: &str = "//#### "; @@ -957,35 +1028,6 @@ pub fn contains_comment(text: &str) -> bool { CharClasses::new(text.chars()).any(|(kind, _)| kind.is_comment()) } -/// Remove trailing spaces from the specified snippet. We do not remove spaces -/// inside strings or comments. -pub fn remove_trailing_white_spaces(text: &str) -> String { - let mut buffer = String::with_capacity(text.len()); - let mut space_buffer = String::with_capacity(128); - for (char_kind, c) in CharClasses::new(text.chars()) { - match c { - '\n' => { - if char_kind == FullCodeCharKind::InString { - buffer.push_str(&space_buffer); - } - space_buffer.clear(); - buffer.push('\n'); - } - _ if c.is_whitespace() => { - space_buffer.push(c); - } - _ => { - if !space_buffer.is_empty() { - buffer.push_str(&space_buffer); - space_buffer.clear(); - } - buffer.push(c); - } - } - } - buffer -} - pub struct CharClasses where T: Iterator, @@ -1780,12 +1822,6 @@ mod test { check("\"/* abc", "abc", Some(4)); } - #[test] - fn test_remove_trailing_white_spaces() { - let s = " r#\"\n test\n \"#"; - assert_eq!(remove_trailing_white_spaces(&s), s); - } - #[test] fn test_filter_normal_code() { let s = r#" diff --git a/src/formatting.rs b/src/formatting.rs index d940c35f1c734..d2748b35736ce 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -528,7 +528,8 @@ impl<'a> FormatLines<'a> { && !self.is_skipped_line() && self.should_report_error(kind, &error_kind) { - self.push_err(error_kind, kind.is_comment(), self.is_string); + let is_string = self.is_string; + self.push_err(error_kind, kind.is_comment(), is_string); } } diff --git a/src/lib.rs b/src/lib.rs index a8958abab39df..ff006325be0f6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -8,8 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -#![feature(nll)] - #[macro_use] extern crate derive_new; extern crate atty; diff --git a/src/macros.rs b/src/macros.rs index a7a6a7fc6b7a8..10a037e35a3b0 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -32,10 +32,7 @@ use syntax::tokenstream::{Cursor, ThinTokenStream, TokenStream, TokenTree}; use syntax::ThinVec; use syntax::{ast, ptr}; -use comment::{ - contains_comment, remove_trailing_white_spaces, CharClasses, FindUncommented, FullCodeCharKind, - LineClasses, -}; +use comment::{contains_comment, CharClasses, FindUncommented, FullCodeCharKind, LineClasses}; use expr::rewrite_array; use lists::{itemize_list, write_list, ListFormatting}; use overflow; @@ -43,7 +40,7 @@ use rewrite::{Rewrite, RewriteContext}; use shape::{Indent, Shape}; use source_map::SpanUtils; use spanned::Spanned; -use utils::{format_visibility, mk_sp, rewrite_ident, wrap_str}; +use utils::{format_visibility, mk_sp, remove_trailing_white_spaces, rewrite_ident, wrap_str}; use visitor::FmtVisitor; const FORCED_BRACKET_MACROS: &[&str] = &["vec!"]; diff --git a/src/test/mod.rs b/src/test/mod.rs index 1967c74da4515..aa3e729bb43a4 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -281,7 +281,7 @@ fn stdin_formatting_smoke_test() { session.format(input).unwrap(); assert!(session.has_no_errors()); } - //eprintln!("{:?}", ); + #[cfg(not(windows))] assert_eq!(buf, "fn main() {}\n".as_bytes()); #[cfg(windows)] diff --git a/src/utils.rs b/src/utils.rs index 3d6f1ad230ddb..d90bcc5931dc0 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -20,7 +20,7 @@ use syntax::ast::{ use syntax::ptr; use syntax::source_map::{BytePos, Span, NO_EXPANSION}; -use comment::filter_normal_code; +use comment::{filter_normal_code, CharClasses, FullCodeCharKind}; use rewrite::RewriteContext; use shape::Shape; @@ -452,3 +452,38 @@ pub fn is_block_expr(context: &RewriteContext, expr: &ast::Expr, repr: &str) -> _ => false, } } + +/// Remove trailing spaces from the specified snippet. We do not remove spaces +/// inside strings or comments. +pub fn remove_trailing_white_spaces(text: &str) -> String { + let mut buffer = String::with_capacity(text.len()); + let mut space_buffer = String::with_capacity(128); + for (char_kind, c) in CharClasses::new(text.chars()) { + match c { + '\n' => { + if char_kind == FullCodeCharKind::InString { + buffer.push_str(&space_buffer); + } + space_buffer.clear(); + buffer.push('\n'); + } + _ if c.is_whitespace() => { + space_buffer.push(c); + } + _ => { + if !space_buffer.is_empty() { + buffer.push_str(&space_buffer); + space_buffer.clear(); + } + buffer.push(c); + } + } + } + buffer +} + +#[test] +fn test_remove_trailing_white_spaces() { + let s = " r#\"\n test\n \"#"; + assert_eq!(remove_trailing_white_spaces(&s), s); +} diff --git a/src/visitor.rs b/src/visitor.rs index 63e2c6bdcac57..a6f926736ccf2 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -791,9 +791,15 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { where F: Fn(&RewriteContext) -> Option, { - let context = self.get_context(); - let result = f(&context); - self.macro_rewrite_failure |= *context.macro_rewrite_failure.borrow(); + // FIXME borrow checker fighting - can be simplified a lot with NLL. + let (result, mrf) = { + let context = self.get_context(); + let result = f(&context); + let mrf = &context.macro_rewrite_failure.borrow(); + (result, *std::ops::Deref::deref(mrf)) + }; + + self.macro_rewrite_failure |= mrf; result }