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: [PATCH 1/6] 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 + .../ra_assists/src/handlers/unwrap_block.rs | 435 ++++++++++++++++++ crates/ra_assists/src/lib.rs | 2 + crates/ra_syntax/src/ast/expr_extensions.rs | 2 +- docs/user/assists.md | 18 + 5 files changed, 475 insertions(+), 1 deletion(-) create mode 100644 crates/ra_assists/src/handlers/unwrap_block.rs diff --git a/crates/ra_assists/src/doc_tests/generated.rs b/crates/ra_assists/src/doc_tests/generated.rs index e4fa9ee366..6fe95da6ab 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 0000000000..b98601f1c0 --- /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 64bd87afbd..c5df86600f 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, diff --git a/crates/ra_syntax/src/ast/expr_extensions.rs b/crates/ra_syntax/src/ast/expr_extensions.rs index 93aa3d45fa..1c1134bc5d 100644 --- a/crates/ra_syntax/src/ast/expr_extensions.rs +++ b/crates/ra_syntax/src/ast/expr_extensions.rs @@ -43,7 +43,7 @@ impl ast::IfExpr { Some(res) } - fn blocks(&self) -> AstChildren { + pub fn blocks(&self) -> AstChildren { support::children(self.syntax()) } } diff --git a/docs/user/assists.md b/docs/user/assists.md index 6c69436223..02323772c0 100644 --- a/docs/user/assists.md +++ b/docs/user/assists.md @@ -695,3 +695,21 @@ use std::┃collections::HashMap; // AFTER use std::{collections::HashMap}; ``` + +## `unwrap_block` + +Removes the `mut` keyword. + +```rust +// BEFORE +fn foo() { + if true {┃ + println!("foo"); + } +} + +// AFTER +fn foo() { + ┃println!("foo"); +} +``` 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: [PATCH 2/6] 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 ++-- docs/user/assists.md | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/ra_assists/src/doc_tests/generated.rs b/crates/ra_assists/src/doc_tests/generated.rs index 6fe95da6ab..b53b97e89a 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 b98601f1c0..35d87bc9e7 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 { diff --git a/docs/user/assists.md b/docs/user/assists.md index 02323772c0..a421aa0c3c 100644 --- a/docs/user/assists.md +++ b/docs/user/assists.md @@ -698,7 +698,7 @@ use std::{collections::HashMap}; ## `unwrap_block` -Removes the `mut` keyword. +This assist removes if...else, for, while and loop control statements to just keep the body. ```rust // BEFORE @@ -710,6 +710,6 @@ fn foo() { // AFTER fn foo() { - ┃println!("foo"); + println!("foo"); } ``` 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: [PATCH 3/6] Add unwrap block assist #4156 Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- .../ra_assists/src/handlers/unwrap_block.rs | 69 +++++++------------ 1 file changed, 23 insertions(+), 46 deletions(-) diff --git a/crates/ra_assists/src/handlers/unwrap_block.rs b/crates/ra_assists/src/handlers/unwrap_block.rs index 35d87bc9e7..71d6d462b6 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}; From 65234e8828defc0a56cb1d5e20793b5163b5330d Mon Sep 17 00:00:00 2001 From: Andrew Chin Date: Fri, 1 May 2020 18:59:19 -0400 Subject: [PATCH 4/6] Remove `workspaceLoaded` setting The `workspaceLoaded` notification setting was originally designed to control the display of a popup message that said: "workspace loaded, {} rust packages" This popup was removed and replaced by a much sleeker message in the VSCode status bar that provides a real-time status while loading: rust-analyzer: {}/{} packages This was done as part of #3587 The new status-bar indicator is unobtrusive and shouldn't need to be disabled. So this setting is removed. --- crates/rust-analyzer/src/config.rs | 7 +------ crates/rust-analyzer/src/main_loop.rs | 3 +-- editors/code/package.json | 5 ----- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 177da94cc9..15b7c69121 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -49,7 +49,6 @@ pub enum FilesWatcher { #[derive(Debug, Clone)] pub struct NotificationsConfig { - pub workspace_loaded: bool, pub cargo_toml_not_found: bool, } @@ -83,10 +82,7 @@ impl Default for Config { lru_capacity: None, proc_macro_srv: None, files: FilesConfig { watcher: FilesWatcher::Notify, exclude: Vec::new() }, - notifications: NotificationsConfig { - workspace_loaded: true, - cargo_toml_not_found: true, - }, + notifications: NotificationsConfig { cargo_toml_not_found: true }, cargo: CargoConfig::default(), rustfmt: RustfmtConfig::Rustfmt { extra_args: Vec::new() }, @@ -129,7 +125,6 @@ impl Config { Some("client") => FilesWatcher::Client, Some("notify") | _ => FilesWatcher::Notify }; - set(value, "/notifications/workspaceLoaded", &mut self.notifications.workspace_loaded); set(value, "/notifications/cargoTomlNotFound", &mut self.notifications.cargo_toml_not_found); set(value, "/cargo/noDefaultFeatures", &mut self.cargo.no_default_features); diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index 0a0e616c9c..3bc2e0a462 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -415,8 +415,7 @@ fn loop_turn( }); } - let show_progress = - !loop_state.workspace_loaded && world_state.config.notifications.workspace_loaded; + let show_progress = !loop_state.workspace_loaded; if !loop_state.workspace_loaded && loop_state.roots_scanned == loop_state.roots_total diff --git a/editors/code/package.json b/editors/code/package.json index d30673791f..7ef727b9d0 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -205,11 +205,6 @@ "default": [], "description": "Paths to exclude from analysis." }, - "rust-analyzer.notifications.workspaceLoaded": { - "type": "boolean", - "default": true, - "markdownDescription": "Whether to show `workspace loaded` message." - }, "rust-analyzer.notifications.cargoTomlNotFound": { "type": "boolean", "default": true, 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: [PATCH 5/6] Add unwrap block assist #4156 Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- .../ra_assists/src/handlers/unwrap_block.rs | 89 +++---------------- 1 file changed, 13 insertions(+), 76 deletions(-) diff --git a/crates/ra_assists/src/handlers/unwrap_block.rs b/crates/ra_assists/src/handlers/unwrap_block.rs index 71d6d462b6..8912ce6451 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( 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: [PATCH 6/6] 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(-) diff --git a/crates/ra_assists/src/handlers/unwrap_block.rs b/crates/ra_assists/src/handlers/unwrap_block.rs index 8912ce6451..58649c47eb 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 }