From 27a2f9d5948edc7d2ba451bdd0f49c4ab19a2a21 Mon Sep 17 00:00:00 2001 From: Giga Bowser <45986823+Giga-Bowser@users.noreply.github.com> Date: Sun, 8 Dec 2024 17:14:22 -0500 Subject: [PATCH 1/6] minor: Add `item_const` constructor to `SyntaxFactory` --- .../src/ast/syntax_factory/constructors.rs | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/crates/syntax/src/ast/syntax_factory/constructors.rs b/crates/syntax/src/ast/syntax_factory/constructors.rs index e86c291f76..d2ab30fb47 100644 --- a/crates/syntax/src/ast/syntax_factory/constructors.rs +++ b/crates/syntax/src/ast/syntax_factory/constructors.rs @@ -188,6 +188,33 @@ impl SyntaxFactory { ast } + pub fn item_const( + &self, + visibility: Option, + name: ast::Name, + ty: ast::Type, + expr: ast::Expr, + ) -> ast::Const { + let ast = make::item_const(visibility.clone(), name.clone(), ty.clone(), expr.clone()) + .clone_for_update(); + + if let Some(mut mapping) = self.mappings() { + let mut builder = SyntaxMappingBuilder::new(ast.syntax().clone()); + if let Some(visibility) = visibility { + builder.map_node( + visibility.syntax().clone(), + ast.visibility().unwrap().syntax().clone(), + ); + } + builder.map_node(name.syntax().clone(), ast.name().unwrap().syntax().clone()); + builder.map_node(ty.syntax().clone(), ast.ty().unwrap().syntax().clone()); + builder.map_node(expr.syntax().clone(), ast.body().unwrap().syntax().clone()); + builder.finish(&mut mapping); + } + + ast + } + pub fn turbofish_generic_arg_list( &self, args: impl IntoIterator + Clone, From 17e482b1a954b0c37eb5afea4f2227a338dfcbac Mon Sep 17 00:00:00 2001 From: Giga Bowser <45986823+Giga-Bowser@users.noreply.github.com> Date: Sun, 8 Dec 2024 17:31:43 -0500 Subject: [PATCH 2/6] internal: Move `is_body_const` to `ide_assists::utils` --- .../src/handlers/promote_local_to_const.rs | 44 ++----------------- crates/ide-assists/src/utils.rs | 44 +++++++++++++++++-- 2 files changed, 44 insertions(+), 44 deletions(-) diff --git a/crates/ide-assists/src/handlers/promote_local_to_const.rs b/crates/ide-assists/src/handlers/promote_local_to_const.rs index 7c2dc0e0c1..0cc771ff39 100644 --- a/crates/ide-assists/src/handlers/promote_local_to_const.rs +++ b/crates/ide-assists/src/handlers/promote_local_to_const.rs @@ -1,19 +1,17 @@ -use hir::{HirDisplay, ModuleDef, PathResolution, Semantics}; +use hir::HirDisplay; use ide_db::{ assists::{AssistId, AssistKind}, defs::Definition, - syntax_helpers::node_ext::preorder_expr, - RootDatabase, }; use stdx::to_upper_snake_case; use syntax::{ ast::{self, make, HasName}, - ted, AstNode, WalkEvent, + ted, AstNode, }; use crate::{ assist_context::{AssistContext, Assists}, - utils, + utils::{self}, }; // Assist: promote_local_to_const @@ -63,7 +61,7 @@ pub(crate) fn promote_local_to_const(acc: &mut Assists, ctx: &AssistContext<'_>) }; let initializer = let_stmt.initializer()?; - if !is_body_const(&ctx.sema, &initializer) { + if !utils::is_body_const(&ctx.sema, &initializer) { cov_mark::hit!(promote_local_non_const); return None; } @@ -103,40 +101,6 @@ pub(crate) fn promote_local_to_const(acc: &mut Assists, ctx: &AssistContext<'_>) ) } -fn is_body_const(sema: &Semantics<'_, RootDatabase>, expr: &ast::Expr) -> bool { - let mut is_const = true; - preorder_expr(expr, &mut |ev| { - let expr = match ev { - WalkEvent::Enter(_) if !is_const => return true, - WalkEvent::Enter(expr) => expr, - WalkEvent::Leave(_) => return false, - }; - match expr { - ast::Expr::CallExpr(call) => { - if let Some(ast::Expr::PathExpr(path_expr)) = call.expr() { - if let Some(PathResolution::Def(ModuleDef::Function(func))) = - path_expr.path().and_then(|path| sema.resolve_path(&path)) - { - is_const &= func.is_const(sema.db); - } - } - } - ast::Expr::MethodCallExpr(call) => { - is_const &= - sema.resolve_method_call(&call).map(|it| it.is_const(sema.db)).unwrap_or(true) - } - ast::Expr::ForExpr(_) - | ast::Expr::ReturnExpr(_) - | ast::Expr::TryExpr(_) - | ast::Expr::YieldExpr(_) - | ast::Expr::AwaitExpr(_) => is_const = false, - _ => (), - } - !is_const - }); - is_const -} - #[cfg(test)] mod tests { use crate::tests::{check_assist, check_assist_not_applicable}; diff --git a/crates/ide-assists/src/utils.rs b/crates/ide-assists/src/utils.rs index 0830017bd0..3c26b04359 100644 --- a/crates/ide-assists/src/utils.rs +++ b/crates/ide-assists/src/utils.rs @@ -3,11 +3,13 @@ pub(crate) use gen_trait_fn_body::gen_trait_fn_body; use hir::{ db::{ExpandDatabase, HirDatabase}, - HasAttrs as HirHasAttrs, HirDisplay, InFile, Semantics, + HasAttrs as HirHasAttrs, HirDisplay, InFile, ModuleDef, PathResolution, Semantics, }; use ide_db::{ - famous_defs::FamousDefs, path_transform::PathTransform, - syntax_helpers::prettify_macro_expansion, RootDatabase, + famous_defs::FamousDefs, + path_transform::PathTransform, + syntax_helpers::{node_ext::preorder_expr, prettify_macro_expansion}, + RootDatabase, }; use stdx::format_to; use syntax::{ @@ -19,7 +21,7 @@ use syntax::{ }, ted, AstNode, AstToken, Direction, Edition, NodeOrToken, SourceFile, SyntaxKind::*, - SyntaxNode, SyntaxToken, TextRange, TextSize, T, + SyntaxNode, SyntaxToken, TextRange, TextSize, WalkEvent, T, }; use crate::assist_context::{AssistContext, SourceChangeBuilder}; @@ -966,3 +968,37 @@ pub(crate) fn tt_from_syntax(node: SyntaxNode) -> Vec, expr: &ast::Expr) -> bool { + let mut is_const = true; + preorder_expr(expr, &mut |ev| { + let expr = match ev { + WalkEvent::Enter(_) if !is_const => return true, + WalkEvent::Enter(expr) => expr, + WalkEvent::Leave(_) => return false, + }; + match expr { + ast::Expr::CallExpr(call) => { + if let Some(ast::Expr::PathExpr(path_expr)) = call.expr() { + if let Some(PathResolution::Def(ModuleDef::Function(func))) = + path_expr.path().and_then(|path| sema.resolve_path(&path)) + { + is_const &= func.is_const(sema.db); + } + } + } + ast::Expr::MethodCallExpr(call) => { + is_const &= + sema.resolve_method_call(&call).map(|it| it.is_const(sema.db)).unwrap_or(true) + } + ast::Expr::ForExpr(_) + | ast::Expr::ReturnExpr(_) + | ast::Expr::TryExpr(_) + | ast::Expr::YieldExpr(_) + | ast::Expr::AwaitExpr(_) => is_const = false, + _ => (), + } + !is_const + }); + is_const +} From 1979d3f9b50f0831dfc64e3f20adfed518583c04 Mon Sep 17 00:00:00 2001 From: Giga Bowser <45986823+Giga-Bowser@users.noreply.github.com> Date: Mon, 9 Dec 2024 16:23:36 -0500 Subject: [PATCH 3/6] feat: Add an assist to extract an expression into a constant --- .../src/handlers/extract_variable.rs | 896 ++++++++++++++---- crates/ide-assists/src/tests.rs | 122 ++- crates/ide-assists/src/tests/generated.rs | 18 + 3 files changed, 847 insertions(+), 189 deletions(-) diff --git a/crates/ide-assists/src/handlers/extract_variable.rs b/crates/ide-assists/src/handlers/extract_variable.rs index 61dc72e0b3..6670eac6a7 100644 --- a/crates/ide-assists/src/handlers/extract_variable.rs +++ b/crates/ide-assists/src/handlers/extract_variable.rs @@ -1,4 +1,4 @@ -use hir::TypeInfo; +use hir::{HirDisplay, TypeInfo}; use ide_db::syntax_helpers::suggest_name; use syntax::{ ast::{ @@ -7,11 +7,11 @@ use syntax::{ }, syntax_editor::Position, NodeOrToken, - SyntaxKind::{BLOCK_EXPR, BREAK_EXPR, COMMENT, LOOP_EXPR, MATCH_GUARD, PATH_EXPR, RETURN_EXPR}, + SyntaxKind::{self}, SyntaxNode, T, }; -use crate::{AssistContext, AssistId, AssistKind, Assists}; +use crate::{utils::is_body_const, AssistContext, AssistId, AssistKind, Assists}; // Assist: extract_variable // @@ -29,6 +29,23 @@ use crate::{AssistContext, AssistId, AssistKind, Assists}; // var_name * 4; // } // ``` + +// Assist: extract_constant +// +// Extracts subexpression into a constant. +// +// ``` +// fn main() { +// $0(1 + 2)$0 * 4; +// } +// ``` +// -> +// ``` +// fn main() { +// const $0VAR_NAME: i32 = 1 + 2; +// VAR_NAME * 4; +// } +// ``` pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { let node = if ctx.has_empty_selection() { if let Some(t) = ctx.token_at_offset().find(|it| it.kind() == T![;]) { @@ -41,7 +58,7 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op } else { match ctx.covering_element() { NodeOrToken::Node(it) => it, - NodeOrToken::Token(it) if it.kind() == COMMENT => { + NodeOrToken::Token(it) if it.kind() == SyntaxKind::COMMENT => { cov_mark::hit!(extract_var_in_comment_is_not_applicable); return None; } @@ -87,27 +104,39 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op } _ => false, }; - - let anchor = Anchor::from(&to_extract)?; + let module = ctx.sema.scope(to_extract.syntax())?.module(); let target = to_extract.syntax().text_range(); - acc.add( - AssistId("extract_variable", AssistKind::RefactorExtract), - "Extract into variable", - target, - move |edit| { - let field_shorthand = to_extract - .syntax() - .parent() - .and_then(ast::RecordExprField::cast) - .filter(|field| field.name_ref().is_some()); + let needs_mut = match &parent { + Some(ast::Expr::RefExpr(expr)) => expr.mut_token().is_some(), + _ => needs_adjust && !needs_ref && ty.as_ref().is_some_and(|ty| ty.is_mutable_reference()), + }; + for kind in ExtractionKind::ALL { + let Some(anchor) = Anchor::from(&to_extract, kind) else { + continue; + }; + let ty_string = match kind { + ExtractionKind::Constant => { + let Some(ty) = ty.clone() else { + continue; + }; - let (var_name, expr_replace) = match field_shorthand { - Some(field) => (field.to_string(), field.syntax().clone()), - None => ( - suggest_name::for_variable(&to_extract, &ctx.sema), - to_extract.syntax().clone(), - ), - }; + // We can't mutably reference a const, nor can we define + // one using a non-const expression or one of unknown type + if needs_mut || !is_body_const(&ctx.sema, &to_extract_no_ref) || ty.is_unknown() { + continue; + } + + let Ok(type_string) = ty.display_source_code(ctx.db(), module.into(), false) else { + continue; + }; + + type_string + } + _ => "".to_owned(), + }; + + acc.add(kind.assist_id(), kind.label(), target, |edit| { + let (var_name, expr_replace) = kind.get_name_and_expr(ctx, &to_extract); let make = SyntaxFactory::new(); let mut editor = edit.make_editor(&expr_replace); @@ -120,35 +149,31 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op editor.add_annotation(pat_name.syntax().clone(), tabstop); } - let ident_pat = match parent { - Some(ast::Expr::RefExpr(expr)) if expr.mut_token().is_some() => { - make.ident_pat(false, true, pat_name) - } - _ if needs_adjust - && !needs_ref - && ty.as_ref().is_some_and(|ty| ty.is_mutable_reference()) => - { - make.ident_pat(false, true, pat_name) - } - _ => make.ident_pat(false, false, pat_name), - }; - - let to_extract_no_ref = match ty.as_ref().filter(|_| needs_ref) { + let initializer = match ty.as_ref().filter(|_| needs_ref) { Some(receiver_type) if receiver_type.is_mutable_reference() => { - make.expr_ref(to_extract_no_ref, true) + make.expr_ref(to_extract_no_ref.clone(), true) } Some(receiver_type) if receiver_type.is_reference() => { - make.expr_ref(to_extract_no_ref, false) + make.expr_ref(to_extract_no_ref.clone(), false) } - _ => to_extract_no_ref, + _ => to_extract_no_ref.clone(), }; - let let_stmt = make.let_stmt(ident_pat.into(), None, Some(to_extract_no_ref)); + let new_stmt: ast::Stmt = match kind { + ExtractionKind::Variable => { + let ident_pat = make.ident_pat(false, needs_mut, pat_name); + make.let_stmt(ident_pat.into(), None, Some(initializer)).into() + } + ExtractionKind::Constant => { + let ast_ty = make.ty(&ty_string); + ast::Item::Const(make.item_const(None, pat_name, ast_ty, initializer)).into() + } + }; - match anchor { + match &anchor { Anchor::Before(place) => { let prev_ws = place.prev_sibling_or_token().and_then(|it| it.into_token()); - let indent_to = IndentLevel::from_node(&place); + let indent_to = IndentLevel::from_node(place); // Adjust ws to insert depending on if this is all inline or on separate lines let trailing_ws = if prev_ws.is_some_and(|it| it.text().starts_with('\n')) { @@ -160,7 +185,7 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op editor.insert_all( Position::before(place), vec![ - let_stmt.syntax().clone().into(), + new_stmt.syntax().clone().into(), make::tokens::whitespace(&trailing_ws).into(), ], ); @@ -170,7 +195,7 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op Anchor::Replace(stmt) => { cov_mark::hit!(test_extract_var_expr_stmt); - editor.replace(stmt.syntax(), let_stmt.syntax()); + editor.replace(stmt.syntax(), new_stmt.syntax()); } Anchor::WrapInBlock(to_wrap) => { let indent_to = to_wrap.indent_level(); @@ -178,11 +203,11 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op let block = if to_wrap.syntax() == &expr_replace { // Since `expr_replace` is the same that needs to be wrapped in a block, // we can just directly replace it with a block - make.block_expr([let_stmt.into()], Some(name_expr)) + make.block_expr([new_stmt], Some(name_expr)) } else { // `expr_replace` is a descendant of `to_wrap`, so we just replace it with `name_expr`. editor.replace(expr_replace, name_expr.syntax()); - make.block_expr([let_stmt.into()], Some(to_wrap.clone())) + make.block_expr([new_stmt], Some(to_wrap.clone())) }; editor.replace(to_wrap.syntax(), block.syntax()); @@ -195,8 +220,10 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op editor.add_mappings(make.finish_with_mappings()); edit.add_file_edits(ctx.file_id(), editor); edit.rename(); - }, - ) + }); + } + + Some(()) } fn peel_parens(mut expr: ast::Expr) -> ast::Expr { @@ -211,17 +238,67 @@ fn peel_parens(mut expr: ast::Expr) -> ast::Expr { /// In general that's true for any expression, but in some cases that would produce invalid code. fn valid_target_expr(node: SyntaxNode) -> Option { match node.kind() { - PATH_EXPR | LOOP_EXPR => None, - BREAK_EXPR => ast::BreakExpr::cast(node).and_then(|e| e.expr()), - RETURN_EXPR => ast::ReturnExpr::cast(node).and_then(|e| e.expr()), - BLOCK_EXPR => { + SyntaxKind::PATH_EXPR | SyntaxKind::LOOP_EXPR => None, + SyntaxKind::BREAK_EXPR => ast::BreakExpr::cast(node).and_then(|e| e.expr()), + SyntaxKind::RETURN_EXPR => ast::ReturnExpr::cast(node).and_then(|e| e.expr()), + SyntaxKind::BLOCK_EXPR => { ast::BlockExpr::cast(node).filter(|it| it.is_standalone()).map(ast::Expr::from) } _ => ast::Expr::cast(node), } } -#[derive(Debug)] +enum ExtractionKind { + Variable, + Constant, +} + +impl ExtractionKind { + const ALL: &'static [ExtractionKind] = &[ExtractionKind::Variable, ExtractionKind::Constant]; + + fn assist_id(&self) -> AssistId { + let s = match self { + ExtractionKind::Variable => "extract_variable", + ExtractionKind::Constant => "extract_constant", + }; + + AssistId(s, AssistKind::RefactorExtract) + } + + fn label(&self) -> &'static str { + match self { + ExtractionKind::Variable => "Extract into variable", + ExtractionKind::Constant => "Extract into constant", + } + } + + fn get_name_and_expr( + &self, + ctx: &AssistContext<'_>, + to_extract: &ast::Expr, + ) -> (String, SyntaxNode) { + let field_shorthand = to_extract + .syntax() + .parent() + .and_then(ast::RecordExprField::cast) + .filter(|field| field.name_ref().is_some()); + let (var_name, expr_replace) = match field_shorthand { + Some(field) => (field.to_string(), field.syntax().clone()), + None => { + (suggest_name::for_variable(to_extract, &ctx.sema), to_extract.syntax().clone()) + } + }; + + let var_name = match self { + ExtractionKind::Variable => var_name, + ExtractionKind::Constant => var_name.to_uppercase(), + }; + + (var_name, expr_replace) + } +} + +#[derive(Debug, Clone)] enum Anchor { Before(SyntaxNode), Replace(ast::ExprStmt), @@ -229,8 +306,8 @@ enum Anchor { } impl Anchor { - fn from(to_extract: &ast::Expr) -> Option { - to_extract + fn from(to_extract: &ast::Expr, kind: &ExtractionKind) -> Option { + let result = to_extract .syntax() .ancestors() .take_while(|it| !ast::Item::can_cast(it.kind()) || ast::MacroCall::can_cast(it.kind())) @@ -253,7 +330,7 @@ impl Anchor { return parent.body().map(Anchor::WrapInBlock); } if let Some(parent) = ast::MatchArm::cast(parent) { - if node.kind() == MATCH_GUARD { + if node.kind() == SyntaxKind::MATCH_GUARD { cov_mark::hit!(test_extract_var_in_match_guard); } else { cov_mark::hit!(test_extract_var_in_match_arm_no_block); @@ -271,19 +348,57 @@ impl Anchor { return Some(Anchor::Before(node)); } None - }) + }); + + match kind { + ExtractionKind::Constant if result.is_none() => { + to_extract.syntax().ancestors().find_map(|node| { + let item = ast::Item::cast(node.clone())?; + let parent = item.syntax().parent()?; + match parent.kind() { + SyntaxKind::ITEM_LIST + | SyntaxKind::SOURCE_FILE + | SyntaxKind::ASSOC_ITEM_LIST + | SyntaxKind::STMT_LIST => Some(Anchor::Before(node)), + _ => None, + } + }) + } + _ => result, + } } } #[cfg(test)] mod tests { - use crate::tests::{check_assist, check_assist_not_applicable, check_assist_target}; + // NOTE: We use check_assist_by_label, but not check_assist_not_applicable_by_label + // because all of our not-applicable tests should behave that way for both assists + // extract_variable offers, and check_assist_not_applicable ensures neither is offered + use crate::tests::{ + check_assist_by_label, check_assist_not_applicable, check_assist_not_applicable_by_label, + check_assist_target, + }; use super::*; #[test] - fn test_extract_var_simple_without_select() { - check_assist( + fn now_bad() { + // unknown type + check_assist_not_applicable_by_label( + extract_variable, + r#" +fn main() { + let a = Some(2); + a.is_some();$0 +} +"#, + "Extract into constant", + ); + } + + #[test] + fn extract_var_simple_without_select() { + check_assist_by_label( extract_variable, r#" fn main() -> i32 { @@ -304,9 +419,10 @@ fn main() -> i32 { var_name } "#, + "Extract into variable", ); - check_assist( + check_assist_by_label( extract_variable, r#" fn foo() -> i32 { 1 } @@ -320,9 +436,10 @@ fn main() { let $0foo = foo(); } "#, + "Extract into variable", ); - check_assist( + check_assist_by_label( extract_variable, r#" fn main() { @@ -336,9 +453,10 @@ fn main() { let $0is_some = a.is_some(); } "#, + "Extract into variable", ); - check_assist( + check_assist_by_label( extract_variable, r#" fn main() { @@ -350,9 +468,10 @@ fn main() { let $0var_name = "hello"; } "#, + "Extract into variable", ); - check_assist( + check_assist_by_label( extract_variable, r#" fn main() { @@ -364,9 +483,10 @@ fn main() { let $0var_name = 1 + 2; } "#, + "Extract into variable", ); - check_assist( + check_assist_by_label( extract_variable, r#" fn main() { @@ -384,11 +504,107 @@ fn main() { }; } "#, + "Extract into variable", ); } #[test] - fn test_extract_var_unit_expr_without_select_not_applicable() { + fn extract_const_simple_without_select() { + check_assist_by_label( + extract_variable, + r#" +fn main() -> i32 { + if true { + 1 + } else { + 2 + }$0 +} +"#, + r#" +fn main() -> i32 { + const $0VAR_NAME: i32 = if true { + 1 + } else { + 2 + }; + VAR_NAME +} +"#, + "Extract into constant", + ); + + check_assist_by_label( + extract_variable, + r#" +const fn foo() -> i32 { 1 } +fn main() { + foo();$0 +} +"#, + r#" +const fn foo() -> i32 { 1 } +fn main() { + const $0FOO: i32 = foo(); +} +"#, + "Extract into constant", + ); + + check_assist_by_label( + extract_variable, + r#" +fn main() { + "hello"$0; +} +"#, + r#" +fn main() { + const $0VAR_NAME: &str = "hello"; +} +"#, + "Extract into constant", + ); + + check_assist_by_label( + extract_variable, + r#" +fn main() { + 1 + 2$0; +} +"#, + r#" +fn main() { + const $0VAR_NAME: i32 = 1 + 2; +} +"#, + "Extract into constant", + ); + + check_assist_by_label( + extract_variable, + r#" +fn main() { + match () { + () if true => 1, + _ => 2, + };$0 +} +"#, + r#" +fn main() { + const $0VAR_NAME: i32 = match () { + () if true => 1, + _ => 2, + }; +} +"#, + "Extract into constant", + ); + } + + #[test] + fn extract_var_unit_expr_without_select_not_applicable() { check_assist_not_applicable( extract_variable, r#" @@ -414,8 +630,8 @@ fn foo() { } #[test] - fn test_extract_var_simple() { - check_assist( + fn extract_var_simple() { + check_assist_by_label( extract_variable, r#" fn foo() { @@ -426,19 +642,37 @@ fn foo() { let $0var_name = 1 + 1; foo(var_name); }"#, + "Extract into variable", + ); + } + + #[test] + fn extract_const_simple() { + check_assist_by_label( + extract_variable, + r#" +fn foo() { + foo($01 + 1$0); +}"#, + r#" +fn foo() { + const $0VAR_NAME: i32 = 1 + 1; + foo(VAR_NAME); +}"#, + "Extract into constant", ); } #[test] fn extract_var_in_comment_is_not_applicable() { cov_mark::check!(extract_var_in_comment_is_not_applicable); - check_assist_not_applicable(extract_variable, "fn main() { 1 + /* $0comment$0 */ 1; }"); + check_assist_not_applicable(extract_variable, r#"fn main() { 1 + /* $0comment$0 */ 1; }"#); } #[test] - fn test_extract_var_expr_stmt() { + fn extract_var_expr_stmt() { cov_mark::check!(test_extract_var_expr_stmt); - check_assist( + check_assist_by_label( extract_variable, r#" fn foo() { @@ -448,42 +682,94 @@ fn foo() { fn foo() { let $0var_name = 1 + 1; }"#, + "Extract into variable", ); - check_assist( + check_assist_by_label( extract_variable, - r" + r#" fn foo() { $0{ let x = 0; x }$0; something_else(); -}", - r" +}"#, + r#" fn foo() { let $0var_name = { let x = 0; x }; something_else(); -}", +}"#, + "Extract into variable", ); } #[test] - fn test_extract_var_part_of_expr_stmt() { - check_assist( + fn extract_const_expr_stmt() { + cov_mark::check!(test_extract_var_expr_stmt); + check_assist_by_label( extract_variable, - r" + r#" +fn foo() { + $0 1 + 1$0; +}"#, + r#" +fn foo() { + const $0VAR_NAME: i32 = 1 + 1; +}"#, + "Extract into constant", + ); + // This is hilarious but as far as I know, it's valid + check_assist_by_label( + extract_variable, + r#" +fn foo() { + $0{ let x = 0; x }$0; + something_else(); +}"#, + r#" +fn foo() { + const $0VAR_NAME: i32 = { let x = 0; x }; + something_else(); +}"#, + "Extract into constant", + ); + } + + #[test] + fn extract_var_part_of_expr_stmt() { + check_assist_by_label( + extract_variable, + r#" fn foo() { $01$0 + 1; -}", - r" +}"#, + r#" fn foo() { let $0var_name = 1; var_name + 1; -}", +}"#, + "Extract into variable", ); } #[test] - fn test_extract_var_last_expr() { + fn extract_const_part_of_expr_stmt() { + check_assist_by_label( + extract_variable, + r#" +fn foo() { + $01$0 + 1; +}"#, + r#" +fn foo() { + const $0VAR_NAME: i32 = 1; + VAR_NAME + 1; +}"#, + "Extract into constant", + ); + } + + #[test] + fn extract_var_last_expr() { cov_mark::check!(test_extract_var_last_expr); - check_assist( + check_assist_by_label( extract_variable, r#" fn foo() { @@ -496,8 +782,9 @@ fn foo() { bar(var_name) } "#, + "Extract into variable", ); - check_assist( + check_assist_by_label( extract_variable, r#" fn foo() -> i32 { @@ -518,13 +805,57 @@ fn bar(i: i32) -> i32 { i } "#, + "Extract into variable", ) } #[test] - fn test_extract_var_in_match_arm_no_block() { + fn extract_const_last_expr() { + cov_mark::check!(test_extract_var_last_expr); + check_assist_by_label( + extract_variable, + r#" +fn foo() { + bar($01 + 1$0) +} +"#, + r#" +fn foo() { + const $0VAR_NAME: i32 = 1 + 1; + bar(VAR_NAME) +} +"#, + "Extract into constant", + ); + check_assist_by_label( + extract_variable, + r#" +fn foo() -> i32 { + $0bar(1 + 1)$0 +} + +const fn bar(i: i32) -> i32 { + i +} +"#, + r#" +fn foo() -> i32 { + const $0BAR: i32 = bar(1 + 1); + BAR +} + +const fn bar(i: i32) -> i32 { + i +} +"#, + "Extract into constant", + ) + } + + #[test] + fn extract_var_in_match_arm_no_block() { cov_mark::check!(test_extract_var_in_match_arm_no_block); - check_assist( + check_assist_by_label( extract_variable, r#" fn main() { @@ -547,12 +878,13 @@ fn main() { }; } "#, + "Extract into variable", ); } #[test] - fn test_extract_var_in_match_arm_with_block() { - check_assist( + fn extract_var_in_match_arm_with_block() { + check_assist_by_label( extract_variable, r#" fn main() { @@ -579,13 +911,14 @@ fn main() { }; } "#, + "Extract into variable", ); } #[test] - fn test_extract_var_in_match_guard() { + fn extract_var_in_match_guard() { cov_mark::check!(test_extract_var_in_match_guard); - check_assist( + check_assist_by_label( extract_variable, r#" fn main() { @@ -604,13 +937,14 @@ fn main() { }; } "#, + "Extract into variable", ); } #[test] - fn test_extract_var_in_closure_no_block() { + fn extract_var_in_closure_no_block() { cov_mark::check!(test_extract_var_in_closure_no_block); - check_assist( + check_assist_by_label( extract_variable, r#" fn main() { @@ -625,12 +959,13 @@ fn main() { }; } "#, + "Extract into variable", ); } #[test] - fn test_extract_var_in_closure_with_block() { - check_assist( + fn extract_var_in_closure_with_block() { + check_assist_by_label( extract_variable, r#" fn main() { @@ -642,104 +977,110 @@ fn main() { let lambda = |x: u32| { let $0var_name = x * 2; var_name }; } "#, + "Extract into variable", ); } #[test] - fn test_extract_var_path_simple() { - check_assist( + fn extract_var_path_simple() { + check_assist_by_label( extract_variable, - " + r#" fn main() { let o = $0Some(true)$0; } -", - " +"#, + r#" fn main() { let $0var_name = Some(true); let o = var_name; } -", +"#, + "Extract into variable", ); } #[test] - fn test_extract_var_path_method() { - check_assist( + fn extract_var_path_method() { + check_assist_by_label( extract_variable, - " + r#" fn main() { let v = $0bar.foo()$0; } -", - " +"#, + r#" fn main() { let $0foo = bar.foo(); let v = foo; } -", +"#, + "Extract into variable", ); } #[test] - fn test_extract_var_return() { - check_assist( + fn extract_var_return() { + check_assist_by_label( extract_variable, - " + r#" fn foo() -> u32 { $0return 2 + 2$0; } -", - " +"#, + r#" fn foo() -> u32 { let $0var_name = 2 + 2; return var_name; } -", +"#, + "Extract into variable", ); } #[test] - fn test_extract_var_does_not_add_extra_whitespace() { - check_assist( + fn extract_var_does_not_add_extra_whitespace() { + check_assist_by_label( extract_variable, - " + r#" fn foo() -> u32 { $0return 2 + 2$0; } -", - " +"#, + r#" fn foo() -> u32 { let $0var_name = 2 + 2; return var_name; } -", +"#, + "Extract into variable", ); - check_assist( + check_assist_by_label( extract_variable, - " + r#" fn foo() -> u32 { $0return 2 + 2$0; } -", - " +"#, + r#" fn foo() -> u32 { let $0var_name = 2 + 2; return var_name; } -", +"#, + "Extract into variable", ); - check_assist( + check_assist_by_label( extract_variable, - " + r#" fn foo() -> u32 { let foo = 1; @@ -748,8 +1089,8 @@ fn foo() -> u32 { $0return 2 + 2$0; } -", - " +"#, + r#" fn foo() -> u32 { let foo = 1; @@ -759,53 +1100,56 @@ fn foo() -> u32 { let $0var_name = 2 + 2; return var_name; } -", +"#, + "Extract into variable", ); } #[test] - fn test_extract_var_break() { - check_assist( + fn extract_var_break() { + check_assist_by_label( extract_variable, - " + r#" fn main() { let result = loop { $0break 2 + 2$0; }; } -", - " +"#, + r#" fn main() { let result = loop { let $0var_name = 2 + 2; break var_name; }; } -", +"#, + "Extract into variable", ); } #[test] - fn test_extract_var_for_cast() { - check_assist( + fn extract_var_for_cast() { + check_assist_by_label( extract_variable, - " + r#" fn main() { let v = $00f32 as u32$0; } -", - " +"#, + r#" fn main() { let $0var_name = 0f32 as u32; let v = var_name; } -", +"#, + "Extract into variable", ); } #[test] fn extract_var_field_shorthand() { - check_assist( + check_assist_by_label( extract_variable, r#" struct S { @@ -826,12 +1170,13 @@ fn main() { S { foo } } "#, + "Extract into variable", ) } #[test] fn extract_var_name_from_type() { - check_assist( + check_assist_by_label( extract_variable, r#" struct Test(i32); @@ -848,12 +1193,13 @@ fn foo() -> Test { test } "#, + "Extract into variable", ) } #[test] fn extract_var_name_from_parameter() { - check_assist( + check_assist_by_label( extract_variable, r#" fn bar(test: u32, size: u32) @@ -870,12 +1216,13 @@ fn foo() { bar(1, size); } "#, + "Extract into variable", ) } #[test] fn extract_var_parameter_name_has_precedence_over_type() { - check_assist( + check_assist_by_label( extract_variable, r#" struct TextSize(u32); @@ -894,12 +1241,13 @@ fn foo() { bar(1, size); } "#, + "Extract into variable", ) } #[test] fn extract_var_name_from_function() { - check_assist( + check_assist_by_label( extract_variable, r#" fn is_required(test: u32, size: u32) -> bool @@ -916,12 +1264,13 @@ fn foo() -> bool { is_required } "#, + "Extract into variable", ) } #[test] fn extract_var_name_from_method() { - check_assist( + check_assist_by_label( extract_variable, r#" struct S; @@ -944,12 +1293,13 @@ fn foo() -> u32 { bar } "#, + "Extract into variable", ) } #[test] fn extract_var_name_from_method_param() { - check_assist( + check_assist_by_label( extract_variable, r#" struct S; @@ -972,12 +1322,13 @@ fn foo() { S.bar(n, 2) } "#, + "Extract into variable", ) } #[test] fn extract_var_name_from_ufcs_method_param() { - check_assist( + check_assist_by_label( extract_variable, r#" struct S; @@ -1000,12 +1351,13 @@ fn foo() { S::bar(&S, n, 2) } "#, + "Extract into variable", ) } #[test] fn extract_var_parameter_name_has_precedence_over_function() { - check_assist( + check_assist_by_label( extract_variable, r#" fn bar(test: u32, size: u32) @@ -1022,14 +1374,15 @@ fn foo() { bar(1, size); } "#, + "Extract into variable", ) } #[test] fn extract_macro_call() { - check_assist( + check_assist_by_label( extract_variable, - r" + r#" struct Vec; macro_rules! vec { () => {Vec} @@ -1037,8 +1390,8 @@ macro_rules! vec { fn main() { let _ = $0vec![]$0; } -", - r" +"#, + r#" struct Vec; macro_rules! vec { () => {Vec} @@ -1047,22 +1400,47 @@ fn main() { let $0vec = vec![]; let _ = vec; } -", +"#, + "Extract into variable", + ); + + check_assist_by_label( + extract_variable, + r#" +struct Vec; +macro_rules! vec { + () => {Vec} +} +fn main() { + let _ = $0vec![]$0; +} +"#, + r#" +struct Vec; +macro_rules! vec { + () => {Vec} +} +fn main() { + const $0VEC: Vec = vec![]; + let _ = VEC; +} +"#, + "Extract into constant", ); } #[test] - fn test_extract_var_for_return_not_applicable() { + fn extract_var_for_return_not_applicable() { check_assist_not_applicable(extract_variable, "fn foo() { $0return$0; } "); } #[test] - fn test_extract_var_for_break_not_applicable() { + fn extract_var_for_break_not_applicable() { check_assist_not_applicable(extract_variable, "fn main() { loop { $0break$0; }; }"); } #[test] - fn test_extract_var_unit_expr_not_applicable() { + fn extract_var_unit_expr_not_applicable() { check_assist_not_applicable( extract_variable, r#" @@ -1080,11 +1458,11 @@ fn foo() { // FIXME: This is not quite correct, but good enough(tm) for the sorting heuristic #[test] fn extract_var_target() { - check_assist_target(extract_variable, "fn foo() -> u32 { $0return 2 + 2$0; }", "2 + 2"); + check_assist_target(extract_variable, r#"fn foo() -> u32 { $0return 2 + 2$0; }"#, "2 + 2"); check_assist_target( extract_variable, - " + r#" fn main() { let x = true; let tuple = match x { @@ -1092,24 +1470,128 @@ fn main() { _ => (0, false) }; } -", +"#, "2 + 2", ); } #[test] fn extract_var_no_block_body() { - check_assist_not_applicable( + check_assist_not_applicable_by_label( extract_variable, - r" + r#" const X: usize = $0100$0; -", +"#, + "Extract into variable", ); } #[test] - fn test_extract_var_mutable_reference_parameter() { - check_assist( + fn extract_const_no_block_body() { + check_assist_by_label( + extract_variable, + r#" +const fn foo(x: i32) -> i32 { + x +} + +const FOO: i32 = foo($0100$0); +"#, + r#" +const fn foo(x: i32) -> i32 { + x +} + +const $0X: i32 = 100; +const FOO: i32 = foo(X); +"#, + "Extract into constant", + ); + + check_assist_by_label( + extract_variable, + r#" +mod foo { + enum Foo { + Bar, + Baz = $042$0, + } +} +"#, + r#" +mod foo { + const $0VAR_NAME: isize = 42; + enum Foo { + Bar, + Baz = VAR_NAME, + } +} +"#, + "Extract into constant", + ); + + check_assist_by_label( + extract_variable, + r#" +const fn foo(x: i32) -> i32 { + x +} + +trait Hello { + const World: i32; +} + +struct Bar; +impl Hello for Bar { + const World = foo($042$0); +} +"#, + r#" +const fn foo(x: i32) -> i32 { + x +} + +trait Hello { + const World: i32; +} + +struct Bar; +impl Hello for Bar { + const $0X: i32 = 42; + const World = foo(X); +} +"#, + "Extract into constant", + ); + + check_assist_by_label( + extract_variable, + r#" +const fn foo(x: i32) -> i32 { + x +} + +fn bar() { + const BAZ: i32 = foo($042$0); +} +"#, + r#" +const fn foo(x: i32) -> i32 { + x +} + +fn bar() { + const $0X: i32 = 42; + const BAZ: i32 = foo(X); +} +"#, + "Extract into constant", + ); + } + + #[test] + fn extract_var_mutable_reference_parameter() { + check_assist_by_label( extract_variable, r#" struct S { @@ -1138,12 +1620,34 @@ fn foo(s: &mut S) { let $0vec = &mut s.vec; vec.push(0); }"#, + "Extract into variable", ); } #[test] - fn test_extract_var_mutable_reference_parameter_deep_nesting() { - check_assist( + fn dont_extract_const_mutable_reference_parameter() { + check_assist_not_applicable_by_label( + extract_variable, + r#" +struct S { + vec: Vec +} + +struct Vec; +impl Vec { + fn push(&mut self, _:usize) {} +} + +fn foo(s: &mut S) { + $0s.vec$0.push(0); +}"#, + "Extract into const", + ); + } + + #[test] + fn extract_var_mutable_reference_parameter_deep_nesting() { + check_assist_by_label( extract_variable, r#" struct Y { @@ -1182,12 +1686,13 @@ fn foo(f: &mut Y) { let $0vec = &mut f.field.field.vec; vec.push(0); }"#, + "Extract into variable", ); } #[test] - fn test_extract_var_reference_parameter() { - check_assist( + fn extract_var_reference_parameter() { + check_assist_by_label( extract_variable, r#" struct X; @@ -1222,12 +1727,13 @@ fn foo(s: &S) { let $0x = &s.sub; x.do_thing(); }"#, + "Extract into variable", ); } #[test] - fn test_extract_var_index_deref() { - check_assist( + fn extract_var_index_deref() { + check_assist_by_label( extract_variable, r#" //- minicore: index @@ -1261,12 +1767,13 @@ fn foo(s: &S) { let $0sub = &s.sub; sub[0]; }"#, + "Extract into variable", ); } #[test] - fn test_extract_var_reference_parameter_deep_nesting() { - check_assist( + fn extract_var_reference_parameter_deep_nesting() { + check_assist_by_label( extract_variable, r#" struct Z; @@ -1315,12 +1822,13 @@ fn foo(s: &S) { let $0z = &s.sub.field.field; z.do_thing(); }"#, + "Extract into variable", ); } #[test] - fn test_extract_var_regular_parameter() { - check_assist( + fn extract_var_regular_parameter() { + check_assist_by_label( extract_variable, r#" struct X; @@ -1355,12 +1863,13 @@ fn foo(s: S) { let $0x = &s.sub; x.do_thing(); }"#, + "Extract into variable", ); } #[test] - fn test_extract_var_mutable_reference_local() { - check_assist( + fn extract_var_mutable_reference_local() { + check_assist_by_label( extract_variable, r#" struct X; @@ -1421,12 +1930,13 @@ fn foo() { let $0x = &local.sub; x.do_thing(); }"#, + "Extract into variable", ); } #[test] - fn test_extract_var_reference_local() { - check_assist( + fn extract_var_reference_local() { + check_assist_by_label( extract_variable, r#" struct X; @@ -1487,12 +1997,13 @@ fn foo() { let $0x = &local.sub; x.do_thing(); }"#, + "Extract into variable", ); } #[test] - fn test_extract_var_for_mutable_borrow() { - check_assist( + fn extract_var_for_mutable_borrow() { + check_assist_by_label( extract_variable, r#" fn foo() { @@ -1503,12 +2014,25 @@ fn foo() { let mut $0var_name = 0; let v = &mut var_name; }"#, + "Extract into variable", + ); + } + + #[test] + fn dont_extract_const_for_mutable_borrow() { + check_assist_not_applicable_by_label( + extract_variable, + r#" +fn foo() { + let v = &mut $00$0; +}"#, + "Extract into constant", ); } #[test] fn generates_no_ref_on_calls() { - check_assist( + check_assist_by_label( extract_variable, r#" struct S; @@ -1529,12 +2053,13 @@ fn foo() { let mut $0bar = bar(); bar.do_work(); }"#, + "Extract into variable", ); } #[test] fn generates_no_ref_for_deref() { - check_assist( + check_assist_by_label( extract_variable, r#" struct S; @@ -1559,6 +2084,7 @@ fn foo() { s.do_work(); } "#, + "Extract into variable", ); } } diff --git a/crates/ide-assists/src/tests.rs b/crates/ide-assists/src/tests.rs index 6469957fe1..4f7f03764f 100644 --- a/crates/ide-assists/src/tests.rs +++ b/crates/ide-assists/src/tests.rs @@ -363,6 +363,7 @@ pub fn test_some_range(a: int) -> bool { expect![[r#" Convert integer base Extract into variable + Extract into constant Extract into function Replace if let with match "#]] @@ -392,6 +393,7 @@ pub fn test_some_range(a: int) -> bool { expect![[r#" Convert integer base Extract into variable + Extract into constant Extract into function Replace if let with match "#]] @@ -406,6 +408,7 @@ pub fn test_some_range(a: int) -> bool { expect![[r#" Extract into variable + Extract into constant Extract into function "#]] .assert_eq(&expected); @@ -440,7 +443,7 @@ pub fn test_some_range(a: int) -> bool { { let assists = assists(&db, &cfg, AssistResolveStrategy::None, frange.into()); - assert_eq!(2, assists.len()); + assert_eq!(3, assists.len()); let mut assists = assists.into_iter(); let extract_into_variable_assist = assists.next().unwrap(); @@ -459,6 +462,22 @@ pub fn test_some_range(a: int) -> bool { "#]] .assert_debug_eq(&extract_into_variable_assist); + let extract_into_constant_assist = assists.next().unwrap(); + expect![[r#" + Assist { + id: AssistId( + "extract_constant", + RefactorExtract, + ), + label: "Extract into constant", + group: None, + target: 59..60, + source_change: None, + command: None, + } + "#]] + .assert_debug_eq(&extract_into_constant_assist); + let extract_into_function_assist = assists.next().unwrap(); expect![[r#" Assist { @@ -486,7 +505,7 @@ pub fn test_some_range(a: int) -> bool { }), frange.into(), ); - assert_eq!(2, assists.len()); + assert_eq!(3, assists.len()); let mut assists = assists.into_iter(); let extract_into_variable_assist = assists.next().unwrap(); @@ -505,6 +524,22 @@ pub fn test_some_range(a: int) -> bool { "#]] .assert_debug_eq(&extract_into_variable_assist); + let extract_into_constant_assist = assists.next().unwrap(); + expect![[r#" + Assist { + id: AssistId( + "extract_constant", + RefactorExtract, + ), + label: "Extract into constant", + group: None, + target: 59..60, + source_change: None, + command: None, + } + "#]] + .assert_debug_eq(&extract_into_constant_assist); + let extract_into_function_assist = assists.next().unwrap(); expect![[r#" Assist { @@ -532,7 +567,7 @@ pub fn test_some_range(a: int) -> bool { }), frange.into(), ); - assert_eq!(2, assists.len()); + assert_eq!(3, assists.len()); let mut assists = assists.into_iter(); let extract_into_variable_assist = assists.next().unwrap(); @@ -594,6 +629,22 @@ pub fn test_some_range(a: int) -> bool { "#]] .assert_debug_eq(&extract_into_variable_assist); + let extract_into_constant_assist = assists.next().unwrap(); + expect![[r#" + Assist { + id: AssistId( + "extract_constant", + RefactorExtract, + ), + label: "Extract into constant", + group: None, + target: 59..60, + source_change: None, + command: None, + } + "#]] + .assert_debug_eq(&extract_into_constant_assist); + let extract_into_function_assist = assists.next().unwrap(); expect![[r#" Assist { @@ -613,7 +664,7 @@ pub fn test_some_range(a: int) -> bool { { let assists = assists(&db, &cfg, AssistResolveStrategy::All, frange.into()); - assert_eq!(2, assists.len()); + assert_eq!(3, assists.len()); let mut assists = assists.into_iter(); let extract_into_variable_assist = assists.next().unwrap(); @@ -675,6 +726,69 @@ pub fn test_some_range(a: int) -> bool { "#]] .assert_debug_eq(&extract_into_variable_assist); + let extract_into_constant_assist = assists.next().unwrap(); + expect![[r#" + Assist { + id: AssistId( + "extract_constant", + RefactorExtract, + ), + label: "Extract into constant", + group: None, + target: 59..60, + source_change: Some( + SourceChange { + source_file_edits: { + FileId( + 0, + ): ( + TextEdit { + indels: [ + Indel { + insert: "const", + delete: 45..47, + }, + Indel { + insert: "VAR_NAME:", + delete: 48..60, + }, + Indel { + insert: "i32", + delete: 61..81, + }, + Indel { + insert: "=", + delete: 82..86, + }, + Indel { + insert: "5;\n if let 2..6 = VAR_NAME {\n true\n } else {\n false\n }", + delete: 87..108, + }, + ], + }, + Some( + SnippetEdit( + [ + ( + 0, + 51..51, + ), + ], + ), + ), + ), + }, + file_system_edits: [], + is_snippet: true, + }, + ), + command: Some( + Rename, + ), + } + "#]] + .assert_debug_eq(&extract_into_constant_assist); + let extract_into_function_assist = assists.next().unwrap(); expect![[r#" Assist { diff --git a/crates/ide-assists/src/tests/generated.rs b/crates/ide-assists/src/tests/generated.rs index 69ea200db1..c1799b48ed 100644 --- a/crates/ide-assists/src/tests/generated.rs +++ b/crates/ide-assists/src/tests/generated.rs @@ -932,6 +932,24 @@ enum TheEnum { ) } +#[test] +fn doctest_extract_constant() { + check_doc_test( + "extract_constant", + r#####" +fn main() { + $0(1 + 2)$0 * 4; +} +"#####, + r#####" +fn main() { + const $0VAR_NAME: i32 = 1 + 2; + VAR_NAME * 4; +} +"#####, + ) +} + #[test] fn doctest_extract_expressions_from_format_string() { check_doc_test( From 135e71fcb33e565a037bfa6e56704ba2c8a61f9a Mon Sep 17 00:00:00 2001 From: Giga Bowser <45986823+Giga-Bowser@users.noreply.github.com> Date: Wed, 11 Dec 2024 10:32:32 -0500 Subject: [PATCH 4/6] minor: Add `item_static` constructor to `SyntaxFactory` --- crates/syntax/src/ast/make.rs | 24 ++++++++++- .../src/ast/syntax_factory/constructors.rs | 40 +++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index 05c2a8354d..eb96ab6ef5 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -895,7 +895,29 @@ pub fn item_const( None => String::new(), Some(it) => format!("{it} "), }; - ast_from_text(&format!("{visibility} const {name}: {ty} = {expr};")) + ast_from_text(&format!("{visibility}const {name}: {ty} = {expr};")) +} + +pub fn item_static( + visibility: Option, + is_unsafe: bool, + is_mut: bool, + name: ast::Name, + ty: ast::Type, + expr: Option, +) -> ast::Static { + let visibility = match visibility { + None => String::new(), + Some(it) => format!("{it} "), + }; + let is_unsafe = if is_unsafe { "unsafe " } else { "" }; + let is_mut = if is_mut { "mut " } else { "" }; + let expr = match expr { + Some(it) => &format!(" = {it}"), + None => "", + }; + + ast_from_text(&format!("{visibility}{is_unsafe}static {is_mut}{name}: {ty}{expr};")) } pub fn unnamed_param(ty: ast::Type) -> ast::Param { diff --git a/crates/syntax/src/ast/syntax_factory/constructors.rs b/crates/syntax/src/ast/syntax_factory/constructors.rs index d2ab30fb47..280c5c25cb 100644 --- a/crates/syntax/src/ast/syntax_factory/constructors.rs +++ b/crates/syntax/src/ast/syntax_factory/constructors.rs @@ -215,6 +215,46 @@ impl SyntaxFactory { ast } + pub fn item_static( + &self, + visibility: Option, + is_unsafe: bool, + is_mut: bool, + name: ast::Name, + ty: ast::Type, + expr: Option, + ) -> ast::Static { + let ast = make::item_static( + visibility.clone(), + is_unsafe, + is_mut, + name.clone(), + ty.clone(), + expr.clone(), + ) + .clone_for_update(); + + if let Some(mut mapping) = self.mappings() { + let mut builder = SyntaxMappingBuilder::new(ast.syntax().clone()); + if let Some(visibility) = visibility { + builder.map_node( + visibility.syntax().clone(), + ast.visibility().unwrap().syntax().clone(), + ); + } + + builder.map_node(name.syntax().clone(), ast.name().unwrap().syntax().clone()); + builder.map_node(ty.syntax().clone(), ast.ty().unwrap().syntax().clone()); + + if let Some(expr) = expr { + builder.map_node(expr.syntax().clone(), ast.body().unwrap().syntax().clone()); + } + builder.finish(&mut mapping); + } + + ast + } + pub fn turbofish_generic_arg_list( &self, args: impl IntoIterator + Clone, From 0cad614b3b202ded9a48d7ed07f36284ec393caf Mon Sep 17 00:00:00 2001 From: Giga Bowser <45986823+Giga-Bowser@users.noreply.github.com> Date: Wed, 11 Dec 2024 14:04:54 -0500 Subject: [PATCH 5/6] feat: Add an assist to extract an expression into a static --- .../src/handlers/extract_variable.rs | 588 +++++++++++++++--- crates/ide-assists/src/tests.rs | 192 +++++- crates/ide-assists/src/tests/generated.rs | 18 + 3 files changed, 682 insertions(+), 116 deletions(-) diff --git a/crates/ide-assists/src/handlers/extract_variable.rs b/crates/ide-assists/src/handlers/extract_variable.rs index 6670eac6a7..a8d71ed7f4 100644 --- a/crates/ide-assists/src/handlers/extract_variable.rs +++ b/crates/ide-assists/src/handlers/extract_variable.rs @@ -1,14 +1,12 @@ use hir::{HirDisplay, TypeInfo}; -use ide_db::syntax_helpers::suggest_name; +use ide_db::{assists::GroupLabel, syntax_helpers::suggest_name}; use syntax::{ ast::{ self, edit::IndentLevel, edit_in_place::Indent, make, syntax_factory::SyntaxFactory, AstNode, }, syntax_editor::Position, - NodeOrToken, - SyntaxKind::{self}, - SyntaxNode, T, + NodeOrToken, SyntaxKind, SyntaxNode, T, }; use crate::{utils::is_body_const, AssistContext, AssistId, AssistKind, Assists}; @@ -46,6 +44,23 @@ use crate::{utils::is_body_const, AssistContext, AssistId, AssistKind, Assists}; // VAR_NAME * 4; // } // ``` + +// Assist: extract_static +// +// Extracts subexpression into a static. +// +// ``` +// fn main() { +// $0(1 + 2)$0 * 4; +// } +// ``` +// -> +// ``` +// fn main() { +// static $0VAR_NAME: i32 = 1 + 2; +// VAR_NAME * 4; +// } +// ``` pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { let node = if ctx.has_empty_selection() { if let Some(t) = ctx.token_at_offset().find(|it| it.kind() == T![;]) { @@ -114,15 +129,20 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op let Some(anchor) = Anchor::from(&to_extract, kind) else { continue; }; + let ty_string = match kind { - ExtractionKind::Constant => { + ExtractionKind::Constant | ExtractionKind::Static => { let Some(ty) = ty.clone() else { continue; }; // We can't mutably reference a const, nor can we define // one using a non-const expression or one of unknown type - if needs_mut || !is_body_const(&ctx.sema, &to_extract_no_ref) || ty.is_unknown() { + if needs_mut + || !is_body_const(&ctx.sema, &to_extract_no_ref) + || ty.is_unknown() + || ty.is_mutable_reference() + { continue; } @@ -135,92 +155,111 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op _ => "".to_owned(), }; - acc.add(kind.assist_id(), kind.label(), target, |edit| { - let (var_name, expr_replace) = kind.get_name_and_expr(ctx, &to_extract); + acc.add_group( + &GroupLabel("Extract into...".to_owned()), + kind.assist_id(), + kind.label(), + target, + |edit| { + let (var_name, expr_replace) = kind.get_name_and_expr(ctx, &to_extract); - let make = SyntaxFactory::new(); - let mut editor = edit.make_editor(&expr_replace); + let make = SyntaxFactory::new(); + let mut editor = edit.make_editor(&expr_replace); - let pat_name = make.name(&var_name); - let name_expr = make.expr_path(make::ext::ident_path(&var_name)); + let pat_name = make.name(&var_name); + let name_expr = make.expr_path(make::ext::ident_path(&var_name)); - if let Some(cap) = ctx.config.snippet_cap { - let tabstop = edit.make_tabstop_before(cap); - editor.add_annotation(pat_name.syntax().clone(), tabstop); - } - - let initializer = match ty.as_ref().filter(|_| needs_ref) { - Some(receiver_type) if receiver_type.is_mutable_reference() => { - make.expr_ref(to_extract_no_ref.clone(), true) + if let Some(cap) = ctx.config.snippet_cap { + let tabstop = edit.make_tabstop_before(cap); + editor.add_annotation(pat_name.syntax().clone(), tabstop); } - Some(receiver_type) if receiver_type.is_reference() => { - make.expr_ref(to_extract_no_ref.clone(), false) - } - _ => to_extract_no_ref.clone(), - }; - let new_stmt: ast::Stmt = match kind { - ExtractionKind::Variable => { - let ident_pat = make.ident_pat(false, needs_mut, pat_name); - make.let_stmt(ident_pat.into(), None, Some(initializer)).into() - } - ExtractionKind::Constant => { - let ast_ty = make.ty(&ty_string); - ast::Item::Const(make.item_const(None, pat_name, ast_ty, initializer)).into() - } - }; + let initializer = match ty.as_ref().filter(|_| needs_ref) { + Some(receiver_type) if receiver_type.is_mutable_reference() => { + make.expr_ref(to_extract_no_ref.clone(), true) + } + Some(receiver_type) if receiver_type.is_reference() => { + make.expr_ref(to_extract_no_ref.clone(), false) + } + _ => to_extract_no_ref.clone(), + }; - match &anchor { - Anchor::Before(place) => { - let prev_ws = place.prev_sibling_or_token().and_then(|it| it.into_token()); - let indent_to = IndentLevel::from_node(place); + let new_stmt: ast::Stmt = match kind { + ExtractionKind::Variable => { + let ident_pat = make.ident_pat(false, needs_mut, pat_name); + make.let_stmt(ident_pat.into(), None, Some(initializer)).into() + } + ExtractionKind::Constant => { + let ast_ty = make.ty(&ty_string); + ast::Item::Const(make.item_const(None, pat_name, ast_ty, initializer)) + .into() + } + ExtractionKind::Static => { + let ast_ty = make.ty(&ty_string); + ast::Item::Static(make.item_static( + None, + false, + false, + pat_name, + ast_ty, + Some(initializer), + )) + .into() + } + }; - // Adjust ws to insert depending on if this is all inline or on separate lines - let trailing_ws = if prev_ws.is_some_and(|it| it.text().starts_with('\n')) { - format!("\n{indent_to}") - } else { - " ".to_owned() - }; + match &anchor { + Anchor::Before(place) => { + let prev_ws = place.prev_sibling_or_token().and_then(|it| it.into_token()); + let indent_to = IndentLevel::from_node(place); - editor.insert_all( - Position::before(place), - vec![ - new_stmt.syntax().clone().into(), - make::tokens::whitespace(&trailing_ws).into(), - ], - ); + // Adjust ws to insert depending on if this is all inline or on separate lines + let trailing_ws = if prev_ws.is_some_and(|it| it.text().starts_with('\n')) { + format!("\n{indent_to}") + } else { + " ".to_owned() + }; - editor.replace(expr_replace, name_expr.syntax()); - } - Anchor::Replace(stmt) => { - cov_mark::hit!(test_extract_var_expr_stmt); + editor.insert_all( + Position::before(place), + vec![ + new_stmt.syntax().clone().into(), + make::tokens::whitespace(&trailing_ws).into(), + ], + ); - editor.replace(stmt.syntax(), new_stmt.syntax()); - } - Anchor::WrapInBlock(to_wrap) => { - let indent_to = to_wrap.indent_level(); - - let block = if to_wrap.syntax() == &expr_replace { - // Since `expr_replace` is the same that needs to be wrapped in a block, - // we can just directly replace it with a block - make.block_expr([new_stmt], Some(name_expr)) - } else { - // `expr_replace` is a descendant of `to_wrap`, so we just replace it with `name_expr`. editor.replace(expr_replace, name_expr.syntax()); - make.block_expr([new_stmt], Some(to_wrap.clone())) - }; + } + Anchor::Replace(stmt) => { + cov_mark::hit!(test_extract_var_expr_stmt); - editor.replace(to_wrap.syntax(), block.syntax()); + editor.replace(stmt.syntax(), new_stmt.syntax()); + } + Anchor::WrapInBlock(to_wrap) => { + let indent_to = to_wrap.indent_level(); - // fixup indentation of block - block.indent(indent_to); + let block = if to_wrap.syntax() == &expr_replace { + // Since `expr_replace` is the same that needs to be wrapped in a block, + // we can just directly replace it with a block + make.block_expr([new_stmt], Some(name_expr)) + } else { + // `expr_replace` is a descendant of `to_wrap`, so we just replace it with `name_expr`. + editor.replace(expr_replace, name_expr.syntax()); + make.block_expr([new_stmt], Some(to_wrap.clone())) + }; + + editor.replace(to_wrap.syntax(), block.syntax()); + + // fixup indentation of block + block.indent(indent_to); + } } - } - editor.add_mappings(make.finish_with_mappings()); - edit.add_file_edits(ctx.file_id(), editor); - edit.rename(); - }); + editor.add_mappings(make.finish_with_mappings()); + edit.add_file_edits(ctx.file_id(), editor); + edit.rename(); + }, + ); } Some(()) @@ -251,15 +290,18 @@ fn valid_target_expr(node: SyntaxNode) -> Option { enum ExtractionKind { Variable, Constant, + Static, } impl ExtractionKind { - const ALL: &'static [ExtractionKind] = &[ExtractionKind::Variable, ExtractionKind::Constant]; + const ALL: &'static [ExtractionKind] = + &[ExtractionKind::Variable, ExtractionKind::Constant, ExtractionKind::Static]; fn assist_id(&self) -> AssistId { let s = match self { ExtractionKind::Variable => "extract_variable", ExtractionKind::Constant => "extract_constant", + ExtractionKind::Static => "extract_static", }; AssistId(s, AssistKind::RefactorExtract) @@ -269,6 +311,7 @@ impl ExtractionKind { match self { ExtractionKind::Variable => "Extract into variable", ExtractionKind::Constant => "Extract into constant", + ExtractionKind::Static => "Extract into static", } } @@ -291,7 +334,7 @@ impl ExtractionKind { let var_name = match self { ExtractionKind::Variable => var_name, - ExtractionKind::Constant => var_name.to_uppercase(), + ExtractionKind::Constant | ExtractionKind::Static => var_name.to_uppercase(), }; (var_name, expr_replace) @@ -351,7 +394,7 @@ impl Anchor { }); match kind { - ExtractionKind::Constant if result.is_none() => { + ExtractionKind::Constant | ExtractionKind::Static if result.is_none() => { to_extract.syntax().ancestors().find_map(|node| { let item = ast::Item::cast(node.clone())?; let parent = item.syntax().parent()?; @@ -381,21 +424,6 @@ mod tests { use super::*; - #[test] - fn now_bad() { - // unknown type - check_assist_not_applicable_by_label( - extract_variable, - r#" -fn main() { - let a = Some(2); - a.is_some();$0 -} -"#, - "Extract into constant", - ); - } - #[test] fn extract_var_simple_without_select() { check_assist_by_label( @@ -604,7 +632,102 @@ fn main() { } #[test] - fn extract_var_unit_expr_without_select_not_applicable() { + fn extract_static_simple_without_select() { + check_assist_by_label( + extract_variable, + r#" +fn main() -> i32 { + if true { + 1 + } else { + 2 + }$0 +} +"#, + r#" +fn main() -> i32 { + static $0VAR_NAME: i32 = if true { + 1 + } else { + 2 + }; + VAR_NAME +} +"#, + "Extract into static", + ); + + check_assist_by_label( + extract_variable, + r#" +const fn foo() -> i32 { 1 } +fn main() { + foo();$0 +} +"#, + r#" +const fn foo() -> i32 { 1 } +fn main() { + static $0FOO: i32 = foo(); +} +"#, + "Extract into static", + ); + + check_assist_by_label( + extract_variable, + r#" +fn main() { + "hello"$0; +} +"#, + r#" +fn main() { + static $0VAR_NAME: &str = "hello"; +} +"#, + "Extract into static", + ); + + check_assist_by_label( + extract_variable, + r#" +fn main() { + 1 + 2$0; +} +"#, + r#" +fn main() { + static $0VAR_NAME: i32 = 1 + 2; +} +"#, + "Extract into static", + ); + + check_assist_by_label( + extract_variable, + r#" +fn main() { + match () { + () if true => 1, + _ => 2, + };$0 +} +"#, + r#" +fn main() { + static $0VAR_NAME: i32 = match () { + () if true => 1, + _ => 2, + }; +} +"#, + "Extract into static", + ); + } + + #[test] + fn dont_extract_unit_expr_without_select() { check_assist_not_applicable( extract_variable, r#" @@ -664,7 +787,24 @@ fn foo() { } #[test] - fn extract_var_in_comment_is_not_applicable() { + fn extract_static_simple() { + check_assist_by_label( + extract_variable, + r#" +fn foo() { + foo($01 + 1$0); +}"#, + r#" +fn foo() { + static $0VAR_NAME: i32 = 1 + 1; + foo(VAR_NAME); +}"#, + "Extract into static", + ); + } + + #[test] + fn dont_extract_in_comment() { cov_mark::check!(extract_var_in_comment_is_not_applicable); check_assist_not_applicable(extract_variable, r#"fn main() { 1 + /* $0comment$0 */ 1; }"#); } @@ -732,6 +872,38 @@ fn foo() { ); } + #[test] + fn extract_static_expr_stmt() { + cov_mark::check!(test_extract_var_expr_stmt); + check_assist_by_label( + extract_variable, + r#" +fn foo() { + $0 1 + 1$0; +}"#, + r#" +fn foo() { + static $0VAR_NAME: i32 = 1 + 1; +}"#, + "Extract into static", + ); + // This is hilarious but as far as I know, it's valid + check_assist_by_label( + extract_variable, + r#" +fn foo() { + $0{ let x = 0; x }$0; + something_else(); +}"#, + r#" +fn foo() { + static $0VAR_NAME: i32 = { let x = 0; x }; + something_else(); +}"#, + "Extract into static", + ); + } + #[test] fn extract_var_part_of_expr_stmt() { check_assist_by_label( @@ -766,6 +938,23 @@ fn foo() { ); } + #[test] + fn extract_static_part_of_expr_stmt() { + check_assist_by_label( + extract_variable, + r#" +fn foo() { + $01$0 + 1; +}"#, + r#" +fn foo() { + static $0VAR_NAME: i32 = 1; + VAR_NAME + 1; +}"#, + "Extract into static", + ); + } + #[test] fn extract_var_last_expr() { cov_mark::check!(test_extract_var_last_expr); @@ -852,6 +1041,49 @@ const fn bar(i: i32) -> i32 { ) } + #[test] + fn extract_static_last_expr() { + cov_mark::check!(test_extract_var_last_expr); + check_assist_by_label( + extract_variable, + r#" +fn foo() { + bar($01 + 1$0) +} +"#, + r#" +fn foo() { + static $0VAR_NAME: i32 = 1 + 1; + bar(VAR_NAME) +} +"#, + "Extract into static", + ); + check_assist_by_label( + extract_variable, + r#" +fn foo() -> i32 { + $0bar(1 + 1)$0 +} + +const fn bar(i: i32) -> i32 { + i +} +"#, + r#" +fn foo() -> i32 { + static $0BAR: i32 = bar(1 + 1); + BAR +} + +const fn bar(i: i32) -> i32 { + i +} +"#, + "Extract into static", + ) + } + #[test] fn extract_var_in_match_arm_no_block() { cov_mark::check!(test_extract_var_in_match_arm_no_block); @@ -1427,6 +1659,30 @@ fn main() { "#, "Extract into constant", ); + + check_assist_by_label( + extract_variable, + r#" +struct Vec; +macro_rules! vec { + () => {Vec} +} +fn main() { + let _ = $0vec![]$0; +} +"#, + r#" +struct Vec; +macro_rules! vec { + () => {Vec} +} +fn main() { + static $0VEC: Vec = vec![]; + let _ = VEC; +} +"#, + "Extract into static", + ); } #[test] @@ -1589,6 +1845,109 @@ fn bar() { ); } + #[test] + fn extract_static_no_block_body() { + check_assist_by_label( + extract_variable, + r#" +const fn foo(x: i32) -> i32 { + x +} + +const FOO: i32 = foo($0100$0); +"#, + r#" +const fn foo(x: i32) -> i32 { + x +} + +static $0X: i32 = 100; +const FOO: i32 = foo(X); +"#, + "Extract into static", + ); + + check_assist_by_label( + extract_variable, + r#" +mod foo { + enum Foo { + Bar, + Baz = $042$0, + } +} +"#, + r#" +mod foo { + static $0VAR_NAME: isize = 42; + enum Foo { + Bar, + Baz = VAR_NAME, + } +} +"#, + "Extract into static", + ); + + check_assist_by_label( + extract_variable, + r#" +const fn foo(x: i32) -> i32 { + x +} + +trait Hello { + const World: i32; +} + +struct Bar; +impl Hello for Bar { + const World = foo($042$0); +} +"#, + r#" +const fn foo(x: i32) -> i32 { + x +} + +trait Hello { + const World: i32; +} + +struct Bar; +impl Hello for Bar { + static $0X: i32 = 42; + const World = foo(X); +} +"#, + "Extract into static", + ); + + check_assist_by_label( + extract_variable, + r#" +const fn foo(x: i32) -> i32 { + x +} + +fn bar() { + const BAZ: i32 = foo($042$0); +} +"#, + r#" +const fn foo(x: i32) -> i32 { + x +} + +fn bar() { + static $0X: i32 = 42; + const BAZ: i32 = foo(X); +} +"#, + "Extract into static", + ); + } + #[test] fn extract_var_mutable_reference_parameter() { check_assist_by_label( @@ -1641,7 +2000,28 @@ impl Vec { fn foo(s: &mut S) { $0s.vec$0.push(0); }"#, - "Extract into const", + "Extract into constant", + ); + } + + #[test] + fn dont_extract_static_mutable_reference_parameter() { + check_assist_not_applicable_by_label( + extract_variable, + r#" +struct S { + vec: Vec +} + +struct Vec; +impl Vec { + fn push(&mut self, _:usize) {} +} + +fn foo(s: &mut S) { + $0s.vec$0.push(0); +}"#, + "Extract into static", ); } @@ -2030,6 +2410,18 @@ fn foo() { ); } + #[test] + fn dont_extract_static_for_mutable_borrow() { + check_assist_not_applicable_by_label( + extract_variable, + r#" +fn foo() { + let v = &mut $00$0; +}"#, + "Extract into static", + ); + } + #[test] fn generates_no_ref_on_calls() { check_assist_by_label( diff --git a/crates/ide-assists/src/tests.rs b/crates/ide-assists/src/tests.rs index 4f7f03764f..e517dd4682 100644 --- a/crates/ide-assists/src/tests.rs +++ b/crates/ide-assists/src/tests.rs @@ -362,8 +362,7 @@ pub fn test_some_range(a: int) -> bool { expect![[r#" Convert integer base - Extract into variable - Extract into constant + Extract into... Extract into function Replace if let with match "#]] @@ -392,8 +391,7 @@ pub fn test_some_range(a: int) -> bool { expect![[r#" Convert integer base - Extract into variable - Extract into constant + Extract into... Extract into function Replace if let with match "#]] @@ -407,8 +405,7 @@ pub fn test_some_range(a: int) -> bool { let expected = labels(&assists); expect![[r#" - Extract into variable - Extract into constant + Extract into... Extract into function "#]] .assert_eq(&expected); @@ -443,7 +440,7 @@ pub fn test_some_range(a: int) -> bool { { let assists = assists(&db, &cfg, AssistResolveStrategy::None, frange.into()); - assert_eq!(3, assists.len()); + assert_eq!(4, assists.len()); let mut assists = assists.into_iter(); let extract_into_variable_assist = assists.next().unwrap(); @@ -454,7 +451,11 @@ pub fn test_some_range(a: int) -> bool { RefactorExtract, ), label: "Extract into variable", - group: None, + group: Some( + GroupLabel( + "Extract into...", + ), + ), target: 59..60, source_change: None, command: None, @@ -470,7 +471,11 @@ pub fn test_some_range(a: int) -> bool { RefactorExtract, ), label: "Extract into constant", - group: None, + group: Some( + GroupLabel( + "Extract into...", + ), + ), target: 59..60, source_change: None, command: None, @@ -478,6 +483,26 @@ pub fn test_some_range(a: int) -> bool { "#]] .assert_debug_eq(&extract_into_constant_assist); + let extract_into_static_assist = assists.next().unwrap(); + expect![[r#" + Assist { + id: AssistId( + "extract_static", + RefactorExtract, + ), + label: "Extract into static", + group: Some( + GroupLabel( + "Extract into...", + ), + ), + target: 59..60, + source_change: None, + command: None, + } + "#]] + .assert_debug_eq(&extract_into_static_assist); + let extract_into_function_assist = assists.next().unwrap(); expect![[r#" Assist { @@ -505,7 +530,7 @@ pub fn test_some_range(a: int) -> bool { }), frange.into(), ); - assert_eq!(3, assists.len()); + assert_eq!(4, assists.len()); let mut assists = assists.into_iter(); let extract_into_variable_assist = assists.next().unwrap(); @@ -516,7 +541,11 @@ pub fn test_some_range(a: int) -> bool { RefactorExtract, ), label: "Extract into variable", - group: None, + group: Some( + GroupLabel( + "Extract into...", + ), + ), target: 59..60, source_change: None, command: None, @@ -532,7 +561,11 @@ pub fn test_some_range(a: int) -> bool { RefactorExtract, ), label: "Extract into constant", - group: None, + group: Some( + GroupLabel( + "Extract into...", + ), + ), target: 59..60, source_change: None, command: None, @@ -540,6 +573,26 @@ pub fn test_some_range(a: int) -> bool { "#]] .assert_debug_eq(&extract_into_constant_assist); + let extract_into_static_assist = assists.next().unwrap(); + expect![[r#" + Assist { + id: AssistId( + "extract_static", + RefactorExtract, + ), + label: "Extract into static", + group: Some( + GroupLabel( + "Extract into...", + ), + ), + target: 59..60, + source_change: None, + command: None, + } + "#]] + .assert_debug_eq(&extract_into_static_assist); + let extract_into_function_assist = assists.next().unwrap(); expect![[r#" Assist { @@ -567,7 +620,7 @@ pub fn test_some_range(a: int) -> bool { }), frange.into(), ); - assert_eq!(3, assists.len()); + assert_eq!(4, assists.len()); let mut assists = assists.into_iter(); let extract_into_variable_assist = assists.next().unwrap(); @@ -578,7 +631,11 @@ pub fn test_some_range(a: int) -> bool { RefactorExtract, ), label: "Extract into variable", - group: None, + group: Some( + GroupLabel( + "Extract into...", + ), + ), target: 59..60, source_change: Some( SourceChange { @@ -637,7 +694,11 @@ pub fn test_some_range(a: int) -> bool { RefactorExtract, ), label: "Extract into constant", - group: None, + group: Some( + GroupLabel( + "Extract into...", + ), + ), target: 59..60, source_change: None, command: None, @@ -645,6 +706,26 @@ pub fn test_some_range(a: int) -> bool { "#]] .assert_debug_eq(&extract_into_constant_assist); + let extract_into_static_assist = assists.next().unwrap(); + expect![[r#" + Assist { + id: AssistId( + "extract_static", + RefactorExtract, + ), + label: "Extract into static", + group: Some( + GroupLabel( + "Extract into...", + ), + ), + target: 59..60, + source_change: None, + command: None, + } + "#]] + .assert_debug_eq(&extract_into_static_assist); + let extract_into_function_assist = assists.next().unwrap(); expect![[r#" Assist { @@ -664,7 +745,7 @@ pub fn test_some_range(a: int) -> bool { { let assists = assists(&db, &cfg, AssistResolveStrategy::All, frange.into()); - assert_eq!(3, assists.len()); + assert_eq!(4, assists.len()); let mut assists = assists.into_iter(); let extract_into_variable_assist = assists.next().unwrap(); @@ -675,7 +756,11 @@ pub fn test_some_range(a: int) -> bool { RefactorExtract, ), label: "Extract into variable", - group: None, + group: Some( + GroupLabel( + "Extract into...", + ), + ), target: 59..60, source_change: Some( SourceChange { @@ -734,7 +819,11 @@ pub fn test_some_range(a: int) -> bool { RefactorExtract, ), label: "Extract into constant", - group: None, + group: Some( + GroupLabel( + "Extract into...", + ), + ), target: 59..60, source_change: Some( SourceChange { @@ -789,6 +878,73 @@ pub fn test_some_range(a: int) -> bool { "#]] .assert_debug_eq(&extract_into_constant_assist); + let extract_into_static_assist = assists.next().unwrap(); + expect![[r#" + Assist { + id: AssistId( + "extract_static", + RefactorExtract, + ), + label: "Extract into static", + group: Some( + GroupLabel( + "Extract into...", + ), + ), + target: 59..60, + source_change: Some( + SourceChange { + source_file_edits: { + FileId( + 0, + ): ( + TextEdit { + indels: [ + Indel { + insert: "static", + delete: 45..47, + }, + Indel { + insert: "VAR_NAME:", + delete: 48..60, + }, + Indel { + insert: "i32", + delete: 61..81, + }, + Indel { + insert: "=", + delete: 82..86, + }, + Indel { + insert: "5;\n if let 2..6 = VAR_NAME {\n true\n } else {\n false\n }", + delete: 87..108, + }, + ], + }, + Some( + SnippetEdit( + [ + ( + 0, + 52..52, + ), + ], + ), + ), + ), + }, + file_system_edits: [], + is_snippet: true, + }, + ), + command: Some( + Rename, + ), + } + "#]] + .assert_debug_eq(&extract_into_static_assist); + let extract_into_function_assist = assists.next().unwrap(); expect![[r#" Assist { diff --git a/crates/ide-assists/src/tests/generated.rs b/crates/ide-assists/src/tests/generated.rs index c1799b48ed..87c3d166ee 100644 --- a/crates/ide-assists/src/tests/generated.rs +++ b/crates/ide-assists/src/tests/generated.rs @@ -1024,6 +1024,24 @@ fn bar(name: i32) -> i32 { ) } +#[test] +fn doctest_extract_static() { + check_doc_test( + "extract_static", + r#####" +fn main() { + $0(1 + 2)$0 * 4; +} +"#####, + r#####" +fn main() { + static $0VAR_NAME: i32 = 1 + 2; + VAR_NAME * 4; +} +"#####, + ) +} + #[test] fn doctest_extract_struct_from_enum_variant() { check_doc_test( From 21782b9a8dff8e9dd9e51fed23a017d89b0b2ff9 Mon Sep 17 00:00:00 2001 From: Giga Bowser <45986823+Giga-Bowser@users.noreply.github.com> Date: Wed, 11 Dec 2024 14:11:20 -0500 Subject: [PATCH 6/6] minor: Group `extract_function` with other extraction assists --- .../src/handlers/extract_function.rs | 4 ++- crates/ide-assists/src/tests.rs | 27 ++++++++++++++----- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/crates/ide-assists/src/handlers/extract_function.rs b/crates/ide-assists/src/handlers/extract_function.rs index 6937d33ebc..2e363b0b62 100644 --- a/crates/ide-assists/src/handlers/extract_function.rs +++ b/crates/ide-assists/src/handlers/extract_function.rs @@ -7,6 +7,7 @@ use hir::{ TypeInfo, TypeParam, }; use ide_db::{ + assists::GroupLabel, defs::{Definition, NameRefClass}, famous_defs::FamousDefs, helpers::mod_path_to_ast, @@ -104,7 +105,8 @@ pub(crate) fn extract_function(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op let scope = ImportScope::find_insert_use_container(&node, &ctx.sema)?; - acc.add( + acc.add_group( + &GroupLabel("Extract into...".to_owned()), AssistId("extract_function", crate::AssistKind::RefactorExtract), "Extract into function", target_range, diff --git a/crates/ide-assists/src/tests.rs b/crates/ide-assists/src/tests.rs index e517dd4682..0b1ff87c5c 100644 --- a/crates/ide-assists/src/tests.rs +++ b/crates/ide-assists/src/tests.rs @@ -363,7 +363,6 @@ pub fn test_some_range(a: int) -> bool { expect![[r#" Convert integer base Extract into... - Extract into function Replace if let with match "#]] .assert_eq(&expected); @@ -392,7 +391,6 @@ pub fn test_some_range(a: int) -> bool { expect![[r#" Convert integer base Extract into... - Extract into function Replace if let with match "#]] .assert_eq(&expected); @@ -406,7 +404,6 @@ pub fn test_some_range(a: int) -> bool { expect![[r#" Extract into... - Extract into function "#]] .assert_eq(&expected); } @@ -511,7 +508,11 @@ pub fn test_some_range(a: int) -> bool { RefactorExtract, ), label: "Extract into function", - group: None, + group: Some( + GroupLabel( + "Extract into...", + ), + ), target: 59..60, source_change: None, command: None, @@ -601,7 +602,11 @@ pub fn test_some_range(a: int) -> bool { RefactorExtract, ), label: "Extract into function", - group: None, + group: Some( + GroupLabel( + "Extract into...", + ), + ), target: 59..60, source_change: None, command: None, @@ -734,7 +739,11 @@ pub fn test_some_range(a: int) -> bool { RefactorExtract, ), label: "Extract into function", - group: None, + group: Some( + GroupLabel( + "Extract into...", + ), + ), target: 59..60, source_change: None, command: None, @@ -953,7 +962,11 @@ pub fn test_some_range(a: int) -> bool { RefactorExtract, ), label: "Extract into function", - group: None, + group: Some( + GroupLabel( + "Extract into...", + ), + ), target: 59..60, source_change: Some( SourceChange {