From e8d7bcc35507425f384cff25feb564ac41a5c5a7 Mon Sep 17 00:00:00 2001 From: Yoshua Wuyts Date: Tue, 9 Feb 2021 12:30:13 +0100 Subject: Add getter/setter assists Finish implementing `generate_setter` assists Make `generate_impl_text` util generic generate getter methods Fix getter / setter naming It's now in-line with the Rust API naming guidelines: https://rust-lang.github.io/api-guidelines/naming.html#getter-names-follow-rust-convention-c-getter apply clippy Improve examples --- .../src/handlers/generate_enum_match_method.rs | 14 +- crates/assists/src/handlers/generate_getter.rs | 156 ++++++++++++++++++++ crates/assists/src/handlers/generate_getter_mut.rs | 159 ++++++++++++++++++++ crates/assists/src/handlers/generate_new.rs | 69 ++------- crates/assists/src/handlers/generate_setter.rs | 162 +++++++++++++++++++++ crates/assists/src/lib.rs | 6 + crates/assists/src/tests.rs | 3 + crates/assists/src/tests/generated.rs | 71 +++++++++ crates/assists/src/utils.rs | 31 +++- 9 files changed, 603 insertions(+), 68 deletions(-) create mode 100644 crates/assists/src/handlers/generate_getter.rs create mode 100644 crates/assists/src/handlers/generate_getter_mut.rs create mode 100644 crates/assists/src/handlers/generate_setter.rs diff --git a/crates/assists/src/handlers/generate_enum_match_method.rs b/crates/assists/src/handlers/generate_enum_match_method.rs index 9d6b161c9..c3ff38b66 100644 --- a/crates/assists/src/handlers/generate_enum_match_method.rs +++ b/crates/assists/src/handlers/generate_enum_match_method.rs @@ -4,7 +4,7 @@ use syntax::ast::{self, AstNode, NameOwner}; use test_utils::mark; use crate::{ - utils::{find_impl_block, find_struct_impl}, + utils::{find_impl_block, find_struct_impl, generate_impl_text}, AssistContext, AssistId, AssistKind, Assists, }; @@ -82,7 +82,7 @@ pub(crate) fn generate_enum_match_method(acc: &mut Assists, ctx: &AssistContext) let start_offset = impl_def .and_then(|impl_def| find_impl_block(impl_def, &mut buf)) .unwrap_or_else(|| { - buf = generate_impl_text(&parent_enum, &buf); + buf = generate_impl_text(&ast::Adt::Enum(parent_enum.clone()), &buf); parent_enum.syntax().text_range().end() }); @@ -91,16 +91,6 @@ pub(crate) fn generate_enum_match_method(acc: &mut Assists, ctx: &AssistContext) ) } -// Generates the surrounding `impl Type { }` including type and lifetime -// parameters -fn generate_impl_text(strukt: &ast::Enum, code: &str) -> String { - let mut buf = String::with_capacity(code.len()); - buf.push_str("\n\nimpl "); - buf.push_str(strukt.name().unwrap().text()); - format_to!(buf, " {{\n{}\n}}", code); - buf -} - #[cfg(test)] mod tests { use test_utils::mark; diff --git a/crates/assists/src/handlers/generate_getter.rs b/crates/assists/src/handlers/generate_getter.rs new file mode 100644 index 000000000..b63dfce41 --- /dev/null +++ b/crates/assists/src/handlers/generate_getter.rs @@ -0,0 +1,156 @@ +use stdx::{format_to, to_lower_snake_case}; +use syntax::ast::VisibilityOwner; +use syntax::ast::{self, AstNode, NameOwner}; + +use crate::{ + utils::{find_impl_block, find_struct_impl, generate_impl_text}, + AssistContext, AssistId, AssistKind, Assists, +}; + +// Assist: generate_getter +// +// Generate a getter method. +// +// ``` +// struct Person { +// nam$0e: String, +// } +// ``` +// -> +// ``` +// struct Person { +// name: String, +// } +// +// impl Person { +// /// Get a reference to the person's name. +// fn name(&self) -> &String { +// &self.name +// } +// } +// ``` +pub(crate) fn generate_getter(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + let strukt = ctx.find_node_at_offset::()?; + let field = ctx.find_node_at_offset::()?; + + let strukt_name = strukt.name()?; + let field_name = field.name()?; + let field_ty = field.ty()?; + + // Return early if we've found an existing fn + let fn_name = to_lower_snake_case(&field_name.to_string()); + let impl_def = find_struct_impl(&ctx, &ast::Adt::Struct(strukt.clone()), fn_name.as_str())?; + + let target = field.syntax().text_range(); + acc.add( + AssistId("generate_getter", AssistKind::Generate), + "Generate a getter method", + target, + |builder| { + let mut buf = String::with_capacity(512); + + let fn_name_spaced = fn_name.replace('_', " "); + let strukt_name_spaced = + to_lower_snake_case(&strukt_name.to_string()).replace('_', " "); + + if impl_def.is_some() { + buf.push('\n'); + } + + let vis = strukt.visibility().map_or(String::new(), |v| format!("{} ", v)); + format_to!( + buf, + " /// Get a reference to the {}'s {}. + {}fn {}(&self) -> &{} {{ + &self.{} + }}", + strukt_name_spaced, + fn_name_spaced, + vis, + fn_name, + field_ty, + fn_name, + ); + + let start_offset = impl_def + .and_then(|impl_def| find_impl_block(impl_def, &mut buf)) + .unwrap_or_else(|| { + buf = generate_impl_text(&ast::Adt::Struct(strukt.clone()), &buf); + strukt.syntax().text_range().end() + }); + + builder.insert(start_offset, buf); + }, + ) +} + +#[cfg(test)] +mod tests { + use crate::tests::{check_assist, check_assist_not_applicable}; + + use super::*; + + fn check_not_applicable(ra_fixture: &str) { + check_assist_not_applicable(generate_getter, ra_fixture) + } + + #[test] + fn test_generate_getter_from_field() { + check_assist( + generate_getter, + r#" +struct Context { + dat$0a: T, +}"#, + r#" +struct Context { + data: T, +} + +impl Context { + /// Get a reference to the context's data. + fn data(&self) -> &T { + &self.data + } +}"#, + ); + } + + #[test] + fn test_generate_getter_already_implemented() { + check_not_applicable( + r#" +struct Context { + dat$0a: T, +} + +impl Context { + fn data(&self) -> &T { + &self.data + } +}"#, + ); + } + + #[test] + fn test_generate_getter_from_field_with_visibility_marker() { + check_assist( + generate_getter, + r#" +pub(crate) struct Context { + dat$0a: T, +}"#, + r#" +pub(crate) struct Context { + data: T, +} + +impl Context { + /// Get a reference to the context's data. + pub(crate) fn data(&self) -> &T { + &self.data + } +}"#, + ); + } +} diff --git a/crates/assists/src/handlers/generate_getter_mut.rs b/crates/assists/src/handlers/generate_getter_mut.rs new file mode 100644 index 000000000..b5085035e --- /dev/null +++ b/crates/assists/src/handlers/generate_getter_mut.rs @@ -0,0 +1,159 @@ +use stdx::{format_to, to_lower_snake_case}; +use syntax::ast::VisibilityOwner; +use syntax::ast::{self, AstNode, NameOwner}; + +use crate::{ + utils::{find_impl_block, find_struct_impl, generate_impl_text}, + AssistContext, AssistId, AssistKind, Assists, +}; + +// Assist: generate_getter_mut +// +// Generate a mut getter method. +// +// ``` +// struct Person { +// nam$0e: String, +// } +// ``` +// -> +// ``` +// struct Person { +// name: String, +// } +// +// impl Person { +// /// Get a mutable reference to the person's name. +// fn name_mut(&mut self) -> &mut String { +// &mut self.name +// } +// } +// ``` +pub(crate) fn generate_getter_mut(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + let strukt = ctx.find_node_at_offset::()?; + let field = ctx.find_node_at_offset::()?; + + let strukt_name = strukt.name()?; + let field_name = field.name()?; + let field_ty = field.ty()?; + + // Return early if we've found an existing fn + let fn_name = to_lower_snake_case(&field_name.to_string()); + let impl_def = find_struct_impl( + &ctx, + &ast::Adt::Struct(strukt.clone()), + format!("{}_mut", fn_name).as_str(), + )?; + + let target = field.syntax().text_range(); + acc.add( + AssistId("generate_getter_mut", AssistKind::Generate), + "Generate a mut getter method", + target, + |builder| { + let mut buf = String::with_capacity(512); + let fn_name_spaced = fn_name.replace('_', " "); + let strukt_name_spaced = + to_lower_snake_case(&strukt_name.to_string()).replace('_', " "); + + if impl_def.is_some() { + buf.push('\n'); + } + + let vis = strukt.visibility().map_or(String::new(), |v| format!("{} ", v)); + format_to!( + buf, + " /// Get a mutable reference to the {}'s {}. + {}fn {}_mut(&mut self) -> &mut {} {{ + &mut self.{} + }}", + strukt_name_spaced, + fn_name_spaced, + vis, + fn_name, + field_ty, + fn_name, + ); + + let start_offset = impl_def + .and_then(|impl_def| find_impl_block(impl_def, &mut buf)) + .unwrap_or_else(|| { + buf = generate_impl_text(&ast::Adt::Struct(strukt.clone()), &buf); + strukt.syntax().text_range().end() + }); + + builder.insert(start_offset, buf); + }, + ) +} + +#[cfg(test)] +mod tests { + use crate::tests::{check_assist, check_assist_not_applicable}; + + use super::*; + + fn check_not_applicable(ra_fixture: &str) { + check_assist_not_applicable(generate_getter_mut, ra_fixture) + } + + #[test] + fn test_generate_getter_mut_from_field() { + check_assist( + generate_getter_mut, + r#" +struct Context { + dat$0a: T, +}"#, + r#" +struct Context { + data: T, +} + +impl Context { + /// Get a mutable reference to the context's data. + fn data_mut(&mut self) -> &mut T { + &mut self.data + } +}"#, + ); + } + + #[test] + fn test_generate_getter_mut_already_implemented() { + check_not_applicable( + r#" +struct Context { + dat$0a: T, +} + +impl Context { + fn data_mut(&mut self) -> &mut T { + &mut self.data + } +}"#, + ); + } + + #[test] + fn test_generate_getter_mut_from_field_with_visibility_marker() { + check_assist( + generate_getter_mut, + r#" +pub(crate) struct Context { + dat$0a: T, +}"#, + r#" +pub(crate) struct Context { + data: T, +} + +impl Context { + /// Get a mutable reference to the context's data. + pub(crate) fn data_mut(&mut self) -> &mut T { + &mut self.data + } +}"#, + ); + } +} diff --git a/crates/assists/src/handlers/generate_new.rs b/crates/assists/src/handlers/generate_new.rs index a9203d33f..c29077225 100644 --- a/crates/assists/src/handlers/generate_new.rs +++ b/crates/assists/src/handlers/generate_new.rs @@ -1,12 +1,10 @@ +use ast::Adt; use itertools::Itertools; use stdx::format_to; -use syntax::{ - ast::{self, AstNode, GenericParamsOwner, NameOwner, StructKind, VisibilityOwner}, - SmolStr, -}; +use syntax::ast::{self, AstNode, NameOwner, StructKind, VisibilityOwner}; use crate::{ - utils::{find_impl_block, find_struct_impl}, + utils::{find_impl_block, find_struct_impl, generate_impl_text}, AssistContext, AssistId, AssistKind, Assists, }; @@ -28,7 +26,6 @@ use crate::{ // impl Ctx { // fn $0new(data: T) -> Self { Self { data } } // } -// // ``` pub(crate) fn generate_new(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let strukt = ctx.find_node_at_offset::()?; @@ -40,7 +37,7 @@ pub(crate) fn generate_new(acc: &mut Assists, ctx: &AssistContext) -> Option<()> }; // Return early if we've found an existing new fn - let impl_def = find_struct_impl(&ctx, &ast::Adt::Struct(strukt.clone()), "new")?; + let impl_def = find_struct_impl(&ctx, &Adt::Struct(strukt.clone()), "new")?; let target = strukt.syntax().text_range(); acc.add(AssistId("generate_new", AssistKind::Generate), "Generate `new`", target, |builder| { @@ -63,7 +60,7 @@ pub(crate) fn generate_new(acc: &mut Assists, ctx: &AssistContext) -> Option<()> let start_offset = impl_def .and_then(|impl_def| find_impl_block(impl_def, &mut buf)) .unwrap_or_else(|| { - buf = generate_impl_text(&strukt, &buf); + buf = generate_impl_text(&Adt::Struct(strukt.clone()), &buf); strukt.syntax().text_range().end() }); @@ -77,32 +74,6 @@ pub(crate) fn generate_new(acc: &mut Assists, ctx: &AssistContext) -> Option<()> }) } -// Generates the surrounding `impl Type { }` including type and lifetime -// parameters -fn generate_impl_text(strukt: &ast::Struct, code: &str) -> String { - let type_params = strukt.generic_param_list(); - let mut buf = String::with_capacity(code.len()); - buf.push_str("\n\nimpl"); - if let Some(type_params) = &type_params { - format_to!(buf, "{}", type_params.syntax()); - } - buf.push(' '); - buf.push_str(strukt.name().unwrap().text()); - if let Some(type_params) = type_params { - let lifetime_params = type_params - .lifetime_params() - .filter_map(|it| it.lifetime()) - .map(|it| SmolStr::from(it.text())); - let type_params = - type_params.type_params().filter_map(|it| it.name()).map(|it| SmolStr::from(it.text())); - format_to!(buf, "<{}>", lifetime_params.chain(type_params).format(", ")) - } - - format_to!(buf, " {{\n{}\n}}\n", code); - - buf -} - #[cfg(test)] mod tests { use crate::tests::{check_assist, check_assist_not_applicable, check_assist_target}; @@ -120,8 +91,7 @@ mod tests { impl Foo { fn $0new() -> Self { Self { } } -} -", +}", ); check_assist( generate_new, @@ -130,8 +100,7 @@ impl Foo { impl Foo { fn $0new() -> Self { Self { } } -} -", +}", ); check_assist( generate_new, @@ -140,8 +109,7 @@ impl Foo { impl<'a, T: Foo<'a>> Foo<'a, T> { fn $0new() -> Self { Self { } } -} -", +}", ); check_assist( generate_new, @@ -150,8 +118,7 @@ impl<'a, T: Foo<'a>> Foo<'a, T> { impl Foo { fn $0new(baz: String) -> Self { Self { baz } } -} -", +}", ); check_assist( generate_new, @@ -160,8 +127,7 @@ impl Foo { impl Foo { fn $0new(baz: String, qux: Vec) -> Self { Self { baz, qux } } -} -", +}", ); // Check that visibility modifiers don't get brought in for fields @@ -172,8 +138,7 @@ impl Foo { impl Foo { fn $0new(baz: String, qux: Vec) -> Self { Self { baz, qux } } -} -", +}", ); // Check that it reuses existing impls @@ -240,8 +205,7 @@ impl Foo { impl Foo { pub fn $0new() -> Self { Self { } } -} -", +}", ); check_assist( generate_new, @@ -250,8 +214,7 @@ impl Foo { impl Foo { pub(crate) fn $0new() -> Self { Self { } } -} -", +}", ); } @@ -322,8 +285,7 @@ impl Source { pub fn map U, U>(self, f: F) -> Source { Source { file_id: self.file_id, ast: f(self.ast) } } -} -"##, +}"##, r##" pub struct AstId { file_id: HirFileId, @@ -347,8 +309,7 @@ impl Source { pub fn map U, U>(self, f: F) -> Source { Source { file_id: self.file_id, ast: f(self.ast) } } -} -"##, +}"##, ); } } diff --git a/crates/assists/src/handlers/generate_setter.rs b/crates/assists/src/handlers/generate_setter.rs new file mode 100644 index 000000000..c9043a162 --- /dev/null +++ b/crates/assists/src/handlers/generate_setter.rs @@ -0,0 +1,162 @@ +use stdx::{format_to, to_lower_snake_case}; +use syntax::ast::VisibilityOwner; +use syntax::ast::{self, AstNode, NameOwner}; + +use crate::{ + utils::{find_impl_block, find_struct_impl, generate_impl_text}, + AssistContext, AssistId, AssistKind, Assists, +}; + +// Assist: generate_setter +// +// Generate a setter method. +// +// ``` +// struct Person { +// nam$0e: String, +// } +// ``` +// -> +// ``` +// struct Person { +// name: String, +// } +// +// impl Person { +// /// Set the person's name. +// fn set_name(&mut self, name: String) { +// self.name = name; +// } +// } +// ``` +pub(crate) fn generate_setter(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + let strukt = ctx.find_node_at_offset::()?; + let field = ctx.find_node_at_offset::()?; + + let strukt_name = strukt.name()?; + let field_name = field.name()?; + let field_ty = field.ty()?; + + // Return early if we've found an existing fn + let fn_name = to_lower_snake_case(&field_name.to_string()); + let impl_def = find_struct_impl( + &ctx, + &ast::Adt::Struct(strukt.clone()), + format!("set_{}", fn_name).as_str(), + )?; + + let target = field.syntax().text_range(); + acc.add( + AssistId("generate_setter", AssistKind::Generate), + "Generate a setter method", + target, + |builder| { + let mut buf = String::with_capacity(512); + + let fn_name_spaced = fn_name.replace('_', " "); + let strukt_name_spaced = + to_lower_snake_case(&strukt_name.to_string()).replace('_', " "); + + if impl_def.is_some() { + buf.push('\n'); + } + + let vis = strukt.visibility().map_or(String::new(), |v| format!("{} ", v)); + format_to!( + buf, + " /// Set the {}'s {}. + {}fn set_{}(&mut self, {}: {}) {{ + self.{} = {}; + }}", + strukt_name_spaced, + fn_name_spaced, + vis, + fn_name, + fn_name, + field_ty, + fn_name, + fn_name, + ); + + let start_offset = impl_def + .and_then(|impl_def| find_impl_block(impl_def, &mut buf)) + .unwrap_or_else(|| { + buf = generate_impl_text(&ast::Adt::Struct(strukt.clone()), &buf); + strukt.syntax().text_range().end() + }); + + builder.insert(start_offset, buf); + }, + ) +} + +#[cfg(test)] +mod tests { + use crate::tests::{check_assist, check_assist_not_applicable}; + + use super::*; + + fn check_not_applicable(ra_fixture: &str) { + check_assist_not_applicable(generate_setter, ra_fixture) + } + + #[test] + fn test_generate_setter_from_field() { + check_assist( + generate_setter, + r#" +struct Person { + dat$0a: T, +}"#, + r#" +struct Person { + data: T, +} + +impl Person { + /// Set the person's data. + fn set_data(&mut self, data: T) { + self.data = data; + } +}"#, + ); + } + + #[test] + fn test_generate_setter_already_implemented() { + check_not_applicable( + r#" +struct Person { + dat$0a: T, +} + +impl Person { + fn set_data(&mut self, data: T) { + self.data = data; + } +}"#, + ); + } + + #[test] + fn test_generate_setter_from_field_with_visibility_marker() { + check_assist( + generate_setter, + r#" +pub(crate) struct Person { + dat$0a: T, +}"#, + r#" +pub(crate) struct Person { + data: T, +} + +impl Person { + /// Set the person's data. + pub(crate) fn set_data(&mut self, data: T) { + self.data = data; + } +}"#, + ); + } +} diff --git a/crates/assists/src/lib.rs b/crates/assists/src/lib.rs index 83fbf6986..957efa6b9 100644 --- a/crates/assists/src/lib.rs +++ b/crates/assists/src/lib.rs @@ -130,8 +130,11 @@ mod handlers { mod generate_enum_match_method; mod generate_from_impl_for_enum; mod generate_function; + mod generate_getter; + mod generate_getter_mut; mod generate_impl; mod generate_new; + mod generate_setter; mod infer_function_return_type; mod inline_function; mod inline_local_variable; @@ -189,8 +192,11 @@ mod handlers { generate_enum_match_method::generate_enum_match_method, generate_from_impl_for_enum::generate_from_impl_for_enum, generate_function::generate_function, + generate_getter::generate_getter, + generate_getter_mut::generate_getter_mut, generate_impl::generate_impl, generate_new::generate_new, + generate_setter::generate_setter, infer_function_return_type::infer_function_return_type, inline_function::inline_function, inline_local_variable::inline_local_variable, diff --git a/crates/assists/src/tests.rs b/crates/assists/src/tests.rs index 32bd8698b..1bdcb9ede 100644 --- a/crates/assists/src/tests.rs +++ b/crates/assists/src/tests.rs @@ -169,6 +169,9 @@ fn assist_order_field_struct() { let mut assists = assists.iter(); assert_eq!(assists.next().expect("expected assist").label, "Change visibility to pub(crate)"); + assert_eq!(assists.next().expect("expected assist").label, "Generate a getter method"); + assert_eq!(assists.next().expect("expected assist").label, "Generate a mut getter method"); + assert_eq!(assists.next().expect("expected assist").label, "Generate a setter method"); assert_eq!(assists.next().expect("expected assist").label, "Add `#[derive]`"); } diff --git a/crates/assists/src/tests/generated.rs b/crates/assists/src/tests/generated.rs index 0dbb05f2a..6f2b22bc2 100644 --- a/crates/assists/src/tests/generated.rs +++ b/crates/assists/src/tests/generated.rs @@ -533,6 +533,54 @@ fn bar(arg: &str, baz: Baz) ${0:-> ()} { ) } +#[test] +fn doctest_generate_getter() { + check_doc_test( + "generate_getter", + r#####" +struct Person { + nam$0e: String, +} +"#####, + r#####" +struct Person { + name: String, +} + +impl Person { + /// Get a reference to the person's name. + fn name(&self) -> &String { + &self.name + } +} +"#####, + ) +} + +#[test] +fn doctest_generate_getter_mut() { + check_doc_test( + "generate_getter_mut", + r#####" +struct Person { + nam$0e: String, +} +"#####, + r#####" +struct Person { + name: String, +} + +impl Person { + /// Get a mutable reference to the person's name. + fn name_mut(&mut self) -> &mut String { + &mut self.name + } +} +"#####, + ) +} + #[test] fn doctest_generate_impl() { check_doc_test( @@ -571,7 +619,30 @@ struct Ctx { impl Ctx { fn $0new(data: T) -> Self { Self { data } } } +"#####, + ) +} +#[test] +fn doctest_generate_setter() { + check_doc_test( + "generate_setter", + r#####" +struct Person { + nam$0e: String, +} +"#####, + r#####" +struct Person { + name: String, +} + +impl Person { + /// Set the person's name. + fn set_name(&mut self, name: String) { + self.name = name; + } +} "#####, ) } diff --git a/crates/assists/src/utils.rs b/crates/assists/src/utils.rs index cd80c2958..643dade23 100644 --- a/crates/assists/src/utils.rs +++ b/crates/assists/src/utils.rs @@ -5,12 +5,13 @@ use std::ops; use hir::{Adt, HasSource}; use ide_db::{helpers::SnippetCap, RootDatabase}; use itertools::Itertools; +use stdx::format_to; use syntax::{ ast::edit::AstNodeEdit, ast::AttrsOwner, ast::NameOwner, - ast::{self, edit, make, ArgListOwner}, - AstNode, Direction, + ast::{self, edit, make, ArgListOwner, GenericParamsOwner}, + AstNode, Direction, SmolStr, SyntaxKind::*, SyntaxNode, TextSize, T, }; @@ -354,3 +355,29 @@ pub(crate) fn find_impl_block(impl_def: ast::Impl, buf: &mut String) -> Option }` including type and lifetime +// parameters +pub(crate) fn generate_impl_text(adt: &ast::Adt, code: &str) -> String { + let type_params = adt.generic_param_list(); + let mut buf = String::with_capacity(code.len()); + buf.push_str("\n\nimpl"); + if let Some(type_params) = &type_params { + format_to!(buf, "{}", type_params.syntax()); + } + buf.push(' '); + buf.push_str(adt.name().unwrap().text()); + if let Some(type_params) = type_params { + let lifetime_params = type_params + .lifetime_params() + .filter_map(|it| it.lifetime()) + .map(|it| SmolStr::from(it.text())); + let type_params = + type_params.type_params().filter_map(|it| it.name()).map(|it| SmolStr::from(it.text())); + format_to!(buf, "<{}>", lifetime_params.chain(type_params).format(", ")) + } + + format_to!(buf, " {{\n{}\n}}", code); + + buf +} -- cgit v1.2.3