prepare v1.4.28 release

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]
 

Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 
 name = "rustfmt-nightly"
-version = "1.4.27"
+version = "1.4.28"
 authors = ["Nicholas Cameron <[email protected]>", "The Rustfmt developers"]
 description = "Tool to find and fix Rust formatting issues"
 repository = "https://github.com/rust-lang/rustfmt"

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};
@@ -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<impl Iterator<Item = Span> + '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<String> {
-    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::<Option<Vec<_>>>()?
+        // Collect the results into a single, flat, Vec.
+        .into_iter()
+        .flatten()
+        .collect::<Vec<_>>();
+
+    // 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<I, T, F1, F2, F3>(
-    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<usize>,
-    combine: bool,
-) -> Option<String>
-where
-    I: Iterator<Item = T>,
-    F1: Fn(&T) -> BytePos,
-    F2: Fn(&T) -> BytePos,
-    F3: Fn(&T) -> Option<String>,
-{
-    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::<Vec<_>>();
-    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<String> {
         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