diff --git a/src/functions.rs b/src/functions.rs index cf459931ba73..5b08fa797bcf 100644 --- a/src/functions.rs +++ b/src/functions.rs @@ -14,6 +14,7 @@ use utils::make_indent; use lists::{write_list, ListFormatting, SeparatorTactic, ListTactic}; use visitor::FmtVisitor; use syntax::{ast, abi}; +use syntax::codemap::{self, Span}; use syntax::print::pprust; use syntax::parse::token; @@ -55,115 +56,21 @@ impl<'a> FmtVisitor<'a> { result.push_str(&token::get_ident(ident)); // Generics. - // FIXME convert bounds to where clauses where they get too big or if - // there is a where clause at all. - let lifetimes: &[_] = &generics.lifetimes; - let tys: &[_] = &generics.ty_params; - if lifetimes.len() + tys.len() > 0 { - let budget = MAX_WIDTH - indent - result.len() - 2; - // TODO might need to insert a newline if the generics are really long - result.push('<'); - - let lt_strs = lifetimes.iter().map(|l| self.rewrite_lifetime_def(l)); - let ty_strs = tys.iter().map(|ty| self.rewrite_ty_param(ty)); - let generics_strs: Vec<_> = lt_strs.chain(ty_strs).map(|s| (s, String::new())).collect(); - let fmt = ListFormatting { - tactic: ListTactic::HorizontalVertical, - separator: ",", - trailing_separator: SeparatorTactic::Never, - indent: indent + result.len() + 1, - h_width: budget, - v_width: budget, - }; - result.push_str(&write_list(&generics_strs, &fmt)); - - result.push('>'); - } + result.push_str(&self.rewrite_generics(generics, indent)); let ret_str = self.rewrite_return(&fd.output); // Args. - let args = &fd.inputs; + let (one_line_budget, multi_line_budget, arg_indent) = + self.compute_budgets_for_args(&mut result, indent, ret_str.len(), newline_brace); - let mut budgets = None; - - // Try keeping everything on the same line - if !result.contains("\n") { - // 3 = `() `, space is before ret_string - let mut used_space = indent + result.len() + 3 + ret_str.len(); - if newline_brace { - used_space += 2; - } - let one_line_budget = if used_space > MAX_WIDTH { - 0 - } else { - MAX_WIDTH - used_space - }; - - let used_space = indent + result.len() + 2; - let max_space = IDEAL_WIDTH + LEEWAY; - if used_space < max_space { - budgets = Some((one_line_budget, - // 2 = `()` - max_space - used_space, - indent + result.len() + 1)); - } - } - - // Didn't work. we must force vertical layout and put args on a newline. - if let None = budgets { - result.push('\n'); - result.push_str(&make_indent(indent + 4)); - // 6 = new indent + `()` - let used_space = indent + 6; - let max_space = IDEAL_WIDTH + LEEWAY; - if used_space > max_space { - // Whoops! bankrupt. - // TODO take evasive action, perhaps kill the indent or something. - } else { - // 5 = new indent + `(` - budgets = Some((0, max_space - used_space, indent + 5)); - } - } - - let (one_line_budget, multi_line_budget, arg_indent) = budgets.unwrap(); result.push('('); - - let fmt = ListFormatting { - tactic: ListTactic::HorizontalVertical, - separator: ",", - trailing_separator: SeparatorTactic::Never, - indent: arg_indent, - h_width: one_line_budget, - v_width: multi_line_budget, - }; - // TODO dead spans - let mut arg_strs: Vec<_> = args.iter().map(|a| (self.rewrite_fn_input(a), String::new())).collect(); - // Account for sugary self. - if let Some(explicit_self) = explicit_self { - match explicit_self.node { - ast::ExplicitSelf_::SelfRegion(ref lt, ref m, _) => { - let lt_str = match lt { - &Some(ref l) => format!("{} ", pprust::lifetime_to_string(l)), - &None => String::new(), - }; - let mut_str = match m { - &ast::Mutability::MutMutable => "mut ".to_string(), - &ast::Mutability::MutImmutable => String::new(), - }; - arg_strs[0].0 = format!("&{}{}self", lt_str, mut_str); - } - ast::ExplicitSelf_::SelfExplicit(ref ty, _) => { - arg_strs[0].0 = format!("self: {}", pprust::ty_to_string(ty)); - } - ast::ExplicitSelf_::SelfValue(_) => { - arg_strs[0].0 = "self".to_string(); - } - _ => {} - } - } - result.push_str(&write_list(&arg_strs, &fmt)); - + result.push_str(&self.rewrite_args(&fd.inputs, + explicit_self, + one_line_budget, + multi_line_budget, + arg_indent, + span_for_return(&fd.output))); result.push(')'); // Where clause. @@ -204,6 +111,147 @@ impl<'a> FmtVisitor<'a> { result } + fn rewrite_args(&self, + args: &[ast::Arg], + explicit_self: Option<&ast::ExplicitSelf>, + one_line_budget: usize, + multi_line_budget: usize, + arg_indent: usize, + ret_span: Span) + -> String + { + let mut arg_item_strs: Vec<_> = args.iter().map(|a| self.rewrite_fn_input(a)).collect(); + // Account for sugary self. + let mut min_args = 1; + if let Some(explicit_self) = explicit_self { + match explicit_self.node { + ast::ExplicitSelf_::SelfRegion(ref lt, ref m, _) => { + let lt_str = match lt { + &Some(ref l) => format!("{} ", pprust::lifetime_to_string(l)), + &None => String::new(), + }; + let mut_str = match m { + &ast::Mutability::MutMutable => "mut ".to_string(), + &ast::Mutability::MutImmutable => String::new(), + }; + arg_item_strs[0] = format!("&{}{}self", lt_str, mut_str); + min_args = 2; + } + ast::ExplicitSelf_::SelfExplicit(ref ty, _) => { + arg_item_strs[0] = format!("self: {}", pprust::ty_to_string(ty)); + } + ast::ExplicitSelf_::SelfValue(_) => { + arg_item_strs[0] = "self".to_string(); + min_args = 2; + } + _ => {} + } + } + + // Comments between args + let mut arg_comments = Vec::new(); + if min_args == 2 { + arg_comments.push("".to_string()); + } + // TODO if there are no args, there might still be a comment, but without + // spans for the comment or parens, there is no chance of getting it right. + // You also don't get to put a comment on self, unless it is explicit. + if args.len() >= min_args { + let mut prev_end = args[min_args-1].ty.span.hi; + for arg in &args[min_args..] { + let cur_start = arg.pat.span.lo; + let snippet = self.snippet(codemap::mk_sp(prev_end, cur_start)); + let mut snippet = snippet.trim(); + if snippet.starts_with(",") { + snippet = snippet[1..].trim(); + } else if snippet.ends_with(",") { + snippet = snippet[..snippet.len()-1].trim(); + } + arg_comments.push(snippet.to_string()); + prev_end = arg.ty.span.hi; + } + // Get the last commment. + // FIXME If you thought the crap with the commas was ugly, just wait. + // This is awful. We're going to look from the last arg span to the + // start of the return type span, then we drop everything after the + // first closing paren. Obviously, this will break if there is a + // closing paren in the comment. + // The fix is comments in the AST or a span for the closing paren. + let snippet = self.snippet(codemap::mk_sp(prev_end, ret_span.lo)); + let snippet = snippet.trim(); + let snippet = &snippet[..snippet.find(")").unwrap()]; + let snippet = snippet.trim(); + arg_comments.push(snippet.to_string()); + } + + debug!("comments: {:?}", arg_comments); + + assert_eq!(arg_item_strs.len(), arg_comments.len()); + let arg_strs: Vec<_> = arg_item_strs.into_iter().zip(arg_comments.into_iter()).collect(); + + let fmt = ListFormatting { + tactic: ListTactic::HorizontalVertical, + separator: ",", + trailing_separator: SeparatorTactic::Never, + indent: arg_indent, + h_width: one_line_budget, + v_width: multi_line_budget, + }; + + write_list(&arg_strs, &fmt) + } + + fn compute_budgets_for_args(&self, + result: &mut String, + indent: usize, + ret_str_len: usize, + newline_brace: bool) + -> (usize, usize, usize) + { + let mut budgets = None; + + // Try keeping everything on the same line + if !result.contains("\n") { + // 3 = `() `, space is before ret_string + let mut used_space = indent + result.len() + 3 + ret_str_len; + if newline_brace { + used_space += 2; + } + let one_line_budget = if used_space > MAX_WIDTH { + 0 + } else { + MAX_WIDTH - used_space + }; + + let used_space = indent + result.len() + 2; + let max_space = IDEAL_WIDTH + LEEWAY; + if used_space < max_space { + budgets = Some((one_line_budget, + // 2 = `()` + max_space - used_space, + indent + result.len() + 1)); + } + } + + // Didn't work. we must force vertical layout and put args on a newline. + if let None = budgets { + result.push('\n'); + result.push_str(&make_indent(indent + 4)); + // 6 = new indent + `()` + let used_space = indent + 6; + let max_space = IDEAL_WIDTH + LEEWAY; + if used_space > max_space { + // Whoops! bankrupt. + // TODO take evasive action, perhaps kill the indent or something. + } else { + // 5 = new indent + `(` + budgets = Some((0, max_space - used_space, indent + 5)); + } + } + + budgets.unwrap() + } + fn newline_for_brace(&self, where_clause: &ast::WhereClause) -> bool { match FN_BRACE_STYLE { BraceStyle::AlwaysNextLine => true, @@ -212,6 +260,36 @@ impl<'a> FmtVisitor<'a> { } } + fn rewrite_generics(&self, generics: &ast::Generics, indent: usize) -> 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(); + let lifetimes: &[_] = &generics.lifetimes; + let tys: &[_] = &generics.ty_params; + if lifetimes.len() + tys.len() > 0 { + let budget = MAX_WIDTH - indent - result.len() - 2; + // TODO might need to insert a newline if the generics are really long + result.push('<'); + + let lt_strs = lifetimes.iter().map(|l| self.rewrite_lifetime_def(l)); + let ty_strs = tys.iter().map(|ty| self.rewrite_ty_param(ty)); + let generics_strs: Vec<_> = lt_strs.chain(ty_strs).map(|s| (s, String::new())).collect(); + let fmt = ListFormatting { + tactic: ListTactic::HorizontalVertical, + separator: ",", + trailing_separator: SeparatorTactic::Never, + indent: indent + result.len() + 1, + h_width: budget, + v_width: budget, + }; + result.push_str(&write_list(&generics_strs, &fmt)); + + result.push('>'); + } + + result + } + fn rewrite_where_clause(&self, where_clause: &ast::WhereClause, indent: usize) -> String { let mut result = String::new(); if where_clause.predicates.len() == 0 { @@ -252,3 +330,11 @@ impl<'a> FmtVisitor<'a> { pprust::ty_to_string(&arg.ty)) } } + +fn span_for_return(ret: &ast::FunctionRetTy) -> Span { + match *ret { + ast::FunctionRetTy::NoReturn(ref span) | + ast::FunctionRetTy::DefaultReturn(ref span) => span.clone(), + ast::FunctionRetTy::Return(ref ty) => ty.span, + } +} diff --git a/src/imports.rs b/src/imports.rs index 34cab06b2fc6..7e64308eafc6 100644 --- a/src/imports.rs +++ b/src/imports.rs @@ -18,6 +18,10 @@ use syntax::print::pprust; use IDEAL_WIDTH; +// TODO change import lists with one item to a single import +// remove empty lists (if they're even possible) +// TODO (some day) remove unused imports, expand globs, compress many single imports into a list import + impl<'a> FmtVisitor<'a> { // Basically just pretty prints a multi-item import. pub fn rewrite_use_list(&mut self, diff --git a/src/lists.rs b/src/lists.rs index 0cfa592f03ef..01bad45b6aff 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -41,30 +41,28 @@ pub struct ListFormatting<'a> { } // Format a list of strings into a string. -pub fn write_list<'b>(items:&[(String, String)], formatting: &ListFormatting<'b>) -> String { +// Precondition: all strings in items are trimmed. +pub fn write_list<'b>(items: &[(String, String)], formatting: &ListFormatting<'b>) -> String { if items.len() == 0 { return String::new(); } let mut tactic = formatting.tactic; - let h_width = formatting.h_width; - let v_width = formatting.v_width; - let sep_len = formatting.separator.len(); - // Conservatively overestimates because of the changing separator tactic. let sep_count = if formatting.trailing_separator != SeparatorTactic::Never { items.len() } else { items.len() - 1 }; + let sep_len = formatting.separator.len(); + let total_sep_len = (sep_len + 1) * sep_count; - // TODO count dead space too. - let total_width = items.iter().map(|&(ref s, _)| s.len()).fold(0, |a, l| a + l); + let total_width = calculate_width(items); // Check if we need to fallback from horizontal listing, if possible. if tactic == ListTactic::HorizontalVertical { - if (total_width + (sep_len + 1) * sep_count) > h_width { + if total_width + total_sep_len > formatting.h_width { tactic = ListTactic::Vertical; } else { tactic = ListTactic::Horizontal; @@ -73,16 +71,12 @@ pub fn write_list<'b>(items:&[(String, String)], formatting: &ListFormatting<'b> // Now that we know how we will layout, we can decide for sure if there // will be a trailing separator. - let trailing_separator = match formatting.trailing_separator { - SeparatorTactic::Always => true, - SeparatorTactic::Vertical => tactic == ListTactic::Vertical, - SeparatorTactic::Never => false, - }; + let trailing_separator = needs_trailing_separator(formatting.trailing_separator, tactic); // Create a buffer for the result. // TODO could use a StringBuffer or rope for this let alloc_width = if tactic == ListTactic::Horizontal { - total_width + (sep_len + 1) * sep_count + total_width + total_sep_len } else { total_width + items.len() * (formatting.indent + 1) }; @@ -90,7 +84,7 @@ pub fn write_list<'b>(items:&[(String, String)], formatting: &ListFormatting<'b> let mut line_len = 0; let indent_str = &make_indent(formatting.indent); - for (i, &(ref item, _)) in items.iter().enumerate() { + for (i, &(ref item, ref comment)) in items.iter().enumerate() { let first = i == 0; let separate = i != items.len() - 1 || trailing_separator; @@ -108,7 +102,7 @@ pub fn write_list<'b>(items:&[(String, String)], formatting: &ListFormatting<'b> item_width += sep_len; } - if line_len > 0 && line_len + item_width > v_width { + if line_len > 0 && line_len + item_width > formatting.v_width { result.push('\n'); result.push_str(indent_str); line_len = 0; @@ -126,11 +120,42 @@ pub fn write_list<'b>(items:&[(String, String)], formatting: &ListFormatting<'b> result.push_str(item); + if tactic != ListTactic::Vertical && comment.len() > 0 { + result.push(' '); + result.push_str(comment); + } + if separate { result.push_str(formatting.separator); } - // TODO dead spans + + if tactic == ListTactic::Vertical && comment.len() > 0 { + result.push(' '); + result.push_str(comment); + } } result } + +fn needs_trailing_separator(separator_tactic: SeparatorTactic, list_tactic: ListTactic) -> bool { + match separator_tactic { + SeparatorTactic::Always => true, + SeparatorTactic::Vertical => list_tactic == ListTactic::Vertical, + SeparatorTactic::Never => false, + } +} + +fn calculate_width(items:&[(String, String)]) -> usize { + let missed_width = items.iter().map(|&(_, ref s)| { + let text_len = s.trim().len(); + if text_len > 0 { + // We'll put a space before any comment. + text_len + 1 + } else { + text_len + } + }).fold(0, |a, l| a + l); + let item_width = items.iter().map(|&(ref s, _)| s.len()).fold(0, |a, l| a + l); + missed_width + item_width +} diff --git a/src/mod.rs b/src/mod.rs index 5c179dc910e7..23fe92438c30 100644 --- a/src/mod.rs +++ b/src/mod.rs @@ -21,10 +21,12 @@ // TODO priorities // Fix fns and methods properly -// dead spans +// dead spans (comments) - in generics // // Smoke testing till we can use it // no newline at the end of doc.rs +// fmt_skip annotations +// take config options from a file #[macro_use] extern crate log; @@ -107,7 +109,10 @@ fn fmt_ast<'a>(krate: &ast::Crate, codemap: &'a CodeMap) -> ChangeSet<'a> { visitor.changes } -// Formatting done on a char by char basis. +// Formatting done on a char by char or line by line basis. +// TODO warn on TODOs and FIXMEs without an issue number +// TODO warn on bad license +// TODO other stuff for parity with make tidy fn fmt_lines(changes: &mut ChangeSet) { // Iterate over the chars in the change set. for (f, text) in changes.text() { @@ -124,7 +129,7 @@ fn fmt_lines(changes: &mut ChangeSet) { } // Check for any line width errors we couldn't correct. if line_len > MAX_WIDTH { - // FIXME store the error rather than reporting immediately. + // TODO store the error rather than reporting immediately. println!("Rustfmt couldn't fix (sorry). {}:{}: line longer than {} characters", f, cur_line, MAX_WIDTH); } @@ -144,7 +149,7 @@ fn fmt_lines(changes: &mut ChangeSet) { } for &(l, _, _) in trims.iter() { - // FIXME store the error rather than reporting immediately. + // TODO store the error rather than reporting immediately. println!("Rustfmt left trailing whitespace at {}:{} (sorry)", f, l); } }