Add try macro to try shorthand conversion tests

Author: marcusklaas

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<String> {
     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,
-        }
-    }
-
-    let mut subexpr_list = vec![expr];
+fn make_subexpr_list(expr: &ast::Expr, context: &RewriteContext) -> (ast::Expr, Vec<ast::Expr>) {
+    let mut subexpr_list = vec![expr.clone()];
 
-    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<ast::Expr> {
     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,

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";
 }

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<S: Into<String>>(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);
-
-            result.push_str(&&try_opt!(rhs));
+            let new_rhs = ex.rewrite(&inner_context, max_width, new_offset);
+
+            // 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);
+                }
+            }
         }
     }
 

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<String> {
+    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<ast::Expr> {
+    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());

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

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?
+}

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

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?
+}