From 73bef854ab854ab1a289944966444453e6f4aadf Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 29 Apr 2020 10:38:18 +0200 Subject: Move shared assist code to utils --- .../src/handlers/replace_let_with_if_let.rs | 16 ++----- .../src/handlers/replace_unwrap_with_match.rs | 52 ++++++++-------------- crates/ra_assists/src/utils.rs | 15 ++++++- 3 files changed, 35 insertions(+), 48 deletions(-) (limited to 'crates/ra_assists/src') diff --git a/crates/ra_assists/src/handlers/replace_let_with_if_let.rs b/crates/ra_assists/src/handlers/replace_let_with_if_let.rs index bdbaae389..fb134f677 100644 --- a/crates/ra_assists/src/handlers/replace_let_with_if_let.rs +++ b/crates/ra_assists/src/handlers/replace_let_with_if_let.rs @@ -1,6 +1,5 @@ use std::iter::once; -use hir::Adt; use ra_syntax::{ ast::{ self, @@ -12,6 +11,7 @@ use ra_syntax::{ use crate::{ assist_ctx::{Assist, AssistCtx}, + utils::happy_try_variant, AssistId, }; @@ -45,20 +45,10 @@ pub(crate) fn replace_let_with_if_let(ctx: AssistCtx) -> Option { let init = let_stmt.initializer()?; let original_pat = let_stmt.pat()?; let ty = ctx.sema.type_of_expr(&init)?; - let enum_ = match ty.as_adt() { - Some(Adt::Enum(it)) => it, - _ => return None, - }; - let happy_case = - [("Result", "Ok"), ("Option", "Some")].iter().find_map(|(known_type, happy_case)| { - if &enum_.name(ctx.db).to_string() == known_type { - return Some(happy_case); - } - None - }); + let happy_variant = happy_try_variant(ctx.sema, &ty); ctx.add_assist(AssistId("replace_let_with_if_let"), "Replace with if-let", |edit| { - let with_placeholder: ast::Pat = match happy_case { + let with_placeholder: ast::Pat = match happy_variant { None => make::placeholder_pat().into(), Some(var_name) => make::tuple_struct_pat( make::path_unqualified(make::path_segment(make::name_ref(var_name))), diff --git a/crates/ra_assists/src/handlers/replace_unwrap_with_match.rs b/crates/ra_assists/src/handlers/replace_unwrap_with_match.rs index 62cb7a763..89211b44a 100644 --- a/crates/ra_assists/src/handlers/replace_unwrap_with_match.rs +++ b/crates/ra_assists/src/handlers/replace_unwrap_with_match.rs @@ -1,12 +1,11 @@ use std::iter; use ra_syntax::{ - ast::{self, make}, + ast::{self, edit::IndentLevel, make}, AstNode, }; -use crate::{Assist, AssistCtx, AssistId}; -use ast::edit::IndentLevel; +use crate::{utils::happy_try_variant, Assist, AssistCtx, AssistId}; // Assist: replace_unwrap_with_match // @@ -38,42 +37,27 @@ pub(crate) fn replace_unwrap_with_match(ctx: AssistCtx) -> Option { } let caller = method_call.expr()?; let ty = ctx.sema.type_of_expr(&caller)?; + let happy_variant = happy_try_variant(ctx.sema, &ty)?; - let type_name = ty.as_adt()?.name(ctx.sema.db).to_string(); + ctx.add_assist(AssistId("replace_unwrap_with_match"), "Replace unwrap with match", |edit| { + let ok_path = make::path_unqualified(make::path_segment(make::name_ref(happy_variant))); + let it = make::bind_pat(make::name("a")).into(); + let ok_tuple = make::tuple_struct_pat(ok_path, iter::once(it)).into(); - for (unwrap_type, variant_name) in [("Result", "Ok"), ("Option", "Some")].iter() { - if &type_name == unwrap_type { - return ctx.add_assist( - AssistId("replace_unwrap_with_match"), - "Replace unwrap with match", - |edit| { - let ok_path = - make::path_unqualified(make::path_segment(make::name_ref(variant_name))); - let it = make::bind_pat(make::name("a")).into(); - let ok_tuple = make::tuple_struct_pat(ok_path, iter::once(it)).into(); + let bind_path = make::path_unqualified(make::path_segment(make::name_ref("a"))); + let ok_arm = make::match_arm(iter::once(ok_tuple), make::expr_path(bind_path)); - let bind_path = make::path_unqualified(make::path_segment(make::name_ref("a"))); - let ok_arm = make::match_arm(iter::once(ok_tuple), make::expr_path(bind_path)); + let unreachable_call = make::unreachable_macro_call().into(); + let err_arm = make::match_arm(iter::once(make::placeholder_pat().into()), unreachable_call); - let unreachable_call = make::unreachable_macro_call().into(); - let err_arm = make::match_arm( - iter::once(make::placeholder_pat().into()), - unreachable_call, - ); + let match_arm_list = make::match_arm_list(vec![ok_arm, err_arm]); + let match_expr = make::expr_match(caller.clone(), match_arm_list); + let match_expr = IndentLevel::from_node(method_call.syntax()).increase_indent(match_expr); - let match_arm_list = make::match_arm_list(vec![ok_arm, err_arm]); - let match_expr = make::expr_match(caller.clone(), match_arm_list); - let match_expr = - IndentLevel::from_node(method_call.syntax()).increase_indent(match_expr); - - edit.target(method_call.syntax().text_range()); - edit.set_cursor(caller.syntax().text_range().start()); - edit.replace_ast::(method_call.into(), match_expr); - }, - ); - } - } - None + edit.target(method_call.syntax().text_range()); + edit.set_cursor(caller.syntax().text_range().start()); + edit.replace_ast::(method_call.into(), match_expr); + }) } #[cfg(test)] diff --git a/crates/ra_assists/src/utils.rs b/crates/ra_assists/src/utils.rs index 3d6c59bda..316c8b20f 100644 --- a/crates/ra_assists/src/utils.rs +++ b/crates/ra_assists/src/utils.rs @@ -1,7 +1,7 @@ //! Assorted functions shared by several assists. pub(crate) mod insert_use; -use hir::Semantics; +use hir::{Adt, Semantics, Type}; use ra_ide_db::RootDatabase; use ra_syntax::{ ast::{self, make, NameOwner}, @@ -99,3 +99,16 @@ fn invert_special_case(expr: &ast::Expr) -> Option { _ => None, } } + +pub(crate) fn happy_try_variant(sema: &Semantics, ty: &Type) -> Option<&'static str> { + let enum_ = match ty.as_adt() { + Some(Adt::Enum(it)) => it, + _ => return None, + }; + [("Result", "Ok"), ("Option", "Some")].iter().find_map(|(known_type, happy_case)| { + if &enum_.name(sema.db).to_string() == known_type { + return Some(*happy_case); + } + None + }) +} -- cgit v1.2.3 From 7c3c289dab46d1dbe9196549c81307f291e631f7 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 29 Apr 2020 11:57:06 +0200 Subject: Use specific pattern when translating if-let-else to match We *probably* should actually use the same machinery here, as we do for fill match arms, but just special-casing options and results seems to be a good first step. --- .../src/handlers/replace_if_let_with_match.rs | 78 ++++++++++++++++++++-- .../src/handlers/replace_let_with_if_let.rs | 4 +- .../src/handlers/replace_unwrap_with_match.rs | 4 +- crates/ra_assists/src/utils.rs | 57 +++++++++++++--- 4 files changed, 123 insertions(+), 20 deletions(-) (limited to 'crates/ra_assists/src') diff --git a/crates/ra_assists/src/handlers/replace_if_let_with_match.rs b/crates/ra_assists/src/handlers/replace_if_let_with_match.rs index 0a0a88f3d..9841f6980 100644 --- a/crates/ra_assists/src/handlers/replace_if_let_with_match.rs +++ b/crates/ra_assists/src/handlers/replace_if_let_with_match.rs @@ -1,11 +1,10 @@ use ra_fmt::unwrap_trivial_block; use ra_syntax::{ - ast::{self, make}, + ast::{self, edit::IndentLevel, make}, AstNode, }; -use crate::{Assist, AssistCtx, AssistId}; -use ast::edit::IndentLevel; +use crate::{utils::TryEnum, Assist, AssistCtx, AssistId}; // Assist: replace_if_let_with_match // @@ -44,15 +43,21 @@ pub(crate) fn replace_if_let_with_match(ctx: AssistCtx) -> Option { ast::ElseBranch::IfExpr(_) => return None, }; - ctx.add_assist(AssistId("replace_if_let_with_match"), "Replace with match", |edit| { + let sema = ctx.sema; + ctx.add_assist(AssistId("replace_if_let_with_match"), "Replace with match", move |edit| { let match_expr = { let then_arm = { let then_expr = unwrap_trivial_block(then_block); - make::match_arm(vec![pat], then_expr) + make::match_arm(vec![pat.clone()], then_expr) }; let else_arm = { + let pattern = sema + .type_of_pat(&pat) + .and_then(|ty| TryEnum::from_ty(sema, &ty)) + .map(|it| it.sad_pattern()) + .unwrap_or_else(|| make::placeholder_pat().into()); let else_expr = unwrap_trivial_block(else_block); - make::match_arm(vec![make::placeholder_pat().into()], else_expr) + make::match_arm(vec![pattern], else_expr) }; make::expr_match(expr, make::match_arm_list(vec![then_arm, else_arm])) }; @@ -68,6 +73,7 @@ pub(crate) fn replace_if_let_with_match(ctx: AssistCtx) -> Option { #[cfg(test)] mod tests { use super::*; + use crate::helpers::{check_assist, check_assist_target}; #[test] @@ -145,4 +151,64 @@ impl VariantData { }", ); } + + #[test] + fn special_case_option() { + check_assist( + replace_if_let_with_match, + r#" +enum Option { Some(T), None } +use Option::*; + +fn foo(x: Option) { + <|>if let Some(x) = x { + println!("{}", x) + } else { + println!("none") + } +} + "#, + r#" +enum Option { Some(T), None } +use Option::*; + +fn foo(x: Option) { + <|>match x { + Some(x) => println!("{}", x), + None => println!("none"), + } +} + "#, + ); + } + + #[test] + fn special_case_result() { + check_assist( + replace_if_let_with_match, + r#" +enum Result { Ok(T), Err(E) } +use Result::*; + +fn foo(x: Result) { + <|>if let Ok(x) = x { + println!("{}", x) + } else { + println!("none") + } +} + "#, + r#" +enum Result { Ok(T), Err(E) } +use Result::*; + +fn foo(x: Result) { + <|>match x { + Ok(x) => println!("{}", x), + Err(_) => println!("none"), + } +} + "#, + ); + } } diff --git a/crates/ra_assists/src/handlers/replace_let_with_if_let.rs b/crates/ra_assists/src/handlers/replace_let_with_if_let.rs index fb134f677..0cf23b754 100644 --- a/crates/ra_assists/src/handlers/replace_let_with_if_let.rs +++ b/crates/ra_assists/src/handlers/replace_let_with_if_let.rs @@ -11,7 +11,7 @@ use ra_syntax::{ use crate::{ assist_ctx::{Assist, AssistCtx}, - utils::happy_try_variant, + utils::TryEnum, AssistId, }; @@ -45,7 +45,7 @@ pub(crate) fn replace_let_with_if_let(ctx: AssistCtx) -> Option { let init = let_stmt.initializer()?; let original_pat = let_stmt.pat()?; let ty = ctx.sema.type_of_expr(&init)?; - let happy_variant = happy_try_variant(ctx.sema, &ty); + let happy_variant = TryEnum::from_ty(ctx.sema, &ty).map(|it| it.happy_case()); ctx.add_assist(AssistId("replace_let_with_if_let"), "Replace with if-let", |edit| { let with_placeholder: ast::Pat = match happy_variant { diff --git a/crates/ra_assists/src/handlers/replace_unwrap_with_match.rs b/crates/ra_assists/src/handlers/replace_unwrap_with_match.rs index 89211b44a..62d4ea522 100644 --- a/crates/ra_assists/src/handlers/replace_unwrap_with_match.rs +++ b/crates/ra_assists/src/handlers/replace_unwrap_with_match.rs @@ -5,7 +5,7 @@ use ra_syntax::{ AstNode, }; -use crate::{utils::happy_try_variant, Assist, AssistCtx, AssistId}; +use crate::{utils::TryEnum, Assist, AssistCtx, AssistId}; // Assist: replace_unwrap_with_match // @@ -37,7 +37,7 @@ pub(crate) fn replace_unwrap_with_match(ctx: AssistCtx) -> Option { } let caller = method_call.expr()?; let ty = ctx.sema.type_of_expr(&caller)?; - let happy_variant = happy_try_variant(ctx.sema, &ty)?; + let happy_variant = TryEnum::from_ty(ctx.sema, &ty)?.happy_case(); ctx.add_assist(AssistId("replace_unwrap_with_match"), "Replace unwrap with match", |edit| { let ok_path = make::path_unqualified(make::path_segment(make::name_ref(happy_variant))); diff --git a/crates/ra_assists/src/utils.rs b/crates/ra_assists/src/utils.rs index 316c8b20f..61f8bd1dc 100644 --- a/crates/ra_assists/src/utils.rs +++ b/crates/ra_assists/src/utils.rs @@ -1,6 +1,8 @@ //! Assorted functions shared by several assists. pub(crate) mod insert_use; +use std::iter; + use hir::{Adt, Semantics, Type}; use ra_ide_db::RootDatabase; use ra_syntax::{ @@ -100,15 +102,50 @@ fn invert_special_case(expr: &ast::Expr) -> Option { } } -pub(crate) fn happy_try_variant(sema: &Semantics, ty: &Type) -> Option<&'static str> { - let enum_ = match ty.as_adt() { - Some(Adt::Enum(it)) => it, - _ => return None, - }; - [("Result", "Ok"), ("Option", "Some")].iter().find_map(|(known_type, happy_case)| { - if &enum_.name(sema.db).to_string() == known_type { - return Some(*happy_case); +#[derive(Clone, Copy)] +pub(crate) enum TryEnum { + Result, + Option, +} + +impl TryEnum { + const ALL: [TryEnum; 2] = [TryEnum::Option, TryEnum::Result]; + + pub(crate) fn from_ty(sema: &Semantics, ty: &Type) -> Option { + let enum_ = match ty.as_adt() { + Some(Adt::Enum(it)) => it, + _ => return None, + }; + TryEnum::ALL.iter().find_map(|&var| { + if &enum_.name(sema.db).to_string() == var.type_name() { + return Some(var); + } + None + }) + } + + pub(crate) fn happy_case(self) -> &'static str { + match self { + TryEnum::Result => "Ok", + TryEnum::Option => "Some", } - None - }) + } + + pub(crate) fn sad_pattern(self) -> ast::Pat { + match self { + TryEnum::Result => make::tuple_struct_pat( + make::path_unqualified(make::path_segment(make::name_ref("Err"))), + iter::once(make::placeholder_pat().into()), + ) + .into(), + TryEnum::Option => make::bind_pat(make::name("None")).into(), + } + } + + fn type_name(self) -> &'static str { + match self { + TryEnum::Result => "Result", + TryEnum::Option => "Option", + } + } } -- cgit v1.2.3 From 76733f0cd456005295e60da8c45d74c8c48f177c Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Wed, 29 Apr 2020 13:52:55 +0200 Subject: Add unwrap block assist #4156 Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- crates/ra_assists/src/doc_tests/generated.rs | 19 ++ crates/ra_assists/src/handlers/unwrap_block.rs | 435 +++++++++++++++++++++++++ crates/ra_assists/src/lib.rs | 2 + 3 files changed, 456 insertions(+) create mode 100644 crates/ra_assists/src/handlers/unwrap_block.rs (limited to 'crates/ra_assists/src') diff --git a/crates/ra_assists/src/doc_tests/generated.rs b/crates/ra_assists/src/doc_tests/generated.rs index e4fa9ee36..6fe95da6a 100644 --- a/crates/ra_assists/src/doc_tests/generated.rs +++ b/crates/ra_assists/src/doc_tests/generated.rs @@ -726,3 +726,22 @@ use std::{collections::HashMap}; "#####, ) } + +#[test] +fn doctest_unwrap_block() { + check( + "unwrap_block", + r#####" +fn foo() { + if true {<|> + println!("foo"); + } +} +"#####, + r#####" +fn foo() { + <|>println!("foo"); +} +"#####, + ) +} diff --git a/crates/ra_assists/src/handlers/unwrap_block.rs b/crates/ra_assists/src/handlers/unwrap_block.rs new file mode 100644 index 000000000..b98601f1c --- /dev/null +++ b/crates/ra_assists/src/handlers/unwrap_block.rs @@ -0,0 +1,435 @@ +use crate::{Assist, AssistCtx, AssistId}; + +use ast::LoopBodyOwner; +use ra_fmt::unwrap_trivial_block; +use ra_syntax::{ast, AstNode}; + +// Assist: unwrap_block +// +// Removes the `mut` keyword. +// +// ``` +// fn foo() { +// if true {<|> +// println!("foo"); +// } +// } +// ``` +// -> +// ``` +// fn foo() { +// <|>println!("foo"); +// } +// ``` +pub(crate) fn unwrap_block(ctx: AssistCtx) -> Option { + let res = if let Some(if_expr) = ctx.find_node_at_offset::() { + // if expression + let mut expr_to_unwrap: Option = None; + for block_expr in if_expr.blocks() { + if let Some(block) = block_expr.block() { + let cursor_in_range = + block.l_curly_token()?.text_range().contains_range(ctx.frange.range); + + if cursor_in_range { + let exprto = unwrap_trivial_block(block_expr); + expr_to_unwrap = Some(exprto); + break; + } + } + } + let expr_to_unwrap = expr_to_unwrap?; + // Find if we are in a else if block + let ancestor = ctx + .sema + .ancestors_with_macros(if_expr.syntax().clone()) + .skip(1) + .find_map(ast::IfExpr::cast); + + if let Some(ancestor) = ancestor { + Some((ast::Expr::IfExpr(ancestor), expr_to_unwrap)) + } else { + Some((ast::Expr::IfExpr(if_expr), expr_to_unwrap)) + } + } else if let Some(for_expr) = ctx.find_node_at_offset::() { + // for expression + let block_expr = for_expr.loop_body()?; + let block = block_expr.block()?; + let cursor_in_range = block.l_curly_token()?.text_range().contains_range(ctx.frange.range); + + if cursor_in_range { + let expr_to_unwrap = unwrap_trivial_block(block_expr); + + Some((ast::Expr::ForExpr(for_expr), expr_to_unwrap)) + } else { + None + } + } else if let Some(while_expr) = ctx.find_node_at_offset::() { + // while expression + let block_expr = while_expr.loop_body()?; + let block = block_expr.block()?; + let cursor_in_range = block.l_curly_token()?.text_range().contains_range(ctx.frange.range); + + if cursor_in_range { + let expr_to_unwrap = unwrap_trivial_block(block_expr); + + Some((ast::Expr::WhileExpr(while_expr), expr_to_unwrap)) + } else { + None + } + } else if let Some(loop_expr) = ctx.find_node_at_offset::() { + // loop expression + let block_expr = loop_expr.loop_body()?; + let block = block_expr.block()?; + let cursor_in_range = block.l_curly_token()?.text_range().contains_range(ctx.frange.range); + + if cursor_in_range { + let expr_to_unwrap = unwrap_trivial_block(block_expr); + + Some((ast::Expr::LoopExpr(loop_expr), expr_to_unwrap)) + } else { + None + } + } else { + None + }; + + let (expr, expr_to_unwrap) = res?; + ctx.add_assist(AssistId("unwrap_block"), "Unwrap block", |edit| { + edit.set_cursor(expr.syntax().text_range().start()); + edit.target(expr_to_unwrap.syntax().text_range()); + + let pat_start: &[_] = &[' ', '{', '\n']; + let expr_to_unwrap = expr_to_unwrap.to_string(); + let expr_string = expr_to_unwrap.trim_start_matches(pat_start); + let mut expr_string_lines: Vec<&str> = expr_string.lines().collect(); + expr_string_lines.pop(); // Delete last line + + let expr_string = expr_string_lines + .into_iter() + .map(|line| line.replacen(" ", "", 1)) // Delete indentation + .collect::>() + .join("\n"); + + edit.replace(expr.syntax().text_range(), expr_string); + }) +} + +#[cfg(test)] +mod tests { + use crate::helpers::{check_assist, check_assist_not_applicable}; + + use super::*; + + #[test] + fn simple_if() { + check_assist( + unwrap_block, + r#" + fn main() { + bar(); + if true {<|> + foo(); + + //comment + bar(); + } else { + println!("bar"); + } + } + "#, + r#" + fn main() { + bar(); + <|>foo(); + + //comment + bar(); + } + "#, + ); + } + + #[test] + fn simple_if_else() { + check_assist( + unwrap_block, + r#" + fn main() { + bar(); + if true { + foo(); + + //comment + bar(); + } else {<|> + println!("bar"); + } + } + "#, + r#" + fn main() { + bar(); + <|>println!("bar"); + } + "#, + ); + } + + #[test] + fn simple_if_else_if() { + check_assist( + unwrap_block, + r#" + fn main() { + //bar(); + if true { + println!("true"); + + //comment + //bar(); + } else if false {<|> + println!("bar"); + } else { + println!("foo"); + } + } + "#, + r#" + fn main() { + //bar(); + <|>println!("bar"); + } + "#, + ); + } + + #[test] + fn simple_if_bad_cursor_position() { + check_assist_not_applicable( + unwrap_block, + r#" + fn main() { + bar();<|> + if true { + foo(); + + //comment + bar(); + } else { + println!("bar"); + } + } + "#, + ); + } + + #[test] + fn issue_example_with_if() { + check_assist( + unwrap_block, + r#" + fn complete_enum_variants(acc: &mut Completions, ctx: &CompletionContext, ty: &Type) { + if let Some(ty) = &ctx.expected_type {<|> + if let Some(Adt::Enum(enum_data)) = ty.as_adt() { + let variants = enum_data.variants(ctx.db); + + let module = if let Some(module) = ctx.scope().module() { + // Compute path from the completion site if available. + module + } else { + // Otherwise fall back to the enum's definition site. + enum_data.module(ctx.db) + }; + + for variant in variants { + if let Some(path) = module.find_use_path(ctx.db, ModuleDef::from(variant)) { + // Variants with trivial paths are already added by the existing completion logic, + // so we should avoid adding these twice + if path.segments.len() > 1 { + acc.add_enum_variant(ctx, variant, Some(path.to_string())); + } + } + } + } + } + } + "#, + r#" + fn complete_enum_variants(acc: &mut Completions, ctx: &CompletionContext, ty: &Type) { + <|>if let Some(Adt::Enum(enum_data)) = ty.as_adt() { + let variants = enum_data.variants(ctx.db); + + let module = if let Some(module) = ctx.scope().module() { + // Compute path from the completion site if available. + module + } else { + // Otherwise fall back to the enum's definition site. + enum_data.module(ctx.db) + }; + + for variant in variants { + if let Some(path) = module.find_use_path(ctx.db, ModuleDef::from(variant)) { + // Variants with trivial paths are already added by the existing completion logic, + // so we should avoid adding these twice + if path.segments.len() > 1 { + acc.add_enum_variant(ctx, variant, Some(path.to_string())); + } + } + } + } + } + "#, + ); + } + + #[test] + fn simple_for() { + check_assist( + unwrap_block, + r#" + fn main() { + for i in 0..5 {<|> + if true { + foo(); + + //comment + bar(); + } else { + println!("bar"); + } + } + } + "#, + r#" + fn main() { + <|>if true { + foo(); + + //comment + bar(); + } else { + println!("bar"); + } + } + "#, + ); + } + + #[test] + fn simple_if_in_for() { + check_assist( + unwrap_block, + r#" + fn main() { + for i in 0..5 { + if true {<|> + foo(); + + //comment + bar(); + } else { + println!("bar"); + } + } + } + "#, + r#" + fn main() { + for i in 0..5 { + <|>foo(); + + //comment + bar(); + } + } + "#, + ); + } + + #[test] + fn simple_loop() { + check_assist( + unwrap_block, + r#" + fn main() { + loop {<|> + if true { + foo(); + + //comment + bar(); + } else { + println!("bar"); + } + } + } + "#, + r#" + fn main() { + <|>if true { + foo(); + + //comment + bar(); + } else { + println!("bar"); + } + } + "#, + ); + } + + #[test] + fn simple_while() { + check_assist( + unwrap_block, + r#" + fn main() { + while true {<|> + if true { + foo(); + + //comment + bar(); + } else { + println!("bar"); + } + } + } + "#, + r#" + fn main() { + <|>if true { + foo(); + + //comment + bar(); + } else { + println!("bar"); + } + } + "#, + ); + } + + #[test] + fn simple_if_in_while_bad_cursor_position() { + check_assist_not_applicable( + unwrap_block, + r#" + fn main() { + while true { + if true { + foo();<|> + + //comment + bar(); + } else { + println!("bar"); + } + } + } + "#, + ); + } +} diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index 64bd87afb..c5df86600 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -143,6 +143,7 @@ mod handlers { mod split_import; mod add_from_impl_for_enum; mod reorder_fields; + mod unwrap_block; pub(crate) fn all() -> &'static [AssistHandler] { &[ @@ -181,6 +182,7 @@ mod handlers { replace_unwrap_with_match::replace_unwrap_with_match, split_import::split_import, add_from_impl_for_enum::add_from_impl_for_enum, + unwrap_block::unwrap_block, // These are manually sorted for better priorities add_missing_impl_members::add_missing_impl_members, add_missing_impl_members::add_missing_default_members, -- cgit v1.2.3 From b4dd4752570d5f1ba24f7ffda69be4b1c935cd04 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 29 Apr 2020 14:49:54 +0200 Subject: More principled approach for finding From trait --- .../src/handlers/add_from_impl_for_enum.rs | 70 +++++++++++----------- crates/ra_assists/src/marks.rs | 1 + crates/ra_assists/src/utils.rs | 60 ++++++++++++++++++- 3 files changed, 94 insertions(+), 37 deletions(-) (limited to 'crates/ra_assists/src') diff --git a/crates/ra_assists/src/handlers/add_from_impl_for_enum.rs b/crates/ra_assists/src/handlers/add_from_impl_for_enum.rs index 03806724a..49deb6701 100644 --- a/crates/ra_assists/src/handlers/add_from_impl_for_enum.rs +++ b/crates/ra_assists/src/handlers/add_from_impl_for_enum.rs @@ -1,11 +1,12 @@ +use ra_ide_db::RootDatabase; use ra_syntax::{ ast::{self, AstNode, NameOwner}, TextSize, }; use stdx::format_to; -use crate::{Assist, AssistCtx, AssistId}; -use ra_ide_db::RootDatabase; +use crate::{utils::FamousDefs, Assist, AssistCtx, AssistId}; +use test_utils::tested_by; // Assist add_from_impl_for_enum // @@ -41,7 +42,8 @@ pub(crate) fn add_from_impl_for_enum(ctx: AssistCtx) -> Option { _ => return None, }; - if already_has_from_impl(ctx.sema, &variant) { + if existing_from_impl(ctx.sema, &variant).is_some() { + tested_by!(test_add_from_impl_already_exists); return None; } @@ -70,41 +72,33 @@ impl From<{0}> for {1} {{ ) } -fn already_has_from_impl( +fn existing_from_impl( sema: &'_ hir::Semantics<'_, RootDatabase>, variant: &ast::EnumVariant, -) -> bool { - let scope = sema.scope(&variant.syntax()); +) -> Option<()> { + let variant = sema.to_def(variant)?; + let enum_ = variant.parent_enum(sema.db); + let krate = enum_.module(sema.db).krate(); - let from_path = ast::make::path_from_text("From"); - let from_hir_path = match hir::Path::from_ast(from_path) { - Some(p) => p, - None => return false, - }; - let from_trait = match scope.resolve_hir_path(&from_hir_path) { - Some(hir::PathResolution::Def(hir::ModuleDef::Trait(t))) => t, - _ => return false, - }; + let from_trait = FamousDefs(sema, krate).core_convert_From()?; - let e: hir::Enum = match sema.to_def(&variant.parent_enum()) { - Some(e) => e, - None => return false, - }; - let e_ty = e.ty(sema.db); + let enum_type = enum_.ty(sema.db); - let hir_enum_var: hir::EnumVariant = match sema.to_def(variant) { - Some(ev) => ev, - None => return false, - }; - let var_ty = hir_enum_var.fields(sema.db)[0].signature_ty(sema.db); + let wrapped_type = variant.fields(sema.db).get(0)?.signature_ty(sema.db); - e_ty.impls_trait(sema.db, from_trait, &[var_ty]) + if enum_type.impls_trait(sema.db, from_trait, &[wrapped_type]) { + Some(()) + } else { + None + } } #[cfg(test)] mod tests { use super::*; + use crate::helpers::{check_assist, check_assist_not_applicable}; + use test_utils::covers; #[test] fn test_add_from_impl_for_enum() { @@ -136,36 +130,40 @@ mod tests { ); } + fn check_not_applicable(ra_fixture: &str) { + let fixture = + format!("//- main.rs crate:main deps:core\n{}\n{}", ra_fixture, FamousDefs::FIXTURE); + check_assist_not_applicable(add_from_impl_for_enum, &fixture) + } + #[test] fn test_add_from_impl_no_element() { - check_assist_not_applicable(add_from_impl_for_enum, "enum A { <|>One }"); + check_not_applicable("enum A { <|>One }"); } #[test] fn test_add_from_impl_more_than_one_element_in_tuple() { - check_assist_not_applicable(add_from_impl_for_enum, "enum A { <|>One(u32, String) }"); + check_not_applicable("enum A { <|>One(u32, String) }"); } #[test] fn test_add_from_impl_struct_variant() { - check_assist_not_applicable(add_from_impl_for_enum, "enum A { <|>One { x: u32 } }"); + check_not_applicable("enum A { <|>One { x: u32 } }"); } #[test] fn test_add_from_impl_already_exists() { - check_assist_not_applicable( - add_from_impl_for_enum, - r#"enum A { <|>One(u32), } + covers!(test_add_from_impl_already_exists); + check_not_applicable( + r#" +enum A { <|>One(u32), } impl From for A { fn from(v: u32) -> Self { A::One(v) } } - -pub trait From { - fn from(T) -> Self; -}"#, +"#, ); } diff --git a/crates/ra_assists/src/marks.rs b/crates/ra_assists/src/marks.rs index 6c2a2b8b6..8d910205f 100644 --- a/crates/ra_assists/src/marks.rs +++ b/crates/ra_assists/src/marks.rs @@ -8,4 +8,5 @@ test_utils::marks![ test_not_inline_mut_variable test_not_applicable_if_variable_unused change_visibility_field_false_positive + test_add_from_impl_already_exists ]; diff --git a/crates/ra_assists/src/utils.rs b/crates/ra_assists/src/utils.rs index 61f8bd1dc..efd988697 100644 --- a/crates/ra_assists/src/utils.rs +++ b/crates/ra_assists/src/utils.rs @@ -3,7 +3,7 @@ pub(crate) mod insert_use; use std::iter; -use hir::{Adt, Semantics, Type}; +use hir::{Adt, Crate, Semantics, Trait, Type}; use ra_ide_db::RootDatabase; use ra_syntax::{ ast::{self, make, NameOwner}, @@ -149,3 +149,61 @@ impl TryEnum { } } } + +/// Helps with finding well-know things inside the standard library. This is +/// somewhat similar to the known paths infra inside hir, but it different; We +/// want to make sure that IDE specific paths don't become interesting inside +/// the compiler itself as well. +pub(crate) struct FamousDefs<'a, 'b>(pub(crate) &'a Semantics<'b, RootDatabase>, pub(crate) Crate); + +#[allow(non_snake_case)] +impl FamousDefs<'_, '_> { + #[cfg(test)] + pub(crate) const FIXTURE: &'static str = r#" +//- /libcore.rs crate:core +pub mod convert{ + pub trait From { + fn from(T) -> Self; + } +} + +pub mod prelude { pub use crate::convert::From } +#[prelude_import] +pub use prelude::*; +"#; + + pub(crate) fn core_convert_From(&self) -> Option { + self.find_trait("core:convert:From") + } + + fn find_trait(&self, path: &str) -> Option { + let db = self.0.db; + let mut path = path.split(':'); + let trait_ = path.next_back()?; + let std_crate = path.next()?; + let std_crate = self + .1 + .dependencies(db) + .into_iter() + .find(|dep| &dep.name.to_string() == std_crate)? + .krate; + + let mut module = std_crate.root_module(db)?; + for segment in path { + module = module.children(db).find_map(|child| { + let name = child.name(db)?; + if &name.to_string() == segment { + Some(child) + } else { + None + } + })?; + } + let def = + module.scope(db, None).into_iter().find(|(name, _def)| &name.to_string() == trait_)?.1; + match def { + hir::ScopeDef::ModuleDef(hir::ModuleDef::Trait(it)) => Some(it), + _ => None, + } + } +} -- cgit v1.2.3 From bbe22640b8d52354c3de3e126c9fcda5b1b174fd Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Wed, 29 Apr 2020 14:53:47 +0200 Subject: Add unwrap block assist #4156 Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- crates/ra_assists/src/doc_tests/generated.rs | 2 +- crates/ra_assists/src/handlers/unwrap_block.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'crates/ra_assists/src') diff --git a/crates/ra_assists/src/doc_tests/generated.rs b/crates/ra_assists/src/doc_tests/generated.rs index 6fe95da6a..b53b97e89 100644 --- a/crates/ra_assists/src/doc_tests/generated.rs +++ b/crates/ra_assists/src/doc_tests/generated.rs @@ -740,7 +740,7 @@ fn foo() { "#####, r#####" fn foo() { - <|>println!("foo"); + println!("foo"); } "#####, ) diff --git a/crates/ra_assists/src/handlers/unwrap_block.rs b/crates/ra_assists/src/handlers/unwrap_block.rs index b98601f1c..35d87bc9e 100644 --- a/crates/ra_assists/src/handlers/unwrap_block.rs +++ b/crates/ra_assists/src/handlers/unwrap_block.rs @@ -6,7 +6,7 @@ use ra_syntax::{ast, AstNode}; // Assist: unwrap_block // -// Removes the `mut` keyword. +// This assist removes if...else, for, while and loop control statements to just keep the body. // // ``` // fn foo() { @@ -18,7 +18,7 @@ use ra_syntax::{ast, AstNode}; // -> // ``` // fn foo() { -// <|>println!("foo"); +// println!("foo"); // } // ``` pub(crate) fn unwrap_block(ctx: AssistCtx) -> Option { -- cgit v1.2.3 From bdcf6f56589f8367c8cc82f3f4f045dcaf53748d Mon Sep 17 00:00:00 2001 From: Edwin Cheng Date: Thu, 30 Apr 2020 18:20:13 +0800 Subject: Introduce LowerCtx for path lowering --- crates/ra_assists/src/assist_ctx.rs | 9 ++++++--- crates/ra_assists/src/ast_transform.rs | 2 ++ .../ra_assists/src/handlers/replace_qualified_name_with_use.rs | 2 +- 3 files changed, 9 insertions(+), 4 deletions(-) (limited to 'crates/ra_assists/src') diff --git a/crates/ra_assists/src/assist_ctx.rs b/crates/ra_assists/src/assist_ctx.rs index 2fe7c3de3..3155a469b 100644 --- a/crates/ra_assists/src/assist_ctx.rs +++ b/crates/ra_assists/src/assist_ctx.rs @@ -1,12 +1,12 @@ //! This module defines `AssistCtx` -- the API surface that is exposed to assists. use hir::Semantics; -use ra_db::FileRange; +use ra_db::{FileRange, Upcast}; use ra_fmt::{leading_indent, reindent}; use ra_ide_db::RootDatabase; use ra_syntax::{ algo::{self, find_covering_element, find_node_at_offset}, - AstNode, SourceFile, SyntaxElement, SyntaxKind, SyntaxNode, SyntaxToken, TextRange, TextSize, - TokenAtOffset, + ast, AstNode, SourceFile, SyntaxElement, SyntaxKind, SyntaxNode, SyntaxToken, TextRange, + TextSize, TokenAtOffset, }; use ra_text_edit::TextEditBuilder; @@ -136,6 +136,9 @@ impl<'a> AssistCtx<'a> { pub(crate) fn covering_node_for_range(&self, range: TextRange) -> SyntaxElement { find_covering_element(self.source_file.syntax(), range) } + pub(crate) fn lower_path(&self, path: ast::Path) -> Option { + hir::Path::from_src(path, &hir::Hygiene::new(self.db.upcast(), self.frange.file_id.into())) + } } pub(crate) struct AssistGroup<'a> { diff --git a/crates/ra_assists/src/ast_transform.rs b/crates/ra_assists/src/ast_transform.rs index 52b4c82db..9ac65ab39 100644 --- a/crates/ra_assists/src/ast_transform.rs +++ b/crates/ra_assists/src/ast_transform.rs @@ -85,6 +85,7 @@ impl<'a> SubstituteTypeParams<'a> { ast::TypeRef::PathType(path_type) => path_type.path()?, _ => return None, }; + // FIXME: use `hir::Path::from_src` instead. let path = hir::Path::from_ast(path)?; let resolution = self.source_scope.resolve_hir_path(&path)?; match resolution { @@ -128,6 +129,7 @@ impl<'a> QualifyPaths<'a> { // don't try to qualify `Fn(Foo) -> Bar` paths, they are in prelude anyway return None; } + // FIXME: use `hir::Path::from_src` instead. let hir_path = hir::Path::from_ast(p.clone()); let resolution = self.source_scope.resolve_hir_path(&hir_path?)?; match resolution { diff --git a/crates/ra_assists/src/handlers/replace_qualified_name_with_use.rs b/crates/ra_assists/src/handlers/replace_qualified_name_with_use.rs index 2f02df303..ad59db392 100644 --- a/crates/ra_assists/src/handlers/replace_qualified_name_with_use.rs +++ b/crates/ra_assists/src/handlers/replace_qualified_name_with_use.rs @@ -27,7 +27,7 @@ pub(crate) fn replace_qualified_name_with_use(ctx: AssistCtx) -> Option return None; } - let hir_path = hir::Path::from_ast(path.clone())?; + let hir_path = ctx.lower_path(path.clone())?; let segments = collect_hir_path_segments(&hir_path)?; if segments.len() < 2 { return None; -- cgit v1.2.3 From 44f5e2048ce00b0b417be95c16bae9a9ced1a5e8 Mon Sep 17 00:00:00 2001 From: Edwin Cheng Date: Fri, 1 May 2020 19:58:47 +0800 Subject: Remove lower_path from AssistCtx to Semantic --- crates/ra_assists/src/assist_ctx.rs | 9 +++------ .../ra_assists/src/handlers/replace_qualified_name_with_use.rs | 2 +- 2 files changed, 4 insertions(+), 7 deletions(-) (limited to 'crates/ra_assists/src') diff --git a/crates/ra_assists/src/assist_ctx.rs b/crates/ra_assists/src/assist_ctx.rs index 3155a469b..2fe7c3de3 100644 --- a/crates/ra_assists/src/assist_ctx.rs +++ b/crates/ra_assists/src/assist_ctx.rs @@ -1,12 +1,12 @@ //! This module defines `AssistCtx` -- the API surface that is exposed to assists. use hir::Semantics; -use ra_db::{FileRange, Upcast}; +use ra_db::FileRange; use ra_fmt::{leading_indent, reindent}; use ra_ide_db::RootDatabase; use ra_syntax::{ algo::{self, find_covering_element, find_node_at_offset}, - ast, AstNode, SourceFile, SyntaxElement, SyntaxKind, SyntaxNode, SyntaxToken, TextRange, - TextSize, TokenAtOffset, + AstNode, SourceFile, SyntaxElement, SyntaxKind, SyntaxNode, SyntaxToken, TextRange, TextSize, + TokenAtOffset, }; use ra_text_edit::TextEditBuilder; @@ -136,9 +136,6 @@ impl<'a> AssistCtx<'a> { pub(crate) fn covering_node_for_range(&self, range: TextRange) -> SyntaxElement { find_covering_element(self.source_file.syntax(), range) } - pub(crate) fn lower_path(&self, path: ast::Path) -> Option { - hir::Path::from_src(path, &hir::Hygiene::new(self.db.upcast(), self.frange.file_id.into())) - } } pub(crate) struct AssistGroup<'a> { diff --git a/crates/ra_assists/src/handlers/replace_qualified_name_with_use.rs b/crates/ra_assists/src/handlers/replace_qualified_name_with_use.rs index ad59db392..918e8dd8d 100644 --- a/crates/ra_assists/src/handlers/replace_qualified_name_with_use.rs +++ b/crates/ra_assists/src/handlers/replace_qualified_name_with_use.rs @@ -27,7 +27,7 @@ pub(crate) fn replace_qualified_name_with_use(ctx: AssistCtx) -> Option return None; } - let hir_path = ctx.lower_path(path.clone())?; + let hir_path = ctx.sema.lower_path(&path)?; let segments = collect_hir_path_segments(&hir_path)?; if segments.len() < 2 { return None; -- cgit v1.2.3 From df7899e47a83fb5544d09d2db9405762d3ce29b7 Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Fri, 1 May 2020 16:41:29 +0200 Subject: Add unwrap block assist #4156 Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- crates/ra_assists/src/handlers/unwrap_block.rs | 69 +++++++++----------------- 1 file changed, 23 insertions(+), 46 deletions(-) (limited to 'crates/ra_assists/src') diff --git a/crates/ra_assists/src/handlers/unwrap_block.rs b/crates/ra_assists/src/handlers/unwrap_block.rs index 35d87bc9e..71d6d462b 100644 --- a/crates/ra_assists/src/handlers/unwrap_block.rs +++ b/crates/ra_assists/src/handlers/unwrap_block.rs @@ -1,8 +1,8 @@ use crate::{Assist, AssistCtx, AssistId}; -use ast::LoopBodyOwner; +use ast::{BlockExpr, Expr, LoopBodyOwner}; use ra_fmt::unwrap_trivial_block; -use ra_syntax::{ast, AstNode}; +use ra_syntax::{ast, AstNode, TextRange}; // Assist: unwrap_block // @@ -26,24 +26,14 @@ pub(crate) fn unwrap_block(ctx: AssistCtx) -> Option { // if expression let mut expr_to_unwrap: Option = None; for block_expr in if_expr.blocks() { - if let Some(block) = block_expr.block() { - let cursor_in_range = - block.l_curly_token()?.text_range().contains_range(ctx.frange.range); - - if cursor_in_range { - let exprto = unwrap_trivial_block(block_expr); - expr_to_unwrap = Some(exprto); - break; - } + if let Some(expr) = excract_expr(ctx.frange.range, block_expr) { + expr_to_unwrap = Some(expr); + break; } } let expr_to_unwrap = expr_to_unwrap?; // Find if we are in a else if block - let ancestor = ctx - .sema - .ancestors_with_macros(if_expr.syntax().clone()) - .skip(1) - .find_map(ast::IfExpr::cast); + let ancestor = if_expr.syntax().ancestors().skip(1).find_map(ast::IfExpr::cast); if let Some(ancestor) = ancestor { Some((ast::Expr::IfExpr(ancestor), expr_to_unwrap)) @@ -53,42 +43,18 @@ pub(crate) fn unwrap_block(ctx: AssistCtx) -> Option { } else if let Some(for_expr) = ctx.find_node_at_offset::() { // for expression let block_expr = for_expr.loop_body()?; - let block = block_expr.block()?; - let cursor_in_range = block.l_curly_token()?.text_range().contains_range(ctx.frange.range); - - if cursor_in_range { - let expr_to_unwrap = unwrap_trivial_block(block_expr); - - Some((ast::Expr::ForExpr(for_expr), expr_to_unwrap)) - } else { - None - } + excract_expr(ctx.frange.range, block_expr) + .map(|expr_to_unwrap| (ast::Expr::ForExpr(for_expr), expr_to_unwrap)) } else if let Some(while_expr) = ctx.find_node_at_offset::() { // while expression let block_expr = while_expr.loop_body()?; - let block = block_expr.block()?; - let cursor_in_range = block.l_curly_token()?.text_range().contains_range(ctx.frange.range); - - if cursor_in_range { - let expr_to_unwrap = unwrap_trivial_block(block_expr); - - Some((ast::Expr::WhileExpr(while_expr), expr_to_unwrap)) - } else { - None - } + excract_expr(ctx.frange.range, block_expr) + .map(|expr_to_unwrap| (ast::Expr::WhileExpr(while_expr), expr_to_unwrap)) } else if let Some(loop_expr) = ctx.find_node_at_offset::() { // loop expression let block_expr = loop_expr.loop_body()?; - let block = block_expr.block()?; - let cursor_in_range = block.l_curly_token()?.text_range().contains_range(ctx.frange.range); - - if cursor_in_range { - let expr_to_unwrap = unwrap_trivial_block(block_expr); - - Some((ast::Expr::LoopExpr(loop_expr), expr_to_unwrap)) - } else { - None - } + excract_expr(ctx.frange.range, block_expr) + .map(|expr_to_unwrap| (ast::Expr::LoopExpr(loop_expr), expr_to_unwrap)) } else { None }; @@ -114,6 +80,17 @@ pub(crate) fn unwrap_block(ctx: AssistCtx) -> Option { }) } +fn excract_expr(cursor_range: TextRange, block_expr: BlockExpr) -> Option { + let block = block_expr.block()?; + let cursor_in_range = block.l_curly_token()?.text_range().contains_range(cursor_range); + + if cursor_in_range { + Some(unwrap_trivial_block(block_expr)) + } else { + None + } +} + #[cfg(test)] mod tests { use crate::helpers::{check_assist, check_assist_not_applicable}; -- cgit v1.2.3 From 4f2134cc33f07c09fe166cec42971828843bc0ef Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 2 May 2020 01:18:19 +0200 Subject: Introduce EffectExpr --- crates/ra_assists/src/handlers/early_return.rs | 16 ++++++++-------- crates/ra_assists/src/handlers/inline_local_variable.rs | 1 + crates/ra_assists/src/handlers/introduce_variable.rs | 2 +- crates/ra_assists/src/handlers/move_guard.rs | 4 ++-- 4 files changed, 12 insertions(+), 11 deletions(-) (limited to 'crates/ra_assists/src') diff --git a/crates/ra_assists/src/handlers/early_return.rs b/crates/ra_assists/src/handlers/early_return.rs index ea6c56f8c..eede2fe91 100644 --- a/crates/ra_assists/src/handlers/early_return.rs +++ b/crates/ra_assists/src/handlers/early_return.rs @@ -2,7 +2,7 @@ use std::{iter::once, ops::RangeInclusive}; use ra_syntax::{ algo::replace_children, - ast::{self, edit::IndentLevel, make, Block, Pat::TupleStructPat}, + ast::{self, edit::IndentLevel, make}, AstNode, SyntaxKind::{FN_DEF, LOOP_EXPR, L_CURLY, R_CURLY, WHILE_EXPR, WHITESPACE}, SyntaxNode, @@ -47,7 +47,7 @@ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Option { // Check if there is an IfLet that we can handle. let if_let_pat = match cond.pat() { None => None, // No IfLet, supported. - Some(TupleStructPat(pat)) if pat.args().count() == 1 => { + Some(ast::Pat::TupleStructPat(pat)) if pat.args().count() == 1 => { let path = pat.path()?; match path.qualifier() { None => { @@ -61,9 +61,9 @@ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Option { }; let cond_expr = cond.expr()?; - let then_block = if_expr.then_branch()?.block()?; + let then_block = if_expr.then_branch()?; - let parent_block = if_expr.syntax().parent()?.ancestors().find_map(ast::Block::cast)?; + let parent_block = if_expr.syntax().parent()?.ancestors().find_map(ast::BlockExpr::cast)?; if parent_block.expr()? != if_expr.clone().into() { return None; @@ -80,7 +80,7 @@ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Option { return None; } - let parent_container = parent_block.syntax().parent()?.parent()?; + let parent_container = parent_block.syntax().parent()?; let early_expression: ast::Expr = match parent_container.kind() { WHILE_EXPR | LOOP_EXPR => make::expr_continue(), @@ -144,13 +144,13 @@ pub(crate) fn convert_to_guarded_return(ctx: AssistCtx) -> Option { } }; edit.target(if_expr.syntax().text_range()); - edit.replace_ast(parent_block, ast::Block::cast(new_block).unwrap()); + edit.replace_ast(parent_block, ast::BlockExpr::cast(new_block).unwrap()); edit.set_cursor(cursor_position); fn replace( new_expr: &SyntaxNode, - then_block: &Block, - parent_block: &Block, + then_block: &ast::BlockExpr, + parent_block: &ast::BlockExpr, if_expr: &ast::IfExpr, ) -> SyntaxNode { let then_block_items = IndentLevel::from(1).decrease_indent(then_block.clone()); diff --git a/crates/ra_assists/src/handlers/inline_local_variable.rs b/crates/ra_assists/src/handlers/inline_local_variable.rs index f5702f6e0..60ec536a7 100644 --- a/crates/ra_assists/src/handlers/inline_local_variable.rs +++ b/crates/ra_assists/src/handlers/inline_local_variable.rs @@ -89,6 +89,7 @@ pub(crate) fn inline_local_variable(ctx: AssistCtx) -> Option { | (ast::Expr::ParenExpr(_), _) | (ast::Expr::PathExpr(_), _) | (ast::Expr::BlockExpr(_), _) + | (ast::Expr::EffectExpr(_), _) | (_, ast::Expr::CallExpr(_)) | (_, ast::Expr::TupleExpr(_)) | (_, ast::Expr::ArrayExpr(_)) diff --git a/crates/ra_assists/src/handlers/introduce_variable.rs b/crates/ra_assists/src/handlers/introduce_variable.rs index eda9ac296..39c656305 100644 --- a/crates/ra_assists/src/handlers/introduce_variable.rs +++ b/crates/ra_assists/src/handlers/introduce_variable.rs @@ -111,7 +111,7 @@ fn valid_target_expr(node: SyntaxNode) -> Option { /// expression like a lambda or match arm. fn anchor_stmt(expr: ast::Expr) -> Option<(SyntaxNode, bool)> { expr.syntax().ancestors().find_map(|node| { - if let Some(expr) = node.parent().and_then(ast::Block::cast).and_then(|it| it.expr()) { + if let Some(expr) = node.parent().and_then(ast::BlockExpr::cast).and_then(|it| it.expr()) { if expr.syntax() == &node { tested_by!(test_introduce_var_last_expr); return Some((node, false)); diff --git a/crates/ra_assists/src/handlers/move_guard.rs b/crates/ra_assists/src/handlers/move_guard.rs index d5ccdd91c..b084dd9ee 100644 --- a/crates/ra_assists/src/handlers/move_guard.rs +++ b/crates/ra_assists/src/handlers/move_guard.rs @@ -113,9 +113,9 @@ pub(crate) fn move_arm_cond_to_match_guard(ctx: AssistCtx) -> Option { "Move condition to match guard", |edit| { edit.target(if_expr.syntax().text_range()); - let then_only_expr = then_block.block().and_then(|it| it.statements().next()).is_none(); + let then_only_expr = then_block.statements().next().is_none(); - match &then_block.block().and_then(|it| it.expr()) { + match &then_block.expr() { Some(then_expr) if then_only_expr => { edit.replace(if_expr.syntax().text_range(), then_expr.syntax().text()) } -- cgit v1.2.3 From b73dbbfbf2cad646eb3f8e3342a1c390a874dc53 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 2 May 2020 11:50:43 +0200 Subject: Add missing members generates indented blocks --- crates/ra_assists/src/doc_tests/generated.rs | 4 +- .../src/handlers/add_missing_impl_members.rs | 194 ++++++++++++--------- 2 files changed, 119 insertions(+), 79 deletions(-) (limited to 'crates/ra_assists/src') diff --git a/crates/ra_assists/src/doc_tests/generated.rs b/crates/ra_assists/src/doc_tests/generated.rs index e4fa9ee36..d6a34b609 100644 --- a/crates/ra_assists/src/doc_tests/generated.rs +++ b/crates/ra_assists/src/doc_tests/generated.rs @@ -180,7 +180,9 @@ trait Trait { } impl Trait for () { - fn foo(&self) -> u32 { todo!() } + fn foo(&self) -> u32 { + todo!() + } } "#####, diff --git a/crates/ra_assists/src/handlers/add_missing_impl_members.rs b/crates/ra_assists/src/handlers/add_missing_impl_members.rs index 2d6d44980..e466c9a86 100644 --- a/crates/ra_assists/src/handlers/add_missing_impl_members.rs +++ b/crates/ra_assists/src/handlers/add_missing_impl_members.rs @@ -1,6 +1,10 @@ use hir::HasSource; use ra_syntax::{ - ast::{self, edit, make, AstNode, NameOwner}, + ast::{ + self, + edit::{self, IndentLevel}, + make, AstNode, NameOwner, + }, SmolStr, }; @@ -40,7 +44,9 @@ enum AddMissingImplMembersMode { // } // // impl Trait for () { -// fn foo(&self) -> u32 { todo!() } +// fn foo(&self) -> u32 { +// todo!() +// } // // } // ``` @@ -165,7 +171,9 @@ fn add_missing_impl_members_inner( fn add_body(fn_def: ast::FnDef) -> ast::FnDef { if fn_def.body().is_none() { - fn_def.with_body(make::block_from_expr(make::expr_todo())) + let body = make::block_expr(None, Some(make::expr_todo())); + let body = IndentLevel(1).increase_indent(body); + fn_def.with_body(body) } else { fn_def } @@ -181,7 +189,7 @@ mod tests { fn test_add_missing_impl_members() { check_assist( add_missing_impl_members, - " + r#" trait Foo { type Output; @@ -197,8 +205,8 @@ struct S; impl Foo for S { fn bar(&self) {} <|> -}", - " +}"#, + r#" trait Foo { type Output; @@ -215,10 +223,14 @@ impl Foo for S { fn bar(&self) {} <|>type Output; const CONST: usize = 42; - fn foo(&self) { todo!() } - fn baz(&self) { todo!() } + fn foo(&self) { + todo!() + } + fn baz(&self) { + todo!() + } -}", +}"#, ); } @@ -226,7 +238,7 @@ impl Foo for S { fn test_copied_overriden_members() { check_assist( add_missing_impl_members, - " + r#" trait Foo { fn foo(&self); fn bar(&self) -> bool { true } @@ -238,8 +250,8 @@ struct S; impl Foo for S { fn bar(&self) {} <|> -}", - " +}"#, + r#" trait Foo { fn foo(&self); fn bar(&self) -> bool { true } @@ -250,9 +262,11 @@ struct S; impl Foo for S { fn bar(&self) {} - <|>fn foo(&self) { todo!() } + <|>fn foo(&self) { + todo!() + } -}", +}"#, ); } @@ -260,16 +274,18 @@ impl Foo for S { fn test_empty_impl_def() { check_assist( add_missing_impl_members, - " + r#" trait Foo { fn foo(&self); } struct S; -impl Foo for S { <|> }", - " +impl Foo for S { <|> }"#, + r#" trait Foo { fn foo(&self); } struct S; impl Foo for S { - <|>fn foo(&self) { todo!() } -}", + <|>fn foo(&self) { + todo!() + } +}"#, ); } @@ -277,16 +293,18 @@ impl Foo for S { fn fill_in_type_params_1() { check_assist( add_missing_impl_members, - " + r#" trait Foo { fn foo(&self, t: T) -> &T; } struct S; -impl Foo for S { <|> }", - " +impl Foo for S { <|> }"#, + r#" trait Foo { fn foo(&self, t: T) -> &T; } struct S; impl Foo for S { - <|>fn foo(&self, t: u32) -> &u32 { todo!() } -}", + <|>fn foo(&self, t: u32) -> &u32 { + todo!() + } +}"#, ); } @@ -294,16 +312,18 @@ impl Foo for S { fn fill_in_type_params_2() { check_assist( add_missing_impl_members, - " + r#" trait Foo { fn foo(&self, t: T) -> &T; } struct S; -impl Foo for S { <|> }", - " +impl Foo for S { <|> }"#, + r#" trait Foo { fn foo(&self, t: T) -> &T; } struct S; impl Foo for S { - <|>fn foo(&self, t: U) -> &U { todo!() } -}", + <|>fn foo(&self, t: U) -> &U { + todo!() + } +}"#, ); } @@ -311,16 +331,18 @@ impl Foo for S { fn test_cursor_after_empty_impl_def() { check_assist( add_missing_impl_members, - " + r#" trait Foo { fn foo(&self); } struct S; -impl Foo for S {}<|>", - " +impl Foo for S {}<|>"#, + r#" trait Foo { fn foo(&self); } struct S; impl Foo for S { - <|>fn foo(&self) { todo!() } -}", + <|>fn foo(&self) { + todo!() + } +}"#, ) } @@ -328,22 +350,24 @@ impl Foo for S { fn test_qualify_path_1() { check_assist( add_missing_impl_members, - " + r#" mod foo { pub struct Bar; trait Foo { fn foo(&self, bar: Bar); } } struct S; -impl foo::Foo for S { <|> }", - " +impl foo::Foo for S { <|> }"#, + r#" mod foo { pub struct Bar; trait Foo { fn foo(&self, bar: Bar); } } struct S; impl foo::Foo for S { - <|>fn foo(&self, bar: foo::Bar) { todo!() } -}", + <|>fn foo(&self, bar: foo::Bar) { + todo!() + } +}"#, ); } @@ -351,22 +375,24 @@ impl foo::Foo for S { fn test_qualify_path_generic() { check_assist( add_missing_impl_members, - " + r#" mod foo { pub struct Bar; trait Foo { fn foo(&self, bar: Bar); } } struct S; -impl foo::Foo for S { <|> }", - " +impl foo::Foo for S { <|> }"#, + r#" mod foo { pub struct Bar; trait Foo { fn foo(&self, bar: Bar); } } struct S; impl foo::Foo for S { - <|>fn foo(&self, bar: foo::Bar) { todo!() } -}", + <|>fn foo(&self, bar: foo::Bar) { + todo!() + } +}"#, ); } @@ -374,22 +400,24 @@ impl foo::Foo for S { fn test_qualify_path_and_substitute_param() { check_assist( add_missing_impl_members, - " + r#" mod foo { pub struct Bar; trait Foo { fn foo(&self, bar: Bar); } } struct S; -impl foo::Foo for S { <|> }", - " +impl foo::Foo for S { <|> }"#, + r#" mod foo { pub struct Bar; trait Foo { fn foo(&self, bar: Bar); } } struct S; impl foo::Foo for S { - <|>fn foo(&self, bar: foo::Bar) { todo!() } -}", + <|>fn foo(&self, bar: foo::Bar) { + todo!() + } +}"#, ); } @@ -398,15 +426,15 @@ impl foo::Foo for S { // when substituting params, the substituted param should not be qualified! check_assist( add_missing_impl_members, - " + r#" mod foo { trait Foo { fn foo(&self, bar: T); } pub struct Param; } struct Param; struct S; -impl foo::Foo for S { <|> }", - " +impl foo::Foo for S { <|> }"#, + r#" mod foo { trait Foo { fn foo(&self, bar: T); } pub struct Param; @@ -414,8 +442,10 @@ mod foo { struct Param; struct S; impl foo::Foo for S { - <|>fn foo(&self, bar: Param) { todo!() } -}", + <|>fn foo(&self, bar: Param) { + todo!() + } +}"#, ); } @@ -423,15 +453,15 @@ impl foo::Foo for S { fn test_qualify_path_associated_item() { check_assist( add_missing_impl_members, - " + r#" mod foo { pub struct Bar; impl Bar { type Assoc = u32; } trait Foo { fn foo(&self, bar: Bar::Assoc); } } struct S; -impl foo::Foo for S { <|> }", - " +impl foo::Foo for S { <|> }"#, + r#" mod foo { pub struct Bar; impl Bar { type Assoc = u32; } @@ -439,8 +469,10 @@ mod foo { } struct S; impl foo::Foo for S { - <|>fn foo(&self, bar: foo::Bar::Assoc) { todo!() } -}", + <|>fn foo(&self, bar: foo::Bar::Assoc) { + todo!() + } +}"#, ); } @@ -448,15 +480,15 @@ impl foo::Foo for S { fn test_qualify_path_nested() { check_assist( add_missing_impl_members, - " + r#" mod foo { pub struct Bar; pub struct Baz; trait Foo { fn foo(&self, bar: Bar); } } struct S; -impl foo::Foo for S { <|> }", - " +impl foo::Foo for S { <|> }"#, + r#" mod foo { pub struct Bar; pub struct Baz; @@ -464,8 +496,10 @@ mod foo { } struct S; impl foo::Foo for S { - <|>fn foo(&self, bar: foo::Bar) { todo!() } -}", + <|>fn foo(&self, bar: foo::Bar) { + todo!() + } +}"#, ); } @@ -473,22 +507,24 @@ impl foo::Foo for S { fn test_qualify_path_fn_trait_notation() { check_assist( add_missing_impl_members, - " + r#" mod foo { pub trait Fn { type Output; } trait Foo { fn foo(&self, bar: dyn Fn(u32) -> i32); } } struct S; -impl foo::Foo for S { <|> }", - " +impl foo::Foo for S { <|> }"#, + r#" mod foo { pub trait Fn { type Output; } trait Foo { fn foo(&self, bar: dyn Fn(u32) -> i32); } } struct S; impl foo::Foo for S { - <|>fn foo(&self, bar: dyn Fn(u32) -> i32) { todo!() } -}", + <|>fn foo(&self, bar: dyn Fn(u32) -> i32) { + todo!() + } +}"#, ); } @@ -496,10 +532,10 @@ impl foo::Foo for S { fn test_empty_trait() { check_assist_not_applicable( add_missing_impl_members, - " + r#" trait Foo; struct S; -impl Foo for S { <|> }", +impl Foo for S { <|> }"#, ) } @@ -507,13 +543,13 @@ impl Foo for S { <|> }", fn test_ignore_unnamed_trait_members_and_default_methods() { check_assist_not_applicable( add_missing_impl_members, - " + r#" trait Foo { fn (arg: u32); fn valid(some: u32) -> bool { false } } struct S; -impl Foo for S { <|> }", +impl Foo for S { <|> }"#, ) } @@ -544,7 +580,9 @@ trait Foo { struct S; impl Foo for S { <|>type Output; - fn foo(&self) { todo!() } + fn foo(&self) { + todo!() + } }"#, ) } @@ -553,7 +591,7 @@ impl Foo for S { fn test_default_methods() { check_assist( add_missing_default_members, - " + r#" trait Foo { type Output; @@ -563,8 +601,8 @@ trait Foo { fn foo(some: u32) -> bool; } struct S; -impl Foo for S { <|> }", - " +impl Foo for S { <|> }"#, + r#" trait Foo { type Output; @@ -576,7 +614,7 @@ trait Foo { struct S; impl Foo for S { <|>fn valid(some: u32) -> bool { false } -}", +}"#, ) } } -- cgit v1.2.3 From eea21738ab9e0b7438d03f7b2efc18c15cc30cf2 Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Sat, 2 May 2020 12:20:39 +0200 Subject: Add unwrap block assist #4156 Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- crates/ra_assists/src/handlers/unwrap_block.rs | 89 ++++---------------------- 1 file changed, 13 insertions(+), 76 deletions(-) (limited to 'crates/ra_assists/src') diff --git a/crates/ra_assists/src/handlers/unwrap_block.rs b/crates/ra_assists/src/handlers/unwrap_block.rs index 71d6d462b..8912ce645 100644 --- a/crates/ra_assists/src/handlers/unwrap_block.rs +++ b/crates/ra_assists/src/handlers/unwrap_block.rs @@ -1,8 +1,8 @@ use crate::{Assist, AssistCtx, AssistId}; -use ast::{BlockExpr, Expr, LoopBodyOwner}; +use ast::{BlockExpr, Expr, ForExpr, IfExpr, LoopBodyOwner, LoopExpr, WhileExpr}; use ra_fmt::unwrap_trivial_block; -use ra_syntax::{ast, AstNode, TextRange}; +use ra_syntax::{ast, AstNode, TextRange, T}; // Assist: unwrap_block // @@ -22,15 +22,11 @@ use ra_syntax::{ast, AstNode, TextRange}; // } // ``` pub(crate) fn unwrap_block(ctx: AssistCtx) -> Option { - let res = if let Some(if_expr) = ctx.find_node_at_offset::() { + let l_curly_token = ctx.find_token_at_offset(T!['{'])?; + + let res = if let Some(if_expr) = l_curly_token.ancestors().find_map(IfExpr::cast) { // if expression - let mut expr_to_unwrap: Option = None; - for block_expr in if_expr.blocks() { - if let Some(expr) = excract_expr(ctx.frange.range, block_expr) { - expr_to_unwrap = Some(expr); - break; - } - } + let expr_to_unwrap = if_expr.blocks().find_map(|expr| extract_expr(ctx.frange.range, expr)); let expr_to_unwrap = expr_to_unwrap?; // Find if we are in a else if block let ancestor = if_expr.syntax().ancestors().skip(1).find_map(ast::IfExpr::cast); @@ -40,20 +36,20 @@ pub(crate) fn unwrap_block(ctx: AssistCtx) -> Option { } else { Some((ast::Expr::IfExpr(if_expr), expr_to_unwrap)) } - } else if let Some(for_expr) = ctx.find_node_at_offset::() { + } else if let Some(for_expr) = l_curly_token.ancestors().find_map(ForExpr::cast) { // for expression let block_expr = for_expr.loop_body()?; - excract_expr(ctx.frange.range, block_expr) + extract_expr(ctx.frange.range, block_expr) .map(|expr_to_unwrap| (ast::Expr::ForExpr(for_expr), expr_to_unwrap)) - } else if let Some(while_expr) = ctx.find_node_at_offset::() { + } else if let Some(while_expr) = l_curly_token.ancestors().find_map(WhileExpr::cast) { // while expression let block_expr = while_expr.loop_body()?; - excract_expr(ctx.frange.range, block_expr) + extract_expr(ctx.frange.range, block_expr) .map(|expr_to_unwrap| (ast::Expr::WhileExpr(while_expr), expr_to_unwrap)) - } else if let Some(loop_expr) = ctx.find_node_at_offset::() { + } else if let Some(loop_expr) = l_curly_token.ancestors().find_map(LoopExpr::cast) { // loop expression let block_expr = loop_expr.loop_body()?; - excract_expr(ctx.frange.range, block_expr) + extract_expr(ctx.frange.range, block_expr) .map(|expr_to_unwrap| (ast::Expr::LoopExpr(loop_expr), expr_to_unwrap)) } else { None @@ -80,7 +76,7 @@ pub(crate) fn unwrap_block(ctx: AssistCtx) -> Option { }) } -fn excract_expr(cursor_range: TextRange, block_expr: BlockExpr) -> Option { +fn extract_expr(cursor_range: TextRange, block_expr: BlockExpr) -> Option { let block = block_expr.block()?; let cursor_in_range = block.l_curly_token()?.text_range().contains_range(cursor_range); @@ -200,65 +196,6 @@ mod tests { ); } - #[test] - fn issue_example_with_if() { - check_assist( - unwrap_block, - r#" - fn complete_enum_variants(acc: &mut Completions, ctx: &CompletionContext, ty: &Type) { - if let Some(ty) = &ctx.expected_type {<|> - if let Some(Adt::Enum(enum_data)) = ty.as_adt() { - let variants = enum_data.variants(ctx.db); - - let module = if let Some(module) = ctx.scope().module() { - // Compute path from the completion site if available. - module - } else { - // Otherwise fall back to the enum's definition site. - enum_data.module(ctx.db) - }; - - for variant in variants { - if let Some(path) = module.find_use_path(ctx.db, ModuleDef::from(variant)) { - // Variants with trivial paths are already added by the existing completion logic, - // so we should avoid adding these twice - if path.segments.len() > 1 { - acc.add_enum_variant(ctx, variant, Some(path.to_string())); - } - } - } - } - } - } - "#, - r#" - fn complete_enum_variants(acc: &mut Completions, ctx: &CompletionContext, ty: &Type) { - <|>if let Some(Adt::Enum(enum_data)) = ty.as_adt() { - let variants = enum_data.variants(ctx.db); - - let module = if let Some(module) = ctx.scope().module() { - // Compute path from the completion site if available. - module - } else { - // Otherwise fall back to the enum's definition site. - enum_data.module(ctx.db) - }; - - for variant in variants { - if let Some(path) = module.find_use_path(ctx.db, ModuleDef::from(variant)) { - // Variants with trivial paths are already added by the existing completion logic, - // so we should avoid adding these twice - if path.segments.len() > 1 { - acc.add_enum_variant(ctx, variant, Some(path.to_string())); - } - } - } - } - } - "#, - ); - } - #[test] fn simple_for() { check_assist( -- cgit v1.2.3 From 6d5f3922f7cf6d6c02521ad947abd63ab4764fca Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Sat, 2 May 2020 12:31:11 +0200 Subject: Add unwrap block assist #4156 Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- crates/ra_assists/src/handlers/unwrap_block.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'crates/ra_assists/src') diff --git a/crates/ra_assists/src/handlers/unwrap_block.rs b/crates/ra_assists/src/handlers/unwrap_block.rs index 8912ce645..58649c47e 100644 --- a/crates/ra_assists/src/handlers/unwrap_block.rs +++ b/crates/ra_assists/src/handlers/unwrap_block.rs @@ -76,12 +76,11 @@ pub(crate) fn unwrap_block(ctx: AssistCtx) -> Option { }) } -fn extract_expr(cursor_range: TextRange, block_expr: BlockExpr) -> Option { - let block = block_expr.block()?; +fn extract_expr(cursor_range: TextRange, block: BlockExpr) -> Option { let cursor_in_range = block.l_curly_token()?.text_range().contains_range(cursor_range); if cursor_in_range { - Some(unwrap_trivial_block(block_expr)) + Some(unwrap_trivial_block(block)) } else { None } -- cgit v1.2.3