Unnamed repository; edit this file 'description' to name the repository.
Auto merge of #12274 - jonas-schievink:move-getter-docs-generation, r=jonas-schievink
feat: Handle getters and setters in documentation template assist
The assist can now turn this:
```rust
pub struct S;
impl S {
pub fn data_mut$0(&mut self) -> &mut [u8] { &mut [] }
}
```
into
```rust
pub struct S;
impl S {
/// Returns a mutable reference to the data.
///
/// # Examples
///
/// ```
/// use test::S;
///
/// let mut s = ;
/// assert_eq!(s.data_mut(), );
/// assert_eq!(s, );
/// ```
pub fn data_mut(&mut self) -> &mut [u8] { &mut [] }
}
```
And similarly for by-value or immutable getters, and for setters. Previously the intro line would be empty.
This PR also removes the documentation generation function from the "Generate getter/setter" assist, since that is better handled by applying the 2 assists in sequence. cc https://github.com/rust-lang/rust-analyzer/issues/12273
| -rw-r--r-- | crates/hir/src/lib.rs | 1 | ||||
| -rw-r--r-- | crates/ide-assists/src/handlers/generate_documentation_template.rs | 265 | ||||
| -rw-r--r-- | crates/ide-assists/src/handlers/generate_getter.rs | 35 | ||||
| -rw-r--r-- | crates/ide-assists/src/tests/generated.rs | 2 | ||||
| -rw-r--r-- | crates/ide-assists/src/utils.rs | 4 |
5 files changed, 252 insertions, 55 deletions
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 4f8fc6bf56..12e06bf4ac 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -1466,6 +1466,7 @@ impl Function { } // Note: logically, this belongs to `hir_ty`, but we are not using it there yet. +#[derive(Clone, Copy, PartialEq, Eq)] pub enum Access { Shared, Exclusive, diff --git a/crates/ide-assists/src/handlers/generate_documentation_template.rs b/crates/ide-assists/src/handlers/generate_documentation_template.rs index 3eaa445d32..7864ca2a4e 100644 --- a/crates/ide-assists/src/handlers/generate_documentation_template.rs +++ b/crates/ide-assists/src/handlers/generate_documentation_template.rs @@ -60,7 +60,7 @@ pub(crate) fn generate_documentation_template( text_range, |builder| { // Introduction / short function description before the sections - let mut doc_lines = vec![introduction_builder(&ast_func, ctx)]; + let mut doc_lines = vec![introduction_builder(&ast_func, ctx).unwrap_or(".".into())]; // Then come the sections if let Some(mut lines) = examples_builder(&ast_func, ctx) { doc_lines.push("".into()); @@ -78,26 +78,64 @@ pub(crate) fn generate_documentation_template( } /// Builds an introduction, trying to be smart if the function is `::new()` -fn introduction_builder(ast_func: &ast::Fn, ctx: &AssistContext) -> String { - || -> Option<String> { - let hir_func = ctx.sema.to_def(ast_func)?; - let container = hir_func.as_assoc_item(ctx.db())?.container(ctx.db()); - if let hir::AssocItemContainer::Impl(implementation) = container { - let ret_ty = hir_func.ret_type(ctx.db()); - let self_ty = implementation.self_ty(ctx.db()); - - let is_new = ast_func.name()?.to_string() == "new"; - match is_new && ret_ty == self_ty { - true => { - Some(format!("Creates a new [`{}`].", self_type_without_lifetimes(ast_func)?)) +fn introduction_builder(ast_func: &ast::Fn, ctx: &AssistContext) -> Option<String> { + let hir_func = ctx.sema.to_def(ast_func)?; + let container = hir_func.as_assoc_item(ctx.db())?.container(ctx.db()); + if let hir::AssocItemContainer::Impl(imp) = container { + let ret_ty = hir_func.ret_type(ctx.db()); + let self_ty = imp.self_ty(ctx.db()); + let name = ast_func.name()?.to_string(); + + let intro_for_new = || { + let is_new = name == "new"; + if is_new && ret_ty == self_ty { + Some(format!("Creates a new [`{}`].", self_type_without_lifetimes(ast_func)?)) + } else { + None + } + }; + + let intro_for_getter = || match ( + hir_func.self_param(ctx.sema.db), + &*hir_func.params_without_self(ctx.sema.db), + ) { + (Some(self_param), []) if self_param.access(ctx.sema.db) != hir::Access::Owned => { + if name.starts_with("as_") || name.starts_with("to_") || name == "get" { + return None; } - false => None, + let what = name.trim_end_matches("_mut").replace('_', " "); + let reference = if ret_ty.is_mutable_reference() { + " a mutable reference to" + } else if ret_ty.is_reference() { + " a reference to" + } else { + "" + }; + Some(format!("Returns{reference} the {what}.")) } - } else { - None + _ => None, + }; + + let intro_for_setter = || { + if !name.starts_with("set_") { + return None; + } + + let what = name.trim_start_matches("set_").replace('_', " "); + Some(format!("Sets the {what}.")) + }; + + if let Some(intro) = intro_for_new() { + return Some(intro); + } + if let Some(intro) = intro_for_getter() { + return Some(intro); } - }() - .unwrap_or_else(|| ".".into()) + if let Some(intro) = intro_for_setter() { + return Some(intro); + } + } + None } /// Builds an `# Examples` section. An option is returned to be able to manage an error in the AST. @@ -1223,4 +1261,195 @@ impl<T> MyGenericStruct<T> { "#, ); } + + #[test] + fn generates_intro_for_getters() { + check_assist( + generate_documentation_template, + r#" +pub struct S; +impl S { + pub fn speed$0(&self) -> f32 { 0.0 } +} +"#, + r#" +pub struct S; +impl S { + /// Returns the speed. + /// + /// # Examples + /// + /// ``` + /// use test::S; + /// + /// let s = ; + /// assert_eq!(s.speed(), ); + /// ``` + pub fn speed(&self) -> f32 { 0.0 } +} +"#, + ); + check_assist( + generate_documentation_template, + r#" +pub struct S; +impl S { + pub fn data$0(&self) -> &[u8] { &[] } +} +"#, + r#" +pub struct S; +impl S { + /// Returns a reference to the data. + /// + /// # Examples + /// + /// ``` + /// use test::S; + /// + /// let s = ; + /// assert_eq!(s.data(), ); + /// ``` + pub fn data(&self) -> &[u8] { &[] } +} +"#, + ); + check_assist( + generate_documentation_template, + r#" +pub struct S; +impl S { + pub fn data$0(&mut self) -> &mut [u8] { &mut [] } +} +"#, + r#" +pub struct S; +impl S { + /// Returns a mutable reference to the data. + /// + /// # Examples + /// + /// ``` + /// use test::S; + /// + /// let mut s = ; + /// assert_eq!(s.data(), ); + /// assert_eq!(s, ); + /// ``` + pub fn data(&mut self) -> &mut [u8] { &mut [] } +} +"#, + ); + check_assist( + generate_documentation_template, + r#" +pub struct S; +impl S { + pub fn data_mut$0(&mut self) -> &mut [u8] { &mut [] } +} +"#, + r#" +pub struct S; +impl S { + /// Returns a mutable reference to the data. + /// + /// # Examples + /// + /// ``` + /// use test::S; + /// + /// let mut s = ; + /// assert_eq!(s.data_mut(), ); + /// assert_eq!(s, ); + /// ``` + pub fn data_mut(&mut self) -> &mut [u8] { &mut [] } +} +"#, + ); + } + + #[test] + fn no_getter_intro_for_prefixed_methods() { + check_assist( + generate_documentation_template, + r#" +pub struct S; +impl S { + pub fn as_bytes$0(&self) -> &[u8] { &[] } +} +"#, + r#" +pub struct S; +impl S { + /// . + /// + /// # Examples + /// + /// ``` + /// use test::S; + /// + /// let s = ; + /// assert_eq!(s.as_bytes(), ); + /// ``` + pub fn as_bytes(&self) -> &[u8] { &[] } +} +"#, + ); + } + + #[test] + fn generates_intro_for_setters() { + check_assist( + generate_documentation_template, + r#" +pub struct S; +impl S { + pub fn set_data$0(&mut self, data: Vec<u8>) {} +} +"#, + r#" +pub struct S; +impl S { + /// Sets the data. + /// + /// # Examples + /// + /// ``` + /// use test::S; + /// + /// let mut s = ; + /// s.set_data(data); + /// assert_eq!(s, ); + /// ``` + pub fn set_data(&mut self, data: Vec<u8>) {} +} +"#, + ); + check_assist( + generate_documentation_template, + r#" +pub struct S; +impl S { + pub fn set_domain_name$0(&mut self, name: String) {} +} +"#, + r#" +pub struct S; +impl S { + /// Sets the domain name. + /// + /// # Examples + /// + /// ``` + /// use test::S; + /// + /// let mut s = ; + /// s.set_domain_name(name); + /// assert_eq!(s, ); + /// ``` + pub fn set_domain_name(&mut self, name: String) {} +} +"#, + ); + } } diff --git a/crates/ide-assists/src/handlers/generate_getter.rs b/crates/ide-assists/src/handlers/generate_getter.rs index 9513c112f3..fead5c9a12 100644 --- a/crates/ide-assists/src/handlers/generate_getter.rs +++ b/crates/ide-assists/src/handlers/generate_getter.rs @@ -38,7 +38,6 @@ use crate::{ // } // // impl Person { -// /// Get a reference to the person's name. // #[must_use] // fn $0name(&self) -> &str { // self.name.as_ref() @@ -65,7 +64,6 @@ pub(crate) fn generate_getter(acc: &mut Assists, ctx: &AssistContext) -> Option< // } // // impl Person { -// /// Get a mutable reference to the person's name. // #[must_use] // fn $0name_mut(&mut self) -> &mut String { // &mut self.name @@ -84,7 +82,6 @@ pub(crate) fn generate_getter_impl( let strukt = ctx.find_node_at_offset::<ast::Struct>()?; let field = ctx.find_node_at_offset::<ast::RecordField>()?; - let strukt_name = strukt.name()?; let field_name = field.name()?; let field_ty = field.ty()?; @@ -114,12 +111,8 @@ pub(crate) fn generate_getter_impl( } let vis = strukt.visibility().map_or(String::new(), |v| format!("{} ", v)); - let (ty, body, description) = if mutable { - ( - format!("&mut {}", field_ty), - format!("&mut self.{}", field_name), - "a mutable reference to ", - ) + let (ty, body) = if mutable { + (format!("&mut {}", field_ty), format!("&mut self.{}", field_name)) } else { (|| { let krate = ctx.sema.scope(field_ty.syntax())?.krate(); @@ -132,25 +125,18 @@ pub(crate) fn generate_getter_impl( ( conversion.convert_type(ctx.db()), conversion.getter(field_name.to_string()), - if conversion.is_copy() { "" } else { "a reference to " }, ) }) })() - .unwrap_or_else(|| { - (format!("&{}", field_ty), format!("&self.{}", field_name), "a reference to ") - }) + .unwrap_or_else(|| (format!("&{}", field_ty), format!("&self.{}", field_name))) }; format_to!( buf, - " /// Get {}the {}'s {}. - #[must_use] + " #[must_use] {}fn {}(&{}self) -> {} {{ {} }}", - description, - to_lower_snake_case(&strukt_name.to_string()).replace('_', " "), - fn_name.trim_end_matches("_mut").replace('_', " "), vis, fn_name, mutable.then(|| "mut ").unwrap_or_default(), @@ -196,7 +182,6 @@ struct Context { } impl Context { - /// Get a reference to the context's data. #[must_use] fn $0data(&self) -> &Data { &self.data @@ -218,7 +203,6 @@ struct Context { } impl Context { - /// Get a mutable reference to the context's data. #[must_use] fn $0data_mut(&mut self) -> &mut Data { &mut self.data @@ -277,7 +261,6 @@ pub(crate) struct Context { } impl Context { - /// Get a reference to the context's data. #[must_use] pub(crate) fn $0data(&self) -> &Data { &self.data @@ -298,7 +281,6 @@ struct Context { } impl Context { - /// Get a reference to the context's data. #[must_use] fn data(&self) -> &Data { &self.data @@ -312,13 +294,11 @@ struct Context { } impl Context { - /// Get a reference to the context's data. #[must_use] fn data(&self) -> &Data { &self.data } - /// Get a reference to the context's count. #[must_use] fn $0count(&self) -> &usize { &self.count @@ -345,7 +325,6 @@ pub struct String; struct S { foo: String } impl S { - /// Get a reference to the s's foo. #[must_use] fn $0foo(&self) -> &String { &self.foo @@ -370,7 +349,6 @@ struct S { foo: $0bool } struct S { foo: bool } impl S { - /// Get the s's foo. #[must_use] fn $0foo(&self) -> bool { self.foo @@ -404,7 +382,6 @@ impl AsRef<str> for String { struct S { foo: String } impl S { - /// Get a reference to the s's foo. #[must_use] fn $0foo(&self) -> &str { self.foo.as_ref() @@ -442,7 +419,6 @@ impl<T> AsRef<T> for Box<T> { struct S { foo: Box<Sweets> } impl S { - /// Get a reference to the s's foo. #[must_use] fn $0foo(&self) -> &Sweets { self.foo.as_ref() @@ -476,7 +452,6 @@ impl<T> AsRef<[T]> for Vec<T> { struct S { foo: Vec<()> } impl S { - /// Get a reference to the s's foo. #[must_use] fn $0foo(&self) -> &[()] { self.foo.as_ref() @@ -500,7 +475,6 @@ struct Failure; struct S { foo: Option<Failure> } impl S { - /// Get a reference to the s's foo. #[must_use] fn $0foo(&self) -> Option<&Failure> { self.foo.as_ref() @@ -524,7 +498,6 @@ struct Context { } impl Context { - /// Get a reference to the context's data. #[must_use] fn $0data(&self) -> Result<&bool, &i32> { self.data.as_ref() diff --git a/crates/ide-assists/src/tests/generated.rs b/crates/ide-assists/src/tests/generated.rs index eed50a8562..099d7a43a7 100644 --- a/crates/ide-assists/src/tests/generated.rs +++ b/crates/ide-assists/src/tests/generated.rs @@ -1036,7 +1036,6 @@ struct Person { } impl Person { - /// Get a reference to the person's name. #[must_use] fn $0name(&self) -> &str { self.name.as_ref() @@ -1061,7 +1060,6 @@ struct Person { } impl Person { - /// Get a mutable reference to the person's name. #[must_use] fn $0name_mut(&mut self) -> &mut String { &mut self.name diff --git a/crates/ide-assists/src/utils.rs b/crates/ide-assists/src/utils.rs index 0add950718..1231cf64a0 100644 --- a/crates/ide-assists/src/utils.rs +++ b/crates/ide-assists/src/utils.rs @@ -568,10 +568,6 @@ impl ReferenceConversion { | ReferenceConversionType::Result => format!("self.{}.as_ref()", field_name), } } - - pub(crate) fn is_copy(&self) -> bool { - matches!(self.conversion, ReferenceConversionType::Copy) - } } // FIXME: It should return a new hir::Type, but currently constructing new types is too cumbersome |