Merge pull request #354 from nrc/max-fn

Use vertical formatting when function args width > limit
This commit is contained in:
Nick Cameron 2015-09-26 14:05:22 +12:00
commit 9a5688f7c2
17 changed files with 158 additions and 46 deletions

View file

@ -126,7 +126,10 @@ pub fn rewrite_chain(mut expr: &ast::Expr,
&connector[..]
};
Some(format!("{}{}{}", parent_rewrite, first_connector, rewrites.join(&connector)))
Some(format!("{}{}{}",
parent_rewrite,
first_connector,
rewrites.join(&connector)))
}
fn pop_expr_chain<'a>(expr: &'a ast::Expr) -> Option<&'a ast::Expr> {
@ -151,7 +154,13 @@ fn rewrite_chain_expr(expr: &ast::Expr,
match expr.node {
ast::Expr_::ExprMethodCall(ref method_name, ref types, ref expressions) => {
let inner = &RewriteContext { block_indent: offset, ..*context };
rewrite_method_call(method_name.node, types, expressions, span, inner, width, offset)
rewrite_method_call(method_name.node,
types,
expressions,
span,
inner,
width,
offset)
}
ast::Expr_::ExprField(_, ref field) => {
Some(format!(".{}", field.node))

View file

@ -343,8 +343,10 @@ mod test {
#[test]
fn test_uncommented() {
assert_eq!(&uncommented("abc/*...*/"), "abc");
assert_eq!(&uncommented("// .... /* \n../* /* *** / */ */a/* // */c\n"), "..ac\n");
assert_eq!(&uncommented("abc \" /* */\" qsdf"), "abc \" /* */\" qsdf");
assert_eq!(&uncommented("// .... /* \n../* /* *** / */ */a/* // */c\n"),
"..ac\n");
assert_eq!(&uncommented("abc \" /* */\" qsdf"),
"abc \" /* */\" qsdf");
}
#[test]
@ -365,7 +367,9 @@ mod test {
check("/*/ */test", "test", Some(6));
check("//test\ntest", "test", Some(7));
check("/* comment only */", "whatever", None);
check("/* comment */ some text /* more commentary */ result", "result", Some(46));
check("/* comment */ some text /* more commentary */ result",
"result",
Some(46));
check("sup // sup", "p", Some(2));
check("sup", "x", None);
check(r#"π? /**/ π is nice!"#, r#"π is nice"#, Some(9));

View file

@ -215,9 +215,11 @@ macro_rules! create_config {
create_config! {
max_width: usize, "Maximum width of each line",
ideal_width: usize, "Ideal width of each line",
leeway: usize, "Leeway of line width",
ideal_width: usize, "Ideal width of each line (only used for comments)",
leeway: usize, "Leeway of line width (deprecated)",
tab_spaces: usize, "Number of spaces per tab",
list_width: usize, "Maximum width in a struct literal or function \
call before faling back to vertical formatting",
newline_style: NewlineStyle, "Unix or Windows line endings",
fn_brace_style: BraceStyle, "Brace style for functions",
fn_return_indent: ReturnIndent, "Location of return type in function declaration",
@ -256,6 +258,7 @@ impl Default for Config {
ideal_width: 80,
leeway: 5,
tab_spaces: 4,
list_width: 50,
newline_style: NewlineStyle::Unix,
fn_brace_style: BraceStyle::SameLineWhere,
fn_return_indent: ReturnIndent::WithArgs,

View file

@ -32,7 +32,11 @@ impl Rewrite for ast::Expr {
fn rewrite(&self, context: &RewriteContext, width: usize, offset: Indent) -> Option<String> {
match self.node {
ast::Expr_::ExprVec(ref expr_vec) => {
rewrite_array(expr_vec.iter().map(|e| &**e), self.span, context, width, offset)
rewrite_array(expr_vec.iter().map(|e| &**e),
self.span,
context,
width,
offset)
}
ast::Expr_::ExprLit(ref l) => {
match l.node {
@ -158,7 +162,10 @@ impl Rewrite for ast::Expr {
offset))
}
ast::Expr_::ExprRet(None) => {
wrap_str("return".to_owned(), context.config.max_width, width, offset)
wrap_str("return".to_owned(),
context.config.max_width,
width,
offset)
}
ast::Expr_::ExprRet(Some(ref expr)) => {
rewrite_unary_prefix(context, "return ", &expr, width, offset)
@ -176,7 +183,10 @@ impl Rewrite for ast::Expr {
ast::Expr_::ExprIndex(..) |
ast::Expr_::ExprInlineAsm(..) |
ast::Expr_::ExprRepeat(..) => {
wrap_str(context.snippet(self.span), context.config.max_width, width, offset)
wrap_str(context.snippet(self.span),
context.config.max_width,
width,
offset)
}
}
}
@ -507,7 +517,13 @@ impl<'a> Rewrite for Loop<'a> {
// FIXME: this drops any comment between "loop" and the block.
self.block
.rewrite(context, width, offset)
.map(|result| format!("{}{}{} {}", label_string, self.keyword, pat_expr_string, result))
.map(|result| {
format!("{}{}{} {}",
label_string,
self.keyword,
pat_expr_string,
result)
})
}
}
@ -639,7 +655,10 @@ fn single_line_if_else(context: &RewriteContext,
let fits_line = fixed_cost + pat_expr_str.len() + if_str.len() + else_str.len() <= width;
if fits_line && !if_str.contains('\n') && !else_str.contains('\n') {
return Some(format!("if {} {{ {} }} else {{ {} }}", pat_expr_str, if_str, else_str));
return Some(format!("if {} {{ {} }} else {{ {} }}",
pat_expr_str,
if_str,
else_str));
}
}
@ -1108,7 +1127,9 @@ fn rewrite_paren(context: &RewriteContext,
width: usize,
offset: Indent)
-> Option<String> {
debug!("rewrite_paren, width: {}, offset: {:?}", width, offset);
debug!("rewrite_paren, width: {}, offset: {:?}",
width,
offset);
// 1 is for opening paren, 2 is for opening+closing, we want to keep the closing
// paren on the same line as the subexpr.
let subexpr_str = subexpr.rewrite(context, try_opt!(width.checked_sub(2)), offset + 1);
@ -1124,7 +1145,9 @@ fn rewrite_struct_lit<'a>(context: &RewriteContext,
width: usize,
offset: Indent)
-> Option<String> {
debug!("rewrite_struct_lit: width {}, offset {:?}", width, offset);
debug!("rewrite_struct_lit: width {}, offset {:?}",
width,
offset);
assert!(!fields.is_empty() || base.is_some());
enum StructLitField<'a> {
@ -1229,10 +1252,15 @@ fn rewrite_struct_lit<'a>(context: &RewriteContext,
.block_indent(context.config)
.to_string(context.config);
let outer_indent = context.block_indent.to_string(context.config);
Some(format!("{} {{\n{}{}\n{}}}", path_str, inner_indent, fields_str, outer_indent))
Some(format!("{} {{\n{}{}\n{}}}",
path_str,
inner_indent,
fields_str,
outer_indent))
};
match (context.config.struct_lit_style, context.config.struct_lit_multiline_style) {
match (context.config.struct_lit_style,
context.config.struct_lit_multiline_style) {
(StructLitStyle::Block, _) if fields_str.contains('\n') || fields_str.len() > h_budget =>
format_on_newline(),
(StructLitStyle::Block, MultilineStyle::ForceMulti) => format_on_newline(),
@ -1250,8 +1278,9 @@ fn rewrite_field(context: &RewriteContext,
-> Option<String> {
let name = &field.ident.node.to_string();
let overhead = name.len() + 2;
let expr = field.expr
.rewrite(context, try_opt!(width.checked_sub(overhead)), offset + overhead);
let expr = field.expr.rewrite(context,
try_opt!(width.checked_sub(overhead)),
offset + overhead);
expr.map(|s| format!("{}: {}", name, s))
}
@ -1261,7 +1290,9 @@ fn rewrite_tuple_lit(context: &RewriteContext,
width: usize,
offset: Indent)
-> Option<String> {
debug!("rewrite_tuple_lit: width: {}, offset: {:?}", width, offset);
debug!("rewrite_tuple_lit: width: {}, offset: {:?}",
width,
offset);
let indent = offset + 1;
// In case of length 1, need a trailing comma
if items.len() == 1 {
@ -1352,7 +1383,9 @@ fn rewrite_unary_prefix(context: &RewriteContext,
width: usize,
offset: Indent)
-> Option<String> {
expr.rewrite(context, try_opt!(width.checked_sub(prefix.len())), offset + prefix.len())
expr.rewrite(context,
try_opt!(width.checked_sub(prefix.len())),
offset + prefix.len())
.map(|r| format!("{}{}", prefix, r))
}
@ -1385,7 +1418,9 @@ fn rewrite_assignment(context: &RewriteContext,
// 1 = space between lhs and operator.
let max_width = try_opt!(width.checked_sub(operator_str.len() + 1));
let lhs_str = format!("{} {}", try_opt!(lhs.rewrite(context, max_width, offset)), operator_str);
let lhs_str = format!("{} {}",
try_opt!(lhs.rewrite(context, max_width, offset)),
operator_str);
rewrite_assign_rhs(&context, lhs_str, rhs, width, offset)
}

View file

@ -115,7 +115,8 @@ fn write_file(text: &StringBuffer,
try!(write_system_newlines(&mut v, text, config));
let fmt_text = String::from_utf8(v).unwrap();
let diff = make_diff(&ori_text, &fmt_text, 3);
print_diff(diff, |line_num| format!("\nDiff at line {}:", line_num));
print_diff(diff,
|line_num| format!("\nDiff at line {}:", line_num));
}
WriteMode::Return => {
// io::Write is not implemented for String, working around with

View file

@ -224,12 +224,14 @@ impl BadIssueSeeker {
fn find_unnumbered_issue() {
fn check_fail(text: &str, failing_pos: usize) {
let mut seeker = BadIssueSeeker::new(ReportTactic::Unnumbered, ReportTactic::Unnumbered);
assert_eq!(Some(failing_pos), text.chars().position(|c| seeker.inspect(c).is_some()));
assert_eq!(Some(failing_pos),
text.chars().position(|c| seeker.inspect(c).is_some()));
}
fn check_pass(text: &str) {
let mut seeker = BadIssueSeeker::new(ReportTactic::Unnumbered, ReportTactic::Unnumbered);
assert_eq!(None, text.chars().position(|c| seeker.inspect(c).is_some()));
assert_eq!(None,
text.chars().position(|c| seeker.inspect(c).is_some()));
}
check_fail("TODO\n", 4);
@ -256,11 +258,17 @@ 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]

View file

@ -307,7 +307,9 @@ impl<'a> FmtVisitor<'a> {
let context = self.get_context();
let ret_str = fd.output
.rewrite(&context, self.config.max_width - indent.width(), indent)
.rewrite(&context,
self.config.max_width - indent.width(),
indent)
.unwrap();
// Args.
@ -948,7 +950,7 @@ impl<'a> FmtVisitor<'a> {
item.item = ty;
}
let fmt = ListFormatting::for_fn(h_budget, offset, self.config);
let fmt = ListFormatting::for_item(h_budget, offset, self.config);
let list_str = try_opt!(write_list(&items, &fmt));
Some(format!("<{}>", list_str))
@ -1011,7 +1013,9 @@ impl<'a> FmtVisitor<'a> {
// 9 = " where ".len() + " {".len()
if density == Density::Tall || preds_str.contains('\n') ||
indent.width() + 9 + preds_str.len() > self.config.max_width {
Some(format!("\n{}where {}", (indent + extra_indent).to_string(self.config), preds_str))
Some(format!("\n{}where {}",
(indent + extra_indent).to_string(self.config),
preds_str))
} else {
Some(format!(" where {}", preds_str))
}
@ -1041,7 +1045,10 @@ impl Rewrite for ast::Arg {
fn rewrite(&self, context: &RewriteContext, width: usize, offset: Indent) -> Option<String> {
if is_named_arg(self) {
if let ast::Ty_::TyInfer = self.ty.node {
wrap_str(pprust::pat_to_string(&self.pat), context.config.max_width, width, offset)
wrap_str(pprust::pat_to_string(&self.pat),
context.config.max_width,
width,
offset)
} else {
let mut result = pprust::pat_to_string(&self.pat);
result.push_str(": ");

View file

@ -115,7 +115,8 @@ impl Indent {
pub fn to_string(&self, config: &Config) -> String {
let (num_tabs, num_spaces) = if config.hard_tabs {
(self.block_indent / config.tab_spaces, self.alignment)
(self.block_indent / config.tab_spaces,
self.alignment)
} else {
(0, self.block_indent + self.alignment)
};
@ -146,7 +147,8 @@ impl Sub for Indent {
type Output = Indent;
fn sub(self, rhs: Indent) -> Indent {
Indent::new(self.block_indent - rhs.block_indent, self.alignment - rhs.alignment)
Indent::new(self.block_indent - rhs.block_indent,
self.alignment - rhs.alignment)
}
}

View file

@ -26,6 +26,8 @@ pub enum ListTactic {
Horizontal,
// Try Horizontal layout, if that fails then vertical
HorizontalVertical,
// HorizontalVertical with a soft limit of n characters.
LimitedHorizontalVertical(usize),
// Pack as many items as possible per row over (possibly) many rows.
Mixed,
}
@ -59,6 +61,19 @@ pub struct ListFormatting<'a> {
impl<'a> ListFormatting<'a> {
pub fn for_fn(width: usize, offset: Indent, config: &'a Config) -> ListFormatting<'a> {
ListFormatting {
tactic: ListTactic::LimitedHorizontalVertical(config.list_width),
separator: ",",
trailing_separator: SeparatorTactic::Never,
indent: offset,
h_width: width,
v_width: width,
ends_with_newline: false,
config: config,
}
}
pub fn for_item(width: usize, offset: Indent, config: &'a Config) -> ListFormatting<'a> {
ListFormatting {
tactic: ListTactic::HorizontalVertical,
separator: ",",
@ -118,6 +133,13 @@ pub fn write_list<'b>(items: &[ListItem], formatting: &ListFormatting<'b>) -> Op
let fits_single = total_width + total_sep_len <= formatting.h_width;
// Check if we need to fallback from horizontal listing, if possible.
if let ListTactic::LimitedHorizontalVertical(limit) = tactic {
if total_width > limit {
tactic = ListTactic::Vertical;
} else {
tactic = ListTactic::HorizontalVertical;
}
}
if tactic == ListTactic::HorizontalVertical {
debug!("write_list: total_width: {}, total_sep_len: {}, h_width: {}",
total_width,

View file

@ -103,7 +103,10 @@ pub fn rewrite_macro(mac: &ast::Mac,
}
MacroStyle::Braces => {
// Skip macro invocations with braces, for now.
wrap_str(context.snippet(mac.span), context.config.max_width, width, offset)
wrap_str(context.snippet(mac.span),
context.config.max_width,
width,
offset)
}
}
}

View file

@ -16,7 +16,8 @@ impl<'a> FmtVisitor<'a> {
// TODO these format_missing methods are ugly. Refactor and add unit tests
// for the central whitespace stripping loop.
pub fn format_missing(&mut self, end: BytePos) {
self.format_missing_inner(end, |this, last_snippet, _| this.buffer.push_str(last_snippet))
self.format_missing_inner(end,
|this, last_snippet, _| this.buffer.push_str(last_snippet))
}
pub fn format_missing_with_indent(&mut self, end: BytePos) {

View file

@ -25,7 +25,10 @@ pub fn list_files<'a>(krate: &'a ast::Crate,
-> HashMap<PathBuf, &'a ast::Mod> {
let mut result = HashMap::new();
let root_filename: PathBuf = codemap.span_to_filename(krate.span).into();
list_submodules(&krate.module, root_filename.parent().unwrap(), codemap, &mut result);
list_submodules(&krate.module,
root_filename.parent().unwrap(),
codemap,
&mut result);
result.insert(root_filename, &krate.module);
result
}

View file

@ -205,7 +205,9 @@ fn rewrite_segment(segment: &ast::PathSegment,
.collect::<Vec<_>>();
let next_span_lo = param_list.last().unwrap().get_span().hi + BytePos(1);
let list_lo = span_after(codemap::mk_sp(*span_lo, span_hi), "<", context.codemap);
let list_lo = span_after(codemap::mk_sp(*span_lo, span_hi),
"<",
context.codemap);
let separator = get_path_separator(context.codemap, *span_lo, list_lo);
// 1 for <
@ -230,7 +232,7 @@ fn rewrite_segment(segment: &ast::PathSegment,
list_lo,
span_hi);
let fmt = ListFormatting::for_fn(list_width, offset + extra_offset, context.config);
let fmt = ListFormatting::for_item(list_width, offset + extra_offset, context.config);
let list_str = try_opt!(write_list(&items.collect::<Vec<_>>(), &fmt));
// Update position of last bracket.
@ -363,7 +365,8 @@ impl Rewrite for ast::TyParamBound {
tref.rewrite(context, width, offset)
}
ast::TyParamBound::TraitTyParamBound(ref tref, ast::TraitBoundModifier::Maybe) => {
Some(format!("?{}", try_opt!(tref.rewrite(context, width - 1, offset + 1))))
Some(format!("?{}",
try_opt!(tref.rewrite(context, width - 1, offset + 1))))
}
ast::TyParamBound::RegionTyParamBound(ref l) => {
Some(pprust::lifetime_to_string(l))

View file

@ -316,7 +316,9 @@ impl<'a> FmtVisitor<'a> {
}
fn format_mod(&mut self, m: &ast::Mod, s: Span, ident: ast::Ident) {
debug!("FmtVisitor::format_mod: ident: {:?}, span: {:?}", ident, s);
debug!("FmtVisitor::format_mod: ident: {:?}, span: {:?}",
ident,
s);
// Decide whether this is an inline mod or an external mod.
let local_file_name = self.codemap.span_to_filename(s);
@ -354,7 +356,9 @@ impl<'a> FmtVisitor<'a> {
overflow_indent: Indent::empty(),
};
// 1 = ";"
match vp.rewrite(&context, self.config.max_width - offset.width() - 1, offset) {
match vp.rewrite(&context,
self.config.max_width - offset.width() - 1,
offset) {
Some(ref s) if s.is_empty() => {
// Format up to last newline
let prev_span = codemap::mk_sp(self.last_pos, span.lo);

View file

@ -89,7 +89,9 @@ fn self_tests() {
warnings += format_report.warning_count();
}
assert!(warnings == 0, "Rustfmt's code generated {} warnings", warnings);
assert!(warnings == 0,
"Rustfmt's code generated {} warnings",
warnings);
}
// For each file, run rustfmt and collect the output.
@ -122,7 +124,8 @@ fn print_mismatches(result: HashMap<String, Vec<Mismatch>>) {
let mut t = term::stdout().unwrap();
for (file_name, diff) in result {
print_diff(diff, |line_num| format!("\nMismatch at {}:{}:", file_name, line_num));
print_diff(diff,
|line_num| format!("\nMismatch at {}:{}:", file_name, line_num));
}
assert!(t.reset().unwrap());

View file

@ -9,7 +9,8 @@ fn main() {
b: WithType, // argument
// ignored
_| {
(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb)
(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,
bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb)
};
let block_body = move |xxxxxxxxxxxxxxxxxxxxxxxxxxxxx,

View file

@ -2,7 +2,9 @@
fn foo() {
let a = (a, a, a, a, a);
let aaaaaaaaaaaaaaaa = (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaa, aaaaaaaaaaaaaa);
let aaaaaaaaaaaaaaaa = (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,
aaaaaaaaaaaaaa,
aaaaaaaaaaaaaa);
let aaaaaaaaaaaaaaaaaaaaaa = (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,
aaaaaaaaaaaaaaaaaaaaaaaaa,
@ -24,6 +26,7 @@ fn a() {
}
fn b() {
((bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb, bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb),
((bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb,
bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb),
bbbbbbbbbbbbbbbbbb)
}