From 4ca51cfbcfe75b27c21bc269780992f55b4c6d84 Mon Sep 17 00:00:00 2001 From: gfreezy Date: Tue, 26 Mar 2019 01:02:06 +0800 Subject: intelligently add parens when inlining local varaibles --- crates/ra_assists/src/assist_ctx.rs | 4 + crates/ra_assists/src/inline_local_variable.rs | 431 ++++++++++++++++++++++--- crates/ra_hir/src/expr.rs | 1 + crates/ra_syntax/src/ast/generated.rs | 10 +- crates/ra_syntax/src/grammar.ron | 1 + 5 files changed, 402 insertions(+), 45 deletions(-) (limited to 'crates') 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) -> 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) -> 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); }", ); } @@ -294,6 +317,326 @@ fn foo() { fn foo() { let mut a<|> = 1 + 1; a + 1; +}", + ); + } + + #[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 { + let bar = Some(1); + let a<|> = bar?; + let b = a * 10; + let c = a as usize; + None +}", + " +fn foo() -> Option { + 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", ], ), -- cgit v1.2.3