From cfdeced2ae7fdcf2354dd834e455541191405826 Mon Sep 17 00:00:00 2001 From: hafiz <20735482+ayazhafiz@users.noreply.github.com> Date: Sun, 10 May 2020 06:54:46 -0500 Subject: [PATCH 01/11] Compare code block line indentation with config whitespace (#4166) Previously the indetation of a line was compared with the configured number of spaces per tab, which could cause lines that were formatted with hard tabs not to be recognized as indented ("\t".len() < " ".len()). Closes #4152 --- src/lib.rs | 4 ++-- tests/target/issue-4152.rs | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) create mode 100644 tests/target/issue-4152.rs diff --git a/src/lib.rs b/src/lib.rs index 753840e065c..da83b285a21 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -371,6 +371,7 @@ fn format_code_block( .rfind('}') .unwrap_or_else(|| formatted.snippet.len()); let mut is_indented = true; + let indent_str = Indent::from_width(config, config.tab_spaces()).to_string(config); for (kind, ref line) in LineClasses::new(&formatted.snippet[FN_MAIN_PREFIX.len()..block_len]) { if !is_first { result.push('\n'); @@ -385,9 +386,8 @@ fn format_code_block( // are too long, or we have failed to format code block. We will be // conservative and just return `None` in this case. return None; - } else if line.len() > config.tab_spaces() { + } else if line.len() > indent_str.len() { // Make sure that the line has leading whitespaces. - let indent_str = Indent::from_width(config, config.tab_spaces()).to_string(config); if line.starts_with(indent_str.as_ref()) { let offset = if config.hard_tabs() { 1 diff --git a/tests/target/issue-4152.rs b/tests/target/issue-4152.rs new file mode 100644 index 00000000000..80f9ff5e304 --- /dev/null +++ b/tests/target/issue-4152.rs @@ -0,0 +1,18 @@ +// rustfmt-hard_tabs: true + +macro_rules! bit { + ($bool:expr) => { + if $bool { + 1; + 1 + } else { + 0; + 0 + } + }; +} +macro_rules! add_one { + ($vec:expr) => {{ + $vec.push(1); + }}; +} From 9702d8ac9059b9203aac321f7947b508c671fdb4 Mon Sep 17 00:00:00 2001 From: "Adam H. Leventhal" Date: Tue, 21 Apr 2020 22:57:54 -0700 Subject: [PATCH 02/11] fixes #4115, #4029, #3898 --- src/attr.rs | 184 ++++++++++++++++++++++++++-------------------------- 1 file changed, 91 insertions(+), 93 deletions(-) diff --git a/src/attr.rs b/src/attr.rs index 48c0685d2c1..9f77941e551 100644 --- a/src/attr.rs +++ b/src/attr.rs @@ -63,15 +63,6 @@ fn is_derive(attr: &ast::Attribute) -> bool { attr.has_name(sym::derive) } -/// Returns the arguments of `#[derive(...)]`. -fn get_derive_spans<'a>(attr: &'a ast::Attribute) -> Option + 'a> { - attr.meta_item_list().map(|meta_item_list| { - meta_item_list - .into_iter() - .map(|nested_meta_item| nested_meta_item.span()) - }) -} - // The shape of the arguments to a function-like attribute. fn argument_shape( left: usize, @@ -100,36 +91,104 @@ fn argument_shape( } fn format_derive( - derive_args: &[Span], - prefix: &str, + derives: &[ast::Attribute], shape: Shape, context: &RewriteContext<'_>, ) -> Option { - let mut result = String::with_capacity(128); - result.push_str(prefix); - result.push_str("[derive("); - - let argument_shape = argument_shape(10 + prefix.len(), 2, false, shape, context)?; - let item_str = format_arg_list( - derive_args.iter(), - |_| DUMMY_SP.lo(), - |_| DUMMY_SP.hi(), - |sp| Some(context.snippet(**sp).to_owned()), - DUMMY_SP, - context, - argument_shape, - // 10 = "[derive()]", 3 = "()" and "]" - shape.offset_left(10 + prefix.len())?.sub_width(3)?, - None, + // Collect all items from all attributes + let all_items = derives + .iter() + .map(|attr| { + // Parse the derive items and extract the span for each item; if any + // attribute is not parseable, none of the attributes will be + // reformatted. + let item_spans = attr.meta_item_list().map(|meta_item_list| { + meta_item_list + .into_iter() + .map(|nested_meta_item| nested_meta_item.span()) + })?; + + let items = itemize_list( + context.snippet_provider, + item_spans, + ")", + ",", + |span| span.lo(), + |span| span.hi(), + |span| Some(context.snippet(*span).to_owned()), + attr.span.lo(), + attr.span.hi(), + false, + ); + + Some(items) + }) + // Fail if any attribute failed. + .collect::>>()? + // Collect the results into a single, flat, Vec. + .into_iter() + .flatten() + .collect::>(); + + // Collect formatting parameters. + let prefix = attr_prefix(&derives[0]); + let argument_shape = argument_shape( + "[derive()]".len() + prefix.len(), + ")]".len(), false, + shape, + context, )?; + let one_line_shape = shape + .offset_left("[derive()]".len() + prefix.len())? + .sub_width("()]".len())?; + let one_line_budget = one_line_shape.width; - result.push_str(&item_str); - if item_str.starts_with('\n') { - result.push(','); + let tactic = definitive_tactic( + &all_items, + ListTactic::HorizontalVertical, + Separator::Comma, + argument_shape.width, + ); + let trailing_separator = match context.config.indent_style() { + // We always add the trailing comma and remove it if it is not needed. + IndentStyle::Block => SeparatorTactic::Always, + IndentStyle::Visual => SeparatorTactic::Never, + }; + + // Format the collection of items. + let fmt = ListFormatting::new(argument_shape, context.config) + .tactic(tactic) + .trailing_separator(trailing_separator) + .ends_with_newline(false); + let item_str = write_list(&all_items, &fmt)?; + + debug!("item_str: '{}'", item_str); + + // Determine if the result will be nested, i.e. if we're using the block + // indent style and either the items are on multiple lines or we've exceeded + // our budget to fit on a single line. + let nested = context.config.indent_style() == IndentStyle::Block + && (item_str.contains('\n') || item_str.len() > one_line_budget); + + // Format the final result. + let mut result = String::with_capacity(128); + result.push_str(prefix); + result.push_str("[derive("); + if nested { + let nested_indent = argument_shape.indent.to_string_with_newline(context.config); + result.push_str(&nested_indent); + result.push_str(&item_str); result.push_str(&shape.indent.to_string_with_newline(context.config)); + } else if let SeparatorTactic::Always = context.config.trailing_comma() { + // Retain the trailing comma. + result.push_str(&item_str); + } else { + // Remove the trailing comma. + result.push_str(&item_str[..item_str.len() - 1]); } result.push_str(")]"); + Some(result) } @@ -255,7 +314,7 @@ impl Rewrite for ast::MetaItem { // width. Since a literal is basically unformattable unless it // is a string literal (and only if `format_strings` is set), // we might be better off ignoring the fact that the attribute - // is longer than the max width and contiue on formatting. + // is longer than the max width and continue on formatting. // See #2479 for example. let value = rewrite_literal(context, literal, lit_shape) .unwrap_or_else(|| context.snippet(literal.span).to_owned()); @@ -265,61 +324,6 @@ impl Rewrite for ast::MetaItem { } } -fn format_arg_list( - list: I, - get_lo: F1, - get_hi: F2, - get_item_string: F3, - span: Span, - context: &RewriteContext<'_>, - shape: Shape, - one_line_shape: Shape, - one_line_limit: Option, - combine: bool, -) -> Option -where - I: Iterator, - F1: Fn(&T) -> BytePos, - F2: Fn(&T) -> BytePos, - F3: Fn(&T) -> Option, -{ - let items = itemize_list( - context.snippet_provider, - list, - ")", - ",", - get_lo, - get_hi, - get_item_string, - span.lo(), - span.hi(), - false, - ); - let item_vec = items.collect::>(); - let tactic = if let Some(limit) = one_line_limit { - ListTactic::LimitedHorizontalVertical(limit) - } else { - ListTactic::HorizontalVertical - }; - - let tactic = definitive_tactic(&item_vec, tactic, Separator::Comma, shape.width); - let fmt = ListFormatting::new(shape, context.config) - .tactic(tactic) - .ends_with_newline(false); - let item_str = write_list(&item_vec, &fmt)?; - - let one_line_budget = one_line_shape.width; - if context.config.indent_style() == IndentStyle::Visual - || combine - || (!item_str.contains('\n') && item_str.len() <= one_line_budget) - { - Some(item_str) - } else { - let nested_indent = shape.indent.to_string_with_newline(context.config); - Some(format!("{}{}", nested_indent, item_str)) - } -} - impl Rewrite for ast::Attribute { fn rewrite(&self, context: &RewriteContext<'_>, shape: Shape) -> Option { let snippet = context.snippet(self.span); @@ -424,13 +428,7 @@ impl<'a> Rewrite for [ast::Attribute] { // Handle derives if we will merge them. if context.config.merge_derives() && is_derive(&attrs[0]) { let derives = take_while_with_pred(context, attrs, is_derive); - let derive_spans: Vec<_> = derives - .iter() - .filter_map(get_derive_spans) - .flatten() - .collect(); - let derive_str = - format_derive(&derive_spans, attr_prefix(&attrs[0]), shape, context)?; + let derive_str = format_derive(derives, shape, context)?; result.push_str(&derive_str); let missing_span = attrs From 3195942539b986dd922898cc922bcdf4053f9fb8 Mon Sep 17 00:00:00 2001 From: "Adam H. Leventhal" Date: Tue, 21 Apr 2020 22:54:08 -0700 Subject: [PATCH 03/11] tests --- tests/target/issue-4029.rs | 7 +++++++ tests/target/issue-4115.rs | 8 ++++++++ 2 files changed, 15 insertions(+) create mode 100644 tests/target/issue-4029.rs create mode 100644 tests/target/issue-4115.rs diff --git a/tests/target/issue-4029.rs b/tests/target/issue-4029.rs new file mode 100644 index 00000000000..314d0180588 --- /dev/null +++ b/tests/target/issue-4029.rs @@ -0,0 +1,7 @@ +// issue #4029 +#[derive(Debug, Clone, Default Hash)] +struct S; + +// issue #3898 +#[derive(Debug, Clone, Default,, Hash)] +struct T; diff --git a/tests/target/issue-4115.rs b/tests/target/issue-4115.rs new file mode 100644 index 00000000000..0dd7bdbd04f --- /dev/null +++ b/tests/target/issue-4115.rs @@ -0,0 +1,8 @@ +#[derive( + A, + B, + C, + D, + // E, +)] +fn foo() {} From a6d8d542451cdc4ee89512367743f8a11d390ae5 Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Sat, 28 Nov 2020 18:03:48 -0600 Subject: [PATCH 04/11] tests: backport an additional test case --- src/attr.rs | 2 +- tests/target/issue_4545.rs | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 tests/target/issue_4545.rs diff --git a/src/attr.rs b/src/attr.rs index 9f77941e551..dab9cabccdc 100644 --- a/src/attr.rs +++ b/src/attr.rs @@ -2,7 +2,7 @@ use rustc_ast::ast; use rustc_ast::attr::HasAttrs; -use rustc_span::{symbol::sym, BytePos, Span, Symbol, DUMMY_SP}; +use rustc_span::{symbol::sym, Span, Symbol}; use self::doc_comment::DocCommentFormatter; use crate::comment::{contains_comment, rewrite_doc_comment, CommentStyle}; diff --git a/tests/target/issue_4545.rs b/tests/target/issue_4545.rs new file mode 100644 index 00000000000..f87c8103657 --- /dev/null +++ b/tests/target/issue_4545.rs @@ -0,0 +1,5 @@ +#[derive(Debug, Foo)] +enum Bar {} + +#[derive(Debug, , Default)] +struct Struct(i32); From c50bfa8f2562177c1c01b12b55a615cb6c601992 Mon Sep 17 00:00:00 2001 From: hafiz <20735482+ayazhafiz@users.noreply.github.com> Date: Sun, 7 Jun 2020 20:31:24 -0500 Subject: [PATCH 05/11] Pick up comments between visibility modifier and item name (#4239) * Pick up comments between visibility modifier and item name I don't think this hurts to fix. #2781, which surfaced this issue, has a number of comments relating to similar but slightly different issues (i.e. dropped comments in other places). I can mark #2781 as closed and then will open new issues for the comments that are not already resolved or tracked. Closes #2781 * fixup! Pick up comments between visibility modifier and item name * fixup! Pick up comments between visibility modifier and item name --- src/items.rs | 47 ++++++++++++++++++++++++++++---------- tests/source/issue-2781.rs | 11 +++++++++ tests/target/issue-2781.rs | 11 +++++++++ 3 files changed, 57 insertions(+), 12 deletions(-) create mode 100644 tests/source/issue-2781.rs create mode 100644 tests/target/issue-2781.rs diff --git a/src/items.rs b/src/items.rs index 039dd1e1ae0..cd8364fd686 100644 --- a/src/items.rs +++ b/src/items.rs @@ -401,7 +401,8 @@ impl<'a> FmtVisitor<'a> { generics: &ast::Generics, span: Span, ) { - let enum_header = format_header(&self.get_context(), "enum ", ident, vis); + let enum_header = + format_header(&self.get_context(), "enum ", ident, vis, self.block_indent); self.push_str(&enum_header); let enum_snippet = self.snippet(span); @@ -952,8 +953,8 @@ pub(crate) struct StructParts<'a> { } impl<'a> StructParts<'a> { - fn format_header(&self, context: &RewriteContext<'_>) -> String { - format_header(context, self.prefix, self.ident, self.vis) + fn format_header(&self, context: &RewriteContext<'_>, offset: Indent) -> String { + format_header(context, self.prefix, self.ident, self.vis, offset) } fn from_variant(variant: &'a ast::Variant) -> Self { @@ -1236,7 +1237,7 @@ fn format_unit_struct( p: &StructParts<'_>, offset: Indent, ) -> Option { - let header_str = format_header(context, p.prefix, p.ident, p.vis); + let header_str = format_header(context, p.prefix, p.ident, p.vis, offset); let generics_str = if let Some(generics) = p.generics { let hi = context.snippet_provider.span_before(p.span, ";"); format_generics( @@ -1265,7 +1266,7 @@ pub(crate) fn format_struct_struct( let mut result = String::with_capacity(1024); let span = struct_parts.span; - let header_str = struct_parts.format_header(context); + let header_str = struct_parts.format_header(context, offset); result.push_str(&header_str); let header_hi = struct_parts.ident.span.hi(); @@ -1401,7 +1402,7 @@ fn format_tuple_struct( let mut result = String::with_capacity(1024); let span = struct_parts.span; - let header_str = struct_parts.format_header(context); + let header_str = struct_parts.format_header(context, offset); result.push_str(&header_str); let body_lo = if fields.is_empty() { @@ -2920,13 +2921,35 @@ fn format_header( item_name: &str, ident: symbol::Ident, vis: &ast::Visibility, + offset: Indent, ) -> String { - format!( - "{}{}{}", - format_visibility(context, vis), - item_name, - rewrite_ident(context, ident) - ) + let mut result = String::with_capacity(128); + let shape = Shape::indented(offset, context.config); + + result.push_str(&format_visibility(context, vis).trim()); + + // Check for a missing comment between the visibility and the item name. + let after_vis = vis.span.hi(); + if let Some(before_item_name) = context + .snippet_provider + .opt_span_before(mk_sp(vis.span().lo(), ident.span.hi()), item_name.trim()) + { + let missing_span = mk_sp(after_vis, before_item_name); + if let Some(result_with_comment) = combine_strs_with_missing_comments( + context, + &result, + item_name, + missing_span, + shape, + /* allow_extend */ true, + ) { + result = result_with_comment; + } + } + + result.push_str(&rewrite_ident(context, ident)); + + result } #[derive(PartialEq, Eq, Clone, Copy)] diff --git a/tests/source/issue-2781.rs b/tests/source/issue-2781.rs new file mode 100644 index 00000000000..2c15b29b6dc --- /dev/null +++ b/tests/source/issue-2781.rs @@ -0,0 +1,11 @@ +pub // Oh, no. A line comment. +struct Foo {} + +pub /* Oh, no. A block comment. */ struct Foo {} + +mod inner { +pub // Oh, no. A line comment. +struct Foo {} + +pub /* Oh, no. A block comment. */ struct Foo {} +} diff --git a/tests/target/issue-2781.rs b/tests/target/issue-2781.rs new file mode 100644 index 00000000000..f144d716be9 --- /dev/null +++ b/tests/target/issue-2781.rs @@ -0,0 +1,11 @@ +pub // Oh, no. A line comment. +struct Foo {} + +pub /* Oh, no. A block comment. */ struct Foo {} + +mod inner { + pub // Oh, no. A line comment. + struct Foo {} + + pub /* Oh, no. A block comment. */ struct Foo {} +} From 3a863fb6539b6847b8b91990c30b81484ea1b390 Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Sat, 28 Nov 2020 18:23:00 -0600 Subject: [PATCH 06/11] fix: apply rustc-ap updates to backported commit --- src/items.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/items.rs b/src/items.rs index cd8364fd686..57156aece33 100644 --- a/src/items.rs +++ b/src/items.rs @@ -2932,7 +2932,7 @@ fn format_header( let after_vis = vis.span.hi(); if let Some(before_item_name) = context .snippet_provider - .opt_span_before(mk_sp(vis.span().lo(), ident.span.hi()), item_name.trim()) + .opt_span_before(mk_sp(vis.span.lo(), ident.span.hi()), item_name.trim()) { let missing_span = mk_sp(after_vis, before_item_name); if let Some(result_with_comment) = combine_strs_with_missing_comments( From a9d59699519df9189043ed1e31e44556018475ff Mon Sep 17 00:00:00 2001 From: WhizSid Date: Fri, 9 Oct 2020 05:57:04 +0530 Subject: [PATCH 07/11] Fixed 'Comment removed between type name and =' issue (#4448) * Fixed Comment removed between type name and = issue * Fixed where clause issue and pass the full span * has_where condition inline * Fixed indentation error on where clause * Removed tmp file --- src/items.rs | 41 +++++++++++++++++++++++++++++++++++--- src/visitor.rs | 4 ++++ tests/source/issue-4244.rs | 16 +++++++++++++++ tests/target/issue-4244.rs | 20 +++++++++++++++++++ 4 files changed, 78 insertions(+), 3 deletions(-) create mode 100644 tests/source/issue-4244.rs create mode 100644 tests/target/issue-4244.rs diff --git a/src/items.rs b/src/items.rs index 57156aece33..6fdca90b12f 100644 --- a/src/items.rs +++ b/src/items.rs @@ -1495,6 +1495,7 @@ fn rewrite_type( generics: &ast::Generics, generic_bounds_opt: Option<&ast::GenericBounds>, rhs: Option<&R>, + span: Span, ) -> Option { let mut result = String::with_capacity(128); result.push_str(&format!("{}type ", format_visibility(context, vis))); @@ -1541,12 +1542,40 @@ fn rewrite_type( if let Some(ty) = rhs { // If there's a where clause, add a newline before the assignment. Otherwise just add a // space. - if !generics.where_clause.predicates.is_empty() { + let has_where = !generics.where_clause.predicates.is_empty(); + if has_where { result.push_str(&indent.to_string_with_newline(context.config)); } else { result.push(' '); } - let lhs = format!("{}=", result); + + let comment_span = context + .snippet_provider + .opt_span_before(span, "=") + .map(|op_lo| mk_sp(generics.where_clause.span.hi(), op_lo)); + + let lhs = match comment_span { + Some(comment_span) + if contains_comment(context.snippet_provider.span_to_snippet(comment_span)?) => + { + let comment_shape = if has_where { + Shape::indented(indent, context.config) + } else { + Shape::indented(indent, context.config) + .block_left(context.config.tab_spaces())? + }; + + combine_strs_with_missing_comments( + context, + result.trim_end(), + "=", + comment_span, + comment_shape, + true, + )? + } + _ => format!("{}=", result), + }; // 1 = `;` let shape = Shape::indented(indent, context.config).sub_width(1)?; @@ -1563,6 +1592,7 @@ pub(crate) fn rewrite_opaque_type( generic_bounds: &ast::GenericBounds, generics: &ast::Generics, vis: &ast::Visibility, + span: Span, ) -> Option { let opaque_type_bounds = OpaqueTypeBounds { generic_bounds }; rewrite_type( @@ -1573,6 +1603,7 @@ pub(crate) fn rewrite_opaque_type( generics, Some(generic_bounds), Some(&opaque_type_bounds), + span, ) } @@ -1801,6 +1832,7 @@ pub(crate) fn rewrite_type_alias( context: &RewriteContext<'_>, indent: Indent, vis: &ast::Visibility, + span: Span, ) -> Option { rewrite_type( context, @@ -1810,6 +1842,7 @@ pub(crate) fn rewrite_type_alias( generics, generic_bounds_opt, ty_opt, + span, ) } @@ -1859,8 +1892,9 @@ pub(crate) fn rewrite_associated_impl_type( generics: &ast::Generics, context: &RewriteContext<'_>, indent: Indent, + span: Span, ) -> Option { - let result = rewrite_type_alias(ident, ty_opt, generics, None, context, indent, vis)?; + let result = rewrite_type_alias(ident, ty_opt, generics, None, context, indent, vis, span)?; match defaultness { ast::Defaultness::Default(..) => Some(format!("default {}", result)), @@ -3118,6 +3152,7 @@ impl Rewrite for ast::ForeignItem { &context, shape.indent, &self.vis, + self.span, ), ast::ForeignItemKind::MacCall(ref mac) => { rewrite_macro(mac, None, context, shape, MacroPosition::Item) diff --git a/src/visitor.rs b/src/visitor.rs index 3b9b0a2f967..fefeffac95c 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -572,6 +572,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { &self.get_context(), self.block_indent, &item.vis, + item.span, ); self.push_rewrite(item.span, rewrite); } @@ -583,6 +584,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { generic_bounds, generics, &item.vis, + item.span, ); self.push_rewrite(item.span, rewrite); } @@ -649,6 +651,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { &self.get_context(), self.block_indent, &ti.vis, + ti.span, ); self.push_rewrite(ti.span, rewrite); } @@ -695,6 +698,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { &generics, &self.get_context(), self.block_indent, + ii.span, ) }; let rewrite = match ty { diff --git a/tests/source/issue-4244.rs b/tests/source/issue-4244.rs new file mode 100644 index 00000000000..34b51085e13 --- /dev/null +++ b/tests/source/issue-4244.rs @@ -0,0 +1,16 @@ +pub struct SS {} + +pub type A /* A Comment */ = SS; + +pub type B // Comment +// B += SS; + +pub type C + /* Comment C */ = SS; + +pub trait D { + type E /* Comment E */ = SS; +} + +type F<'a: 'static, T: Ord + 'static>: Eq + PartialEq where T: 'static + Copy /* x */ = Vec; diff --git a/tests/target/issue-4244.rs b/tests/target/issue-4244.rs new file mode 100644 index 00000000000..8958ba99e80 --- /dev/null +++ b/tests/target/issue-4244.rs @@ -0,0 +1,20 @@ +pub struct SS {} + +pub type A /* A Comment */ = SS; + +pub type B // Comment + // B + = SS; + +pub type C + /* Comment C */ + = SS; + +pub trait D { + type E /* Comment E */ = SS; +} + +type F<'a: 'static, T: Ord + 'static>: Eq + PartialEq +where + T: 'static + Copy, /* x */ += Vec; From 3702e024b5384522503a3574421c4f3066942a5b Mon Sep 17 00:00:00 2001 From: WhizSid Date: Tue, 3 Nov 2020 09:14:15 +0530 Subject: [PATCH 08/11] Fixed comment dropped between & and type issue (#4482) * Fixed comment dropped between & and type issue * Reduced nesting levels and avoided duplications * Removed extra allocations --- src/types.rs | 89 ++++++++++++++++++++++++++------------ tests/source/issue-4245.rs | 26 +++++++++++ tests/target/issue-4245.rs | 34 +++++++++++++++ 3 files changed, 122 insertions(+), 27 deletions(-) create mode 100644 tests/source/issue-4245.rs create mode 100644 tests/target/issue-4245.rs diff --git a/src/types.rs b/src/types.rs index 4e2b87ebfc8..4b936c1baf5 100644 --- a/src/types.rs +++ b/src/types.rs @@ -2,7 +2,7 @@ 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::*; use crate::config::{IndentStyle, TypeDensity, Version}; @@ -648,37 +648,72 @@ 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 { - Some(ref lifetime) => { - let lt_budget = shape.width.checked_sub(2 + mut_len)?; - let lt_str = lifetime.rewrite( + let mut result = String::with_capacity(128); + result.push_str("&"); + let ref_hi = context.snippet_provider.span_after(self.span(), "&"); + let mut cmnt_lo = ref_hi; + + 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 lt_len = lt_str.len(); - let budget = shape.width.checked_sub(2 + mut_len + lt_len)?; - format!( - "&{} {}{}", - lt_str, - mut_str, - mt.ty.rewrite( - context, - Shape::legacy(budget, shape.indent + 2 + mut_len + lt_len) - )? - ) + } else { + result.push_str(<_str); } - None => { - let budget = shape.width.checked_sub(1 + mut_len)?; - format!( - "&{}{}", + result.push_str(" "); + cmnt_lo = lifetime.ident.span.hi(); + } + + 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, + result.trim_end(), mut_str, - mt.ty.rewrite( - context, - Shape::legacy(budget, shape.indent + 1 + mut_len) - )? - ) + before_mut_span, + shape, + true, + )?; + } else { + result.push_str(mut_str); } - }) + cmnt_lo = mut_hi; + } + + 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)?, + before_ty_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 1ca07227df619eaae4c18a0d07aa35d231408606 Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Sat, 28 Nov 2020 18:30:28 -0600 Subject: [PATCH 09/11] fix: backport some imports for cherry-picked commit --- src/types.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/types.rs b/src/types.rs index 4b936c1baf5..e8bd74d5a12 100644 --- a/src/types.rs +++ b/src/types.rs @@ -4,6 +4,7 @@ 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::config::lists::*; use crate::config::{IndentStyle, TypeDensity, Version}; use crate::expr::{format_expr, rewrite_assign_rhs, rewrite_tuple, rewrite_unary_prefix, ExprType}; From ad98e46f36e413410df7820130aeebb9dc3d08a8 Mon Sep 17 00:00:00 2001 From: WhizSid Date: Sun, 18 Oct 2020 22:32:46 +0530 Subject: [PATCH 10/11] Comment between typebounds (#4474) * Test cases and get spans * Fixed type bounds * Fixed issue of test cases * Fixed first test case issue * Removed unwanted whitespaces * Removed tmp files --- src/types.rs | 162 +++++++++++++++++++++++++++---------- tests/source/issue-4243.rs | 21 +++++ tests/target/issue-4243.rs | 28 +++++++ 3 files changed, 167 insertions(+), 44 deletions(-) create mode 100644 tests/source/issue-4243.rs create mode 100644 tests/target/issue-4243.rs diff --git a/src/types.rs b/src/types.rs index e8bd74d5a12..7eeee2ac979 100644 --- a/src/types.rs +++ b/src/types.rs @@ -874,57 +874,131 @@ fn join_bounds( items: &[ast::GenericBound], need_indent: bool, ) -> Option { - debug_assert!(!items.is_empty()); - - // Try to join types in a single line - let joiner = match context.config.type_punctuation_density() { - TypeDensity::Compressed => "+", - TypeDensity::Wide => " + ", - }; - let type_strs = items - .iter() - .map(|item| item.rewrite(context, shape)) - .collect::>>()?; - let result = type_strs.join(joiner); - if items.len() <= 1 || (!result.contains('\n') && result.len() <= shape.width) { - return Some(result); - } + join_bounds_inner(context, shape, items, need_indent, false) +} - // We need to use multiple lines. - let (type_strs, offset) = if need_indent { - // Rewrite with additional indentation. - let nested_shape = shape - .block_indent(context.config.tab_spaces()) - .with_max_width(context.config); - let type_strs = items - .iter() - .map(|item| item.rewrite(context, nested_shape)) - .collect::>>()?; - (type_strs, nested_shape.indent) - } else { - (type_strs, shape.indent) - }; +fn join_bounds_inner( + context: &RewriteContext<'_>, + 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 mut result = String::with_capacity(128); - result.push_str(&type_strs[0]); - let mut can_be_put_on_the_same_line = is_bound_extendable(&result, &items[0]); - let generic_bounds_in_order = is_generic_bounds_in_order(items); - for (bound, bound_str) in items[1..].iter().zip(type_strs[1..].iter()) { - if generic_bounds_in_order && can_be_put_on_the_same_line { - result.push_str(joiner); - } else { - result.push_str(&offset.to_string_with_newline(context.config)); - result.push_str("+ "); - } - result.push_str(bound_str); - can_be_put_on_the_same_line = is_bound_extendable(bound_str, bound); - } - Some(result) + let result = items.iter().enumerate().try_fold( + (String::new(), None, false), + |(strs, prev_trailing_span, prev_extendable), (i, item)| { + let trailing_span = if i < items.len() - 1 { + let hi = context + .snippet_provider + .span_before(mk_sp(items[i + 1].span().lo(), item.span().hi()), "+"); + + Some(mk_sp(item.span().hi(), hi)) + } else { + None + }; + let (leading_span, has_leading_comment) = if i > 0 { + let lo = context + .snippet_provider + .span_after(mk_sp(items[i - 1].span().hi(), item.span().lo()), "+"); + + let span = mk_sp(lo, item.span().lo()); + + let has_comments = contains_comment(context.snippet(span)); + + (Some(mk_sp(lo, item.span().lo())), 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 i > 0 && 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) { + shape + .indent + .to_string_with_newline(context.config) + .to_string() + } else { + 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 (trailing_str, extendable) = if i == 0 { + let bound_str = item.rewrite(context, shape)?; + let bound_str_clone = bound_str.clone(); + (bound_str, is_bound_extendable(&bound_str_clone, item)) + } else { + let bound_str = &item.rewrite(context, shape)?; + match leading_span { + Some(ls) if has_leading_comment => ( + combine_strs_with_missing_comments( + context, joiner, bound_str, ls, shape, true, + )?, + is_bound_extendable(bound_str, item), + ), + _ => ( + String::from(joiner) + bound_str, + is_bound_extendable(bound_str, item), + ), + } + }; + 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(( + String::from(strs) + &trailing_str, + trailing_span, + extendable, + )), + } + }, + )?; + + if !force_newline + && items.len() > 1 + && (result.0.contains('\n') || result.0.len() > shape.width) + { + join_bounds_inner(context, shape, items, need_indent, true) + } else { + Some(result.0) + } } pub(crate) fn can_be_overflowed_type( diff --git a/tests/source/issue-4243.rs b/tests/source/issue-4243.rs new file mode 100644 index 00000000000..d8a27f7a4a4 --- /dev/null +++ b/tests/source/issue-4243.rs @@ -0,0 +1,21 @@ +fn main() { + type A: AA /*AA*/ + /*AB*/ AB ++ AC = AA +/*AA*/ + + /*AB*/ AB+AC; + + type B: BA /*BAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA*/+/*BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB*/ BB + + BC = BA /*BAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA*/ + /*BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB*/ BB+ BC; + + type C: CA // CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA +// CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA + + + // CBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB + // CBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB + CB + CC = CA // CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA + // CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA + + + // CBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB + // CBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB + CB+ CC; +} diff --git a/tests/target/issue-4243.rs b/tests/target/issue-4243.rs new file mode 100644 index 00000000000..67fa1d2a312 --- /dev/null +++ b/tests/target/issue-4243.rs @@ -0,0 +1,28 @@ +fn main() { + type A: AA /*AA*/ + /*AB*/ AB + AC = AA + /*AA*/ + + + /*AB*/ + AB + + AC; + + type B: BA /*BAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA*/ + + /*BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB*/ BB + + BC = BA /*BAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA*/ + + /*BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB*/ BB + + BC; + + type C: CA // CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA + // CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA + + + // CBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB + // CBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB + CB + + CC = CA // CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA + // CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA + + + // CBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB + // CBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB + CB + + CC; +} From e0592d5e654cf1c516e6f823724842123c95fcdf Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Sun, 29 Nov 2020 12:32:15 -0600 Subject: [PATCH 11/11] meta: bump to v1.4.28 --- CHANGELOG.md | 32 +++++++++++++++++++++++++++++++- Cargo.lock | 2 +- Cargo.toml | 2 +- 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ac6638b848..81ea44042d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,36 @@ ## [Unreleased] +## [1.4.28] 2020-11-29 + +### Changed + +- `rustc-ap-*` crates updated to v691.0.0 +- In the event of an invalid inner attribute on a `cfg_if` condition, rustfmt will now attempt to continue and format the imported modules. Previously rustfmt would emit the parser error about an inner attribute being invalid in this position, but for rustfmt's purposes the invalid attribute doesn't prevent nor impact module formatting. + +### Added + +- [`group_imports`][group-imports-config-docs] - a new configuration option that allows users to control the strategy used for grouping imports ([#4107](https://github.com/rust-lang/rustfmt/issues/4107)) + +[group-imports-config-docs]: https://github.com/rust-lang/rustfmt/blob/v1.4.28/Configurations.md#group_imports + +### Fixed +- Formatting of malformed derived attributes is no longer butchered. ([#3898](https://github.com/rust-lang/rustfmt/issues/3898), [#4029](https://github.com/rust-lang/rustfmt/issues/4029), [#4115](https://github.com/rust-lang/rustfmt/issues/4115), [#4545](https://github.com/rust-lang/rustfmt/issues/4545)) +- Correct indentation used in macro branches when `hard_tabs` is enabled. ([#4152](https://github.com/rust-lang/rustfmt/issues/4152)) +- Comments between the visibility modifier and item name are no longer dropped. ([#2781](https://github.com/rust-lang/rustfmt/issues/2781)) +- Comments preceding the assignment operator in type aliases are no longer dropped. ([#4244](https://github.com/rust-lang/rustfmt/issues/4244)) +- Comments between {`&` operator, lifetime, `mut` kw, type} are no longer dropped. ([#4245](https://github.com/rust-lang/rustfmt/issues/4245)) +- Comments between type bounds are no longer dropped. ([#4243](https://github.com/rust-lang/rustfmt/issues/4243)) +- Function headers are no longer dropped on foreign function items. ([#4288](https://github.com/rust-lang/rustfmt/issues/4288)) +- Foreign function blocks are no longer dropped. ([#4313](https://github.com/rust-lang/rustfmt/issues/4313)) +- `where_single_line` is no longer incorrectly applied to multiline function signatures that have no `where` clause. ([#4547](https://github.com/rust-lang/rustfmt/issues/4547)) + +### Install/Download Options +- **crates.io package** - *pending* +- **rustup (nightly)** - *pending* +- **GitHub Release Binaries** - [Release v1.4.28](https://github.com/rust-lang/rustfmt/releases/tag/v1.4.28) +- **Build from source** - [Tag v1.4.28](https://github.com/rust-lang/rustfmt/tree/v1.4.28), see instructions for how to [install rustfmt from source][install-from-source] + ## [1.4.27] 2020-11-16 ### Fixed @@ -10,7 +40,7 @@ ### Install/Download Options - **crates.io package** - *pending* -- **rustup (nightly)** - *pending* +- **rustup (nightly)** - Starting in `2020-11-18` - **GitHub Release Binaries** - [Release v1.4.27](https://github.com/rust-lang/rustfmt/releases/tag/v1.4.27) - **Build from source** - [Tag v1.4.27](https://github.com/rust-lang/rustfmt/tree/v1.4.27), see instructions for how to [install rustfmt from source][install-from-source] diff --git a/Cargo.lock b/Cargo.lock index f29b73235b8..bf0afe6e8bd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1243,7 +1243,7 @@ dependencies = [ [[package]] name = "rustfmt-nightly" -version = "1.4.27" +version = "1.4.28" dependencies = [ "annotate-snippets 0.6.1", "anyhow", diff --git a/Cargo.toml b/Cargo.toml index 95f93be6be7..ecef24f7f73 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "rustfmt-nightly" -version = "1.4.27" +version = "1.4.28" authors = ["Nicholas Cameron ", "The Rustfmt developers"] description = "Tool to find and fix Rust formatting issues" repository = "https://github.com/rust-lang/rustfmt"