handle field attributes when aligning a struct's fields (rust-lang#3513)

rust-lang-ci · Oct 19, 2019 · 5327c36 · 5327c36
1 parent a5d16df
commit 5327c36
Show file tree

Hide file tree

Showing 5 changed files with 102 additions and 25 deletions.
diff --git a/src/items.rs b/src/items.rs
@@ -1617,7 +1617,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
@@ -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};
@@ -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<T: AlignedItem>(
     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))
@@ -255,6 +253,9 @@ fn rewrite_aligned_items_inner<T: AlignedItem>(
     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<T: AlignedItem>(
     context: &RewriteContext<'_>,
     fields: &[T],
@@ -264,25 +265,19 @@ fn group_aligned_items<T: AlignedItem>(
         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)
             .lines()
             .skip(1)
             .collect::<Vec<_>>()
             .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
@@ -0,0 +1,41 @@
+// 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<String>,
+    #[serde(rename = "ClientIP")]
+    client_ip: Option<IpAddr>,
+    object_id: String,
+    actor: Option<Vec<IDType>>,
+    actor_context_id: Option<String>,
+    actor_ip_address: Option<IpAddr>,
+    azure_active_directory_event_type: Option<u8>,
+
+    #[serde(rename = "very")]
+    aaaaa: String,
+    #[serde(rename = "cool")]
+    bb: i32,
+}
+
+#[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<String>,
+    client_ip: Option<IpAddr>,
+    object_id: String,
+    actor: Option<Vec<IDType>>,
+    actor_context_id: Option<String>,
+    actor_ip_address: Option<IpAddr>,
+    azure_active_directory_event_type: Option<u8>,
+}
diff --git a/tests/target/configs/struct_field_align_threshold/20.rs b/tests/target/configs/struct_field_align_threshold/20.rs
@@ -38,12 +38,12 @@ 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,
+    g:     SomeOtherType,
     /// A doc comment on a field
-    h: AThirdType,
+    h:     AThirdType,
     pub i: TypeForPublicField,
 }
 
@@ -66,7 +66,7 @@ struct X {
 pub struct Writebatch<K: Key> {
     #[allow(dead_code)] // only used for holding the internal pointer
     writebatch: RawWritebatch,
-    marker: PhantomData<K>,
+    marker:     PhantomData<K>,
 }
 
 struct Bar;
@@ -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
@@ -0,0 +1,41 @@
+// 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<String>,
+    #[serde(rename = "ClientIP")]
+    client_ip:                         Option<IpAddr>,
+    object_id:                         String,
+    actor:                             Option<Vec<IDType>>,
+    actor_context_id:                  Option<String>,
+    actor_ip_address:                  Option<IpAddr>,
+    azure_active_directory_event_type: Option<u8>,
+
+    #[serde(rename = "very")]
+    aaaaa: String,
+    #[serde(rename = "cool")]
+    bb:    i32,
+}
+
+#[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<String>,
+    client_ip:                         Option<IpAddr>,
+    object_id:                         String,
+    actor:                             Option<Vec<IDType>>,
+    actor_context_id:                  Option<String>,
+    actor_ip_address:                  Option<IpAddr>,
+    azure_active_directory_event_type: Option<u8>,
+}