diff --git a/src/chains.rs b/src/chains.rs index da2a7629bad2..3f306abfdcab 100644 --- a/src/chains.rs +++ b/src/chains.rs @@ -86,18 +86,18 @@ use rewrite::{Rewrite, RewriteContext}; use utils::{wrap_str, first_line_width}; use expr::rewrite_call; use config::BlockIndentStyle; +use macros::convert_try_mac; use syntax::{ast, ptr}; use syntax::codemap::{mk_sp, Span}; - pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, width: usize, offset: Indent) -> Option { let total_span = expr.span; - let (parent, subexpr_list) = make_subexpr_list(expr); + let (parent, subexpr_list) = make_subexpr_list(expr, context); // Parent is the first item in the chain, e.g., `foo` in `foo.bar.baz()`. let parent_block_indent = chain_base_indent(context, offset); @@ -106,19 +106,16 @@ pub fn rewrite_chain(expr: &ast::Expr, // Decide how to layout the rest of the chain. `extend` is true if we can // put the first non-parent item on the same line as the parent. - let (indent, extend) = if !parent_rewrite.contains('\n') && is_continuable(parent) || + let (indent, extend) = if !parent_rewrite.contains('\n') && is_continuable(&parent) || parent_rewrite.len() <= context.config.tab_spaces { -// <<<<<<< HEAD -// // Try and put at least the first two items on the same line. -// (chain_indent(context, offset + Indent::new(0, parent_rewrite.len())), true) -// ======= + let indent = if let ast::ExprKind::Try(..) = subexpr_list.last().unwrap().node { parent_block_indent.block_indent(context.config) } else { - offset + Indent::new(0, parent_rewrite.len()) + chain_indent(context, offset + Indent::new(0, parent_rewrite.len())) }; (indent, true) - } else if is_block_expr(parent, &parent_rewrite) { + } else if is_block_expr(&parent, &parent_rewrite) { // The parent is a block, so align the rest of the chain with the closing // brace. (parent_block_indent, false) @@ -197,11 +194,13 @@ pub fn rewrite_chain(expr: &ast::Expr, offset) } -fn join_rewrites(rewrites: &[String], subexps: &[&ast::Expr], connector: &str) -> String { +fn join_rewrites(rewrites: &[String], subexps: &[ast::Expr], connector: &str) -> String { let mut rewrite_iter = rewrites.iter(); let mut result = rewrite_iter.next().unwrap().clone(); + let mut subexpr_iter = subexps.iter().rev(); + subexpr_iter.next(); - for (rewrite, expr) in rewrite_iter.zip(subexps.iter()) { + for (rewrite, expr) in rewrite_iter.zip(subexpr_iter) { match expr.node { ast::ExprKind::Try(_) => (), _ => result.push_str(connector), @@ -235,21 +234,11 @@ fn is_block_expr(expr: &ast::Expr, repr: &str) -> bool { // Returns the root of the chain and a Vec of the prefixes of the rest of the chain. // E.g., for input `a.b.c` we return (`a`, [`a.b.c`, `a.b`]) -fn make_subexpr_list(mut expr: &ast::Expr) -> (&ast::Expr, Vec<&ast::Expr>) { - fn pop_expr_chain(expr: &ast::Expr) -> Option<&ast::Expr> { - match expr.node { - ast::ExprKind::MethodCall(_, _, ref expressions) => Some(&expressions[0]), - ast::ExprKind::TupField(ref subexpr, _) | - ast::ExprKind::Field(ref subexpr, _) => Some(subexpr), - _ => None, - } - } +fn make_subexpr_list(expr: &ast::Expr, context: &RewriteContext) -> (ast::Expr, Vec) { + let mut subexpr_list = vec![expr.clone()]; - let mut subexpr_list = vec![expr]; - - while let Some(subexpr) = pop_expr_chain(expr) { - subexpr_list.push(subexpr); - expr = subexpr; + while let Some(subexpr) = pop_expr_chain(subexpr_list.last().unwrap(), context) { + subexpr_list.push(subexpr.clone()); } let parent = subexpr_list.pop().unwrap(); @@ -315,16 +304,33 @@ fn rewrite_method_call_with_overflow(expr_kind: &ast::ExprKind, } } -fn pop_expr_chain(expr: &ast::Expr) -> Option<&ast::Expr> { +// Returns the expression's subexpression, if it exists. When the subexpr +// is a try! macro, we'll convert it to shorthand when the option is set. +fn pop_expr_chain(expr: &ast::Expr, context: &RewriteContext) -> Option { match expr.node { - ast::ExprKind::MethodCall(_, _, ref expressions) => Some(&expressions[0]), + ast::ExprKind::MethodCall(_, _, ref expressions) => { + Some(convert_try(&expressions[0], context)) + } ast::ExprKind::TupField(ref subexpr, _) | ast::ExprKind::Field(ref subexpr, _) | - ast::ExprKind::Try(ref subexpr) => Some(subexpr), + ast::ExprKind::Try(ref subexpr) => Some(convert_try(subexpr, context)), _ => None, } } +fn convert_try(expr: &ast::Expr, context: &RewriteContext) -> ast::Expr { + match expr.node { + ast::ExprKind::Mac(ref mac) if context.config.use_try_shorthand => { + if let Some(subexpr) = convert_try_mac(mac, context) { + subexpr + } else { + expr.clone() + } + } + _ => expr.clone(), + } +} + // Rewrite the last element in the chain `expr`. E.g., given `a.b.c` we rewrite // `.c`. fn rewrite_chain_subexpr(expr: &ast::Expr, diff --git a/src/config.rs b/src/config.rs index 333835b66cdc..5e4dbefad45c 100644 --- a/src/config.rs +++ b/src/config.rs @@ -392,6 +392,7 @@ create_config! { match_wildcard_trailing_comma: bool, true, "Put a trailing comma after a wildcard arm"; closure_block_indent_threshold: isize, 5, "How many lines a closure must have before it is \ block indented. -1 means never use block indent."; + use_try_shorthand: bool, false, "Replace uses of the try! macro by the ? shorthand"; write_mode: WriteMode, WriteMode::Replace, "What Write Mode to use when none is supplied: Replace, Overwrite, Display, Diff, Coverage"; } diff --git a/src/expr.rs b/src/expr.rs index ba2ed7aec095..1edacbf765d6 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -148,7 +148,7 @@ impl Rewrite for ast::Expr { ast::ExprKind::Closure(capture, ref fn_decl, ref body, _) => { rewrite_closure(capture, fn_decl, body, self.span, context, width, offset) } - // ast::ExprKind::Try(..) | + ast::ExprKind::Try(..) | ast::ExprKind::Field(..) | ast::ExprKind::TupField(..) | ast::ExprKind::MethodCall(..) => rewrite_chain(self, context, width, offset), @@ -205,15 +205,10 @@ impl Rewrite for ast::Expr { (None, None) => wrap_str(delim.into(), context.config.max_width, width, offset), } } - ast::ExprKind::Try(ref expr) => { - rewrite_unary_suffix(context, "?", &**expr, width, offset) - } // We do not format these expressions yet, but they should still // satisfy our width restrictions. ast::ExprKind::InPlace(..) | - ast::ExprKind::InlineAsm(..) | - // TODO(#848): Handle type ascription - ast::ExprKind::Type(_, _) => { + ast::ExprKind::InlineAsm(..) => { wrap_str(context.snippet(self.span), context.config.max_width, width, @@ -1832,24 +1827,41 @@ pub fn rewrite_assign_rhs>(context: &RewriteContext, let max_width = try_opt!(width.checked_sub(last_line_width + 1)); let rhs = ex.rewrite(&context, max_width, offset + last_line_width + 1); + fn count_line_breaks(src: &str) -> usize { + src.chars().filter(|&x| x == '\n').count() + } + match rhs { - Some(new_str) => { + Some(ref new_str) if count_line_breaks(new_str) < 2 => { result.push(' '); - result.push_str(&new_str) + result.push_str(new_str); } - None => { - // Expression did not fit on the same line as the identifier. Retry - // on the next line. + _ => { + // Expression did not fit on the same line as the identifier or is + // at least three lines big. Try splitting the line and see + // if that works better. let new_offset = offset.block_indent(context.config); - result.push_str(&format!("\n{}", new_offset.to_string(context.config))); - - // FIXME: we probably should related max_width to width instead of - // config.max_width where is the 1 coming from anyway? - let max_width = try_opt!(context.config.max_width.checked_sub(new_offset.width() + 1)); + let max_width = try_opt!((width + offset.width()).checked_sub(new_offset.width())); let inner_context = context.nested_context(); - let rhs = ex.rewrite(&inner_context, max_width, new_offset); + let new_rhs = ex.rewrite(&inner_context, max_width, new_offset); - result.push_str(&&try_opt!(rhs)); + // FIXME: DRY! + match (rhs, new_rhs) { + (Some(ref orig_rhs), Some(ref replacement_rhs)) + if count_line_breaks(orig_rhs) > count_line_breaks(replacement_rhs) + 1 => { + result.push_str(&format!("\n{}", new_offset.to_string(context.config))); + result.push_str(replacement_rhs); + } + (None, Some(ref final_rhs)) => { + result.push_str(&format!("\n{}", new_offset.to_string(context.config))); + result.push_str(final_rhs); + } + (None, None) => return None, + (Some(ref orig_rhs), _) => { + result.push(' '); + result.push_str(orig_rhs); + } + } } } diff --git a/src/macros.rs b/src/macros.rs index a0c7cd51bb79..e9992fb0eb01 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -25,7 +25,7 @@ use syntax::parse::tts_to_parser; use syntax::codemap::{mk_sp, BytePos}; use Indent; -use rewrite::RewriteContext; +use rewrite::{Rewrite, RewriteContext}; use expr::{rewrite_call, rewrite_array}; use comment::{FindUncommented, contains_comment}; use utils::{CodeMapSpanUtils, wrap_str}; @@ -56,6 +56,12 @@ pub fn rewrite_macro(mac: &ast::Mac, width: usize, offset: Indent) -> Option { + if context.config.use_try_shorthand { + if let Some(expr) = convert_try_mac(mac, context) { + return expr.rewrite(context, width, offset); + } + } + let original_style = macro_style(mac, context); let macro_name = match extra_ident { None | @@ -141,6 +147,24 @@ pub fn rewrite_macro(mac: &ast::Mac, } } +/// Tries to convert a macro use into a short hand try expression. Returns None +/// when the macro is not an instance of try! (or parsing the inner expression +/// failed). +pub fn convert_try_mac(mac: &ast::Mac, context: &RewriteContext) -> Option { + if &format!("{}", mac.node.path)[..] == "try" { + let mut parser = tts_to_parser(context.parse_session, mac.node.tts.clone(), Vec::new()); + + Some(ast::Expr { + id: 0, // dummy value + node: ast::ExprKind::Try(try_opt!(parser.parse_expr().ok())), + span: mac.span, // incorrect span, but shouldn't matter too much + attrs: None, + }) + } else { + None + } +} + 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()); diff --git a/tests/source/chains.rs b/tests/source/chains.rs index 8ec5cd191c5e..48de948865bc 100644 --- a/tests/source/chains.rs +++ b/tests/source/chains.rs @@ -120,4 +120,8 @@ fn try_shorthand() { let yyyy = expr?.another?.another?.another?.another?.another?.another?.another?.another?.test(); let zzzz = expr?.another?.another?.another?.another?; let aaa = x ???????????? ?????????????? ???? ????? ?????????????? ????????? ?????????????? ??; + + let y = a.very .loooooooooooooooooooooooooooooooooooooong() .chain() + .inside() .weeeeeeeeeeeeeee()? .test() .0 + .x; } diff --git a/tests/source/try-conversion.rs b/tests/source/try-conversion.rs new file mode 100644 index 000000000000..addf2f5d5e8f --- /dev/null +++ b/tests/source/try-conversion.rs @@ -0,0 +1,11 @@ +// rustfmt-use_try_shorthand: true + +fn main() { + let x = try!(some_expr()); + + let y = try!(a.very.loooooooooooooooooooooooooooooooooooooong().chain().inside().weeeeeeeeeeeeeee()).test().0.x; +} + +fn test() { + a? +} diff --git a/tests/target/chains.rs b/tests/target/chains.rs index 4c38ab7eb528..2298b5e17148 100644 --- a/tests/target/chains.rs +++ b/tests/target/chains.rs @@ -145,4 +145,13 @@ fn try_shorthand() { let yyyy = expr?.another?.another?.another?.another?.another?.another?.another?.another?.test(); let zzzz = expr?.another?.another?.another?.another?; let aaa = x??????????????????????????????????????????????????????????????????????????; + + let y = a.very + .loooooooooooooooooooooooooooooooooooooong() + .chain() + .inside() + .weeeeeeeeeeeeeee()? + .test() + .0 + .x; } diff --git a/tests/target/try-conversion.rs b/tests/target/try-conversion.rs new file mode 100644 index 000000000000..2daaba884907 --- /dev/null +++ b/tests/target/try-conversion.rs @@ -0,0 +1,18 @@ +// rustfmt-use_try_shorthand: true + +fn main() { + let x = some_expr()?; + + let y = a.very + .loooooooooooooooooooooooooooooooooooooong() + .chain() + .inside() + .weeeeeeeeeeeeeee()? + .test() + .0 + .x; +} + +fn test() { + a? +}