diff --git a/crates/ide-assists/src/handlers/generate_delegate_methods.rs b/crates/ide-assists/src/handlers/generate_delegate_methods.rs index 1c2ec79809b..31fc69562c9 100644 --- a/crates/ide-assists/src/handlers/generate_delegate_methods.rs +++ b/crates/ide-assists/src/handlers/generate_delegate_methods.rs @@ -1,13 +1,17 @@ use std::collections::HashSet; use hir::{self, HasCrate, HasSource, HasVisibility}; -use syntax::ast::{self, make, AstNode, HasGenericParams, HasName, HasVisibility as _}; +use syntax::{ + ast::{ + self, edit_in_place::Indent, make, AstNode, HasGenericParams, HasName, HasVisibility as _, + }, + ted, +}; use crate::{ - utils::{convert_param_list_to_arg_list, find_struct_impl, render_snippet, Cursor}, + utils::{convert_param_list_to_arg_list, find_struct_impl}, AssistContext, AssistId, AssistKind, Assists, GroupLabel, }; -use syntax::ast::edit::AstNodeEdit; // Assist: generate_delegate_methods // @@ -96,7 +100,7 @@ pub(crate) fn generate_delegate_methods(acc: &mut Assists, ctx: &AssistContext<' AssistId("generate_delegate_methods", AssistKind::Generate), format!("Generate delegate for `{field_name}.{name}()`",), target, - |builder| { + |edit| { // Create the function let method_source = match method.source(ctx.db()) { Some(source) => source.value, @@ -135,36 +139,12 @@ pub(crate) fn generate_delegate_methods(acc: &mut Assists, ctx: &AssistContext<' is_const, is_unsafe, ) - .indent(ast::edit::IndentLevel(1)) .clone_for_update(); - let cursor = Cursor::Before(f.syntax()); - - // Create or update an impl block, attach the function to it, - // then insert into our code. - match impl_def { - Some(impl_def) => { - // Remember where in our source our `impl` block lives. - let impl_def = impl_def.clone_for_update(); - let old_range = impl_def.syntax().text_range(); - - // Attach the function to the impl block - let assoc_items = impl_def.get_or_create_assoc_item_list(); - assoc_items.add_item(f.clone().into()); - - // Update the impl block. - match ctx.config.snippet_cap { - Some(cap) => { - let snippet = render_snippet(cap, impl_def.syntax(), cursor); - builder.replace_snippet(cap, old_range, snippet); - } - None => { - builder.replace(old_range, impl_def.syntax().to_string()); - } - } - } + // Get the impl to update, or create one if we need to. + let impl_def = match impl_def { + Some(impl_def) => edit.make_mut(impl_def), None => { - // Attach the function to the impl block let name = &strukt_name.to_string(); let params = strukt.generic_param_list(); let ty_params = params.clone(); @@ -178,24 +158,34 @@ pub(crate) fn generate_delegate_methods(acc: &mut Assists, ctx: &AssistContext<' None, ) .clone_for_update(); - let assoc_items = impl_def.get_or_create_assoc_item_list(); - assoc_items.add_item(f.clone().into()); + + // Fixup impl_def indentation + let indent = strukt.indent_level(); + impl_def.reindent_to(indent); // Insert the impl block. - match ctx.config.snippet_cap { - Some(cap) => { - let offset = strukt.syntax().text_range().end(); - let snippet = render_snippet(cap, impl_def.syntax(), cursor); - let snippet = format!("\n\n{snippet}"); - builder.insert_snippet(cap, offset, snippet); - } - None => { - let offset = strukt.syntax().text_range().end(); - let snippet = format!("\n\n{}", impl_def.syntax()); - builder.insert(offset, snippet); - } - } + let strukt = edit.make_mut(strukt.clone()); + ted::insert_all( + ted::Position::after(strukt.syntax()), + vec![ + make::tokens::whitespace(&format!("\n\n{indent}")).into(), + impl_def.syntax().clone().into(), + ], + ); + + impl_def } + }; + + // Fixup function indentation. + // FIXME: Should really be handled by `AssocItemList::add_item` + f.reindent_to(impl_def.indent_level() + 1); + + let assoc_items = impl_def.get_or_create_assoc_item_list(); + assoc_items.add_item(f.clone().into()); + + if let Some(cap) = ctx.config.snippet_cap { + edit.add_tabstop_before(cap, f) } }, )?; @@ -244,6 +234,45 @@ impl Person { ); } + #[test] + fn test_generate_delegate_create_impl_block_match_indent() { + check_assist( + generate_delegate_methods, + r#" +mod indent { + struct Age(u8); + impl Age { + fn age(&self) -> u8 { + self.0 + } + } + + struct Person { + ag$0e: Age, + } +}"#, + r#" +mod indent { + struct Age(u8); + impl Age { + fn age(&self) -> u8 { + self.0 + } + } + + struct Person { + age: Age, + } + + impl Person { + $0fn age(&self) -> u8 { + self.age.age() + } + } +}"#, + ); + } + #[test] fn test_generate_delegate_update_impl_block() { check_assist( @@ -281,6 +310,47 @@ impl Person { ); } + #[test] + fn test_generate_delegate_update_impl_block_match_indent() { + check_assist( + generate_delegate_methods, + r#" +mod indent { + struct Age(u8); + impl Age { + fn age(&self) -> u8 { + self.0 + } + } + + struct Person { + ag$0e: Age, + } + + impl Person {} +}"#, + r#" +mod indent { + struct Age(u8); + impl Age { + fn age(&self) -> u8 { + self.0 + } + } + + struct Person { + age: Age, + } + + impl Person { + $0fn age(&self) -> u8 { + self.age.age() + } + } +}"#, + ); + } + #[test] fn test_generate_delegate_tuple_struct() { check_assist( diff --git a/crates/ide-assists/src/handlers/generate_derive.rs b/crates/ide-assists/src/handlers/generate_derive.rs index 78ac2eb30e5..747f70f9f6f 100644 --- a/crates/ide-assists/src/handlers/generate_derive.rs +++ b/crates/ide-assists/src/handlers/generate_derive.rs @@ -1,7 +1,6 @@ use syntax::{ - ast::{self, edit::IndentLevel, AstNode, HasAttrs}, - SyntaxKind::{COMMENT, WHITESPACE}, - TextSize, + ast::{self, edit_in_place::AttrsOwnerEdit, make, AstNode, HasAttrs}, + T, }; use crate::{AssistContext, AssistId, AssistKind, Assists}; @@ -27,48 +26,37 @@ pub(crate) fn generate_derive(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { let cap = ctx.config.snippet_cap?; let nominal = ctx.find_node_at_offset::()?; - let node_start = derive_insertion_offset(&nominal)?; let target = nominal.syntax().text_range(); - acc.add( - AssistId("generate_derive", AssistKind::Generate), - "Add `#[derive]`", - target, - |builder| { - let derive_attr = nominal - .attrs() - .filter_map(|x| x.as_simple_call()) - .filter(|(name, _arg)| name == "derive") - .map(|(_name, arg)| arg) - .next(); - match derive_attr { - None => { - let indent_level = IndentLevel::from_node(nominal.syntax()); - builder.insert_snippet( - cap, - node_start, - format!("#[derive($0)]\n{indent_level}"), - ); - } - Some(tt) => { - // Just move the cursor. - builder.insert_snippet( - cap, - tt.syntax().text_range().end() - TextSize::of(')'), - "$0", - ) - } - }; - }, - ) -} + acc.add(AssistId("generate_derive", AssistKind::Generate), "Add `#[derive]`", target, |edit| { + let derive_attr = nominal + .attrs() + .filter_map(|x| x.as_simple_call()) + .filter(|(name, _arg)| name == "derive") + .map(|(_name, arg)| arg) + .next(); + match derive_attr { + None => { + let derive = make::attr_outer(make::meta_token_tree( + make::ext::ident_path("derive"), + make::token_tree(T!['('], vec![]).clone_for_update(), + )) + .clone_for_update(); -// Insert `derive` after doc comments. -fn derive_insertion_offset(nominal: &ast::Adt) -> Option { - let non_ws_child = nominal - .syntax() - .children_with_tokens() - .find(|it| it.kind() != COMMENT && it.kind() != WHITESPACE)?; - Some(non_ws_child.text_range().start()) + let nominal = edit.make_mut(nominal); + nominal.add_attr(derive.clone()); + + edit.add_tabstop_before_token( + cap, + derive.meta().unwrap().token_tree().unwrap().r_paren_token().unwrap(), + ); + } + Some(tt) => { + // Just move the cursor. + let tt = edit.make_mut(tt); + edit.add_tabstop_before_token(cap, tt.right_delimiter_token().unwrap()); + } + }; + }) } #[cfg(test)] @@ -114,6 +102,38 @@ fn add_derive_existing() { ); } + #[test] + fn add_derive_existing_with_brackets() { + check_assist( + generate_derive, + " +#[derive[Clone]] +struct Foo { a: i32$0, } +", + " +#[derive[Clone$0]] +struct Foo { a: i32, } +", + ); + } + + #[test] + fn add_derive_existing_missing_delimiter() { + // since `#[derive]` isn't a simple attr call (i.e. `#[derive()]`) + // we don't consider it as a proper derive attr and generate a new + // one instead + check_assist( + generate_derive, + " +#[derive] +struct Foo { a: i32$0, }", + " +#[derive] +#[derive($0)] +struct Foo { a: i32, }", + ); + } + #[test] fn add_derive_new_with_doc_comment() { check_assist( diff --git a/crates/ide-assists/src/handlers/wrap_return_type_in_result.rs b/crates/ide-assists/src/handlers/wrap_return_type_in_result.rs index b6c489eb62e..24c3387457a 100644 --- a/crates/ide-assists/src/handlers/wrap_return_type_in_result.rs +++ b/crates/ide-assists/src/handlers/wrap_return_type_in_result.rs @@ -6,7 +6,7 @@ }; use syntax::{ ast::{self, make, Expr}, - match_ast, AstNode, + match_ast, ted, AstNode, }; use crate::{AssistContext, AssistId, AssistKind, Assists}; @@ -52,8 +52,8 @@ pub(crate) fn wrap_return_type_in_result(acc: &mut Assists, ctx: &AssistContext< AssistId("wrap_return_type_in_result", AssistKind::RefactorRewrite), "Wrap return type in Result", type_ref.syntax().text_range(), - |builder| { - let body = ast::Expr::BlockExpr(body); + |edit| { + let body = edit.make_mut(ast::Expr::BlockExpr(body)); let mut exprs_to_wrap = Vec::new(); let tail_cb = &mut |e: &_| tail_cb_impl(&mut exprs_to_wrap, e); @@ -70,17 +70,24 @@ pub(crate) fn wrap_return_type_in_result(acc: &mut Assists, ctx: &AssistContext< let ok_wrapped = make::expr_call( make::expr_path(make::ext::ident_path("Ok")), make::arg_list(iter::once(ret_expr_arg.clone())), - ); - builder.replace_ast(ret_expr_arg, ok_wrapped); + ) + .clone_for_update(); + ted::replace(ret_expr_arg.syntax(), ok_wrapped.syntax()); } - match ctx.config.snippet_cap { - Some(cap) => { - let snippet = format!("Result<{type_ref}, ${{0:_}}>"); - builder.replace_snippet(cap, type_ref.syntax().text_range(), snippet) - } - None => builder - .replace(type_ref.syntax().text_range(), format!("Result<{type_ref}, _>")), + let new_result_ty = + make::ext::ty_result(type_ref.clone(), make::ty_placeholder()).clone_for_update(); + let old_result_ty = edit.make_mut(type_ref.clone()); + + ted::replace(old_result_ty.syntax(), new_result_ty.syntax()); + + if let Some(cap) = ctx.config.snippet_cap { + let generic_args = new_result_ty + .syntax() + .descendants() + .find_map(ast::GenericArgList::cast) + .unwrap(); + edit.add_placeholder_snippet(cap, generic_args.generic_args().last().unwrap()); } }, ) diff --git a/crates/ide-db/src/source_change.rs b/crates/ide-db/src/source_change.rs index 061fb0f05cf..fad0ca51a02 100644 --- a/crates/ide-db/src/source_change.rs +++ b/crates/ide-db/src/source_change.rs @@ -9,7 +9,10 @@ use base_db::{AnchoredPathBuf, FileId}; use nohash_hasher::IntMap; use stdx::never; -use syntax::{algo, ast, ted, AstNode, SyntaxNode, SyntaxNodePtr, TextRange, TextSize}; +use syntax::{ + algo, ast, ted, AstNode, SyntaxElement, SyntaxNode, SyntaxNodePtr, SyntaxToken, TextRange, + TextSize, +}; use text_edit::{TextEdit, TextEditBuilder}; #[derive(Default, Debug, Clone)] @@ -237,19 +240,31 @@ pub fn trigger_signature_help(&mut self) { /// Adds a tabstop snippet to place the cursor before `node` pub fn add_tabstop_before(&mut self, _cap: SnippetCap, node: impl AstNode) { assert!(node.syntax().parent().is_some()); - self.add_snippet(PlaceSnippet::Before(node.syntax().clone())); + self.add_snippet(PlaceSnippet::Before(node.syntax().clone().into())); } /// Adds a tabstop snippet to place the cursor after `node` pub fn add_tabstop_after(&mut self, _cap: SnippetCap, node: impl AstNode) { assert!(node.syntax().parent().is_some()); - self.add_snippet(PlaceSnippet::After(node.syntax().clone())); + self.add_snippet(PlaceSnippet::After(node.syntax().clone().into())); + } + + /// Adds a tabstop snippet to place the cursor before `token` + pub fn add_tabstop_before_token(&mut self, _cap: SnippetCap, token: SyntaxToken) { + assert!(token.parent().is_some()); + self.add_snippet(PlaceSnippet::Before(token.clone().into())); + } + + /// Adds a tabstop snippet to place the cursor after `token` + pub fn add_tabstop_after_token(&mut self, _cap: SnippetCap, token: SyntaxToken) { + assert!(token.parent().is_some()); + self.add_snippet(PlaceSnippet::After(token.clone().into())); } /// Adds a snippet to move the cursor selected over `node` pub fn add_placeholder_snippet(&mut self, _cap: SnippetCap, node: impl AstNode) { assert!(node.syntax().parent().is_some()); - self.add_snippet(PlaceSnippet::Over(node.syntax().clone())) + self.add_snippet(PlaceSnippet::Over(node.syntax().clone().into())) } fn add_snippet(&mut self, snippet: PlaceSnippet) { @@ -282,38 +297,40 @@ fn from(edit: FileSystemEdit) -> SourceChange { } enum PlaceSnippet { - /// Place a tabstop before a node - Before(SyntaxNode), - /// Place a tabstop before a node - After(SyntaxNode), - /// Place a placeholder snippet in place of the node - Over(SyntaxNode), + /// Place a tabstop before an element + Before(SyntaxElement), + /// Place a tabstop before an element + After(SyntaxElement), + /// Place a placeholder snippet in place of the element + Over(SyntaxElement), } impl PlaceSnippet { - /// Places the snippet before or over a node with the given tab stop index + /// Places the snippet before or over an element with the given tab stop index fn place(self, order: usize) { - // ensure the target node is still attached + // ensure the target element is still attached match &self { - PlaceSnippet::Before(node) | PlaceSnippet::After(node) | PlaceSnippet::Over(node) => { - // node should still be in the tree, but if it isn't + PlaceSnippet::Before(element) + | PlaceSnippet::After(element) + | PlaceSnippet::Over(element) => { + // element should still be in the tree, but if it isn't // then it's okay to just ignore this place - if stdx::never!(node.parent().is_none()) { + if stdx::never!(element.parent().is_none()) { return; } } } match self { - PlaceSnippet::Before(node) => { - ted::insert_raw(ted::Position::before(&node), Self::make_tab_stop(order)); + PlaceSnippet::Before(element) => { + ted::insert_raw(ted::Position::before(&element), Self::make_tab_stop(order)); } - PlaceSnippet::After(node) => { - ted::insert_raw(ted::Position::after(&node), Self::make_tab_stop(order)); + PlaceSnippet::After(element) => { + ted::insert_raw(ted::Position::after(&element), Self::make_tab_stop(order)); } - PlaceSnippet::Over(node) => { - let position = ted::Position::before(&node); - node.detach(); + PlaceSnippet::Over(element) => { + let position = ted::Position::before(&element); + element.detach(); let snippet = ast::SourceFile::parse(&format!("${{{order}:_}}")) .syntax_node() @@ -321,7 +338,7 @@ fn place(self, order: usize) { let placeholder = snippet.descendants().find_map(ast::UnderscoreExpr::cast).unwrap(); - ted::replace(placeholder.syntax(), node); + ted::replace(placeholder.syntax(), element); ted::insert_raw(position, snippet); } diff --git a/crates/syntax/src/ast/edit_in_place.rs b/crates/syntax/src/ast/edit_in_place.rs index 5919609d91e..1f5ed206b4a 100644 --- a/crates/syntax/src/ast/edit_in_place.rs +++ b/crates/syntax/src/ast/edit_in_place.rs @@ -213,6 +213,28 @@ fn remove_attrs_and_docs(node: &SyntaxNode) { } } } + + fn add_attr(&self, attr: ast::Attr) { + add_attr(self.syntax(), attr); + + fn add_attr(node: &SyntaxNode, attr: ast::Attr) { + let indent = IndentLevel::from_node(node); + attr.reindent_to(indent); + + let after_attrs_and_comments = node + .children_with_tokens() + .find(|it| !matches!(it.kind(), WHITESPACE | COMMENT | ATTR)) + .map_or(Position::first_child_of(node), |it| Position::before(it)); + + ted::insert_all( + after_attrs_and_comments, + vec![ + attr.syntax().clone().into(), + make::tokens::whitespace(&format!("\n{indent}")).into(), + ], + ) + } + } } impl AttrsOwnerEdit for T {} diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index 3facd90a11d..4c6db0ef06c 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -10,6 +10,8 @@ //! `parse(format!())` we use internally is an implementation detail -- long //! term, it will be replaced with direct tree manipulation. use itertools::Itertools; +use parser::T; +use rowan::NodeOrToken; use stdx::{format_to, never}; use crate::{ast, utils::is_raw_identifier, AstNode, SourceFile, SyntaxKind, SyntaxToken}; @@ -1030,6 +1032,41 @@ pub fn struct_( ast_from_text(&format!("{visibility}struct {strukt_name}{type_params}{field_list}{semicolon}",)) } +pub fn attr_outer(meta: ast::Meta) -> ast::Attr { + ast_from_text(&format!("#[{meta}]")) +} + +pub fn attr_inner(meta: ast::Meta) -> ast::Attr { + ast_from_text(&format!("#![{meta}]")) +} + +pub fn meta_expr(path: ast::Path, expr: ast::Expr) -> ast::Meta { + ast_from_text(&format!("#[{path} = {expr}]")) +} + +pub fn meta_token_tree(path: ast::Path, tt: ast::TokenTree) -> ast::Meta { + ast_from_text(&format!("#[{path}{tt}]")) +} + +pub fn meta_path(path: ast::Path) -> ast::Meta { + ast_from_text(&format!("#[{path}]")) +} + +pub fn token_tree( + delimiter: SyntaxKind, + tt: Vec>, +) -> ast::TokenTree { + let (l_delimiter, r_delimiter) = match delimiter { + T!['('] => ('(', ')'), + T!['['] => ('[', ']'), + T!['{'] => ('{', '}'), + _ => panic!("invalid delimiter `{delimiter:?}`"), + }; + let tt = tt.into_iter().join(""); + + ast_from_text(&format!("tt!{l_delimiter}{tt}{r_delimiter}")) +} + #[track_caller] fn ast_from_text(text: &str) -> N { let parse = SourceFile::parse(text);