From 8bb0b936b297c2414343b44058758dd67c2fb94c Mon Sep 17 00:00:00 2001 From: WhizSid Date: Wed, 21 Oct 2020 03:27:32 +0530 Subject: [PATCH 1/3] Fixed comment dropped between & and type issue --- src/formatting/types.rs | 118 +++++++++++++++++++++++++++++-------- tests/source/issue-4245.rs | 26 ++++++++ tests/target/issue-4245.rs | 34 +++++++++++ 3 files changed, 155 insertions(+), 23 deletions(-) create mode 100644 tests/source/issue-4245.rs create mode 100644 tests/target/issue-4245.rs diff --git a/src/formatting/types.rs b/src/formatting/types.rs index b4ead894f2f..95868bced13 100644 --- a/src/formatting/types.rs +++ b/src/formatting/types.rs @@ -2,10 +2,11 @@ use std::iter::ExactSizeIterator; use std::ops::Deref; use rustc_ast::ast::{self, FnRetTy, Mutability}; -use rustc_span::{symbol::kw, BytePos, Span}; +use rustc_span::{symbol::kw, BytePos, Pos, Span}; use crate::config::{lists::*, IndentStyle, TypeDensity}; use crate::formatting::{ + comment::{combine_strs_with_missing_comments, contains_comment, FindUncommented}, expr::{ format_expr, rewrite_assign_rhs, rewrite_call, rewrite_tuple, rewrite_unary_prefix, ExprType, @@ -652,37 +653,108 @@ impl Rewrite for ast::Ty { ast::TyKind::Rptr(ref lifetime, ref mt) => { let mut_str = format_mutability(mt.mutbl); let mut_len = mut_str.len(); - Some(match *lifetime { + let mut result = String::with_capacity(128); + result.push_str("&"); + let and_hi = context.snippet_provider.span_after(self.span(), "&"); + + let last_span = match *lifetime { Some(ref lifetime) => { let lt_budget = shape.width.checked_sub(2 + mut_len)?; let lt_str = lifetime.rewrite( context, Shape::legacy(lt_budget, shape.indent + 2 + mut_len), )?; - let lt_len = lt_str.len(); - let budget = shape.width.checked_sub(2 + mut_len + lt_len)?; - format!( - "&{} {}{}", - lt_str, - mut_str, - mt.ty.rewrite( + + let before_lt_span = mk_sp(and_hi, lifetime.ident.span.lo()); + if contains_comment(context.snippet(before_lt_span)) { + result = combine_strs_with_missing_comments( context, - Shape::legacy(budget, shape.indent + 2 + mut_len + lt_len) - )? - ) + &result, + <_str, + before_lt_span, + shape, + true, + )?; + } else { + result.push_str(<_str); + } + result.push_str(" "); + + let after_lt_span = mk_sp(lifetime.ident.span.hi(), mt.ty.span.lo()); + let snip = context.snippet(after_lt_span); + // Mutability span dropped from the compiler + match snip.find_uncommented("mut") { + Some(mut_pos) => { + let mut_lo = + lifetime.ident.span.hi() + BytePos::from_usize(mut_pos); + let mut_hi = + lifetime.ident.span.hi() + BytePos::from_usize(mut_pos + 3); + let before_mut_span = mk_sp(lifetime.ident.span.hi(), mut_lo); + if contains_comment(context.snippet(before_mut_span)) { + result = combine_strs_with_missing_comments( + context, + result.trim_end(), + mut_str, + before_mut_span, + shape, + true, + )?; + } else { + result.push_str(mut_str); + } + mk_sp(mut_hi, mt.ty.span.lo()) + } + None => after_lt_span, + } } - None => { - let budget = shape.width.checked_sub(1 + mut_len)?; - format!( - "&{}{}", - mut_str, - mt.ty.rewrite( - context, - Shape::legacy(budget, shape.indent + 1 + mut_len) - )? - ) + _ => { + let btwn_span = mk_sp( + context.snippet_provider.span_after(self.span(), "&"), + mt.ty.span.lo(), + ); + match context.snippet(btwn_span).find_uncommented("mut") { + Some(pos) => { + let mut_lo = and_hi + BytePos::from_usize(pos); + let mut_hi = and_hi + BytePos::from_usize(pos + 3); + let before_mut_span = mk_sp(and_hi, mut_lo); + if contains_comment(context.snippet(before_mut_span)) { + result = combine_strs_with_missing_comments( + context, + &result, + mut_str, + before_mut_span, + shape, + true, + )?; + } else { + result.push_str(mut_str); + } + mk_sp(mut_hi, mt.ty.span.lo()) + } + None => btwn_span, + } } - }) + }; + + if contains_comment(context.snippet(last_span)) { + result = combine_strs_with_missing_comments( + context, + result.trim_end(), + &mt.ty.rewrite(&context, shape)?, + last_span, + shape, + true, + )?; + } else { + let used_width = last_line_width(&result); + let budget = shape.width.checked_sub(used_width)?; + let ty_str = mt + .ty + .rewrite(&context, Shape::legacy(budget, shape.indent + used_width))?; + result.push_str(&ty_str); + } + + Some(result) } // FIXME: we drop any comments here, even though it's a silly place to put // comments. diff --git a/tests/source/issue-4245.rs b/tests/source/issue-4245.rs new file mode 100644 index 00000000000..57d7e192d0a --- /dev/null +++ b/tests/source/issue-4245.rs @@ -0,0 +1,26 @@ + + +fn a(a: & // Comment + // Another comment + 'a File) {} + +fn b(b: & /* Another Comment */'a File) {} + +fn c(c: &'a /*Comment */ mut /*Comment */ File){} + +fn d(c: & // Comment +'b // Multi Line +// Comment +mut // Multi Line +// Comment +File +) {} + +fn e(c: &// Comment +File) {} + +fn d(c: &// Comment +mut // Multi Line +// Comment +File +) {} diff --git a/tests/target/issue-4245.rs b/tests/target/issue-4245.rs new file mode 100644 index 00000000000..e3d40eb4267 --- /dev/null +++ b/tests/target/issue-4245.rs @@ -0,0 +1,34 @@ +fn a( + a: & // Comment + // Another comment + 'a File, +) { +} + +fn b(b: & /* Another Comment */ 'a File) {} + +fn c(c: &'a /*Comment */ mut /*Comment */ File) {} + +fn d( + c: & // Comment + 'b // Multi Line + // Comment + mut // Multi Line + // Comment + File, +) { +} + +fn e( + c: & // Comment + File, +) { +} + +fn d( + c: & // Comment + mut // Multi Line + // Comment + File, +) { +} From 86863b183cecf50b7d79c693eb86f689c1553524 Mon Sep 17 00:00:00 2001 From: WhizSid Date: Wed, 28 Oct 2020 03:28:38 +0530 Subject: [PATCH 2/3] Reduced nesting levels and avoided duplications --- src/formatting/types.rs | 120 ++++++++++++++-------------------------- 1 file changed, 43 insertions(+), 77 deletions(-) diff --git a/src/formatting/types.rs b/src/formatting/types.rs index f75a9096d54..ab0e17c3a39 100644 --- a/src/formatting/types.rs +++ b/src/formatting/types.rs @@ -655,93 +655,59 @@ impl Rewrite for ast::Ty { let mut_len = mut_str.len(); let mut result = String::with_capacity(128); result.push_str("&"); - let and_hi = context.snippet_provider.span_after(self.span(), "&"); + let ref_hi = context.snippet_provider.span_after(self.span(), "&"); + let mut cmnt_lo = ref_hi; - let last_span = match *lifetime { - Some(ref lifetime) => { - let lt_budget = shape.width.checked_sub(2 + mut_len)?; - let lt_str = lifetime.rewrite( + if let Some(ref lifetime) = *lifetime { + let lt_budget = shape.width.checked_sub(2 + mut_len)?; + let lt_str = lifetime.rewrite( + context, + Shape::legacy(lt_budget, shape.indent + 2 + mut_len), + )?; + let before_lt_span = mk_sp(cmnt_lo, lifetime.ident.span.lo()); + if contains_comment(context.snippet(before_lt_span)) { + result = combine_strs_with_missing_comments( context, - Shape::legacy(lt_budget, shape.indent + 2 + mut_len), + &result, + <_str, + before_lt_span, + shape, + true, )?; - - let before_lt_span = mk_sp(and_hi, lifetime.ident.span.lo()); - if contains_comment(context.snippet(before_lt_span)) { - result = combine_strs_with_missing_comments( - context, - &result, - <_str, - before_lt_span, - shape, - true, - )?; - } else { - result.push_str(<_str); - } - result.push_str(" "); - - let after_lt_span = mk_sp(lifetime.ident.span.hi(), mt.ty.span.lo()); - let snip = context.snippet(after_lt_span); - // Mutability span dropped from the compiler - match snip.find_uncommented("mut") { - Some(mut_pos) => { - let mut_lo = - lifetime.ident.span.hi() + BytePos::from_usize(mut_pos); - let mut_hi = - lifetime.ident.span.hi() + BytePos::from_usize(mut_pos + 3); - let before_mut_span = mk_sp(lifetime.ident.span.hi(), mut_lo); - if contains_comment(context.snippet(before_mut_span)) { - result = combine_strs_with_missing_comments( - context, - result.trim_end(), - mut_str, - before_mut_span, - shape, - true, - )?; - } else { - result.push_str(mut_str); - } - mk_sp(mut_hi, mt.ty.span.lo()) - } - None => after_lt_span, - } + } else { + result.push_str(<_str); } - _ => { - let btwn_span = mk_sp( - context.snippet_provider.span_after(self.span(), "&"), - mt.ty.span.lo(), - ); - match context.snippet(btwn_span).find_uncommented("mut") { - Some(pos) => { - let mut_lo = and_hi + BytePos::from_usize(pos); - let mut_hi = and_hi + BytePos::from_usize(pos + 3); - let before_mut_span = mk_sp(and_hi, mut_lo); - if contains_comment(context.snippet(before_mut_span)) { - result = combine_strs_with_missing_comments( - context, - &result, - mut_str, - before_mut_span, - shape, - true, - )?; - } else { - result.push_str(mut_str); - } - mk_sp(mut_hi, mt.ty.span.lo()) - } - None => btwn_span, - } + result.push_str(" "); + cmnt_lo = lifetime.ident.span.hi(); + } + + let after_lt_span = mk_sp(cmnt_lo, mt.ty.span.lo()); + let snip = context.snippet(after_lt_span); + if let Some(mut_pos) = snip.find_uncommented("mut") { + let mut_lo = cmnt_lo + BytePos::from_usize(mut_pos); + let before_mut_span = mk_sp(cmnt_lo, mut_lo); + if contains_comment(context.snippet(before_mut_span)) { + result = combine_strs_with_missing_comments( + context, + result.trim_end(), + mut_str, + before_mut_span, + shape, + true, + )?; + } else { + result.push_str(mut_str); } - }; + cmnt_lo = cmnt_lo + BytePos::from_usize(mut_pos + 3); + } - if contains_comment(context.snippet(last_span)) { + let before_ty_span = mk_sp(cmnt_lo, mt.ty.span.lo()); + if contains_comment(context.snippet(before_ty_span)) { result = combine_strs_with_missing_comments( context, result.trim_end(), &mt.ty.rewrite(&context, shape)?, - last_span, + before_ty_span, shape, true, )?; From 97a637715578ef3b60a7b55964f493d5f1aa8ae5 Mon Sep 17 00:00:00 2001 From: WhizSid Date: Sun, 1 Nov 2020 02:46:35 +0530 Subject: [PATCH 3/3] Removed extra allocations --- src/formatting/types.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/formatting/types.rs b/src/formatting/types.rs index ab0e17c3a39..0267a4190bd 100644 --- a/src/formatting/types.rs +++ b/src/formatting/types.rs @@ -6,7 +6,7 @@ use rustc_span::{symbol::kw, BytePos, Pos, Span}; use crate::config::{lists::*, IndentStyle, TypeDensity}; use crate::formatting::{ - comment::{combine_strs_with_missing_comments, contains_comment, FindUncommented}, + comment::{combine_strs_with_missing_comments, contains_comment}, expr::{ format_expr, rewrite_assign_rhs, rewrite_call, rewrite_tuple, rewrite_unary_prefix, ExprType, @@ -681,11 +681,9 @@ impl Rewrite for ast::Ty { cmnt_lo = lifetime.ident.span.hi(); } - let after_lt_span = mk_sp(cmnt_lo, mt.ty.span.lo()); - let snip = context.snippet(after_lt_span); - if let Some(mut_pos) = snip.find_uncommented("mut") { - let mut_lo = cmnt_lo + BytePos::from_usize(mut_pos); - let before_mut_span = mk_sp(cmnt_lo, mut_lo); + if ast::Mutability::Mut == mt.mutbl { + let mut_hi = context.snippet_provider.span_after(self.span(), "mut"); + let before_mut_span = mk_sp(cmnt_lo, mut_hi - BytePos::from_usize(3)); if contains_comment(context.snippet(before_mut_span)) { result = combine_strs_with_missing_comments( context, @@ -698,7 +696,7 @@ impl Rewrite for ast::Ty { } else { result.push_str(mut_str); } - cmnt_lo = cmnt_lo + BytePos::from_usize(mut_pos + 3); + cmnt_lo = mut_hi; } let before_ty_span = mk_sp(cmnt_lo, mt.ty.span.lo());