inline the attribute with its item even with the macro_use attribute or when reorder_imports is disabled (#3598)

Author: scampi

src/items.rs

@@ -9,6 +9,7 @@ use syntax::source_map::{self, BytePos, Span};
 use syntax::visit;
 use syntax::{ast, ptr, symbol};
 
+use crate::attr::filter_inline_attrs;
 use crate::comment::{
     combine_strs_with_missing_comments, contains_comment, is_last_comment_block,
     recover_comment_removed, recover_missing_comment_in_span, rewrite_missing_comment,
@@ -2977,29 +2978,71 @@ impl Rewrite for ast::ForeignItem {
     }
 }
 
+/// Rewrite the attributes of an item.
+fn rewrite_attrs(
+    context: &RewriteContext<'_>,
+    item: &ast::Item,
+    item_str: &str,
+    shape: Shape,
+) -> Option<String> {
+    let attrs = filter_inline_attrs(&item.attrs, item.span());
+    let attrs_str = attrs.rewrite(context, shape)?;
+
+    let missed_span = if attrs.is_empty() {
+        mk_sp(item.span.lo(), item.span.lo())
+    } else {
+        mk_sp(attrs[attrs.len() - 1].span.hi(), item.span.lo())
+    };
+
+    let allow_extend = if attrs.len() == 1 {
+        let line_len = attrs_str.len() + 1 + item_str.len();
+        !attrs.first().unwrap().is_sugared_doc
+            && context.config.inline_attribute_width() >= line_len
+    } else {
+        false
+    };
+
+    combine_strs_with_missing_comments(
+        context,
+        &attrs_str,
+        &item_str,
+        missed_span,
+        shape,
+        allow_extend,
+    )
+}
+
 /// Rewrite an inline mod.
-pub(crate) fn rewrite_mod(context: &RewriteContext<'_>, item: &ast::Item) -> String {
+/// The given shape is used to format the mod's attributes.
+pub(crate) fn rewrite_mod(
+    context: &RewriteContext<'_>,
+    item: &ast::Item,
+    attrs_shape: Shape,
+) -> Option<String> {
     let mut result = String::with_capacity(32);
     result.push_str(&*format_visibility(context, &item.vis));
     result.push_str("mod ");
     result.push_str(rewrite_ident(context, item.ident));
     result.push(';');
-    result
+    rewrite_attrs(context, item, &result, attrs_shape)
 }
 
-/// Rewrite `extern crate foo;` WITHOUT attributes.
+/// Rewrite `extern crate foo;`.
+/// The given shape is used to format the extern crate's attributes.
 pub(crate) fn rewrite_extern_crate(
     context: &RewriteContext<'_>,
     item: &ast::Item,
+    attrs_shape: Shape,
 ) -> Option<String> {
     assert!(is_extern_crate(item));
     let new_str = context.snippet(item.span);
-    Some(if contains_comment(new_str) {
+    let item_str = if contains_comment(new_str) {
         new_str.to_owned()
     } else {
         let no_whitespace = &new_str.split_whitespace().collect::<Vec<&str>>().join(" ");
         String::from(&*Regex::new(r"\s;").unwrap().replace(no_whitespace, ";"))
-    })
+    };
+    rewrite_attrs(context, item, &item_str, attrs_shape)
 }
 
 /// Returns `true` for `mod foo;`, false for `mod foo { .. }`.

src/reorder.rs

@@ -11,12 +11,11 @@ use std::cmp::{Ord, Ordering};
 use syntax::{ast, attr, source_map::Span, symbol::sym};
 
 use crate::attr::filter_inline_attrs;
-use crate::comment::combine_strs_with_missing_comments;
 use crate::config::Config;
 use crate::imports::{merge_use_trees, UseTree};
 use crate::items::{is_mod_decl, rewrite_extern_crate, rewrite_mod};
 use crate::lists::{itemize_list, write_list, ListFormatting, ListItem};
-use crate::rewrite::{Rewrite, RewriteContext};
+use crate::rewrite::RewriteContext;
 use crate::shape::Shape;
 use crate::source_map::LineRangeUtils;
 use crate::spanned::Spanned;
@@ -70,37 +69,11 @@ fn rewrite_reorderable_item(
     item: &ast::Item,
     shape: Shape,
 ) -> Option<String> {
-    let attrs = filter_inline_attrs(&item.attrs, item.span());
-    let attrs_str = attrs.rewrite(context, shape)?;
-
-    let missed_span = if attrs.is_empty() {
-        mk_sp(item.span.lo(), item.span.lo())
-    } else {
-        mk_sp(attrs.last().unwrap().span.hi(), item.span.lo())
-    };
-
-    let item_str = match item.node {
-        ast::ItemKind::ExternCrate(..) => rewrite_extern_crate(context, item)?,
-        ast::ItemKind::Mod(..) => rewrite_mod(context, item),
-        _ => return None,
-    };
-
-    let allow_extend = if attrs.len() == 1 {
-        let line_len = attrs_str.len() + 1 + item_str.len();
-        !attrs.first().unwrap().is_sugared_doc
-            && context.config.inline_attribute_width() >= line_len
-    } else {
-        false
-    };
-
-    combine_strs_with_missing_comments(
-        context,
-        &attrs_str,
-        &item_str,
-        missed_span,
-        shape,
-        allow_extend,
-    )
+    match item.node {
+        ast::ItemKind::ExternCrate(..) => rewrite_extern_crate(context, item, shape),
+        ast::ItemKind::Mod(..) => rewrite_mod(context, item, shape),
+        _ => None,
+    }
 }
 
 /// Rewrite a list of items with reordering. Every item in `items` must have

src/visitor.rs

@@ -315,8 +315,8 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
             .append(&mut get_skip_macro_names(&attrs));
 
         let should_visit_node_again = match item.node {
-            // For use items, skip rewriting attributes. Just check for a skip attribute.
-            ast::ItemKind::Use(..) => {
+            // For use/extern crate items, skip rewriting attributes but check for a skip attribute.
+            ast::ItemKind::Use(..) | ast::ItemKind::ExternCrate(_) => {
                 if contains_skip(attrs) {
                     self.push_skipped_with_span(attrs.as_slice(), item.span(), item.span());
                     false
@@ -381,8 +381,13 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
                     self.push_rewrite(item.span, rw);
                 }
                 ast::ItemKind::ExternCrate(_) => {
-                    let rw = rewrite_extern_crate(&self.get_context(), item);
-                    self.push_rewrite(item.span, rw);
+                    let rw = rewrite_extern_crate(&self.get_context(), item, self.shape());
+                    let span = if attrs.is_empty() {
+                        item.span
+                    } else {
+                        mk_sp(attrs[0].span.lo(), item.span.hi())
+                    };
+                    self.push_rewrite(span, rw);
                 }
                 ast::ItemKind::Struct(..) | ast::ItemKind::Union(..) => {
                     self.visit_struct(&StructParts::from_item(item));

tests/source/issue-3585/extern_crate.rs

@@ -0,0 +1,12 @@
+// rustfmt-inline_attribute_width: 100
+
+#[macro_use]
+extern crate static_assertions;
+
+#[cfg(unix)]
+extern crate static_assertions;
+
+// a comment before the attribute
+#[macro_use]
+// some comment after
+extern crate static_assertions;

tests/source/issue-3585/reorder_imports_disabled.rs

@@ -0,0 +1,12 @@
+// rustfmt-inline_attribute_width: 100
+// rustfmt-reorder_imports: false
+
+#[cfg(unix)]
+extern crate crateb;
+#[cfg(unix)]
+extern crate cratea;
+
+#[cfg(unix)]
+use crateb;
+#[cfg(unix)]
+use cratea;

tests/source/issue-3585/reorder_imports_enabled.rs

@@ -0,0 +1,12 @@
+// rustfmt-inline_attribute_width: 100
+// rustfmt-reorder_imports: true
+
+#[cfg(unix)]
+extern crate crateb;
+#[cfg(unix)]
+extern crate cratea;
+
+#[cfg(unix)]
+use crateb;
+#[cfg(unix)]
+use cratea;

tests/source/issue-3585/use.rs

@@ -0,0 +1,7 @@
+// rustfmt-inline_attribute_width: 100
+
+#[macro_use]
+use static_assertions;
+
+#[cfg(unix)]
+use static_assertions;

tests/target/issue-3585/extern_crate.rs

@@ -0,0 +1,10 @@
+// rustfmt-inline_attribute_width: 100
+
+#[macro_use] extern crate static_assertions;
+
+#[cfg(unix)] extern crate static_assertions;
+
+// a comment before the attribute
+#[macro_use]
+// some comment after
+extern crate static_assertions;

tests/target/issue-3585/reorder_imports_disabled.rs

@@ -0,0 +1,8 @@
+// rustfmt-inline_attribute_width: 100
+// rustfmt-reorder_imports: false
+
+#[cfg(unix)] extern crate crateb;
+#[cfg(unix)] extern crate cratea;
+
+#[cfg(unix)] use crateb;
+#[cfg(unix)] use cratea;

tests/target/issue-3585/reorder_imports_enabled.rs

@@ -0,0 +1,8 @@
+// rustfmt-inline_attribute_width: 100
+// rustfmt-reorder_imports: true
+
+#[cfg(unix)] extern crate cratea;
+#[cfg(unix)] extern crate crateb;
+
+#[cfg(unix)] use cratea;
+#[cfg(unix)] use crateb;

tests/target/issue-3585/use.rs

@@ -0,0 +1,5 @@
+// rustfmt-inline_attribute_width: 100
+
+#[macro_use] use static_assertions;
+
+#[cfg(unix)] use static_assertions;