intelligently add parens when inlining local varaibles

5 files changed, 402 insertions, 45 deletions

diff --git a/crates/ra_assists/src/assist_ctx.rs b/crates/ra_assists/src/assist_ctx.rs
index bb5742bd9..e80e35738 100644
--- a/crates/ra_assists/src/assist_ctx.rs
+++ b/crates/ra_assists/src/assist_ctx.rs
@@ -114,6 +114,10 @@ impl<'a, DB: HirDatabase> AssistCtx<'a, DB> {
     pub(crate) fn covering_element(&self) -> SyntaxElement<'a> {
         find_covering_element(self.source_file.syntax(), self.frange.range)
     }
+
+    pub(crate) fn covering_node_for_range(&self, range: TextRange) -> SyntaxElement<'a> {
+        find_covering_element(self.source_file.syntax(), range)
+    }
 }
 
 #[derive(Default)]
diff --git a/crates/ra_assists/src/inline_local_variable.rs b/crates/ra_assists/src/inline_local_variable.rs
index 2258ca139..0d7b884b8 100644
--- a/crates/ra_assists/src/inline_local_variable.rs
+++ b/crates/ra_assists/src/inline_local_variable.rs
@@ -1,7 +1,16 @@
-use hir::db::HirDatabase;
-use hir::source_binder::function_from_child_node;
-use ra_syntax::{ast::{self, AstNode}, TextRange};
-use ra_syntax::ast::{PatKind, ExprKind};
+use hir::{
+    db::HirDatabase,
+    source_binder::function_from_child_node
+};
+use ra_syntax::{
+    ast::{
+        self,
+        AstNode,
+        PatKind,
+        ExprKind
+    },
+    TextRange,
+};
 
 use crate::{Assist, AssistCtx, AssistId};
 use crate::assist_ctx::AssistBuilder;
@@ -15,37 +24,7 @@ pub(crate) fn inline_local_varialbe(mut ctx: AssistCtx<impl HirDatabase>) -> Opt
     if bind_pat.is_mutable() {
         return None;
     }
-    let initializer = let_stmt.initializer()?;
-    let wrap_in_parens = match initializer.kind() {
-        ExprKind::LambdaExpr(_)
-        | ExprKind::IfExpr(_)
-        | ExprKind::LoopExpr(_)
-        | ExprKind::ForExpr(_)
-        | ExprKind::WhileExpr(_)
-        | ExprKind::ContinueExpr(_)
-        | ExprKind::BreakExpr(_)
-        | ExprKind::Label(_)
-        | ExprKind::ReturnExpr(_)
-        | ExprKind::MatchExpr(_)
-        | ExprKind::StructLit(_)
-        | ExprKind::CastExpr(_)
-        | ExprKind::PrefixExpr(_)
-        | ExprKind::RangeExpr(_)
-        | ExprKind::BinExpr(_) => true,
-        ExprKind::CallExpr(_)
-        | ExprKind::IndexExpr(_)
-        | ExprKind::MethodCallExpr(_)
-        | ExprKind::FieldExpr(_)
-        | ExprKind::TryExpr(_)
-        | ExprKind::RefExpr(_)
-        | ExprKind::Literal(_)
-        | ExprKind::TupleExpr(_)
-        | ExprKind::ArrayExpr(_)
-        | ExprKind::ParenExpr(_)
-        | ExprKind::PathExpr(_)
-        | ExprKind::BlockExpr(_) => false,
-    };
-
+    let initializer_expr = let_stmt.initializer();
     let delete_range = if let Some(whitespace) = let_stmt
         .syntax()
         .next_sibling_or_token()
@@ -56,22 +35,66 @@ pub(crate) fn inline_local_varialbe(mut ctx: AssistCtx<impl HirDatabase>) -> Opt
         let_stmt.syntax().range()
     };
 
-    let init_str = if wrap_in_parens {
-        format!("({})", initializer.syntax().text().to_string())
-    } else {
-        initializer.syntax().text().to_string()
-    };
     let function = function_from_child_node(ctx.db, ctx.frange.file_id, bind_pat.syntax())?;
     let scope = function.scopes(ctx.db);
     let refs = scope.find_all_refs(bind_pat);
 
+    let mut wrap_in_parens = vec![true; refs.len()];
+
+    for (i, desc) in refs.iter().enumerate() {
+        let usage_node = ctx
+            .covering_node_for_range(desc.range)
+            .ancestors()
+            .find_map(|node| ast::PathExpr::cast(node))?;
+        let usage_parent_option = usage_node.syntax().parent().and_then(ast::Expr::cast);
+        let usage_parent = match usage_parent_option {
+            Some(u) => u,
+            None => {
+                wrap_in_parens[i] = false;
+                continue;
+            }
+        };
+
+        wrap_in_parens[i] = match (initializer_expr?.kind(), usage_parent.kind()) {
+            (ExprKind::CallExpr(_), _)
+            | (ExprKind::IndexExpr(_), _)
+            | (ExprKind::MethodCallExpr(_), _)
+            | (ExprKind::FieldExpr(_), _)
+            | (ExprKind::TryExpr(_), _)
+            | (ExprKind::RefExpr(_), _)
+            | (ExprKind::Literal(_), _)
+            | (ExprKind::TupleExpr(_), _)
+            | (ExprKind::ArrayExpr(_), _)
+            | (ExprKind::ParenExpr(_), _)
+            | (ExprKind::PathExpr(_), _)
+            | (ExprKind::BlockExpr(_), _)
+            | (_, ExprKind::CallExpr(_))
+            | (_, ExprKind::TupleExpr(_))
+            | (_, ExprKind::ArrayExpr(_))
+            | (_, ExprKind::ParenExpr(_))
+            | (_, ExprKind::ForExpr(_))
+            | (_, ExprKind::WhileExpr(_))
+            | (_, ExprKind::BreakExpr(_))
+            | (_, ExprKind::ReturnExpr(_))
+            | (_, ExprKind::MatchExpr(_)) => false,
+            _ => true,
+        };
+    }
+
+    let init_str = initializer_expr?.syntax().text().to_string();
+    let init_in_paren = format!("({})", &init_str);
+
     ctx.add_action(
         AssistId("inline_local_variable"),
         "inline local variable",
         move |edit: &mut AssistBuilder| {
             edit.delete(delete_range);
-            for desc in refs {
-                edit.replace(desc.range, init_str.clone())
+            for (desc, should_wrap) in refs.iter().zip(wrap_in_parens) {
+                if should_wrap {
+                    edit.replace(desc.range, init_in_paren.clone())
+                } else {
+                    edit.replace(desc.range, init_str.clone())
+                }
             }
             edit.set_cursor(delete_range.start())
         },
@@ -149,7 +172,7 @@ fn foo() {
 
     }
     let b = (1 + 1) * 10;
-    bar((1 + 1));
+    bar(1 + 1);
 }",
         );
     }
@@ -217,7 +240,7 @@ fn foo() {
 
     }
     let b = (bar(1) as u64) * 10;
-    bar((bar(1) as u64));
+    bar(bar(1) as u64);
 }",
         );
     }
@@ -297,4 +320,324 @@ fn foo() {
 }",
         );
     }
+
+    #[test]
+    fn test_call_expr() {
+        check_assist(
+            inline_local_varialbe,
+            "
+fn foo() {
+    let a<|> = bar(10 + 1);
+    let b = a * 10;
+    let c = a as usize;
+}",
+            "
+fn foo() {
+    <|>let b = bar(10 + 1) * 10;
+    let c = bar(10 + 1) as usize;
+}",
+        );
+    }
+
+    #[test]
+    fn test_index_expr() {
+        check_assist(
+            inline_local_varialbe,
+            "
+fn foo() {
+    let x = vec![1, 2, 3];
+    let a<|> = x[0];
+    let b = a * 10;
+    let c = a as usize;
+}",
+            "
+fn foo() {
+    let x = vec![1, 2, 3];
+    <|>let b = x[0] * 10;
+    let c = x[0] as usize;
+}",
+        );
+    }
+
+    #[test]
+    fn test_method_call_expr() {
+        check_assist(
+            inline_local_varialbe,
+            "
+fn foo() {
+    let bar = vec![1];
+    let a<|> = bar.len();
+    let b = a * 10;
+    let c = a as usize;
+}",
+            "
+fn foo() {
+    let bar = vec![1];
+    <|>let b = bar.len() * 10;
+    let c = bar.len() as usize;
+}",
+        );
+    }
+
+    #[test]
+    fn test_field_expr() {
+        check_assist(
+            inline_local_varialbe,
+            "
+struct Bar {
+    foo: usize
+}
+
+fn foo() {
+    let bar = Bar { foo: 1 };
+    let a<|> = bar.foo;
+    let b = a * 10;
+    let c = a as usize;
+}",
+            "
+struct Bar {
+    foo: usize
+}
+
+fn foo() {
+    let bar = Bar { foo: 1 };
+    <|>let b = bar.foo * 10;
+    let c = bar.foo as usize;
+}",
+        );
+    }
+
+    #[test]
+    fn test_try_expr() {
+        check_assist(
+            inline_local_varialbe,
+            "
+fn foo() -> Option<usize> {
+    let bar = Some(1);
+    let a<|> = bar?;
+    let b = a * 10;
+    let c = a as usize;
+    None
+}",
+            "
+fn foo() -> Option<usize> {
+    let bar = Some(1);
+    <|>let b = bar? * 10;
+    let c = bar? as usize;
+    None
+}",
+        );
+    }
+
+    #[test]
+    fn test_ref_expr() {
+        check_assist(
+            inline_local_varialbe,
+            "
+fn foo() {
+    let bar = 10;
+    let a<|> = &bar;
+    let b = a * 10;
+}",
+            "
+fn foo() {
+    let bar = 10;
+    <|>let b = &bar * 10;
+}",
+        );
+    }
+
+    #[test]
+    fn test_tuple_expr() {
+        check_assist(
+            inline_local_varialbe,
+            "
+fn foo() {
+    let a<|> = (10, 20);
+    let b = a[0];
+}",
+            "
+fn foo() {
+    <|>let b = (10, 20)[0];
+}",
+        );
+    }
+
+    #[test]
+    fn test_array_expr() {
+        check_assist(
+            inline_local_varialbe,
+            "
+fn foo() {
+    let a<|> = [1, 2, 3];
+    let b = a.len();
+}",
+            "
+fn foo() {
+    <|>let b = [1, 2, 3].len();
+}",
+        );
+    }
+
+    #[test]
+    fn test_paren() {
+        check_assist(
+            inline_local_varialbe,
+            "
+fn foo() {
+    let a<|> = (10 + 20);
+    let b = a * 10;
+    let c = a as usize;
+}",
+            "
+fn foo() {
+    <|>let b = (10 + 20) * 10;
+    let c = (10 + 20) as usize;
+}",
+        );
+    }
+
+    #[test]
+    fn test_path_expr() {
+        check_assist(
+            inline_local_varialbe,
+            "
+fn foo() {
+    let d = 10;
+    let a<|> = d;
+    let b = a * 10;
+    let c = a as usize;
+}",
+            "
+fn foo() {
+    let d = 10;
+    <|>let b = d * 10;
+    let c = d as usize;
+}",
+        );
+    }
+
+    #[test]
+    fn test_block_expr() {
+        check_assist(
+            inline_local_varialbe,
+            "
+fn foo() {
+    let a<|> = { 10 };
+    let b = a * 10;
+    let c = a as usize;
+}",
+            "
+fn foo() {
+    <|>let b = { 10 } * 10;
+    let c = { 10 } as usize;
+}",
+        );
+    }
+
+    #[test]
+    fn test_used_in_different_expr1() {
+        check_assist(
+            inline_local_varialbe,
+            "
+fn foo() {
+    let a<|> = 10 + 20;
+    let b = a * 10;
+    let c = (a, 20);
+    let d = [a, 10];
+    let e = (a);
+}",
+            "
+fn foo() {
+    <|>let b = (10 + 20) * 10;
+    let c = (10 + 20, 20);
+    let d = [10 + 20, 10];
+    let e = (10 + 20);
+}",
+        );
+    }
+
+    #[test]
+    fn test_used_in_for_expr() {
+        check_assist(
+            inline_local_varialbe,
+            "
+fn foo() {
+    let a<|> = vec![10, 20];
+    for i in a {}
+}",
+            "
+fn foo() {
+    <|>for i in vec![10, 20] {}
+}",
+        );
+    }
+
+    #[test]
+    fn test_used_in_while_expr() {
+        check_assist(
+            inline_local_varialbe,
+            "
+fn foo() {
+    let a<|> = 1 > 0;
+    while a {}
+}",
+            "
+fn foo() {
+    <|>while 1 > 0 {}
+}",
+        );
+    }
+
+    #[test]
+    fn test_used_in_break_expr() {
+        check_assist(
+            inline_local_varialbe,
+            "
+fn foo() {
+    let a<|> = 1 + 1;
+    loop {
+        break a;
+    }
+}",
+            "
+fn foo() {
+    <|>loop {
+        break 1 + 1;
+    }
+}",
+        );
+    }
+
+    #[test]
+    fn test_used_in_return_expr() {
+        check_assist(
+            inline_local_varialbe,
+            "
+fn foo() {
+    let a<|> = 1 > 0;
+    return a;
+}",
+            "
+fn foo() {
+    <|>return 1 > 0;
+}",
+        );
+    }
+
+    #[test]
+    fn test_used_in_match_expr() {
+        check_assist(
+            inline_local_varialbe,
+            "
+fn foo() {
+    let a<|> = 1 > 0;
+    match a {}
+}",
+            "
+fn foo() {
+    <|>match 1 > 0 {}
+}",
+        );
+    }
 }
diff --git a/crates/ra_hir/src/expr.rs b/crates/ra_hir/src/expr.rs
index 2ff4139f9..946c9faf2 100644
--- a/crates/ra_hir/src/expr.rs
+++ b/crates/ra_hir/src/expr.rs
@@ -760,6 +760,7 @@ impl ExprCollector {
             ast::ExprKind::Label(_e) => self.alloc_expr(Expr::Missing, syntax_ptr),
             ast::ExprKind::IndexExpr(_e) => self.alloc_expr(Expr::Missing, syntax_ptr),
             ast::ExprKind::RangeExpr(_e) => self.alloc_expr(Expr::Missing, syntax_ptr),
+            ast::ExprKind::MacroCall(_e) => self.alloc_expr(Expr::Missing, syntax_ptr),
         }
     }
 
diff --git a/crates/ra_syntax/src/ast/generated.rs b/crates/ra_syntax/src/ast/generated.rs
index 4afe1a146..435d90116 100644
--- a/crates/ra_syntax/src/ast/generated.rs
+++ b/crates/ra_syntax/src/ast/generated.rs
@@ -715,6 +715,7 @@ pub enum ExprKind<'a> {
     RangeExpr(&'a RangeExpr),
     BinExpr(&'a BinExpr),
     Literal(&'a Literal),
+    MacroCall(&'a MacroCall),
 }
 impl<'a> From<&'a TupleExpr> for &'a Expr {
     fn from(n: &'a TupleExpr) -> &'a Expr {
@@ -851,6 +852,11 @@ impl<'a> From<&'a Literal> for &'a Expr {
         Expr::cast(&n.syntax).unwrap()
     }
 }
+impl<'a> From<&'a MacroCall> for &'a Expr {
+    fn from(n: &'a MacroCall) -> &'a Expr {
+        Expr::cast(&n.syntax).unwrap()
+    }
+}
 
 
 impl AstNode for Expr {
@@ -882,7 +888,8 @@ impl AstNode for Expr {
             | PREFIX_EXPR
             | RANGE_EXPR
             | BIN_EXPR
-            | LITERAL => Some(Expr::from_repr(syntax.into_repr())),
+            | LITERAL
+            | MACRO_CALL => Some(Expr::from_repr(syntax.into_repr())),
             _ => None,
         }
     }
@@ -924,6 +931,7 @@ impl Expr {
             RANGE_EXPR => ExprKind::RangeExpr(RangeExpr::cast(&self.syntax).unwrap()),
             BIN_EXPR => ExprKind::BinExpr(BinExpr::cast(&self.syntax).unwrap()),
             LITERAL => ExprKind::Literal(Literal::cast(&self.syntax).unwrap()),
+            MACRO_CALL => ExprKind::MacroCall(MacroCall::cast(&self.syntax).unwrap()),
             _ => unreachable!(),
         }
     }
diff --git a/crates/ra_syntax/src/grammar.ron b/crates/ra_syntax/src/grammar.ron
index 6d7a5a1cb..3d97bea7f 100644
--- a/crates/ra_syntax/src/grammar.ron
+++ b/crates/ra_syntax/src/grammar.ron
@@ -494,6 +494,7 @@ Grammar(
                 "RangeExpr",
                 "BinExpr",
                 "Literal",
+                "MacroCall",
             ],
         ),