From 685e0e5ba4d251469aca18e5c8afe0953cb110fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Campinas?= Date: Mon, 15 Apr 2019 23:13:35 +0200 Subject: [PATCH 1/2] handle field attributes when aligning a struct's fields the `items::rewrite_struct_field` method was already handling multiline field names via `last_line_width`, which was rendered moot by the previous logic in `vertical::AlignedItem::struct_field_prefix_max_min_width`. Close #2869 --- src/items.rs | 2 +- src/vertical.rs | 14 ++++---- tests/source/issue-2869.rs | 36 +++++++++++++++++++ .../struct_field_align_threshold/20.rs | 6 ++-- tests/target/issue-2869.rs | 36 +++++++++++++++++++ 5 files changed, 82 insertions(+), 12 deletions(-) create mode 100644 tests/source/issue-2869.rs create mode 100644 tests/target/issue-2869.rs diff --git a/src/items.rs b/src/items.rs index cdf9fa06eef..74af9869c08 100644 --- a/src/items.rs +++ b/src/items.rs @@ -1600,7 +1600,7 @@ pub(crate) fn rewrite_struct_field( shape, attrs_extendable, )?; - let overhead = last_line_width(&attr_prefix); + let overhead = trimmed_last_line_width(&attr_prefix); let lhs_offset = lhs_max_width.saturating_sub(overhead); for _ in 0..lhs_offset { spacing.push(' '); diff --git a/src/vertical.rs b/src/vertical.rs index 62cc7d8b33d..a490b9415be 100644 --- a/src/vertical.rs +++ b/src/vertical.rs @@ -17,7 +17,9 @@ use crate::rewrite::{Rewrite, RewriteContext}; use crate::shape::{Indent, Shape}; use crate::source_map::SpanUtils; use crate::spanned::Spanned; -use crate::utils::{contains_skip, is_attributes_extendable, mk_sp, rewrite_ident}; +use crate::utils::{ + contains_skip, is_attributes_extendable, mk_sp, rewrite_ident, trimmed_last_line_width, +}; pub(crate) trait AlignedItem { fn skip(&self) -> bool; @@ -183,13 +185,9 @@ fn struct_field_prefix_max_min_width( fields .iter() .map(|field| { - field.rewrite_prefix(context, shape).and_then(|field_str| { - if field_str.contains('\n') { - None - } else { - Some(field_str.len()) - } - }) + field + .rewrite_prefix(context, shape) + .map(|field_str| trimmed_last_line_width(&field_str)) }) .fold_options((0, ::std::usize::MAX), |(max_len, min_len), len| { (cmp::max(max_len, len), cmp::min(min_len, len)) diff --git a/tests/source/issue-2869.rs b/tests/source/issue-2869.rs new file mode 100644 index 00000000000..4c5bff3df6f --- /dev/null +++ b/tests/source/issue-2869.rs @@ -0,0 +1,36 @@ +// rustfmt-struct_field_align_threshold: 50 + +#[derive(Serialize, Deserialize, Debug)] +#[serde(rename_all = "PascalCase")] +struct AuditLog1 { + creation_time: String, + id: String, + operation: String, + organization_id: String, + record_type: u32, + result_status: Option, + #[serde(rename = "ClientIP")] + client_ip: Option, + object_id: String, + actor: Option>, + actor_context_id: Option, + actor_ip_address: Option, + azure_active_directory_event_type: Option, +} + +#[derive(Serialize, Deserialize, Debug)] +#[serde(rename_all = "PascalCase")] +struct AuditLog2 { + creation_time: String, + id: String, + operation: String, + organization_id: String, + record_type: u32, + result_status: Option, + client_ip: Option, + object_id: String, + actor: Option>, + actor_context_id: Option, + actor_ip_address: Option, + azure_active_directory_event_type: Option, +} diff --git a/tests/target/configs/struct_field_align_threshold/20.rs b/tests/target/configs/struct_field_align_threshold/20.rs index 848abd3d3cf..2830af3ad5d 100644 --- a/tests/target/configs/struct_field_align_threshold/20.rs +++ b/tests/target/configs/struct_field_align_threshold/20.rs @@ -41,9 +41,9 @@ pub struct Foo { f: SomeType, // Comment beside a field // Comment on a field #[AnAttribute] - g: SomeOtherType, + g: SomeOtherType, /// A doc comment on a field - h: AThirdType, + h: AThirdType, pub i: TypeForPublicField, } @@ -66,7 +66,7 @@ struct X { pub struct Writebatch { #[allow(dead_code)] // only used for holding the internal pointer writebatch: RawWritebatch, - marker: PhantomData, + marker: PhantomData, } struct Bar; diff --git a/tests/target/issue-2869.rs b/tests/target/issue-2869.rs new file mode 100644 index 00000000000..19bca814c03 --- /dev/null +++ b/tests/target/issue-2869.rs @@ -0,0 +1,36 @@ +// rustfmt-struct_field_align_threshold: 50 + +#[derive(Serialize, Deserialize, Debug)] +#[serde(rename_all = "PascalCase")] +struct AuditLog1 { + creation_time: String, + id: String, + operation: String, + organization_id: String, + record_type: u32, + result_status: Option, + #[serde(rename = "ClientIP")] + client_ip: Option, + object_id: String, + actor: Option>, + actor_context_id: Option, + actor_ip_address: Option, + azure_active_directory_event_type: Option, +} + +#[derive(Serialize, Deserialize, Debug)] +#[serde(rename_all = "PascalCase")] +struct AuditLog2 { + creation_time: String, + id: String, + operation: String, + organization_id: String, + record_type: u32, + result_status: Option, + client_ip: Option, + object_id: String, + actor: Option>, + actor_context_id: Option, + actor_ip_address: Option, + azure_active_directory_event_type: Option, +} From 372d45104d1b3e365f6d58cc06ab82c9b60c6a99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Campinas?= Date: Sun, 29 Sep 2019 00:41:05 +0200 Subject: [PATCH 2/2] blank lines separate alignment groups in structs --- src/vertical.rs | 19 ++++++++----------- tests/source/issue-2869.rs | 5 +++++ .../struct_field_align_threshold/20.rs | 4 ++-- tests/target/issue-2869.rs | 5 +++++ 4 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/vertical.rs b/src/vertical.rs index a490b9415be..59b03789fb5 100644 --- a/src/vertical.rs +++ b/src/vertical.rs @@ -6,7 +6,7 @@ use itertools::Itertools; use syntax::ast; use syntax::source_map::{BytePos, Span}; -use crate::comment::{combine_strs_with_missing_comments, contains_comment}; +use crate::comment::combine_strs_with_missing_comments; use crate::config::lists::*; use crate::expr::rewrite_field; use crate::items::{rewrite_struct_field, rewrite_struct_field_prefix}; @@ -253,6 +253,9 @@ fn rewrite_aligned_items_inner( write_list(&items, &fmt) } +/// Returns the index in `fields` up to which a field belongs to the current group. +/// The returned string is the group separator to use when rewriting the fields. +/// Groups are defined by blank lines. fn group_aligned_items( context: &RewriteContext<'_>, fields: &[T], @@ -262,7 +265,6 @@ fn group_aligned_items( if fields[i].skip() { return ("", index); } - // See if there are comments or empty lines between fields. let span = mk_sp(fields[i].get_span().hi(), fields[i + 1].get_span().lo()); let snippet = context .snippet(span) @@ -270,17 +272,12 @@ fn group_aligned_items( .skip(1) .collect::>() .join("\n"); - let spacings = if snippet + let has_blank_line = snippet .lines() .dropping_back(1) - .any(|l| l.trim().is_empty()) - { - "\n" - } else { - "" - }; - if contains_comment(&snippet) || snippet.lines().count() > 1 { - return (spacings, index); + .any(|l| l.trim().is_empty()); + if has_blank_line { + return ("\n", index); } index += 1; } diff --git a/tests/source/issue-2869.rs b/tests/source/issue-2869.rs index 4c5bff3df6f..d18adfb462e 100644 --- a/tests/source/issue-2869.rs +++ b/tests/source/issue-2869.rs @@ -16,6 +16,11 @@ struct AuditLog1 { actor_context_id: Option, actor_ip_address: Option, azure_active_directory_event_type: Option, + + #[serde(rename = "very")] + aaaaa: String, + #[serde(rename = "cool")] + bb: i32, } #[derive(Serialize, Deserialize, Debug)] diff --git a/tests/target/configs/struct_field_align_threshold/20.rs b/tests/target/configs/struct_field_align_threshold/20.rs index 2830af3ad5d..12a523e9d83 100644 --- a/tests/target/configs/struct_field_align_threshold/20.rs +++ b/tests/target/configs/struct_field_align_threshold/20.rs @@ -38,7 +38,7 @@ fn main() { pub struct Foo { #[rustfmt::skip] f : SomeType, // Comment beside a field - f: SomeType, // Comment beside a field + f: SomeType, // Comment beside a field // Comment on a field #[AnAttribute] g: SomeOtherType, @@ -323,7 +323,7 @@ fn main() { // Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec a diam lectus. Sed sit // amet ipsum mauris. Maecenas congue ligula ac quam viverra nec consectetur ante // hendrerit. Donec et mollis dolor. - first: item(), + first: item(), // Praesent et diam eget libero egestas mattis sit amet vitae augue. // Nam tincidunt congue enim, ut porta lorem lacinia consectetur. second: Item, diff --git a/tests/target/issue-2869.rs b/tests/target/issue-2869.rs index 19bca814c03..6a68c2d95fa 100644 --- a/tests/target/issue-2869.rs +++ b/tests/target/issue-2869.rs @@ -16,6 +16,11 @@ struct AuditLog1 { actor_context_id: Option, actor_ip_address: Option, azure_active_directory_event_type: Option, + + #[serde(rename = "very")] + aaaaa: String, + #[serde(rename = "cool")] + bb: i32, } #[derive(Serialize, Deserialize, Debug)]