From 28be77915f5b6750f0390e233a44499f2c3f2547 Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Thu, 17 Oct 2019 19:58:08 -0500 Subject: [PATCH 1/2] fix: nested comments in control flow condition pat --- src/patterns.rs | 73 +++++++++++++++++++++++++++++++++----- tests/source/issue_3853.rs | 35 ++++++++++++++++++ tests/target/issue_3853.rs | 31 ++++++++++++++++ 3 files changed, 130 insertions(+), 9 deletions(-) create mode 100644 tests/source/issue_3853.rs create mode 100644 tests/target/issue_3853.rs diff --git a/src/patterns.rs b/src/patterns.rs index e81631d16624..baa39dbbce64 100644 --- a/src/patterns.rs +++ b/src/patterns.rs @@ -89,26 +89,81 @@ impl Rewrite for Pat { PatKind::Box(ref pat) => rewrite_unary_prefix(context, "box ", &**pat, shape), PatKind::Ident(binding_mode, ident, ref sub_pat) => { let (prefix, mutability) = match binding_mode { - BindingMode::ByRef(mutability) => ("ref ", mutability), + BindingMode::ByRef(mutability) => ("ref", mutability), BindingMode::ByValue(mutability) => ("", mutability), }; - let mut_infix = format_mutability(mutability); + let mut_infix = format_mutability(mutability).trim(); let id_str = rewrite_ident(context, ident); let sub_pat = match *sub_pat { Some(ref p) => { - // 3 - ` @ `. + // 2 - `@ `. let width = shape .width - .checked_sub(prefix.len() + mut_infix.len() + id_str.len() + 3)?; - format!( - " @ {}", - p.rewrite(context, Shape::legacy(width, shape.indent))? - ) + .checked_sub(prefix.len() + mut_infix.len() + id_str.len() + 2)?; + let lo = context.snippet_provider.span_after(self.span, "@"); + combine_strs_with_missing_comments( + context, + "@", + &p.rewrite(context, Shape::legacy(width, shape.indent))?, + mk_sp(lo, p.span.lo()), + shape, + true, + )? } None => "".to_owned(), }; - Some(format!("{}{}{}{}", prefix, mut_infix, id_str, sub_pat)) + // combine prefix and mut + let (first_lo, first) = if !prefix.is_empty() && !mut_infix.is_empty() { + let hi = context.snippet_provider.span_before(self.span, "mut"); + let lo = context.snippet_provider.span_after(self.span, "ref"); + ( + context.snippet_provider.span_after(self.span, "mut"), + combine_strs_with_missing_comments( + context, + prefix, + mut_infix, + mk_sp(lo, hi), + shape, + true, + )?, + ) + } else if !prefix.is_empty() { + ( + context.snippet_provider.span_after(self.span, "ref"), + prefix.to_owned(), + ) + } else if !mut_infix.is_empty() { + ( + context.snippet_provider.span_after(self.span, "mut"), + mut_infix.to_owned(), + ) + } else { + (self.span.lo(), "".to_owned()) + }; + + let next = if !sub_pat.is_empty() { + let hi = context.snippet_provider.span_before(self.span, "@"); + combine_strs_with_missing_comments( + context, + id_str, + &sub_pat, + mk_sp(ident.span.hi(), hi), + shape, + true, + )? + } else { + id_str.to_owned() + }; + + combine_strs_with_missing_comments( + context, + &first, + &next, + mk_sp(first_lo, ident.span.lo()), + shape, + true, + ) } PatKind::Wild => { if 1 <= shape.width { diff --git a/tests/source/issue_3853.rs b/tests/source/issue_3853.rs new file mode 100644 index 000000000000..d2cd9a98a1f9 --- /dev/null +++ b/tests/source/issue_3853.rs @@ -0,0 +1,35 @@ +fn by_ref_with_block_before_ident() { +if let Some(ref /*def*/ state)= foo{ + println!( + "asdfasdfasdf"); } +} + +fn mut_block_before_ident() { +if let Some(mut /*def*/ state ) =foo{ + println!( + "123" ); } +} + +fn ref_and_mut_blocks_before_ident() { +if let Some(ref /*abc*/ + mut /*def*/ state ) = foo { + println!( + "deefefefefefwea" ); } +} + +fn sub_pattern() { + let foo @ /*foo*/ +bar(f) = 42; +} + +fn no_prefix_block_before_ident() { +if let Some( + /*def*/ state ) = foo { + println!( + "129387123123" ); } +} + +fn issue_3853() { +if let Some(ref /*mut*/ state) = foo { + } +} diff --git a/tests/target/issue_3853.rs b/tests/target/issue_3853.rs new file mode 100644 index 000000000000..53b94d3fc007 --- /dev/null +++ b/tests/target/issue_3853.rs @@ -0,0 +1,31 @@ +fn by_ref_with_block_before_ident() { + if let Some(ref /*def*/ state) = foo { + println!("asdfasdfasdf"); + } +} + +fn mut_block_before_ident() { + if let Some(mut /*def*/ state) = foo { + println!("123"); + } +} + +fn ref_and_mut_blocks_before_ident() { + if let Some(ref /*abc*/ mut /*def*/ state) = foo { + println!("deefefefefefwea"); + } +} + +fn sub_pattern() { + let foo @ /*foo*/ bar(f) = 42; +} + +fn no_prefix_block_before_ident() { + if let Some(/*def*/ state) = foo { + println!("129387123123"); + } +} + +fn issue_3853() { + if let Some(ref /*mut*/ state) = foo {} +} From fd6e960648b8a8d46e31bf4989928cae82e9684f Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Thu, 17 Oct 2019 20:04:05 -0500 Subject: [PATCH 2/2] fix: comments between lhs and rhs --- src/comment.rs | 2 +- src/expr.rs | 41 ++++++++++++++++++++++++++++++++------ tests/source/issue_3853.rs | 17 ++++++++++++++++ tests/target/issue_3853.rs | 16 +++++++++++++++ 4 files changed, 69 insertions(+), 7 deletions(-) diff --git a/src/comment.rs b/src/comment.rs index e725adea6bae..c721f07d0199 100644 --- a/src/comment.rs +++ b/src/comment.rs @@ -112,7 +112,7 @@ impl<'a> CommentStyle<'a> { } } -fn comment_style(orig: &str, normalize_comments: bool) -> CommentStyle<'_> { +pub(crate) fn comment_style(orig: &str, normalize_comments: bool) -> CommentStyle<'_> { if !normalize_comments { if orig.starts_with("/**") && !orig.starts_with("/**/") { CommentStyle::DoubleBullet diff --git a/src/expr.rs b/src/expr.rs index 1d9625298c5c..176f78db92ab 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -9,8 +9,8 @@ use syntax::{ast, ptr}; use crate::chains::rewrite_chain; use crate::closures; use crate::comment::{ - combine_strs_with_missing_comments, contains_comment, recover_comment_removed, rewrite_comment, - rewrite_missing_comment, CharClasses, FindUncommented, + combine_strs_with_missing_comments, comment_style, contains_comment, recover_comment_removed, + rewrite_comment, rewrite_missing_comment, CharClasses, FindUncommented, }; use crate::config::lists::*; use crate::config::{Config, ControlBraceStyle, IndentStyle, Version}; @@ -808,7 +808,7 @@ impl<'a> ControlFlow<'a> { debug!("rewrite_pat_expr {:?} {:?} {:?}", shape, self.pat, expr); let cond_shape = shape.offset_left(offset)?; - if !self.pat.is_none() { + if let Some(pat) = self.pat { let matcher = if self.matcher.is_empty() { self.matcher.to_owned() } else { @@ -817,12 +817,41 @@ impl<'a> ControlFlow<'a> { let pat_shape = cond_shape .offset_left(matcher.len())? .sub_width(self.connector.len())?; - let pat_string = if let Some(pat) = self.pat { - pat.rewrite(context, pat_shape)? + let pat_string = pat.rewrite(context, pat_shape)?; + let comments_lo = context + .snippet_provider + .span_after(self.span, self.connector.trim()); + let missing_comments = if let Some(comment) = + rewrite_missing_comment(mk_sp(comments_lo, expr.span.lo()), cond_shape, context) + { + if !self.connector.is_empty() && !comment.is_empty() { + if comment_style(&comment, false).is_line_comment() || comment.contains("\n") { + let newline = &pat_shape + .indent + .block_indent(context.config) + .to_string_with_newline(context.config); + // An extra space is added when the lhs and rhs are joined + // so we need to remove one space from the end to ensure + // the comment and rhs are aligned. + let mut suffix = newline.as_ref().to_string(); + if !suffix.is_empty() { + suffix.truncate(suffix.len() - 1); + } + format!("{}{}{}", newline, comment, suffix) + } else { + format!(" {}", comment) + } + } else { + comment + } } else { "".to_owned() }; - let result = format!("{}{}{}", matcher, pat_string, self.connector); + + let result = format!( + "{}{}{}{}", + matcher, pat_string, self.connector, missing_comments + ); return rewrite_assign_rhs(context, result, expr, cond_shape); } diff --git a/tests/source/issue_3853.rs b/tests/source/issue_3853.rs index d2cd9a98a1f9..c41309bc7882 100644 --- a/tests/source/issue_3853.rs +++ b/tests/source/issue_3853.rs @@ -33,3 +33,20 @@ fn issue_3853() { if let Some(ref /*mut*/ state) = foo { } } + +fn double_slash_comment_between_lhs_and_rhs() { + if let Some(e) = + // self.foo.bar(e, tx) + packet.transaction.state.committed + { + // body + println!( + "a2304712836123"); + } +} + +fn block_comment_between_lhs_and_rhs() { +if let Some(ref /*def*/ mut /*abc*/ state)= /*abc*/foo{ + println!( + "asdfasdfasdf"); } +} diff --git a/tests/target/issue_3853.rs b/tests/target/issue_3853.rs index 53b94d3fc007..eae59eff94e5 100644 --- a/tests/target/issue_3853.rs +++ b/tests/target/issue_3853.rs @@ -29,3 +29,19 @@ fn no_prefix_block_before_ident() { fn issue_3853() { if let Some(ref /*mut*/ state) = foo {} } + +fn double_slash_comment_between_lhs_and_rhs() { + if let Some(e) = + // self.foo.bar(e, tx) + packet.transaction.state.committed + { + // body + println!("a2304712836123"); + } +} + +fn block_comment_between_lhs_and_rhs() { + if let Some(ref /*def*/ mut /*abc*/ state) = /*abc*/ foo { + println!("asdfasdfasdf"); + } +}