Merge #9497

9497: Wrap inlined closures in parens when inlined in an expression in `inline_call` r=Veykril a=Veykril

bors r+

Co-authored-by: Lukas Wirth <lukastw97@gmail.com>

crates/ide_assists/src/handlers/inline_call.rs

@@ -14,25 +14,22 @@
 
 // Assist: inline_call
 //
-// Inlines a function or method body.
+// Inlines a function or method body creating a `let` statement per parameter unless the parameter
+// can be inlined. The parameter will be inlined either if it the supplied argument is a simple local
+// or if the parameter is only accessed inside the function body once.
 //
 // ```
-// fn align(a: u32, b: u32) -> u32 {
+// # //- minicore: option
-//     (a + b - 1) & !(b - 1)
+// fn foo(name: Option<&str>) {
-// }
+//     let name = name.unwrap$0();
-// fn main() {
-//     let x = align$0(1, 2);
 // }
 // ```
 // ->
 // ```
-// fn align(a: u32, b: u32) -> u32 {
+// fn foo(name: Option<&str>) {
-//     (a + b - 1) & !(b - 1)
+//     let name = match name {
-// }
+//             Some(val) => val,
-// fn main() {
+//             None => panic!("called `Option::unwrap()` on a `None` value"),
-//     let x = {
-//         let b = 2;
-//         (1 + b - 1) & !(b - 1)
 //         };
 // }
 // ```
@@ -137,7 +134,7 @@ pub(crate) fn inline_(
                                 .covering_element(range)
                                 .ancestors()
                                 .nth(3)
-                                .filter(|it| ast::PathExpr::can_cast(it.kind())),
+                                .and_then(ast::PathExpr::cast),
                             _ => None,
                         })
                         .collect::<Option<Vec<_>>>()
@@ -163,7 +160,14 @@ pub(crate) fn inline_(
                 match &*usages {
                     // inline single use parameters
                     [usage] => {
-                        ted::replace(usage, expr.syntax().clone_for_update());
+                        let expr = if matches!(expr, ast::Expr::ClosureExpr(_))
+                            && usage.syntax().parent().and_then(ast::Expr::cast).is_some()
+                        {
+                            make::expr_paren(expr)
+                        } else {
+                            expr
+                        };
+                        ted::replace(usage.syntax(), expr.syntax().clone_for_update());
                     }
                     // inline parameters whose expression is a simple local reference
                     [_, ..]
@@ -173,7 +177,7 @@ pub(crate) fn inline_(
                         ) =>
                     {
                         usages.into_iter().for_each(|usage| {
-                            ted::replace(usage, &expr.syntax().clone_for_update());
+                            ted::replace(usage.syntax(), &expr.syntax().clone_for_update());
                         });
                     }
                     // cant inline, emit a let statement
@@ -540,6 +544,56 @@ fn bar(&self) {
         };
     }
 }
+"#,
+        );
+    }
+
+    #[test]
+    fn wraps_closure_in_paren() {
+        check_assist(
+            inline_call,
+            r#"
+fn foo(x: fn()) {
+    x();
+}
+
+fn main() {
+    foo$0(|| {})
+}
+"#,
+            r#"
+fn foo(x: fn()) {
+    x();
+}
+
+fn main() {
+    {
+        (|| {})();
+    }
+}
+"#,
+        );
+        check_assist(
+            inline_call,
+            r#"
+fn foo(x: fn()) {
+    x();
+}
+
+fn main() {
+    foo$0(main)
+}
+"#,
+            r#"
+fn foo(x: fn()) {
+    x();
+}
+
+fn main() {
+    {
+        main();
+    }
+}
 "#,
         );
     }

crates/ide_assists/src/tests/generated.rs

@@ -923,21 +923,16 @@ fn doctest_inline_call() {
     check_doc_test(
         "inline_call",
         r#####"
-fn align(a: u32, b: u32) -> u32 {
+//- minicore: option
-    (a + b - 1) & !(b - 1)
+fn foo(name: Option<&str>) {
-}
+    let name = name.unwrap$0();
-fn main() {
-    let x = align$0(1, 2);
 }
 "#####,
         r#####"
-fn align(a: u32, b: u32) -> u32 {
+fn foo(name: Option<&str>) {
-    (a + b - 1) & !(b - 1)
+    let name = match name {
-}
+            Some(val) => val,
-fn main() {
+            None => panic!("called `Option::unwrap()` on a `None` value"),
-    let x = {
-        let b = 2;
-        (1 + b - 1) & !(b - 1)
         };
 }
 "#####,

crates/ide_db/src/helpers/insert_use.rs

@@ -1,4 +1,7 @@
 //! Handle syntactic aspects of inserting a new `use`.
+#[cfg(test)]
+mod tests;
+
 use std::cmp::Ordering;
 
 use hir::Semantics;
@@ -378,5 +381,3 @@ fn is_inner_comment(token: SyntaxToken) -> bool {
     ast::Comment::cast(token).and_then(|comment| comment.kind().doc)
         == Some(ast::CommentPlacement::Inner)
 }
-#[cfg(test)]
-mod tests;

crates/syntax/src/ted.rs

@@ -161,6 +161,14 @@ fn ws_before(position: &Position, new: &SyntaxElement) -> Option<SyntaxToken> {
         }
     }
 
+    if prev.kind() == T!['{'] && ast::Stmt::can_cast(new.kind()) {
+        if let Some(block_expr) = prev.parent().and_then(ast::BlockExpr::cast) {
+            let mut indent = IndentLevel::from_element(&block_expr.syntax().clone().into());
+            indent.0 += 1;
+            return Some(make::tokens::whitespace(&format!("\n{}", indent)));
+        }
+    }
+
     ws_between(prev, new)
 }
 fn ws_after(position: &Position, new: &SyntaxElement) -> Option<SyntaxToken> {
@@ -187,12 +195,6 @@ fn ws_between(left: &SyntaxElement, right: &SyntaxElement) -> Option<SyntaxToken
         return None;
     }
 
-    if left.kind() == T!['{'] && right.kind() == SyntaxKind::LET_STMT {
-        let mut indent = IndentLevel::from_element(left);
-        indent.0 += 1;
-        return Some(make::tokens::whitespace(&format!("\n{}", indent)));
-    }
-
     if right.kind() == SyntaxKind::USE {
         let mut indent = IndentLevel::from_element(left);
         if left.kind() == SyntaxKind::USE {

crates/test_utils/src/minicore.rs

@@ -330,6 +330,15 @@ pub enum Option<T> {
         #[lang = "Some"]
         Some(T),
     }
+
+    impl<T> Option<T> {
+        pub const fn unwrap(self) -> T {
+            match self {
+                Some(val) => val,
+                None => panic!("called `Option::unwrap()` on a `None` value"),
+            }
+        }
+    }
 }
 // endregion:option