diff --git a/Cargo.toml b/Cargo.toml index 1fcbd4fc402a..097b88644fa1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,7 +7,7 @@ description = "Tool to find and fix Rust formatting issues" repository = "https://github.com/rust-lang-nursery/rustfmt" readme = "README.md" license = "Apache-2.0/MIT" -include = ["src/*.rs", "Cargo.toml", "build.rs", "LICENSE-*"] +include = ["src/**", "Cargo.toml", "build.rs", "LICENSE-*"] build = "build.rs" categories = ["development-tools"] diff --git a/src/comment.rs b/src/comment.rs index 0a716aa308c1..aa078f4dbf7c 100644 --- a/src/comment.rs +++ b/src/comment.rs @@ -153,11 +153,10 @@ pub fn combine_strs_with_missing_comments( let mut one_line_width = last_line_width(prev_str) + first_line_width(next_str) + first_sep.len(); - let original_snippet = context.snippet(span); - let trimmed_snippet = original_snippet.trim(); let indent_str = shape.indent.to_string(context.config); + let missing_comment = try_opt!(rewrite_missing_comment(span, shape, context)); - if trimmed_snippet.is_empty() { + if missing_comment.is_empty() { if allow_extend && prev_str.len() + first_sep.len() + next_str.len() <= shape.width { return Some(format!("{}{}{}", prev_str, first_sep, next_str)); } else { @@ -175,18 +174,13 @@ pub fn combine_strs_with_missing_comments( // Peek the the original source code and find out whether there is a newline between the first // expression and the second expression or the missing comment. We will preserve the orginal // layout whenever possible. + let original_snippet = context.snippet(span); let prefer_same_line = if let Some(pos) = original_snippet.chars().position(|c| c == '/') { !original_snippet[..pos].contains('\n') } else { !original_snippet.contains('\n') }; - let missing_comment = try_opt!(rewrite_comment( - trimmed_snippet, - false, - shape, - context.config - )); one_line_width -= first_sep.len(); let first_sep = if prev_str.is_empty() || missing_comment.is_empty() { String::new() @@ -365,6 +359,50 @@ fn rewrite_comment_inner( Some(result) } +/// Given the span, rewrite the missing comment inside it if available. +/// Note that the given span must only include comments (or leading/trailing whitespaces). +pub fn rewrite_missing_comment( + span: Span, + shape: Shape, + context: &RewriteContext, +) -> Option { + let missing_snippet = context.snippet(span); + let trimmed_snippet = missing_snippet.trim(); + if !trimmed_snippet.is_empty() { + rewrite_comment(trimmed_snippet, false, shape, context.config) + } else { + Some(String::new()) + } +} + +/// Recover the missing comments in the specified span, if available. +/// The layout of the comments will be preserved as long as it does not break the code +/// and its total width does not exceed the max width. +pub fn recover_missing_comment_in_span( + span: Span, + shape: Shape, + context: &RewriteContext, + used_width: usize, +) -> Option { + let missing_comment = try_opt!(rewrite_missing_comment(span, shape, context)); + if missing_comment.is_empty() { + Some(String::new()) + } else { + let missing_snippet = context.snippet(span); + let pos = missing_snippet.chars().position(|c| c == '/').unwrap_or(0); + // 1 = ` ` + let total_width = missing_comment.len() + used_width + 1; + let force_new_line_before_comment = + missing_snippet[..pos].contains('\n') || total_width > context.config.max_width(); + let sep = if force_new_line_before_comment { + format!("\n{}", shape.indent.to_string(context.config)) + } else { + String::from(" ") + }; + Some(format!("{}{}", sep, missing_comment)) + } +} + /// Trims whitespace and aligns to indent, but otherwise does not change comments. fn light_rewrite_comment(orig: &str, offset: Indent, config: &Config) -> Option { let lines: Vec<&str> = orig.lines() diff --git a/src/expr.rs b/src/expr.rs index a42ab973e0d2..1bb910b61b3a 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -20,7 +20,7 @@ use {Indent, Shape, Spanned}; use chains::rewrite_chain; use codemap::{LineRangeUtils, SpanUtils}; use comment::{combine_strs_with_missing_comments, contains_comment, recover_comment_removed, - rewrite_comment, FindUncommented}; + rewrite_comment, rewrite_missing_comment, FindUncommented}; use config::{Config, ControlBraceStyle, IndentStyle, MultilineStyle, Style}; use items::{span_hi_for_arg, span_lo_for_arg}; use lists::{definitive_tactic, itemize_list, shape_for_tactic, struct_lit_formatting, @@ -1195,9 +1195,13 @@ impl<'a> ControlFlow<'a> { mk_sp(self.block.span.lo, self.block.span.lo) }; - // for event in event + // `for event in event` + // Do not include label in the span. + let lo = self.label.map_or(self.span.lo, |label| label.span.hi); let between_kwd_cond = mk_sp( - context.codemap.span_after(self.span, self.keyword.trim()), + context + .codemap + .span_after(mk_sp(lo, self.span.hi), self.keyword.trim()), self.pat .map_or(cond_span.lo, |p| if self.matcher.is_empty() { p.span.lo @@ -1378,21 +1382,13 @@ fn rewrite_label(label: Option) -> String { } fn extract_comment(span: Span, context: &RewriteContext, shape: Shape) -> Option { - let comment_str = context.snippet(span); - if contains_comment(&comment_str) { - let comment = try_opt!(rewrite_comment( - comment_str.trim(), - false, - shape, - context.config, - )); - Some(format!( + match rewrite_missing_comment(span, shape, context) { + Some(ref comment) if !comment.is_empty() => Some(format!( "\n{indent}{}\n{indent}", comment, indent = shape.indent.to_string(context.config) - )) - } else { - None + )), + _ => None, } } diff --git a/src/items.rs b/src/items.rs index b1636ef0123a..94a13a502eb6 100644 --- a/src/items.rs +++ b/src/items.rs @@ -19,7 +19,7 @@ use syntax::codemap::{BytePos, Span}; use {Indent, Shape, Spanned}; use codemap::{LineRangeUtils, SpanUtils}; use comment::{combine_strs_with_missing_comments, contains_comment, recover_comment_removed, - rewrite_comment, FindUncommented}; + recover_missing_comment_in_span, rewrite_missing_comment, FindUncommented}; use config::{BraceStyle, Config, Density, IndentStyle, ReturnIndent, Style}; use expr::{format_expr, is_empty_block, is_simple_block_stmt, rewrite_assign_rhs, rewrite_call_inner, ExprType}; @@ -66,7 +66,7 @@ impl Rewrite for ast::Local { // String that is placed within the assignment pattern and expression. let infix = { - let mut infix = String::new(); + let mut infix = String::with_capacity(32); if let Some(ref ty) = self.ty { let separator = type_annotation_separator(context.config); @@ -517,8 +517,10 @@ pub fn format_impl( where_span_end: Option, ) -> Option { if let ast::ItemKind::Impl(_, _, _, ref generics, _, ref self_ty, ref items) = item.node { - let mut result = String::new(); + let mut result = String::with_capacity(128); let ref_and_type = try_opt!(format_impl_ref_and_type(context, item, offset)); + let indent_str = offset.to_string(context.config); + let sep = format!("\n{}", &indent_str); result.push_str(&ref_and_type); let where_budget = if result.contains('\n') { @@ -543,6 +545,24 @@ pub fn format_impl( option, )); + // If there is no where clause, we may have missing comments between the trait name and + // the opening brace. + if generics.where_clause.predicates.is_empty() { + if let Some(hi) = where_span_end { + match recover_missing_comment_in_span( + mk_sp(self_ty.span.hi, hi), + Shape::indented(offset, context.config), + context, + last_line_width(&result), + ) { + Some(ref missing_comment) if !missing_comment.is_empty() => { + result.push_str(missing_comment); + } + _ => (), + } + } + } + if try_opt!(is_impl_single_line( context, &items, @@ -551,9 +571,8 @@ pub fn format_impl( &item, )) { result.push_str(&where_clause_str); - if where_clause_str.contains('\n') { - let white_space = offset.to_string(context.config); - result.push_str(&format!("\n{}{{\n{}}}", &white_space, &white_space)); + if where_clause_str.contains('\n') || last_line_contains_single_line_comment(&result) { + result.push_str(&format!("{}{{{}}}", &sep, &sep)); } else { result.push_str(" {}"); } @@ -569,14 +588,11 @@ pub fn format_impl( result.push_str(&where_clause_str); match context.config.item_brace_style() { - BraceStyle::AlwaysNextLine => { - result.push('\n'); - result.push_str(&offset.to_string(context.config)); - } + _ if last_line_contains_single_line_comment(&result) => result.push_str(&sep), + BraceStyle::AlwaysNextLine => result.push_str(&sep), BraceStyle::PreferSameLine => result.push(' '), BraceStyle::SameLineWhere => if !where_clause_str.is_empty() { - result.push('\n'); - result.push_str(&offset.to_string(context.config)); + result.push_str(&sep); } else { result.push(' '); }, @@ -610,8 +626,7 @@ pub fn format_impl( } if result.chars().last().unwrap() == '{' { - result.push('\n'); - result.push_str(&offset.to_string(context.config)); + result.push_str(&sep); } result.push('}'); @@ -632,7 +647,7 @@ fn is_impl_single_line( let open_pos = try_opt!(snippet.find_uncommented("{")) + 1; Some( - context.config.impl_empty_single_line() && items.is_empty() && + context.config.impl_empty_single_line() && items.is_empty() && !result.contains('\n') && result.len() + where_clause_str.len() <= context.config.max_width() && !contains_comment(&snippet[open_pos..]), ) @@ -653,7 +668,7 @@ fn format_impl_ref_and_type( _, ) = item.node { - let mut result = String::new(); + let mut result = String::with_capacity(128); result.push_str(&format_visibility(&item.vis)); result.push_str(&format_defaultness(defaultness)); @@ -853,7 +868,7 @@ pub fn format_trait(context: &RewriteContext, item: &ast::Item, offset: Indent) if let ast::ItemKind::Trait(unsafety, ref generics, ref type_param_bounds, ref trait_items) = item.node { - let mut result = String::new(); + let mut result = String::with_capacity(128); let header = format!( "{}{}trait {}", format_visibility(&item.vis), @@ -910,14 +925,7 @@ pub fn format_trait(context: &RewriteContext, item: &ast::Item, offset: Indent) .checked_sub(last_line_width(&result)) ); let pos_before_where = if type_param_bounds.is_empty() { - if generics.where_clause.predicates.is_empty() { - // We do not use this, so it does not matter - item.span.lo - } else { - let snippet = context.snippet(item.span); - let where_pos = snippet.find_uncommented("where"); - item.span.lo + where_pos.map_or(BytePos(0), |p| BytePos(p as u32)) - } + generics.where_clause.span.lo } else { type_param_bounds[type_param_bounds.len() - 1].span().hi }; @@ -946,7 +954,33 @@ pub fn format_trait(context: &RewriteContext, item: &ast::Item, offset: Indent) } result.push_str(&where_clause_str); + if generics.where_clause.predicates.is_empty() { + let item_snippet = context.snippet(item.span); + if let Some(lo) = item_snippet.chars().position(|c| c == '/') { + // 1 = `{` + let comment_hi = body_lo - BytePos(1); + let comment_lo = item.span.lo + BytePos(lo as u32); + if comment_lo < comment_hi { + match recover_missing_comment_in_span( + mk_sp(comment_lo, comment_hi), + Shape::indented(offset, context.config), + context, + last_line_width(&result), + ) { + Some(ref missing_comment) if !missing_comment.is_empty() => { + result.push_str(missing_comment); + } + _ => (), + } + } + } + } + match context.config.item_brace_style() { + _ if last_line_contains_single_line_comment(&result) => { + result.push('\n'); + result.push_str(&offset.to_string(context.config)); + } BraceStyle::AlwaysNextLine => { result.push('\n'); result.push_str(&offset.to_string(context.config)); @@ -1231,7 +1265,7 @@ pub fn rewrite_type_alias( vis: &ast::Visibility, span: Span, ) -> Option { - let mut result = String::new(); + let mut result = String::with_capacity(128); result.push_str(&format_visibility(vis)); result.push_str("type "); @@ -2006,33 +2040,17 @@ fn rewrite_fn_base( // args and `{`. if where_clause_str.is_empty() { if let ast::FunctionRetTy::Default(ret_span) = fd.output { - let sp = mk_sp(args_span.hi, ret_span.hi); - let missing_snippet = context.snippet(sp); - let trimmed_snippet = missing_snippet.trim(); - let missing_comment = if trimmed_snippet.is_empty() { - String::new() - } else { - try_opt!(rewrite_comment( - trimmed_snippet, - false, - Shape::indented(indent, context.config), - context.config, - )) - }; - if !missing_comment.is_empty() { - let pos = missing_snippet.chars().position(|c| c == '/').unwrap_or(0); - // 1 = ` ` - let total_width = missing_comment.len() + last_line_width(&result) + 1; - let force_new_line_before_comment = missing_snippet[..pos].contains('\n') || - total_width > context.config.max_width(); - let sep = if force_new_line_before_comment { - format!("\n{}", indent.to_string(context.config)) - } else { - String::from(" ") - }; - result.push_str(&sep); - result.push_str(&missing_comment); - force_new_line_for_brace = true; + match recover_missing_comment_in_span( + mk_sp(args_span.hi, ret_span.hi), + shape, + context, + last_line_width(&result), + ) { + Some(ref missing_comment) if !missing_comment.is_empty() => { + result.push_str(missing_comment); + force_new_line_for_brace = true; + } + _ => (), } } } @@ -2683,34 +2701,17 @@ fn missing_span_before_after_where( (missing_span_before, missing_span_after) } -fn rewrite_missing_comment_in_where( - context: &RewriteContext, - comment: &str, - shape: Shape, -) -> Option { - let comment = comment.trim(); - if comment.is_empty() { - Some(String::new()) - } else { - rewrite_comment(comment, false, shape, context.config) - } -} - fn rewrite_comments_before_after_where( context: &RewriteContext, span_before_where: Span, span_after_where: Span, shape: Shape, ) -> Option<(String, String)> { - let before_comment = try_opt!(rewrite_missing_comment_in_where( - context, - &context.snippet(span_before_where), - shape, - )); - let after_comment = try_opt!(rewrite_missing_comment_in_where( - context, - &context.snippet(span_after_where), + let before_comment = try_opt!(rewrite_missing_comment(span_before_where, shape, context)); + let after_comment = try_opt!(rewrite_missing_comment( + span_after_where, shape.block_indent(context.config.tab_spaces()), + context, )); Some((before_comment, after_comment)) } diff --git a/src/visitor.rs b/src/visitor.rs index bf1afb0c3f09..f07dad4a6ba5 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -18,7 +18,8 @@ use syntax::parse::ParseSess; use {Indent, Shape, Spanned}; use codemap::{LineRangeUtils, SpanUtils}; -use comment::{contains_comment, CodeCharKind, CommentCodeSlices, FindUncommented}; +use comment::{contains_comment, recover_missing_comment_in_span, CodeCharKind, CommentCodeSlices, + FindUncommented}; use comment::rewrite_comment; use config::{BraceStyle, Config}; use expr::{format_expr, ExprType}; @@ -919,15 +920,17 @@ impl Rewrite for ast::Attribute { impl<'a> Rewrite for [ast::Attribute] { fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option { - let mut result = String::new(); if self.is_empty() { - return Some(result); + return Some(String::new()); } + let mut result = String::with_capacity(128); let indent = shape.indent.to_string(context.config); let mut derive_args = Vec::new(); let mut iter = self.iter().enumerate().peekable(); + let mut insert_new_line = true; + let mut is_prev_sugared_doc = false; while let Some((i, a)) = iter.next() { let a_str = try_opt!(a.rewrite(context, shape)); @@ -937,31 +940,61 @@ impl<'a> Rewrite for [ast::Attribute] { // This particular horror show is to preserve line breaks in between doc // comments. An alternative would be to force such line breaks to start // with the usual doc comment token. - let multi_line = a_str.starts_with("//") && comment.matches('\n').count() > 1; - let comment = comment.trim(); + let (multi_line_before, multi_line_after) = if a.is_sugared_doc || + is_prev_sugared_doc + { + // Look at before and after comment and see if there are any empty lines. + let comment_begin = comment.chars().position(|c| c == '/'); + let len = comment_begin.unwrap_or(comment.len()); + let mlb = comment.chars().take(len).filter(|c| *c == '\n').count() > 1; + let mla = if comment_begin.is_none() { + mlb + } else { + let comment_end = comment.chars().rev().position(|c| !c.is_whitespace()); + let len = comment_end.unwrap(); + comment + .chars() + .rev() + .take(len) + .filter(|c| *c == '\n') + .count() > 1 + }; + (mlb, mla) + } else { + (false, false) + }; + + let comment = try_opt!(recover_missing_comment_in_span( + mk_sp(self[i - 1].span.hi, a.span.lo), + shape.with_max_width(context.config), + context, + 0, + )); + if !comment.is_empty() { - let comment = try_opt!(rewrite_comment( - comment, - false, - Shape::legacy( - context.config.comment_width() - shape.indent.width(), - shape.indent, - ), - context.config, - )); - result.push_str(&indent); + if multi_line_before { + result.push('\n'); + } result.push_str(&comment); result.push('\n'); - } else if multi_line { + if multi_line_after { + result.push('\n') + } + } else if insert_new_line { result.push('\n'); + if multi_line_after { + result.push('\n') + } } + if derive_args.is_empty() { result.push_str(&indent); } + + insert_new_line = true; } // Write the attribute itself. - let mut insert_new_line = true; if context.config.merge_derives() { // If the attribute is `#[derive(...)]`, take the arguments. if let Some(mut args) = get_derive_args(context, a) { @@ -982,9 +1015,7 @@ impl<'a> Rewrite for [ast::Attribute] { result.push_str(&a_str); } - if insert_new_line && i < self.len() - 1 { - result.push('\n'); - } + is_prev_sugared_doc = a.is_sugared_doc; } Some(result) } diff --git a/tests/source/attrib.rs b/tests/source/attrib.rs index 558153719475..fe8a5e3615b9 100644 --- a/tests/source/attrib.rs +++ b/tests/source/attrib.rs @@ -1,6 +1,19 @@ // rustfmt-wrap_comments: true // Test attributes and doc comments are preserved. +//! Doc comment + +#![attribute] + +//! Crate doc comment + +// Comment + +// Comment on attribute +#![the(attribute)] + +// Another comment + #[invalid attribute] fn foo() {} diff --git a/tests/source/trait.rs b/tests/source/trait.rs index 57cf4d799548..7ed858a9ca6c 100644 --- a/tests/source/trait.rs +++ b/tests/source/trait.rs @@ -53,3 +53,7 @@ trait FooBar : Tttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttt trait WhereList where T: Foo, J: Bar {} +trait X /* comment */ {} +trait Y // comment +{ +} diff --git a/tests/target/attrib.rs b/tests/target/attrib.rs index 542ed3676f0d..677c920ff1b3 100644 --- a/tests/target/attrib.rs +++ b/tests/target/attrib.rs @@ -1,6 +1,19 @@ // rustfmt-wrap_comments: true // Test attributes and doc comments are preserved. +//! Doc comment + +#![attribute] + +//! Crate doc comment + +// Comment + +// Comment on attribute +#![the(attribute)] + +// Another comment + #[invalid attribute] fn foo() {} @@ -33,11 +46,12 @@ impl Bar { fn f3(self) -> Dog {} /// Blah blah bing. + #[attrib1] /// Blah blah bing. #[attrib2] - // Another comment that needs rewrite because it's - // tooooooooooooooooooooooooooooooo loooooooooooong. + // Another comment that needs rewrite because it's tooooooooooooooooooooooooooooooo + // loooooooooooong. /// Blah blah bing. fn f4(self) -> Cat {} diff --git a/tests/target/comment.rs b/tests/target/comment.rs index 9b650dad51f6..168fb28edb99 100644 --- a/tests/target/comment.rs +++ b/tests/target/comment.rs @@ -83,6 +83,7 @@ fn some_fn3() // some comment some comment some comment some comment some commen } fn some_fn4() -// some comment some comment some comment some comment some comment some comment some comment +// some comment some comment some comment some comment some comment some comment +// some comment { } diff --git a/tests/target/comment4.rs b/tests/target/comment4.rs index 516b78c4ec42..2916f083ca0f 100644 --- a/tests/target/comment4.rs +++ b/tests/target/comment4.rs @@ -1,5 +1,5 @@ -#![allow(dead_code)] -// bar +#![allow(dead_code)] // bar + //! Doc comment fn test() { // comment diff --git a/tests/target/impl.rs b/tests/target/impl.rs index c0db6769512d..7fce1d9dd765 100644 --- a/tests/target/impl.rs +++ b/tests/target/impl.rs @@ -18,6 +18,11 @@ where } } +impl X /* comment */ {} +impl Y // comment +{ +} + impl Foo for T // comment1 where diff --git a/tests/target/trait.rs b/tests/target/trait.rs index 199ba2665f22..a2ea54e0f03b 100644 --- a/tests/target/trait.rs +++ b/tests/target/trait.rs @@ -79,3 +79,8 @@ where J: Bar, { } + +trait X /* comment */ {} +trait Y // comment +{ +}