Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 48 additions & 5 deletions src/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 { .. }`.
Expand Down
39 changes: 6 additions & 33 deletions src/reorder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
13 changes: 9 additions & 4 deletions src/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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));
Expand Down
12 changes: 12 additions & 0 deletions tests/source/issue-3585/extern_crate.rs
Original file line number Diff line number Diff line change
@@ -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;
12 changes: 12 additions & 0 deletions tests/source/issue-3585/reorder_imports_disabled.rs
Original file line number Diff line number Diff line change
@@ -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;
12 changes: 12 additions & 0 deletions tests/source/issue-3585/reorder_imports_enabled.rs
Original file line number Diff line number Diff line change
@@ -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;
7 changes: 7 additions & 0 deletions tests/source/issue-3585/use.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// rustfmt-inline_attribute_width: 100

#[macro_use]
use static_assertions;

#[cfg(unix)]
use static_assertions;
10 changes: 10 additions & 0 deletions tests/target/issue-3585/extern_crate.rs
Original file line number Diff line number Diff line change
@@ -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;
8 changes: 8 additions & 0 deletions tests/target/issue-3585/reorder_imports_disabled.rs
Original file line number Diff line number Diff line change
@@ -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;
8 changes: 8 additions & 0 deletions tests/target/issue-3585/reorder_imports_enabled.rs
Original file line number Diff line number Diff line change
@@ -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;
5 changes: 5 additions & 0 deletions tests/target/issue-3585/use.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// rustfmt-inline_attribute_width: 100

#[macro_use] use static_assertions;

#[cfg(unix)] use static_assertions;