Merge pull request #75 from nrc/structs

Structs

Author: nrc

src/config.rs

@@ -19,11 +19,12 @@ pub struct Config {
     pub newline_style: ::NewlineStyle,
     pub fn_brace_style: ::BraceStyle,
     pub fn_return_indent: ::ReturnIndent,
+    pub struct_trailing_comma: bool,
+    pub struct_lit_trailing_comma: ::lists::SeparatorTactic,
 }
 
 impl Config {
     fn from_toml(toml: &str) -> Config {
-        println!("About to parse: {}", toml);
         let parsed = toml.parse().unwrap();
         toml::decode(parsed).unwrap()
     }

src/default.toml

@@ -5,3 +5,5 @@ tab_spaces = 4
 newline_style = "Unix"
 fn_brace_style = "SameLineWhere"
 fn_return_indent = "WithArgs"
+struct_trailing_comma = true
+struct_lit_trailing_comma = "Vertical"

src/expr.rs

@@ -13,7 +13,9 @@ use utils::*;
 use lists::{write_list, ListFormatting, SeparatorTactic, ListTactic};
 
 use syntax::{ast, ptr};
-use syntax::codemap::{Span, Pos};
+use syntax::codemap::{Pos, Span};
+use syntax::parse::token;
+use syntax::print::pprust;
 
 use MIN_STRING;
 
@@ -134,6 +136,64 @@ impl<'a> FmtVisitor<'a> {
         format!("({})", subexpr_str)
     }
 
+    fn rewrite_struct_lit(&mut self,
+                          path: &ast::Path,
+                          fields: &[ast::Field],
+                          base: Option<&ast::Expr>,
+                          width: usize,
+                          offset: usize)
+        -> String
+    {
+        debug!("rewrite_struct_lit: width {}, offset {}", width, offset);
+        assert!(fields.len() > 0 || base.is_some());
+
+        let path_str = pprust::path_to_string(path);
+        // Foo { a: Foo } - indent is +3, width is -5.
+        let indent = offset + path_str.len() + 3;
+        let budget = width - (path_str.len() + 5);
+
+        let mut field_strs: Vec<_> =
+            fields.iter().map(|f| self.rewrite_field(f, budget, indent)).collect();
+        if let Some(expr) = base {
+            // Another 2 on the width/indent for the ..
+            field_strs.push(format!("..{}", self.rewrite_expr(expr, budget - 2, indent + 2)))
+        }
+
+        // FIXME comments
+        let field_strs: Vec<_> = field_strs.into_iter().map(|s| (s, String::new())).collect();
+        let tactics = if field_strs.iter().any(|&(ref s, _)| s.contains('\n')) {
+            ListTactic::Vertical
+        } else {
+            ListTactic::HorizontalVertical
+        };
+        let fmt = ListFormatting {
+            tactic: tactics,
+            separator: ",",
+            trailing_separator: if base.is_some() {
+                    SeparatorTactic::Never
+                } else {
+                    config!(struct_lit_trailing_comma)
+                },
+            indent: indent,
+            h_width: budget,
+            v_width: budget,
+        };
+        let fields_str = write_list(&field_strs, &fmt);
+        format!("{} {{ {} }}", path_str, fields_str)
+
+        // FIXME if the usual multi-line layout is too wide, we should fall back to
+        // Foo {
+        //     a: ...,
+        // }
+    }
+
+    fn rewrite_field(&mut self, field: &ast::Field, width: usize, offset: usize) -> String {
+        let name = &token::get_ident(field.ident.node);
+        let overhead = name.len() + 2;
+        let expr = self.rewrite_expr(&field.expr, width - overhead, offset + overhead);
+        format!("{}: {}", name, expr)
+    }
+
     pub fn rewrite_expr(&mut self, expr: &ast::Expr, width: usize, offset: usize) -> String {
         match expr.node {
             ast::Expr_::ExprLit(ref l) => {
@@ -152,6 +212,13 @@ impl<'a> FmtVisitor<'a> {
             ast::Expr_::ExprParen(ref subexpr) => {
                 return self.rewrite_paren(subexpr, width, offset);
             }
+            ast::Expr_::ExprStruct(ref path, ref fields, ref base) => {
+                return self.rewrite_struct_lit(path,
+                                               fields,
+                                               base.as_ref().map(|e| &**e),
+                                               width,
+                                               offset);
+            }
             _ => {}
         }
 

src/imports.rs

@@ -47,7 +47,7 @@ impl<'a> FmtVisitor<'a> {
                             path: &ast::Path,
                             path_list: &[ast::PathListItem],
                             visibility: ast::Visibility) -> String {
-        let path_str = pprust::path_to_string(&path);
+        let path_str = pprust::path_to_string(path);
 
         let vis = match visibility {
             ast::Public => "pub ",

src/functions.rs renamed to src/items.rs

@@ -8,6 +8,8 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+// Formatting top-level items - functions, structs, enums, traits, impls.
+
 use {ReturnIndent, BraceStyle};
 use utils::make_indent;
 use lists::{write_list, ListFormatting, SeparatorTactic, ListTactic};
@@ -123,7 +125,7 @@ impl<'a> FmtVisitor<'a> {
         let generics_indent = indent + result.len();
         result.push_str(&self.rewrite_generics(generics,
                                                generics_indent,
-                                               span_for_return(&fd.output)));
+                                               span_for_return(&fd.output).lo));
 
         let ret_str = self.rewrite_return(&fd.output);
 
@@ -388,7 +390,130 @@ impl<'a> FmtVisitor<'a> {
         }
     }
 
-    fn rewrite_generics(&self, generics: &ast::Generics, indent: usize, ret_span: Span) -> String {
+    pub fn visit_struct(&mut self,
+                        ident: ast::Ident,
+                        vis: ast::Visibility,
+                        struct_def: &ast::StructDef,
+                        generics: &ast::Generics,
+                        span: Span)
+    {
+        let header_str = self.struct_header(ident, vis);
+        self.changes.push_str_span(span, &header_str);
+
+        if struct_def.fields.len() == 0 {
+            assert!(generics.where_clause.predicates.len() == 0,
+                    "No-field struct with where clause?");
+            assert!(generics.lifetimes.len() == 0, "No-field struct with generics?");
+            assert!(generics.ty_params.len() == 0, "No-field struct with generics?");
+
+            self.changes.push_str_span(span, ";");
+            return;
+        }
+
+        let mut generics_buf = String::new();
+        let generics_str = self.rewrite_generics(generics, self.block_indent, struct_def.fields[0].span.lo);
+        generics_buf.push_str(&generics_str);
+
+        if generics.where_clause.predicates.len() > 0 {
+            generics_buf.push_str(&self.rewrite_where_clause(&generics.where_clause,
+                                                             self.block_indent,
+                                                             struct_def.fields[0].span.lo));
+            generics_buf.push_str(&make_indent(self.block_indent));
+            generics_buf.push_str("\n{");
+
+        } else {
+            generics_buf.push_str(" {");
+        }
+        self.changes.push_str_span(span, &generics_buf);
+
+        let struct_snippet = self.snippet(span);
+        // FIXME this will give incorrect results if there is a { in a commet.
+        self.last_pos = span.lo + BytePos(struct_snippet.find('{').unwrap() as u32 + 1);
+
+        self.block_indent += config!(tab_spaces);
+        for (i, f) in struct_def.fields.iter().enumerate() {
+            self.visit_field(f, i == struct_def.fields.len() - 1, span.lo, &struct_snippet);
+        }
+        self.block_indent -= config!(tab_spaces);
+
+        self.format_missing_with_indent(span.lo + BytePos(struct_snippet.rfind('}').unwrap() as u32));
+        self.changes.push_str_span(span, "}");
+    }
+
+    fn struct_header(&self,
+                     ident: ast::Ident,
+                     vis: ast::Visibility)
+        -> String
+    {
+        let vis = if vis == ast::Visibility::Public {
+            "pub "
+        } else {
+            ""
+        };
+
+        format!("{}struct {}", vis, &token::get_ident(ident))
+    }
+
+    // Field of a struct
+    fn visit_field(&mut self,
+                   field: &ast::StructField,
+                   last_field: bool,
+                   // These two args are for missing spans hacks.
+                   struct_start: BytePos,
+                   struct_snippet: &str)
+    {
+        if self.visit_attrs(&field.node.attrs) {
+            return;
+        }
+        self.format_missing_with_indent(field.span.lo);
+
+        let name = match field.node.kind {
+            ast::StructFieldKind::NamedField(ident, _) => Some(token::get_ident(ident)),
+            ast::StructFieldKind::UnnamedField(_) => None,
+        };
+        let vis = match field.node.kind {
+            ast::StructFieldKind::NamedField(_, vis) |
+            ast::StructFieldKind::UnnamedField(vis) => if vis == ast::Visibility::Public {
+                "pub "
+            } else {
+                ""
+            }
+        };
+        let typ = pprust::ty_to_string(&field.node.ty);
+
+        let mut field_str = match name {
+            Some(name) => {
+                let budget = config!(ideal_width) - self.block_indent;
+                // 3 is being conservative and assuming that there will be a trailing comma.
+                if self.block_indent + vis.len() + name.len() + typ.len() + 3 > budget {
+                    format!("{}{}:\n{}{}",
+                            vis,
+                            name,
+                            &make_indent(self.block_indent + config!(tab_spaces)),
+                            typ)
+                } else {
+                    format!("{}{}: {}", vis, name, typ)
+                }
+            }
+            None => format!("{}{}", vis, typ),
+        };
+        if !last_field || config!(struct_trailing_comma) {
+            field_str.push(',');
+        }
+        self.changes.push_str_span(field.span, &field_str);
+
+        // This hack makes sure we only add comments etc. after the comma, and
+        // makes sure we don't repeat any commas.
+        let hi = field.span.hi;
+        // FIXME a comma in a comment will break this hack.
+        let comma_pos = match struct_snippet[(hi.0 - struct_start.0) as usize..].find(',') {
+            Some(i) => i,
+            None => 0,
+        };
+        self.last_pos = hi + BytePos(comma_pos as u32 + 1);
+    }
+
+    fn rewrite_generics(&self, generics: &ast::Generics, indent: usize, span_end: BytePos) -> String {
         // FIXME convert bounds to where clauses where they get too big or if
         // there is a where clause at all.
         let mut result = String::new();
@@ -422,7 +547,7 @@ impl<'a> FmtVisitor<'a> {
                                                    ">",
                                                    |sp| sp.lo,
                                                    |sp| sp.hi,
-                                                   ret_span.lo);
+                                                   span_end);
 
         // If there are // comments, keep them multi-line.
         let mut list_tactic = ListTactic::HorizontalVertical;

src/lib.rs

@@ -50,7 +50,7 @@ use visitor::FmtVisitor;
 mod config;
 mod changes;
 mod visitor;
-mod functions;
+mod items;
 mod missed_spans;
 mod lists;
 mod utils;

src/lists.rs

@@ -9,6 +9,7 @@
 // except according to those terms.
 
 use utils::make_indent;
+use rustc_serialize::{Decodable, Decoder};
 
 #[derive(Eq, PartialEq, Debug, Copy, Clone)]
 pub enum ListTactic {
@@ -29,6 +30,19 @@ pub enum SeparatorTactic {
     Vertical,
 }
 
+// TODO could use a macro for all these Decodable impls.
+impl Decodable for SeparatorTactic {
+    fn decode<D: Decoder>(d: &mut D) -> Result<Self, D::Error> {
+        let s = try!(d.read_str());
+        match &*s {
+            "Always" => Ok(SeparatorTactic::Always),
+            "Never" => Ok(SeparatorTactic::Never),
+            "Vertical" => Ok(SeparatorTactic::Vertical),
+            _ => Err(d.error("Bad variant")),
+        }
+    }
+}
+
 // TODO having some helpful ctors for ListFormatting would be nice.
 pub struct ListFormatting<'a> {
     pub tactic: ListTactic,

src/visitor.rs

@@ -180,6 +180,15 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> {
                 self.changes.push_str_span(item.span, &new_str);
                 self.last_pos = item.span.hi;
             }
+            ast::Item_::ItemStruct(ref def, ref generics) => {
+                self.format_missing_with_indent(item.span.lo);
+                self.visit_struct(item.ident,
+                                  item.vis,
+                                  def,
+                                  generics,
+                                  item.span);
+                self.last_pos = item.span.hi;
+            }
             _ => {
                 visit::walk_item(self, item);
             }
@@ -252,7 +261,7 @@ impl<'a> FmtVisitor<'a> {
     }
 
     // Returns true if we should skip the following item.
-    fn visit_attrs(&mut self, attrs: &[ast::Attribute]) -> bool {
+    pub fn visit_attrs(&mut self, attrs: &[ast::Attribute]) -> bool {
         if attrs.len() == 0 {
             return false;
         }

tests/idem.rs

@@ -57,7 +57,7 @@ fn idempotent_tests() {
 
 // Compare output to input.
 fn print_mismatches(result: HashMap<String, String>) {
-    for (file_name, fmt_text) in result {
+    for (_, fmt_text) in result {
         println!("{}", fmt_text);
     }
 }

tests/idem/struct_lits.rs

@@ -0,0 +1,24 @@
+// Struct literal expressions.
+
+fn main() {
+    let x = Bar;
+
+    // Comment
+    let y = Foo { a: x };
+
+    Foo { a: Bar, b: foo() };
+
+    Foo { a: foo(), b: bar(), ..something };
+
+    Foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo { a: foo(), b: bar() };
+    Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo { a: foo(),
+                                                                               b: bar(), };
+
+    Fooooooooooooooooooooooooooooooooooooooooooooooooooooo { a: foo(),
+                                                             b: bar(),
+                                                             c: bar(),
+                                                             d: bar(),
+                                                             e: bar(),
+                                                             f: bar(),
+                                                             ..baz() };
+}