From 11462550e98c4d9361b9cac9572c4f6eb0353adb Mon Sep 17 00:00:00 2001 From: "Adam H. Leventhal" Date: Tue, 21 Apr 2020 22:57:54 -0700 Subject: [PATCH] fixes #4115, #4029, #3898 --- rustfmt-core/rustfmt-lib/src/attr.rs | 186 +++++++++++++-------------- 1 file changed, 92 insertions(+), 94 deletions(-) diff --git a/rustfmt-core/rustfmt-lib/src/attr.rs b/rustfmt-core/rustfmt-lib/src/attr.rs index 1b83a564a33..6976168ce0a 100644 --- a/rustfmt-core/rustfmt-lib/src/attr.rs +++ b/rustfmt-core/rustfmt-lib/src/attr.rs @@ -2,7 +2,7 @@ use rustc_ast::ast; use rustc_ast::attr::HasAttrs; -use rustc_span::{symbol::sym, BytePos, Span, DUMMY_SP}; +use rustc_span::{symbol::sym, Span}; use self::doc_comment::DocCommentFormatter; use crate::comment::{contains_comment, rewrite_doc_comment, CommentStyle}; @@ -52,15 +52,6 @@ fn is_derive(attr: &ast::Attribute) -> bool { attr.check_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, @@ -89,36 +80,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) } @@ -244,7 +303,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()); @@ -258,61 +317,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); @@ -417,13 +421,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