diff --git a/src/comment.rs b/src/comment.rs index 7b76c232937..570e9309556 100644 --- a/src/comment.rs +++ b/src/comment.rs @@ -159,6 +159,26 @@ pub(crate) fn combine_strs_with_missing_comments( span: Span, shape: Shape, allow_extend: bool, +) -> Option { + combine_lines_with_missing_comments( + context, + prev_str, + next_str, + span, + shape, + false, + allow_extend, + ) +} + +pub(crate) fn combine_lines_with_missing_comments( + context: &RewriteContext<'_>, + prev_str: &str, + next_str: &str, + span: Span, + shape: Shape, + allow_multiline_join: bool, + allow_extend: bool, ) -> Option { trace!( "combine_strs_with_missing_comments `{}` `{}` {:?} {:?}", @@ -171,7 +191,8 @@ pub(crate) fn combine_strs_with_missing_comments( let mut result = String::with_capacity(prev_str.len() + next_str.len() + shape.indent.width() + 128); result.push_str(prev_str); - let mut allow_one_line = !prev_str.contains('\n') && !next_str.contains('\n'); + let contains_nl = prev_str.contains('\n') || next_str.contains('\n'); + let mut allow_line_join = allow_multiline_join || !contains_nl; let first_sep = if prev_str.is_empty() || next_str.is_empty() || trimmed_last_line_width(prev_str) == 0 { "" @@ -213,7 +234,11 @@ pub(crate) fn combine_strs_with_missing_comments( } else { let one_line_width = last_line_width(prev_str) + first_line_width(&missing_comment) + 1; if prefer_same_line && one_line_width <= shape.width { - Cow::from(" ") + if prev_str.ends_with(" ") { + Cow::from("") + } else { + Cow::from(" ") + } } else { indent.to_string_with_newline(config) } @@ -221,14 +246,16 @@ pub(crate) fn combine_strs_with_missing_comments( result.push_str(&first_sep); result.push_str(&missing_comment); - let second_sep = if missing_comment.is_empty() || next_str.is_empty() { + let skip_second_sep = + missing_comment.is_empty() || next_str.is_empty() || next_str.starts_with("\n"); + let second_sep = if skip_second_sep { Cow::from("") } else if missing_comment.starts_with("//") { indent.to_string_with_newline(config) } else { one_line_width += missing_comment.len() + first_sep.len() + 1; - allow_one_line &= !missing_comment.starts_with("//") && !missing_comment.contains('\n'); - if prefer_same_line && allow_one_line && one_line_width <= shape.width { + allow_line_join &= !missing_comment.starts_with("//") && !missing_comment.contains('\n'); + if prefer_same_line && allow_line_join && one_line_width <= shape.width { Cow::from(" ") } else { indent.to_string_with_newline(config) diff --git a/src/expr.rs b/src/expr.rs index 58942e442de..aea195c44b9 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -1938,18 +1938,36 @@ pub(crate) fn rewrite_assign_rhs_with_comments, R: Rewrite>( ) -> Option { let lhs = lhs.into(); let contains_comment = contains_comment(context.snippet(between_span)); - let shape = if contains_comment { - shape.block_left(context.config.tab_spaces())? - } else { - shape - }; - let rhs = rewrite_assign_rhs_expr(context, &lhs, ex, shape, rhs_tactics)?; - if contains_comment { - let rhs = rhs.trim_start(); - combine_strs_with_missing_comments(context, &lhs, rhs, between_span, shape, allow_extend) - } else { - Some(lhs + &rhs) + match (contains_comment, rhs_tactics) { + (true, RhsTactics::ForceNextLineWithoutIndent) => { + let rhs = rewrite_assign_rhs_expr(context, &lhs, ex, shape, rhs_tactics)?; + let rhs = rhs.trim_start(); + + combine_strs_with_missing_comments( + context, + &lhs, + rhs, + between_span, + shape.block_left(context.config.tab_spaces())?, + allow_extend, + ) + } + (true, RhsTactics::Default | RhsTactics::AllowOverflow) => { + let shape = shape.block_left(context.config.tab_spaces())?; + let rhs = rewrite_assign_rhs_expr(context, &lhs, ex, shape, rhs_tactics)?; + let rhs = rhs.trim_start(); + + combine_strs_with_missing_comments( + context, + &lhs, + rhs, + between_span, + shape, + allow_extend, + ) + } + (false, _) => rewrite_assign_rhs_with(context, lhs, ex, shape, rhs_tactics), } } @@ -2014,7 +2032,7 @@ fn shape_from_rhs_tactic( match rhs_tactic { RhsTactics::ForceNextLineWithoutIndent => shape .with_max_width(context.config) - .sub_width(shape.indent.width()), + .sub_width(shape.indent.block_indent(context.config).width()), RhsTactics::Default | RhsTactics::AllowOverflow => { Shape::indented(shape.indent.block_indent(context.config), context.config) .sub_width(shape.rhs_overhead(context.config)) diff --git a/src/items.rs b/src/items.rs index 1c7899b3ac3..86f564b9c93 100644 --- a/src/items.rs +++ b/src/items.rs @@ -1043,28 +1043,56 @@ pub(crate) fn format_trait( let body_lo = context.snippet_provider.span_after(item.span, "{"); let shape = Shape::indented(offset, context.config).offset_left(result.len())?; + + let combine_comment_span = + |lo: BytePos, hi: BytePos, lhs: String, rhs: &str| -> Option { + let span = mk_sp(lo, hi); + + if contains_comment(context.snippet(span)) { + combine_strs_with_missing_comments(context, &lhs, rhs, span, shape, true) + } else { + Some(lhs + rhs) + } + }; + let generics_str = rewrite_generics(context, rewrite_ident(context, item.ident), generics, shape)?; - result.push_str(&generics_str); - // FIXME(#2055): rustfmt fails to format when there are comments between trait bounds. + // Recover the comment before ident. + let comment_lo = context.snippet_provider.span_after(item.span, "trait"); + let comment_hi = item.ident.span.lo(); + result = combine_comment_span(comment_lo, comment_hi, result, &generics_str)?; + if !generic_bounds.is_empty() { - let ident_hi = context - .snippet_provider - .span_after(item.span, &item.ident.as_str()); - let bound_hi = generic_bounds.last().unwrap().span().hi(); - let snippet = context.snippet(mk_sp(ident_hi, bound_hi)); - if contains_comment(snippet) { - return None; - } + // Recover the comment before the colon. + let comment_lo = generics.span.hi(); + let comment_hi = context.snippet_provider.span_before(item.span, ":"); + result = combine_comment_span(comment_lo, comment_hi, result, ":")?; - result = rewrite_assign_rhs_with( + // Recover the comment after the colon. + let comment_lo = context.snippet_provider.span_after(item.span, ":"); + let comment_hi = generic_bounds[0].span().lo(); + let comment_span = mk_sp(comment_lo, comment_hi); + + result = rewrite_assign_rhs_with_comments( context, - result + ":", + &result, generic_bounds, shape, RhsTactics::ForceNextLineWithoutIndent, + comment_span, + true, )?; + + // Recover the comment following the bounds. + let comment_lo = generic_bounds[generic_bounds.len() - 1].span().hi(); + let comment_hi = if generics.where_clause.predicates.is_empty() { + body_lo - BytePos(1) + } else { + generics.where_clause.span.lo() + }; + + result = combine_comment_span(comment_lo, comment_hi, result, "")?; } // Rewrite where-clause. @@ -1073,9 +1101,9 @@ pub(crate) fn format_trait( let where_budget = context.budget(last_line_width(&result)); let pos_before_where = if generic_bounds.is_empty() { - generics.where_clause.span.lo() + generics.span.hi() } else { - generic_bounds[generic_bounds.len() - 1].span().hi() + generics.where_clause.span.lo() }; let option = WhereClauseOption::snuggled(&generics_str); let where_clause_str = rewrite_where_clause( @@ -1085,7 +1113,7 @@ pub(crate) fn format_trait( Shape::legacy(where_budget, offset.block_only()), where_on_new_line, "{", - None, + Some(body_lo), pos_before_where, option, )?; @@ -1100,26 +1128,11 @@ pub(crate) fn format_trait( result.push_str(&where_indent.to_string_with_newline(context.config)); } result.push_str(&where_clause_str); - } else { - let item_snippet = context.snippet(item.span); - if let Some(lo) = item_snippet.find('/') { - // 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); - } - _ => (), - } - } - } + } else if generic_bounds.is_empty() { + // 1 = `{` + let comment_lo = item.ident.span.hi(); + let comment_hi = body_lo - BytePos(1); + result = combine_comment_span(comment_lo, comment_hi, result, "")?; } match context.config.brace_style() { diff --git a/src/types.rs b/src/types.rs index 9ea90c5e46d..9481e0b3513 100644 --- a/src/types.rs +++ b/src/types.rs @@ -4,11 +4,14 @@ use std::ops::Deref; use rustc_ast::ast::{self, FnRetTy, Mutability}; use rustc_span::{symbol::kw, BytePos, Pos, Span}; -use crate::comment::{combine_strs_with_missing_comments, contains_comment}; +use crate::comment::{ + combine_lines_with_missing_comments, combine_strs_with_missing_comments, contains_comment, +}; use crate::config::lists::*; use crate::config::{IndentStyle, TypeDensity, Version}; use crate::expr::{ - format_expr, rewrite_assign_rhs, rewrite_call, rewrite_tuple, rewrite_unary_prefix, ExprType, + format_expr, rewrite_assign_rhs, rewrite_assign_rhs_with_comments, rewrite_call, rewrite_tuple, + rewrite_unary_prefix, ExprType, RhsTactics, }; use crate::lists::{ definitive_tactic, itemize_list, write_list, ListFormatting, ListItem, Separator, @@ -429,7 +432,23 @@ impl Rewrite for ast::WherePredicate { format!("{}{}", type_str, colon) }; - rewrite_assign_rhs(context, lhs, bounds, shape)? + let comment_lo = context.snippet_provider.span_after(self.span(), ":"); + let comment_hi = if bounds.is_empty() { + comment_lo + } else { + bounds[0].span().lo() + }; + let comment_span = mk_sp(comment_lo, comment_hi); + + rewrite_assign_rhs_with_comments( + context, + lhs, + bounds, + shape, + RhsTactics::Default, + comment_span, + true, + )? } ast::WherePredicate::RegionPredicate(ast::WhereRegionPredicate { ref lifetime, @@ -903,6 +922,13 @@ fn is_generic_bounds_in_order(generic_bounds: &[ast::GenericBound]) -> bool { } } +fn is_bound_extendable(s: &str, b: &ast::GenericBound) -> bool { + match b { + ast::GenericBound::Outlives(..) => true, + ast::GenericBound::Trait(..) => last_line_extendable(s), + } +} + fn join_bounds( context: &RewriteContext<'_>, shape: Shape, @@ -914,22 +940,44 @@ fn join_bounds( fn join_bounds_inner( context: &RewriteContext<'_>, - shape: Shape, + orig_shape: Shape, items: &[ast::GenericBound], need_indent: bool, force_newline: bool, ) -> Option { debug_assert!(!items.is_empty()); - let generic_bounds_in_order = is_generic_bounds_in_order(items); - let is_bound_extendable = |s: &str, b: &ast::GenericBound| match b { - ast::GenericBound::Outlives(..) => true, - ast::GenericBound::Trait(..) => last_line_extendable(s), + let shape = if need_indent && force_newline { + orig_shape + .block_indent(context.config.tab_spaces()) + .with_max_width(context.config) + } else { + orig_shape }; + let combine_comment_span = + |span: Option, has_comment: bool, lhs: &str, rhs: &str| -> Option { + match span { + Some(span) if has_comment => { + combine_lines_with_missing_comments(context, lhs, rhs, span, shape, true, true) + } + _ => Some(String::from(lhs) + rhs), + } + }; + + let generic_bounds_in_order = is_generic_bounds_in_order(items); + let density = context.config.type_punctuation_density(); + let result = items.iter().enumerate().try_fold( (String::new(), None, false), |(strs, prev_trailing_span, prev_extendable), (i, item)| { + // Check whether the previous bound had a comment just after it. + let prev_has_trailing_comment = match prev_trailing_span { + Some(ts) => contains_comment(context.snippet(ts)), + _ => false, + }; + + // Check whether there are any comments written just after this bound. let trailing_span = if i < items.len() - 1 { let hi = context .snippet_provider @@ -939,6 +987,8 @@ fn join_bounds_inner( } else { None }; + + // Check whether there are any comments written just before this bound. let (leading_span, has_leading_comment) = if i > 0 { let lo = context .snippet_provider @@ -948,76 +998,52 @@ fn join_bounds_inner( let has_comments = contains_comment(context.snippet(span)); - (Some(mk_sp(lo, item.span().lo())), has_comments) + (Some(span), has_comments) } else { (None, false) }; - let prev_has_trailing_comment = match prev_trailing_span { - Some(ts) => contains_comment(context.snippet(ts)), - _ => false, - }; - let shape = if need_indent && force_newline { - shape - .block_indent(context.config.tab_spaces()) - .with_max_width(context.config) - } else { - shape - }; - let whitespace = if force_newline && (!prev_extendable || !generic_bounds_in_order) { + let continue_lastline = !prev_extendable || !generic_bounds_in_order; + let whitespace = if force_newline && continue_lastline { shape .indent .to_string_with_newline(context.config) .to_string() + } else if prev_has_trailing_comment && has_leading_comment { + String::from("") } else { - String::from(" ") + match density { + TypeDensity::Wide => String::from(" "), + TypeDensity::Compressed => String::from(""), + } }; - let joiner = match context.config.type_punctuation_density() { - TypeDensity::Compressed => String::from("+"), - TypeDensity::Wide => whitespace + "+ ", - }; - let joiner = if has_leading_comment { - joiner.trim_end() - } else { - &joiner - }; - let joiner = if prev_has_trailing_comment { - joiner.trim_start() - } else { - joiner + let bound_str = item.rewrite(context, shape)?; + let extendable = is_bound_extendable(&bound_str, item); + + let joiner = match (i, density, has_leading_comment) { + (0, _, _) => "", + (_, TypeDensity::Wide, false) => "+ ", + (_, TypeDensity::Compressed, false) => "+", + (_, _, true) => "+", }; - let (extendable, trailing_str) = if i == 0 { - let bound_str = item.rewrite(context, shape)?; - (is_bound_extendable(&bound_str, item), bound_str) + let trailing_str = + combine_comment_span(leading_span, has_leading_comment, joiner, &bound_str)?; + + let trailing_str = if i == 0 { + trailing_str } else { - let bound_str = &item.rewrite(context, shape)?; - match leading_span { - Some(ls) if has_leading_comment => ( - is_bound_extendable(bound_str, item), - combine_strs_with_missing_comments( - context, joiner, bound_str, ls, shape, true, - )?, - ), - _ => ( - is_bound_extendable(bound_str, item), - String::from(joiner) + bound_str, - ), - } + whitespace + &trailing_str }; - match prev_trailing_span { - Some(ts) if prev_has_trailing_comment => combine_strs_with_missing_comments( - context, - &strs, - &trailing_str, - ts, - shape, - true, - ) - .map(|v| (v, trailing_span, extendable)), - _ => Some((strs + &trailing_str, trailing_span, extendable)), - } + + combine_comment_span( + prev_trailing_span, + prev_has_trailing_comment, + &strs, + &trailing_str, + ) + .map(|v| (v, trailing_span, extendable)) }, )?; @@ -1025,7 +1051,7 @@ fn join_bounds_inner( && items.len() > 1 && (result.0.contains('\n') || result.0.len() > shape.width) { - join_bounds_inner(context, shape, items, need_indent, true) + join_bounds_inner(context, orig_shape, items, need_indent, true) } else { Some(result.0) } diff --git a/tests/source/issue-2055.rs b/tests/source/issue-2055.rs new file mode 100644 index 00000000000..9856777af6c --- /dev/null +++ b/tests/source/issue-2055.rs @@ -0,0 +1,59 @@ + +pub trait CommentAfterColon1: /* Comment */ Clone +{ + // +} + +pub trait CommentAfterColon2: // Comment +Clone +{ + // +} + +pub trait CommentBeforeColon /*comment*/: Clone +{ + // +} + +pub trait /*comment*/ CommentBeforeIdent1: Clone +{ + // +} + +pub trait // comment +CommentBeforeIdent2: Clone +{ + // +} + +pub trait CommentsShort /*after ident*/: /*BA before*/ BA /*BA after*/ + /*BB before*/ BB /*BB after*/ +{ + // +} + +pub trait CommentsMultiline /*after ident*/: /*BA before*/ BA /*BA after*/ + /*BB before*/ BB /*BB after*/ + /*BC before*/ BC /*BC after*/ + /*BD before*/ BD /*BD after*/ +{ + // +} + +pub trait CommentsShortWhere +where T1: BA /*comment 1*/ + /*comment 2*/ BB + BC, /*comment 3*/ + T2: BA /*comment 4*/ + /*comment 5*/ BB + BC, /*comment 6*/ +{ + // +} + +pub trait CommentsWhere/*before where*/ +where T1: /*comment 1*/ BA /*comment 2*/ + /*comment 3*/ BB /*comment 4*/ + /*comment 5*/ BC /*comment 6*/ + /*comment 7*/ BB /*comment 8*/ + /*comment 9*/ BC, /*comment 10*/ + T2: /*comment 11*/ +{ + // +} + +pub trait KitchenSink/*before colon*/://after colon +FromIterator /*comment 1*/ + /*comment 2*/ Printer<'tcx, Error = fmt::Error, Path = Self, Region = Self, Type = Self, DynExistential = Self, Const = Self, > /*comment 3*/ + /*comment 4*/ fmt::Write /*comment 5*/ + /*comment 6*/ Clone /*comment 7*/ + /*comment 8*/ Default /*comment 9*/ +where T1: /*comment 10*/ BA /*comment 11*/ + /*comment 12*/ BB + BC /*comment 13*/ + /*comment 14*/ BB + /*comment 15*/ BC, /*comment 16*/ +T2: /*comment 17*/ BA /*comment 18*/ + /*comment 19*/ BB + BC /*comment 20*/ + /*comment 21*/ BB + /*comment 22*/ BC, /*comment 23*/ +{ + // +} diff --git a/tests/target/issue-2055.rs b/tests/target/issue-2055.rs new file mode 100644 index 00000000000..a29c92a9974 --- /dev/null +++ b/tests/target/issue-2055.rs @@ -0,0 +1,90 @@ +pub trait CommentAfterColon1: /* Comment */ Clone { + // +} + +pub trait CommentAfterColon2: // Comment + Clone +{ + // +} + +pub trait CommentBeforeColon /*comment*/ : Clone { + // +} + +pub trait /*comment*/ CommentBeforeIdent1: Clone { + // +} + +pub trait // comment +CommentBeforeIdent2: Clone +{ + // +} + +pub trait CommentsShort /*after ident*/ : /*BA before*/ + BA /*BA after*/ + /*BB before*/ BB /*BB after*/ +{ + // +} + +pub trait CommentsMultiline /*after ident*/ : /*BA before*/ + BA /*BA after*/ + + /*BB before*/ BB /*BB after*/ + + /*BC before*/ BC /*BC after*/ + + /*BD before*/ BD /*BD after*/ +{ + // +} + +pub trait CommentsShortWhere +where + T1: BA /*comment 1*/ + /*comment 2*/ BB + BC, /*comment 3*/ + T2: BA /*comment 4*/ + /*comment 5*/ BB + BC, /*comment 6*/ +{ + // +} + +pub trait CommentsWhere +/*before where*/ +where + T1: /*comment 1*/ + BA /*comment 2*/ + + /*comment 3*/ BB /*comment 4*/ + + /*comment 5*/ BC /*comment 6*/ + + /*comment 7*/ BB /*comment 8*/ + + /*comment 9*/ BC, /*comment 10*/ + T2:, /*comment 11*/ +{ + // +} + +pub trait KitchenSink /*before colon*/ : //after colon + FromIterator /*comment 1*/ + + /*comment 2*/ Printer< + 'tcx, + Error = fmt::Error, + Path = Self, + Region = Self, + Type = Self, + DynExistential = Self, + Const = Self, + > /*comment 3*/ + /*comment 4*/ fmt::Write /*comment 5*/ + + /*comment 6*/ Clone /*comment 7*/ + + /*comment 8*/ Default /*comment 9*/ +where + T1: /*comment 10*/ + BA /*comment 11*/ + + /*comment 12*/ BB + + BC /*comment 13*/ + + /*comment 14*/ BB + + /*comment 15*/ BC, /*comment 16*/ + T2: /*comment 17*/ + BA /*comment 18*/ + + /*comment 19*/ BB + + BC /*comment 20*/ + + /*comment 21*/ BB + + /*comment 22*/ BC, /*comment 23*/ +{ + // +} diff --git a/tests/target/trait.rs b/tests/target/trait.rs index 7f067991b26..f824f44bad6 100644 --- a/tests/target/trait.rs +++ b/tests/target/trait.rs @@ -86,11 +86,13 @@ trait Y // comment // #2055 pub trait Foo: -// A and C -A + C -// and B + // A and C + A + + C + // and B + B -{} +{ +} // #2158 trait Foo {