fix span of Try expression when converting from try! macro to the ? keyword (#3940)

Author: scampi

src/chains.rs

@@ -331,33 +331,26 @@ impl Chain {
             }
         }
         while let Some(chain_item) = iter.next() {
-            let comment_snippet = context.snippet(chain_item.span);
-            // FIXME: Figure out the way to get a correct span when converting `try!` to `?`.
-            let handle_comment =
-                !(context.config.use_try_shorthand() || is_tries(comment_snippet.trim()));
-
             // Pre-comment
-            if handle_comment {
-                let pre_comment_span = mk_sp(prev_span_end, chain_item.span.lo());
-                let pre_comment_snippet = trim_tries(context.snippet(pre_comment_span));
-                let (pre_comment, _) = extract_pre_comment(&pre_comment_snippet);
-                match pre_comment {
-                    Some(ref comment) if !comment.is_empty() => {
-                        children.push(ChainItem::comment(
-                            pre_comment_span,
-                            comment.to_owned(),
-                            CommentPosition::Top,
-                        ));
-                    }
-                    _ => (),
+            let pre_comment_span = mk_sp(prev_span_end, chain_item.span.lo());
+            let pre_comment_snippet = trim_tries(context.snippet(pre_comment_span));
+            let (pre_comment, _) = extract_pre_comment(&pre_comment_snippet);
+            match pre_comment {
+                Some(ref comment) if !comment.is_empty() => {
+                    children.push(ChainItem::comment(
+                        pre_comment_span,
+                        comment.to_owned(),
+                        CommentPosition::Top,
+                    ));
                 }
+                _ => (),
             }
 
             prev_span_end = chain_item.span.hi();
             children.push(chain_item);
 
             // Post-comment
-            if !handle_comment || iter.peek().is_none() {
+            if iter.peek().is_none() {
                 continue;
             }
 

src/macros.rs

@@ -1199,9 +1199,13 @@ pub(crate) fn convert_try_mac(mac: &ast::Mac, context: &RewriteContext<'_>) -> O
         let ts: TokenStream = mac.tts.clone();
         let mut parser = new_parser_from_tts(context.parse_sess.inner(), ts.trees().collect());
 
+        let mut kind = parser.parse_expr().ok()?;
+        // take the end pos of mac so that the Try expression includes the closing parenthesis of
+        // the try! macro
+        kind.span = mk_sp(kind.span.lo(), mac.span.hi());
         Some(ast::Expr {
             id: ast::NodeId::root(), // dummy value
-            kind: ast::ExprKind::Try(parser.parse_expr().ok()?),
+            kind: ast::ExprKind::Try(kind),
             span: mac.span, // incorrect span, but shouldn't matter too much
             attrs: ThinVec::new(),
         })

tests/source/issue-3908.rs

@@ -0,0 +1,23 @@
+// rustfmt-use_try_shorthand: true
+
+macro_rules! foo {
+() => {
+bar1()
+// baz1
+.expect("Qux1");
+try!(try!(try!(bar2())
+// baz2a
+.expect("Qux2a"))
+// baz2b
+.expect("Qux2b"));
+bar3()?
+// baz3
+.expect("Qux3")?;
+};
+}
+
+fn foo() {
+try!(try!(bar4())
+// baz4
+.expect("Qux4"));
+}

tests/target/issue-3908.rs

@@ -0,0 +1,23 @@
+// rustfmt-use_try_shorthand: true
+
+macro_rules! foo {
+    () => {
+        bar1()
+            // baz1
+            .expect("Qux1");
+        bar2()?
+            // baz2a
+            .expect("Qux2a")?
+            // baz2b
+            .expect("Qux2b")?;
+        bar3()?
+            // baz3
+            .expect("Qux3")?;
+    };
+}
+
+fn foo() {
+    bar4()?
+        // baz4
+        .expect("Qux4")?;
+}