Format macro invocations

src/comment.rs

@@ -288,6 +288,7 @@ impl<T> Iterator for CharClasses<T> where T: Iterator, T::Item: RichChar {
 mod test {
     use super::{CharClasses, CodeCharKind, contains_comment, rewrite_comment, FindUncommented};
 
+    // TODO(#217): prevent string literal from going over the limit.
     #[test]
     fn format_comments() {
         assert_eq!("/* test */", rewrite_comment(" //test", true, 100, 100));

src/expr.rs

@@ -23,6 +23,7 @@ use comment::{FindUncommented, rewrite_comment, contains_comment};
 use types::rewrite_path;
 use items::{span_lo_for_arg, span_hi_for_arg};
 use chains::rewrite_chain;
+use macros::rewrite_macro;
 
 use syntax::{ast, ptr};
 use syntax::codemap::{CodeMap, Span, BytePos, mk_sp};
@@ -150,6 +151,9 @@ impl Rewrite for ast::Expr {
             ast::Expr_::ExprMethodCall(..) => {
                 rewrite_chain(self, context, width, offset)
             }
+            ast::Expr_::ExprMac(ref mac) => {
+                rewrite_macro(mac, context, width, offset)
+            }
             // We do not format these expressions yet, but they should still
             // satisfy our width restrictions.
             _ => wrap_str(context.snippet(self.span), context.config.max_width, width, offset),
@@ -157,13 +161,13 @@ impl Rewrite for ast::Expr {
     }
 }
 
-fn rewrite_array<'a, I>(expr_iter: I,
-                        span: Span,
-                        context: &RewriteContext,
-                        width: usize,
-                        offset: usize)
-                        -> Option<String>
-    where I: Iterator<Item = &'a ast::Expr> + ExactSizeIterator
+pub fn rewrite_array<'a, I>(expr_iter: I,
+                            span: Span,
+                            context: &RewriteContext,
+                            width: usize,
+                            offset: usize)
+                            -> Option<String>
+    where I: Iterator<Item = &'a ast::Expr>
 {
     // 2 for brackets;
     let max_item_width = try_opt!(width.checked_sub(2));
@@ -727,12 +731,13 @@ impl Rewrite for ast::Arm {
         // Patterns
         // 5 = ` => {`
         let pat_budget = try_opt!(width.checked_sub(5));
-        let pat_strs = try_opt!(pats.iter().map(|p| {
-                                               p.rewrite(context,
-                                                         pat_budget,
-                                                         offset + context.config.tab_spaces)
-                                           })
-                                           .collect::<Option<Vec<_>>>());
+        let pat_strs = try_opt!(pats.iter()
+                                    .map(|p| {
+                                        p.rewrite(context,
+                                                  pat_budget,
+                                                  offset + context.config.tab_spaces)
+                                    })
+                                    .collect::<Option<Vec<_>>>());
 
         let mut total_width = pat_strs.iter().fold(0, |a, p| a + p.len());
         // Add ` | `.len().
@@ -802,9 +807,7 @@ impl Rewrite for ast::Arm {
         }
 
         let body_budget = try_opt!(width.checked_sub(context.config.tab_spaces));
-        let body_str = try_opt!(body.rewrite(context,
-                                             body_budget,
-                                             context.block_indent));
+        let body_str = try_opt!(body.rewrite(context, body_budget, context.block_indent));
         Some(format!("{}{} =>\n{}{},",
                      attr_str.trim_left(),
                      pats_str,

src/issues.rs

@@ -256,17 +256,11 @@ fn find_issue() {
                          ReportTactic::Always,
                          ReportTactic::Never));
 
-    assert!(! is_bad_issue("TODO: no number\n",
-                           ReportTactic::Never,
-                           ReportTactic::Always));
+    assert!(!is_bad_issue("TODO: no number\n", ReportTactic::Never, ReportTactic::Always));
 
-    assert!(is_bad_issue("This is a FIXME(#1)\n",
-                         ReportTactic::Never,
-                         ReportTactic::Always));
+    assert!(is_bad_issue("This is a FIXME(#1)\n", ReportTactic::Never, ReportTactic::Always));
 
-    assert!(! is_bad_issue("bad FIXME\n",
-                           ReportTactic::Always,
-                           ReportTactic::Never));
+    assert!(!is_bad_issue("bad FIXME\n", ReportTactic::Always, ReportTactic::Never));
 }
 
 #[test]
@@ -275,17 +269,19 @@ fn issue_type() {
     let expected = Some(Issue { issue_type: IssueType::Todo, missing_number: false });
 
     assert_eq!(expected,
-               "TODO(#100): more awesomeness".chars()
-                                       .map(|c| seeker.inspect(c))
-                                       .find(Option::is_some)
-                                       .unwrap());
+               "TODO(#100): more awesomeness"
+                   .chars()
+                   .map(|c| seeker.inspect(c))
+                   .find(Option::is_some)
+                   .unwrap());
 
     let mut seeker = BadIssueSeeker::new(ReportTactic::Never, ReportTactic::Unnumbered);
     let expected = Some(Issue { issue_type: IssueType::Fixme, missing_number: true });
 
     assert_eq!(expected,
-               "Test. FIXME: bad, bad, not good".chars()
-                                                .map(|c| seeker.inspect(c))
-                                                .find(Option::is_some)
-                                                .unwrap());
+               "Test. FIXME: bad, bad, not good"
+                   .chars()
+                   .map(|c| seeker.inspect(c))
+                   .find(Option::is_some)
+                   .unwrap());
 }

src/items.rs

@@ -215,7 +215,9 @@ impl<'a> FmtVisitor<'a> {
             self.compute_budgets_for_args(&result, indent, ret_str.len(), newline_brace);
 
         debug!("rewrite_fn: one_line_budget: {}, multi_line_budget: {}, arg_indent: {}",
-               one_line_budget, multi_line_budget, arg_indent);
+               one_line_budget,
+               multi_line_budget,
+               arg_indent);
 
         // Check if vertical layout was forced by compute_budget_for_args.
         if one_line_budget <= 0 {
@@ -426,7 +428,8 @@ impl<'a> FmtVisitor<'a> {
             let used_space = indent + result.len() + 2;
             let max_space = self.config.ideal_width + self.config.leeway;
             debug!("compute_budgets_for_args: used_space: {}, max_space: {}",
-                   used_space, max_space);
+                   used_space,
+                   max_space);
             if used_space < max_space {
                 budgets = Some((one_line_budget,
                                 max_space - used_space,
@@ -563,9 +566,9 @@ impl<'a> FmtVisitor<'a> {
 
                     // Make sure we do not exceed column limit
                     // 4 = " = ,"
-                    assert!(
-                        self.config.max_width >= vis.len() + name.len() + expr_snippet.len() + 4,
-                        "Enum variant exceeded column limit");
+                    assert!(self.config.max_width >=
+                            vis.len() + name.len() + expr_snippet.len() + 4,
+                            "Enum variant exceeded column limit");
                 }
 
                 result
@@ -903,9 +906,7 @@ impl<'a> FmtVisitor<'a> {
         // 9 = " where ".len() + " {".len()
         if density == Density::Tall || preds_str.contains('\n') ||
            indent + 9 + preds_str.len() > self.config.max_width {
-            Some(format!("\n{}where {}",
-                         make_indent(indent + extra_indent),
-                         preds_str))
+            Some(format!("\n{}where {}", make_indent(indent + extra_indent), preds_str))
         } else {
             Some(format!(" where {}", preds_str))
         }

src/lib.rs

@@ -11,6 +11,7 @@
 #![feature(rustc_private)]
 #![feature(custom_attribute)]
 #![feature(slice_splits)]
+#![feature(catch_panic)]
 #![allow(unused_attributes)]
 
 // TODO we're going to allocate a whole bunch of temp Strings, is it worth
@@ -72,6 +73,7 @@ mod comment;
 mod modules;
 pub mod rustfmt_diff;
 mod chains;
+mod macros;
 
 const MIN_STRING: usize = 10;
 // When we get scoped annotations, we should have rustfmt::skip.
@@ -323,8 +325,6 @@ impl<'a> CompilerCalls<'a> for RustFmtCalls {
         panic!("No input supplied to RustFmt");
     }
 
-    #[rustfmt_skip]
-    // FIXME(#195): closure is formatted poorly.
     fn build_controller(&mut self, _: &Session) -> driver::CompileController<'a> {
         let write_mode = self.write_mode;
 
@@ -338,8 +338,8 @@ impl<'a> CompilerCalls<'a> for RustFmtCalls {
             let krate = state.krate.unwrap();
             let codemap = state.session.codemap();
             let mut file_map = fmt_ast(krate, codemap, &*config);
-            // For some reason, the codemap does not include terminating newlines
-            // so we must add one on for each file. This is sad.
+            // For some reason, the codemap does not include terminating
+            // newlines so we must add one on for each file. This is sad.
             filemap::append_newlines(&mut file_map);
             println!("{}", fmt_lines(&mut file_map, &*config));
 

src/lists.rs

@@ -116,7 +116,9 @@ pub fn write_list<'b>(items: &[ListItem], formatting: &ListFormatting<'b>) -> Op
     // Check if we need to fallback from horizontal listing, if possible.
     if tactic == ListTactic::HorizontalVertical {
         debug!("write_list: total_width: {}, total_sep_len: {}, h_width: {}",
-               total_width, total_sep_len, formatting.h_width);
+               total_width,
+               total_sep_len,
+               formatting.h_width);
         tactic = if fits_single && !items.iter().any(ListItem::is_multiline) {
             ListTactic::Horizontal
         } else {

src/macros.rs

@@ -0,0 +1,124 @@
+// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// Format list-like macro invocations. These are invocations whose token trees
+// can be interpreted as expressions and separated by commas.
+// Note that these token trees do not actually have to be interpreted as
+// expressions by the compiler. An example of an invocation we would reformat is
+// foo!( x, y, z ). The token x may represent an identifier in the code, but we
+// interpreted as an expression.
+// Macro uses which are not-list like, such as bar!(key => val), will not be
+// reformated.
+// List-like invocations with parentheses will be formatted as function calls,
+// and those with brackets will be formatted as array literals.
+
+use std::thread;
+
+use syntax::ast;
+use syntax::parse::token::{Eof, Comma, Token};
+use syntax::parse::{ParseSess, tts_to_parser};
+
+use rewrite::RewriteContext;
+use expr::{rewrite_call, rewrite_array};
+use comment::FindUncommented;
+use utils::wrap_str;
+
+// We need to pass `TokenTree`s to our expression parsing thread, but they are
+// not `Send`. We wrap them in a `Send` container to force our will.
+// FIXME: this is a pretty terrible hack. Any other solution would be preferred.
+struct ForceSend<T>(pub T);
+unsafe impl<T> Send for ForceSend<T> {}
+
+// FIXME: use the enum from libsyntax?
+enum MacroStyle {
+    Parens,
+    Brackets,
+    Braces,
+}
+
+pub fn rewrite_macro(mac: &ast::Mac,
+                     context: &RewriteContext,
+                     width: usize,
+                     offset: usize)
+                     -> Option<String> {
+    let ast::Mac_::MacInvocTT(ref path, ref tt_vec, _) = mac.node;
+    let style = macro_style(mac, context);
+    let macro_name = format!("{}!", path);
+
+    if let MacroStyle::Braces = style {
+        return None;
+    } else if tt_vec.is_empty() {
+        return if let MacroStyle::Parens = style {
+            Some(format!("{}()", macro_name))
+        } else {
+            Some(format!("{}[]", macro_name))
+        };
+    }
+
+    let wrapped_tt_vec = ForceSend((*tt_vec).clone());
+    // Wrap expression parsing logic in a thread since the libsyntax parser
+    // panicks on failure, which we do not want to propagate.
+    let expr_vec_result = thread::catch_panic(move || {
+        let parse_session = ParseSess::new();
+        let mut parser = tts_to_parser(&parse_session, wrapped_tt_vec.0, vec![]);
+        let mut expr_vec = vec![];
+
+        loop {
+            expr_vec.push(parser.parse_expr());
+
+            match parser.token {
+                Token::Eof => break,
+                Token::Comma => (),
+                _ => panic!("Macro not list-like, skiping..."),
+            }
+
+            let _ = parser.bump();
+        }
+
+        expr_vec
+    });
+    let expr_vec = try_opt!(expr_vec_result.ok());
+
+    match style {
+        MacroStyle::Parens => {
+            // Format macro invocation as function call.
+            rewrite_call(context, &macro_name, &expr_vec, mac.span, width, offset)
+        }
+        MacroStyle::Brackets => {
+            // Format macro invocation as array literal.
+            let extra_offset = macro_name.len();
+            let rewrite = try_opt!(rewrite_array(expr_vec.iter().map(|x| &**x),
+                                                 mac.span,
+                                                 context,
+                                                 try_opt!(width.checked_sub(extra_offset)),
+                                                 offset + extra_offset));
+            Some(format!("{}{}", macro_name, rewrite))
+        }
+        MacroStyle::Braces => {
+            // Skip macro invocations with braces, for now.
+            wrap_str(context.snippet(mac.span), context.config.max_width, width, offset)
+        }
+    }
+}
+
+fn macro_style(mac: &ast::Mac, context: &RewriteContext) -> MacroStyle {
+    let snippet = context.snippet(mac.span);
+    let paren_pos = snippet.find_uncommented("(").unwrap_or(usize::max_value());
+    let bracket_pos = snippet.find_uncommented("[").unwrap_or(usize::max_value());
+    let brace_pos = snippet.find_uncommented("{").unwrap_or(usize::max_value());
+
+    if paren_pos < bracket_pos && paren_pos < brace_pos {
+        MacroStyle::Parens
+    } else if bracket_pos < brace_pos {
+        MacroStyle::Brackets
+    } else {
+        MacroStyle::Braces
+    }
+}

src/types.rs

@@ -137,7 +137,9 @@ impl<'a> Rewrite for SegmentParam<'a> {
                 try_opt!(ty.rewrite(context, width, offset))
             }
             SegmentParam::Binding(ref binding) => {
-                format!("{} = {}", binding.ident, try_opt!(binding.ty.rewrite(context, width, offset)))
+                format!("{} = {}",
+                        binding.ident,
+                        try_opt!(binding.ty.rewrite(context, width, offset)))
             }
         })
     }
@@ -319,8 +321,10 @@ impl Rewrite for ast::WherePredicate {
                                                                              .. }) => {
                 format!("{}: {}",
                         pprust::lifetime_to_string(lifetime),
-                        bounds.iter().map(pprust::lifetime_to_string)
-                              .collect::<Vec<_>>().join(" + "))
+                        bounds.iter()
+                              .map(pprust::lifetime_to_string)
+                              .collect::<Vec<_>>()
+                              .join(" + "))
             }
             ast::WherePredicate::EqPredicate(ast::WhereEqPredicate { ref path, ref ty, .. }) => {
                 let ty_str = pprust::ty_to_string(ty);
@@ -342,8 +346,11 @@ impl Rewrite for ast::LifetimeDef {
         } else {
             Some(format!("{}: {}",
                          pprust::lifetime_to_string(&self.lifetime),
-                         self.bounds.iter().map(pprust::lifetime_to_string)
-                                    .collect::<Vec<_>>().join(" + ")))
+                         self.bounds
+                             .iter()
+                             .map(pprust::lifetime_to_string)
+                             .collect::<Vec<_>>()
+                             .join(" + ")))
         }
     }
 }
@@ -410,9 +417,9 @@ impl Rewrite for ast::PolyTraitRef {
             // 6 is "for<> ".len()
             let extra_offset = lifetime_str.len() + 6;
             let max_path_width = try_opt!(width.checked_sub(extra_offset));
-            let path_str = try_opt!(self.trait_ref.path.rewrite(context,
-                                                                max_path_width,
-                                                                offset + extra_offset));
+            let path_str = try_opt!(self.trait_ref
+                                        .path
+                                        .rewrite(context, max_path_width, offset + extra_offset));
 
             Some(format!("for<{}> {}", lifetime_str, path_str))
         } else {

src/visitor.rs

@@ -18,6 +18,7 @@ use utils;
 use config::Config;
 use rewrite::{Rewrite, RewriteContext};
 use comment::rewrite_comment;
+use macros::rewrite_macro;
 
 pub struct FmtVisitor<'a> {
     pub codemap: &'a CodeMap,
@@ -73,7 +74,7 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> {
                     self.last_pos = stmt.span.hi;
                 }
             }
-            ast::Stmt_::StmtMac(..) => {
+            ast::Stmt_::StmtMac(ref _mac, _macro_style) => {
                 self.format_missing_with_indent(stmt.span.lo);
                 visit::walk_stmt(self, stmt);
             }
@@ -213,6 +214,12 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> {
                 self.format_missing_with_indent(item.span.lo);
                 self.format_mod(module, item.span, item.ident);
             }
+            ast::Item_::ItemMac(..) => {
+                self.format_missing_with_indent(item.span.lo);
+                // TODO: we cannot format these yet, because of a bad span.
+                // See rust lang issue #28424.
+                // visit::walk_item(self, item);
+            }
             _ => {
                 visit::walk_item(self, item);
             }
@@ -249,7 +256,14 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> {
     }
 
     fn visit_mac(&mut self, mac: &'v ast::Mac) {
-        visit::walk_mac(self, mac)
+        // 1 = ;
+        let width = self.config.max_width - self.block_indent - 1;
+        let rewrite = rewrite_macro(mac, &self.get_context(), width, self.block_indent);
+
+        if let Some(res) = rewrite {
+            self.buffer.push_str(&res);
+            self.last_pos = mac.span.hi;
+        }
     }
 }
 

tests/source/macros.rs

@@ -0,0 +1,30 @@
+itemmacro!(this, is.not() .formatted(yet));
+
+fn main() {
+    foo! ( );
+
+    bar!( a , b , c );
+
+    baz!(1+2+3, quux. kaas());
+
+    quux!(AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA, BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB);
+
+    kaas!(/* comments */ a /* post macro */, b /* another */);
+
+    trailingcomma!( a , b , c , );
+
+    noexpr!( i am not an expression, OK? );
+
+    vec! [ a , b , c];
+
+    vec! [AAAAAA, AAAAAA, AAAAAA, AAAAAA, AAAAAA, AAAAAA, AAAAAA, AAAAAA, AAAAAA,
+          BBBBB, 5, 100-30, 1.33, b, b, b];
+
+    vec! [a /* comment */];
+
+    foo(makro!(1,   3));
+
+    hamkaas!{ () };
+
+    macrowithbraces! {dont,    format, me}
+}

tests/target/macros.rs

@@ -0,0 +1,33 @@
+itemmacro!(this, is.not() .formatted(yet));
+
+fn main() {
+    foo!();
+
+    bar!(a, b, c);
+
+    baz!(1 + 2 + 3, quux.kaas());
+
+    quux!(AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA,
+          BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB);
+
+    kaas!(// comments
+          a, // post macro
+          b /* another */);
+
+    trailingcomma!( a , b , c , );
+
+    noexpr!( i am not an expression, OK? );
+
+    vec![a, b, c];
+
+    vec![AAAAAA, AAAAAA, AAAAAA, AAAAAA, AAAAAA, AAAAAA, AAAAAA, AAAAAA, AAAAAA, BBBBB, 5,
+         100 - 30, 1.33, b, b, b];
+
+    vec![a /* comment */];
+
+    foo(makro!(1, 3));
+
+    hamkaas!{ () };
+
+    macrowithbraces! {dont,    format, me}
+}

tests/target/match.rs

@@ -67,12 +67,20 @@ fn main() {
 fn main() {
     match r {
         Variableeeeeeeeeeeeeeeeee => ("variable",
-                                      vec!("id","name","qualname","value","type","scopeid"),
+                                      vec!("id", "name", "qualname", "value", "type", "scopeid"),
                                       true,
                                       true),
-        Enummmmmmmmmmmmmmmmmmmmm => ("enum", vec!("id","qualname","scopeid","value"), true, true),
+        Enummmmmmmmmmmmmmmmmmmmm => ("enum",
+                                     vec!("id", "qualname", "scopeid", "value"),
+                                     true,
+                                     true),
         Variantttttttttttttttttttttttt => ("variant",
-                                           vec!("id","name","qualname","type","value","scopeid"),
+                                           vec!("id",
+                                                "name",
+                                                "qualname",
+                                                "type",
+                                                "value",
+                                                "scopeid"),
                                            true,
                                            true),
     }