From 4c8259e210bbb6c6c6e4781ce15d42a2f9c43f22 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 23 May 2021 23:13:35 +0300 Subject: [PATCH 1/4] reduce duplication --- .../src/handlers/generate_getter.rs | 173 ++++++++++++++-- .../src/handlers/generate_getter_mut.rs | 195 ------------------ crates/ide_assists/src/lib.rs | 3 +- crates/ide_assists/src/tests.rs | 2 +- 4 files changed, 162 insertions(+), 211 deletions(-) delete mode 100644 crates/ide_assists/src/handlers/generate_getter_mut.rs diff --git a/crates/ide_assists/src/handlers/generate_getter.rs b/crates/ide_assists/src/handlers/generate_getter.rs index df7d1bb9541..e01985112d3 100644 --- a/crates/ide_assists/src/handlers/generate_getter.rs +++ b/crates/ide_assists/src/handlers/generate_getter.rs @@ -29,6 +29,40 @@ // } // ``` pub(crate) fn generate_getter(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + generate_getter_impl(acc, ctx, false) +} + +// 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<()> { + generate_getter_impl(acc, ctx, true) +} + +pub(crate) fn generate_getter_impl( + acc: &mut Assists, + ctx: &AssistContext, + mutable: bool, +) -> Option<()> { let strukt = ctx.find_node_at_offset::()?; let field = ctx.find_node_at_offset::()?; @@ -37,22 +71,26 @@ pub(crate) fn generate_getter(acc: &mut Assists, ctx: &AssistContext) -> Option< 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 mut fn_name = to_lower_snake_case(&field_name.to_string()); + if mutable { + format_to!(fn_name, "_mut"); + } let impl_def = find_struct_impl(&ctx, &ast::Adt::Struct(strukt.clone()), fn_name.as_str())?; + let (id, label) = if mutable { + ("generate_getter_mut", "Generate a mut getter method") + } else { + ("generate_getter", "Generate a getter method") + }; let target = field.syntax().text_range(); acc.add_group( &GroupLabel("Generate getter/setter".to_owned()), - AssistId("generate_getter", AssistKind::Generate), - "Generate a getter method", + AssistId(id, AssistKind::Generate), + label, 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'); } @@ -60,16 +98,18 @@ pub(crate) fn generate_getter(acc: &mut Assists, ctx: &AssistContext) -> Option< let vis = strukt.visibility().map_or(String::new(), |v| format!("{} ", v)); format_to!( buf, - " /// Get a reference to the {}'s {}. - {}fn {}(&self) -> &{} {{ - &self.{} + " /// Get a {}reference to the {}'s {}. + {}fn {}(&{mut_}self) -> &{mut_}{} {{ + &{mut_}self.{} }}", - strukt_name_spaced, - fn_name_spaced, + mutable.then(|| "mutable ").unwrap_or_default(), + to_lower_snake_case(&strukt_name.to_string()).replace('_', " "), + fn_name.trim_end_matches("_mut").replace('_', " "), vis, fn_name, field_ty, - fn_name, + field_name, + mut_ = mutable.then(|| "mut ").unwrap_or_default(), ); let start_offset = impl_def @@ -190,3 +230,110 @@ fn count(&self) -> &usize { ); } } + +#[cfg(test)] +mod tests_mut { + 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 + } +}"#, + ); + } + + #[test] + fn test_multiple_generate_getter_mut() { + check_assist( + generate_getter_mut, + r#" +struct Context { + data: T, + cou$0nt: usize, +} + +impl Context { + /// Get a mutable reference to the context's data. + fn data_mut(&mut self) -> &mut T { + &mut self.data + } +}"#, + r#" +struct Context { + data: T, + count: usize, +} + +impl Context { + /// Get a mutable reference to the context's data. + fn data_mut(&mut self) -> &mut T { + &mut self.data + } + + /// Get a mutable reference to the context's count. + fn count_mut(&mut self) -> &mut usize { + &mut self.count + } +}"#, + ); + } +} diff --git a/crates/ide_assists/src/handlers/generate_getter_mut.rs b/crates/ide_assists/src/handlers/generate_getter_mut.rs deleted file mode 100644 index 821c2eed546..00000000000 --- a/crates/ide_assists/src/handlers/generate_getter_mut.rs +++ /dev/null @@ -1,195 +0,0 @@ -use stdx::{format_to, to_lower_snake_case}; -use syntax::ast::{self, AstNode, NameOwner, VisibilityOwner}; - -use crate::{ - utils::{find_impl_block_end, find_struct_impl, generate_impl_text}, - AssistContext, AssistId, AssistKind, Assists, GroupLabel, -}; - -// 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_group( - &GroupLabel("Generate getter/setter".to_owned()), - 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_end(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 - } -}"#, - ); - } - - #[test] - fn test_multiple_generate_getter_mut() { - check_assist( - generate_getter_mut, - r#" -struct Context { - data: T, - cou$0nt: usize, -} - -impl Context { - /// Get a mutable reference to the context's data. - fn data_mut(&mut self) -> &mut T { - &mut self.data - } -}"#, - r#" -struct Context { - data: T, - count: usize, -} - -impl Context { - /// Get a mutable reference to the context's data. - fn data_mut(&mut self) -> &mut T { - &mut self.data - } - - /// Get a mutable reference to the context's count. - fn count_mut(&mut self) -> &mut usize { - &mut self.count - } -}"#, - ); - } -} diff --git a/crates/ide_assists/src/lib.rs b/crates/ide_assists/src/lib.rs index 4cd82f8c16d..16af72927b0 100644 --- a/crates/ide_assists/src/lib.rs +++ b/crates/ide_assists/src/lib.rs @@ -206,7 +206,6 @@ mod handlers { mod generate_enum_projection_method; mod generate_from_impl_for_enum; mod generate_function; - mod generate_getter_mut; mod generate_getter; mod generate_impl; mod generate_new; @@ -276,8 +275,8 @@ pub(crate) fn all() -> &'static [Handler] { generate_enum_projection_method::generate_enum_try_into_method, generate_from_impl_for_enum::generate_from_impl_for_enum, generate_function::generate_function, - generate_getter_mut::generate_getter_mut, generate_getter::generate_getter, + generate_getter::generate_getter_mut, generate_impl::generate_impl, generate_new::generate_new, generate_setter::generate_setter, diff --git a/crates/ide_assists/src/tests.rs b/crates/ide_assists/src/tests.rs index 6a9231e0787..2b7c2d581e9 100644 --- a/crates/ide_assists/src/tests.rs +++ b/crates/ide_assists/src/tests.rs @@ -215,8 +215,8 @@ fn assist_order_field_struct() { assert_eq!(assists.next().expect("expected assist").label, "Change visibility to pub(crate)"); assert_eq!(assists.next().expect("expected assist").label, "Generate `Deref` impl using `bar`"); - assert_eq!(assists.next().expect("expected assist").label, "Generate a mut getter method"); 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]`"); } From c06599504bc4fcbb3d5024d9a8bdb14f263bad36 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 23 May 2021 23:15:54 +0300 Subject: [PATCH 2/4] remove duplicate tests --- .../src/handlers/generate_getter.rs | 147 +++++------------- 1 file changed, 35 insertions(+), 112 deletions(-) diff --git a/crates/ide_assists/src/handlers/generate_getter.rs b/crates/ide_assists/src/handlers/generate_getter.rs index e01985112d3..fbd47d76111 100644 --- a/crates/ide_assists/src/handlers/generate_getter.rs +++ b/crates/ide_assists/src/handlers/generate_getter.rs @@ -130,10 +130,6 @@ mod tests { 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( @@ -152,13 +148,33 @@ impl Context { fn data(&self) -> &T { &self.data } +}"#, + ); + + 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_already_implemented() { - check_not_applicable( + check_assist_not_applicable( + generate_getter, r#" struct Context { dat$0a: T, @@ -168,6 +184,20 @@ impl Context { fn data(&self) -> &T { &self.data } +}"#, + ); + + check_assist_not_applicable( + generate_getter_mut, + r#" +struct Context { + dat$0a: T, +} + +impl Context { + fn data_mut(&mut self) -> &mut T { + &mut self.data + } }"#, ); } @@ -230,110 +260,3 @@ fn count(&self) -> &usize { ); } } - -#[cfg(test)] -mod tests_mut { - 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 - } -}"#, - ); - } - - #[test] - fn test_multiple_generate_getter_mut() { - check_assist( - generate_getter_mut, - r#" -struct Context { - data: T, - cou$0nt: usize, -} - -impl Context { - /// Get a mutable reference to the context's data. - fn data_mut(&mut self) -> &mut T { - &mut self.data - } -}"#, - r#" -struct Context { - data: T, - count: usize, -} - -impl Context { - /// Get a mutable reference to the context's data. - fn data_mut(&mut self) -> &mut T { - &mut self.data - } - - /// Get a mutable reference to the context's count. - fn count_mut(&mut self) -> &mut usize { - &mut self.count - } -}"#, - ); - } -} From af54b1e248f377853957ada0270e269bedb577b4 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 23 May 2021 23:19:00 +0300 Subject: [PATCH 3/4] minimize tests --- .../src/handlers/generate_getter.rs | 98 ++++++++++--------- 1 file changed, 54 insertions(+), 44 deletions(-) diff --git a/crates/ide_assists/src/handlers/generate_getter.rs b/crates/ide_assists/src/handlers/generate_getter.rs index fbd47d76111..c7739582446 100644 --- a/crates/ide_assists/src/handlers/generate_getter.rs +++ b/crates/ide_assists/src/handlers/generate_getter.rs @@ -135,39 +135,43 @@ fn test_generate_getter_from_field() { check_assist( generate_getter, r#" -struct Context { - dat$0a: T, -}"#, +struct Context { + dat$0a: Data, +} +"#, r#" -struct Context { - data: T, +struct Context { + data: Data, } -impl Context { +impl Context { /// Get a reference to the context's data. - fn data(&self) -> &T { + fn data(&self) -> &Data { &self.data } -}"#, +} +"#, ); check_assist( generate_getter_mut, r#" -struct Context { - dat$0a: T, -}"#, +struct Context { + dat$0a: Data, +} +"#, r#" -struct Context { - data: T, +struct Context { + data: Data, } -impl Context { +impl Context { /// Get a mutable reference to the context's data. - fn data_mut(&mut self) -> &mut T { + fn data_mut(&mut self) -> &mut Data { &mut self.data } -}"#, +} +"#, ); } @@ -176,29 +180,31 @@ fn test_generate_getter_already_implemented() { check_assist_not_applicable( generate_getter, r#" -struct Context { - dat$0a: T, +struct Context { + dat$0a: Data, } -impl Context { - fn data(&self) -> &T { +impl Context { + fn data(&self) -> &Data { &self.data } -}"#, +} +"#, ); check_assist_not_applicable( generate_getter_mut, r#" -struct Context { - dat$0a: T, +struct Context { + dat$0a: Data, } -impl Context { - fn data_mut(&mut self) -> &mut T { +impl Context { + fn data_mut(&mut self) -> &mut Data { &mut self.data } -}"#, +} +"#, ); } @@ -207,20 +213,22 @@ fn test_generate_getter_from_field_with_visibility_marker() { check_assist( generate_getter, r#" -pub(crate) struct Context { - dat$0a: T, -}"#, +pub(crate) struct Context { + dat$0a: Data, +} +"#, r#" -pub(crate) struct Context { - data: T, +pub(crate) struct Context { + data: Data, } -impl Context { +impl Context { /// Get a reference to the context's data. - pub(crate) fn data(&self) -> &T { + pub(crate) fn data(&self) -> &Data { &self.data } -}"#, +} +"#, ); } @@ -229,26 +237,27 @@ fn test_multiple_generate_getter() { check_assist( generate_getter, r#" -struct Context { - data: T, +struct Context { + data: Data, cou$0nt: usize, } -impl Context { +impl Context { /// Get a reference to the context's data. - fn data(&self) -> &T { + fn data(&self) -> &Data { &self.data } -}"#, +} +"#, r#" -struct Context { - data: T, +struct Context { + data: Data, count: usize, } -impl Context { +impl Context { /// Get a reference to the context's data. - fn data(&self) -> &T { + fn data(&self) -> &Data { &self.data } @@ -256,7 +265,8 @@ fn data(&self) -> &T { fn count(&self) -> &usize { &self.count } -}"#, +} +"#, ); } } From 8696c82777f9992fceadc531536bf90b64cf753d Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 23 May 2021 23:22:38 +0300 Subject: [PATCH 4/4] feat: generate getter assist places the cursor at the generated function --- .../src/handlers/generate_getter.rs | 19 ++++++++++++------- crates/ide_assists/src/tests/generated.rs | 4 ++-- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/crates/ide_assists/src/handlers/generate_getter.rs b/crates/ide_assists/src/handlers/generate_getter.rs index c7739582446..9faaaf284b0 100644 --- a/crates/ide_assists/src/handlers/generate_getter.rs +++ b/crates/ide_assists/src/handlers/generate_getter.rs @@ -23,7 +23,7 @@ // // impl Person { // /// Get a reference to the person's name. -// fn name(&self) -> &String { +// fn $0name(&self) -> &String { // &self.name // } // } @@ -49,7 +49,7 @@ pub(crate) fn generate_getter(acc: &mut Assists, ctx: &AssistContext) -> Option< // // impl Person { // /// Get a mutable reference to the person's name. -// fn name_mut(&mut self) -> &mut String { +// fn $0name_mut(&mut self) -> &mut String { // &mut self.name // } // } @@ -119,7 +119,12 @@ pub(crate) fn generate_getter_impl( strukt.syntax().text_range().end() }); - builder.insert(start_offset, buf); + match ctx.config.snippet_cap { + Some(cap) => { + builder.insert_snippet(cap, start_offset, buf.replacen("fn ", "fn $0", 1)) + } + None => builder.insert(start_offset, buf), + } }, ) } @@ -146,7 +151,7 @@ struct Context { impl Context { /// Get a reference to the context's data. - fn data(&self) -> &Data { + fn $0data(&self) -> &Data { &self.data } } @@ -167,7 +172,7 @@ struct Context { impl Context { /// Get a mutable reference to the context's data. - fn data_mut(&mut self) -> &mut Data { + fn $0data_mut(&mut self) -> &mut Data { &mut self.data } } @@ -224,7 +229,7 @@ pub(crate) struct Context { impl Context { /// Get a reference to the context's data. - pub(crate) fn data(&self) -> &Data { + pub(crate) fn $0data(&self) -> &Data { &self.data } } @@ -262,7 +267,7 @@ fn data(&self) -> &Data { } /// Get a reference to the context's count. - fn count(&self) -> &usize { + fn $0count(&self) -> &usize { &self.count } } diff --git a/crates/ide_assists/src/tests/generated.rs b/crates/ide_assists/src/tests/generated.rs index 4406406a278..ffc915fd4d9 100644 --- a/crates/ide_assists/src/tests/generated.rs +++ b/crates/ide_assists/src/tests/generated.rs @@ -786,7 +786,7 @@ struct Person { impl Person { /// Get a reference to the person's name. - fn name(&self) -> &String { + fn $0name(&self) -> &String { &self.name } } @@ -810,7 +810,7 @@ struct Person { impl Person { /// Get a mutable reference to the person's name. - fn name_mut(&mut self) -> &mut String { + fn $0name_mut(&mut self) -> &mut String { &mut self.name } }