user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Keep comments on fn arguments

Browse source
This commit is contained in:
Nick Cameron 2015-04-21 22:50:43 +12:00
parent 4c869a1b9d
commit 854b52dbed
4 changed files with 244 additions and 124 deletions
Download patch file Download diff file Expand all files Collapse all files

292
src/functions.rs
View file

@ -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,
}
}

4
src/imports.rs
View file

@ -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,

59
src/lists.rs
View file

@ -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
}

13
src/mod.rs
View file

@ -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);
}
}