From f1b6e45fbacc8683a0a7da6e338261b943188849 Mon Sep 17 00:00:00 2001
From: Jonas Schievink <jonas.schievink@ferrous-systems.com>
Date: Mon, 16 May 2022 19:10:38 +0200
Subject: [PATCH] Handle getters and setters in documentation template assist
---
crates/hir/src/lib.rs | 1 +
.../generate_documentation_template.rs | 265 ++++++++++++++++--
.../src/handlers/generate_getter.rs | 35 +--
crates/ide-assists/src/tests/generated.rs | 2 -
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 4f8fc6bf5674..12e06bf4aca4 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 3eaa445d322d..7864ca2a4e53 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 9513c112f32e..fead5c9a1238 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 eed50a8562db..099d7a43a7f2 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 0add9507185c..1231cf64a017 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