diff --git a/src/comment.rs b/src/comment.rs index ff7558131d2..42aaaa4f8c9 100644 --- a/src/comment.rs +++ b/src/comment.rs @@ -288,6 +288,7 @@ impl Iterator for CharClasses 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)); diff --git a/src/expr.rs b/src/expr.rs index 1f82c5e10a3..e0ad66ba34f 100644 --- a/src/expr.rs +++ b/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 - where I: Iterator + ExactSizeIterator +pub fn rewrite_array<'a, I>(expr_iter: I, + span: Span, + context: &RewriteContext, + width: usize, + offset: usize) + -> Option + where I: Iterator { // 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::>>()); + let pat_strs = try_opt!(pats.iter() + .map(|p| { + p.rewrite(context, + pat_budget, + offset + context.config.tab_spaces) + }) + .collect::>>()); 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, diff --git a/src/issues.rs b/src/issues.rs index 7d100d6db2a..6b35add20bf 100644 --- a/src/issues.rs +++ b/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()); } diff --git a/src/items.rs b/src/items.rs index 842f2699c2a..6351813f996 100644 --- a/src/items.rs +++ b/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)) } diff --git a/src/lib.rs b/src/lib.rs index e5017387a25..4ab1782700d 100644 --- a/src/lib.rs +++ b/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)); diff --git a/src/lists.rs b/src/lists.rs index 678c84c199a..2b85c994581 100644 --- a/src/lists.rs +++ b/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 { diff --git a/src/macros.rs b/src/macros.rs new file mode 100644 index 00000000000..dd742ad46c8 --- /dev/null +++ b/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 or the MIT license +// , 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(pub T); +unsafe impl Send for ForceSend {} + +// 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 { + 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, ¯o_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 + } +} diff --git a/src/types.rs b/src/types.rs index d474eb22b70..1f5e667c6fa 100644 --- a/src/types.rs +++ b/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::>().join(" + ")) + bounds.iter() + .map(pprust::lifetime_to_string) + .collect::>() + .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::>().join(" + "))) + self.bounds + .iter() + .map(pprust::lifetime_to_string) + .collect::>() + .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 { diff --git a/src/visitor.rs b/src/visitor.rs index 54ada1b7455..f09c198ca33 100644 --- a/src/visitor.rs +++ b/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; + } } } diff --git a/tests/source/macros.rs b/tests/source/macros.rs new file mode 100644 index 00000000000..8487e0c5deb --- /dev/null +++ b/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} +} diff --git a/tests/target/macros.rs b/tests/target/macros.rs new file mode 100644 index 00000000000..ac87b693246 --- /dev/null +++ b/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} +} diff --git a/tests/target/match.rs b/tests/target/match.rs index 5cfbb5a7072..e18c04bf02a 100644 --- a/tests/target/match.rs +++ b/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), }