Merge pull request #2656 from topecongiro/issue-2594

Use consistent formatting for empty enum and struct
rust-lang · Apr 29, 2018 · dae9fb6 · dae9fb6
2 parents fb9a5ad + 3432807
commit dae9fb6
Show file tree

Hide file tree

Showing 8 changed files with 102 additions and 66 deletions.
diff --git a/src/items.rs b/src/items.rs
@@ -33,11 +33,7 @@ use rewrite::{Rewrite, RewriteContext};
 use shape::{Indent, Shape};
 use spanned::Spanned;
 use types::TraitTyParamBounds;
-use utils::{colon_spaces, contains_skip, first_line_width, format_abi, format_auto,
-            format_constness, format_defaultness, format_mutability, format_unsafety,
-            format_visibility, is_attributes_extendable, last_line_contains_single_line_comment,
-            last_line_used_width, last_line_width, mk_sp, semicolon_for_expr, starts_with_newline,
-            stmt_expr, trimmed_last_line_width};
+use utils::*;
 use vertical::rewrite_with_alignment;
 use visitor::FmtVisitor;
 
@@ -458,35 +454,39 @@ impl<'a> FmtVisitor<'a> {
 
         self.last_pos = body_start;
 
-        self.block_indent = self.block_indent.block_indent(self.config);
-        let variant_list = self.format_variant_list(enum_def, body_start, span.hi() - BytePos(1));
-        match variant_list {
-            Some(ref body_str) => self.push_str(body_str),
-            None => self.format_missing_no_indent(span.hi() - BytePos(1)),
-        }
-        self.block_indent = self.block_indent.block_unindent(self.config);
-
-        if variant_list.is_some() || contains_comment(&enum_snippet[brace_pos..]) {
-            let indent_str = self.block_indent.to_string(self.config);
-            self.push_str(&indent_str);
+        match self.format_variant_list(enum_def, body_start, span.hi()) {
+            Some(ref s) if enum_def.variants.is_empty() => self.push_str(s),
+            rw => {
+                self.push_rewrite(mk_sp(body_start, span.hi()), rw);
+                self.block_indent = self.block_indent.block_unindent(self.config);
+            }
         }
-        self.push_str("}");
-        self.last_pos = span.hi();
     }
 
     // Format the body of an enum definition
     fn format_variant_list(
-        &self,
+        &mut self,
         enum_def: &ast::EnumDef,
         body_lo: BytePos,
         body_hi: BytePos,
     ) -> Option<String> {
         if enum_def.variants.is_empty() {
-            return None;
+            let mut buffer = String::with_capacity(128);
+            // 1 = "}"
+            let span = mk_sp(body_lo, body_hi - BytePos(1));
+            format_empty_struct_or_tuple(
+                &self.get_context(),
+                span,
+                self.block_indent,
+                &mut buffer,
+                "",
+                "}",
+            );
+            return Some(buffer);
         }
         let mut result = String::with_capacity(1024);
-        let indentation = self.block_indent.to_string_with_newline(self.config);
-        result.push_str(&indentation);
+        let original_offset = self.block_indent;
+        self.block_indent = self.block_indent.block_indent(self.config);
 
         let itemize_list_with = |one_line_width: usize| {
             itemize_list(
@@ -531,7 +531,8 @@ impl<'a> FmtVisitor<'a> {
 
         let list = write_list(&items, &fmt)?;
         result.push_str(&list);
-        result.push('\n');
+        result.push_str(&original_offset.to_string_with_newline(self.config));
+        result.push('}');
         Some(result)
     }
 
@@ -1195,18 +1196,8 @@ pub fn format_struct_struct(
     }
 
     if fields.is_empty() {
-        let snippet = context.snippet(mk_sp(body_lo, span.hi() - BytePos(1)));
-        if snippet.trim().is_empty() {
-            // `struct S {}`
-        } else if snippet.trim_right_matches(&[' ', '\t'][..]).ends_with('\n') {
-            // fix indent
-            result.push_str(snippet.trim_right());
-            result.push('\n');
-            result.push_str(&offset.to_string(context.config));
-        } else {
-            result.push_str(snippet);
-        }
-        result.push('}');
+        let inner_span = mk_sp(body_lo, span.hi() - BytePos(1));
+        format_empty_struct_or_tuple(context, inner_span, offset, &mut result, "", "}");
         return Some(result);
     }
 
@@ -1247,6 +1238,41 @@ fn get_bytepos_after_visibility(vis: &ast::Visibility, default_span: Span) -> By
     }
 }
 
+// Format tuple or struct without any fields. We need to make sure that the comments
+// inside the delimiters are preserved.
+fn format_empty_struct_or_tuple(
+    context: &RewriteContext,
+    span: Span,
+    offset: Indent,
+    result: &mut String,
+    opener: &str,
+    closer: &str,
+) {
+    // 3 = " {}" or "();"
+    let used_width = last_line_used_width(&result, offset.width()) + 3;
+    if used_width > context.config.max_width() {
+        result.push_str(&offset.to_string_with_newline(context.config))
+    }
+    result.push_str(opener);
+    match rewrite_missing_comment(span, Shape::indented(offset, context.config), context) {
+        Some(ref s) if s.is_empty() => (),
+        Some(ref s) => {
+            if !is_single_line(s) || first_line_contains_single_line_comment(s) {
+                let nested_indent_str = offset
+                    .block_indent(context.config)
+                    .to_string_with_newline(context.config);
+                result.push_str(&nested_indent_str);
+            }
+            result.push_str(s);
+            if last_line_contains_single_line_comment(s) {
+                result.push_str(&offset.to_string_with_newline(context.config));
+            }
+        }
+        None => result.push_str(context.snippet(span)),
+    }
+    result.push_str(closer);
+}
+
 fn format_tuple_struct(
     context: &RewriteContext,
     struct_parts: &StructParts,
@@ -1310,31 +1336,11 @@ fn format_tuple_struct(
     };
 
     if fields.is_empty() {
-        // 3 = `();`
-        let used_width = last_line_used_width(&result, offset.width()) + 3;
-        if used_width > context.config.max_width() {
-            result.push('\n');
-            result.push_str(&offset
-                .block_indent(context.config)
-                .to_string(context.config))
-        }
-        result.push('(');
-        let snippet = context.snippet(mk_sp(
-            body_lo,
-            context
-                .snippet_provider
-                .span_before(mk_sp(body_lo, span.hi()), ")"),
-        ));
-        if snippet.is_empty() {
-            // `struct S ()`
-        } else if snippet.trim_right_matches(&[' ', '\t'][..]).ends_with('\n') {
-            result.push_str(snippet.trim_right());
-            result.push('\n');
-            result.push_str(&offset.to_string(context.config));
-        } else {
-            result.push_str(snippet);
-        }
-        result.push(')');
+        let body_hi = context
+            .snippet_provider
+            .span_before(mk_sp(body_lo, span.hi()), ")");
+        let inner_span = mk_sp(body_lo, body_hi);
+        format_empty_struct_or_tuple(context, inner_span, offset, &mut result, "(", ")");
     } else {
         let shape = Shape::indented(offset, context.config).sub_width(1)?;
         let fields = &fields.iter().collect::<Vec<_>>();

diff --git a/src/utils.rs b/src/utils.rs
@@ -134,6 +134,16 @@ pub fn outer_attributes(attrs: &[ast::Attribute]) -> Vec<ast::Attribute> {
     filter_attributes(attrs, ast::AttrStyle::Outer)
 }
 
+#[inline]
+pub fn is_single_line(s: &str) -> bool {
+    s.chars().find(|&c| c == '\n').is_none()
+}
+
+#[inline]
+pub fn first_line_contains_single_line_comment(s: &str) -> bool {
+    s.lines().next().map_or(false, |l| l.contains("//"))
+}
+
 #[inline]
 pub fn last_line_contains_single_line_comment(s: &str) -> bool {
     s.lines().last().map_or(false, |l| l.contains("//"))

diff --git a/tests/source/enum.rs b/tests/source/enum.rs
@@ -192,3 +192,7 @@ pub enum QlError {
     // (kind, input, expected)
     #[fail(display = "Could not find {}: Found: {}, expected: {:?}", 0, 1, 2)] ResolveError(&'static str, String, Option<String>),
 }
+
+// #2594
+enum Foo {}
+enum Bar { }
diff --git a/tests/target/configs/struct_field_align_threshold/20.rs b/tests/target/configs/struct_field_align_threshold/20.rs
@@ -255,8 +255,12 @@ struct Foo {
 struct Foo {
     // trailing space ->
 }
-struct Foo { /* comment */ }
-struct Foo( /* comment */ );
+struct Foo {
+    // comment
+}
+struct Foo(
+    // comment
+);
 
 struct LongStruct {
     a: A,

diff --git a/tests/target/enum.rs b/tests/target/enum.rs
@@ -258,3 +258,7 @@ pub enum QlError {
     #[fail(display = "Could not find {}: Found: {}, expected: {:?}", 0, 1, 2)]
     ResolveError(&'static str, String, Option<String>),
 }
+
+// #2594
+enum Foo {}
+enum Bar {}
diff --git a/tests/target/issue-977.rs b/tests/target/issue-977.rs
@@ -4,7 +4,9 @@
 trait NameC {
     // comment
 }
-struct FooC { /* comment */ }
+struct FooC {
+    // comment
+}
 enum MooC {
     // comment
 }

diff --git a/tests/target/structs.rs b/tests/target/structs.rs
@@ -228,8 +228,12 @@ struct Foo {
 struct Foo {
     // trailing space ->
 }
-struct Foo { /* comment */ }
-struct Foo( /* comment */ );
+struct Foo {
+    // comment
+}
+struct Foo(
+    // comment
+);
 
 struct LongStruct {
     a: A,

diff --git a/tests/target/unions.rs b/tests/target/unions.rs
@@ -136,7 +136,9 @@ union Foo {
 union Foo {
     // trailing space ->
 }
-union Foo { /* comment */ }
+union Foo {
+    // comment
+}
 
 union LongUnion {
     a: A,