Repository: rust-lang/rust-analyzer
Pull request #12274 (2022-05-16 17:14:04)

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


Patch:
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.
@@ -1220,6 +1258,197 @@ impl<T> MyGenericStruct<T> {
         self.x = new_value;
     }
 }
+"#,
+        );
+    }
+
+    #[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/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


Test patch:
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