From 4a5341e0448d716e2ead16f89449543b63ee6f78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20ALLART?= Date: Wed, 5 Jan 2022 00:23:36 +0100 Subject: [PATCH 1/3] fix(gen-doc-assist): remove lifetimes in description of `new` --- .../generate_documentation_template.rs | 125 +++++++++++++++++- 1 file changed, 124 insertions(+), 1 deletion(-) diff --git a/crates/ide_assists/src/handlers/generate_documentation_template.rs b/crates/ide_assists/src/handlers/generate_documentation_template.rs index 1481eadb519..4558f06fb51 100644 --- a/crates/ide_assists/src/handlers/generate_documentation_template.rs +++ b/crates/ide_assists/src/handlers/generate_documentation_template.rs @@ -88,7 +88,9 @@ fn introduction_builder(ast_func: &ast::Fn, ctx: &AssistContext) -> String { let is_new = ast_func.name()?.to_string() == "new"; match is_new && ret_ty == self_ty { - true => Some(format!("Creates a new [`{}`].", self_type(ast_func)?)), + true => { + Some(format!("Creates a new [`{}`].", lifetimes_removed(&self_type(ast_func)?))) + } false => None, } } else { @@ -230,6 +232,49 @@ fn self_type(ast_func: &ast::Fn) -> Option { .map(|t| (t.to_string())) } +/// Output the same string as the input, removing lifetimes. +/// +/// Lifetimes are detected as starting with a `'` and ending with `,\s*` or before a `>`. +fn lifetimes_removed(with_lifetimes: &str) -> String { + #[derive(Debug)] + enum State { + OutOfLifetime, + AfterLifetime, + InLifetime, + } + + let mut state = State::OutOfLifetime; + let mut without_lifetimes = String::new(); + for c in with_lifetimes.chars() { + match state { + State::OutOfLifetime => { + if c == '\'' { + state = State::InLifetime; + } else { + without_lifetimes.push(c); + } + } + State::InLifetime => { + if c == ',' { + state = State::AfterLifetime; + } else if c == '>' { + without_lifetimes.push(c); + state = State::OutOfLifetime; + } + } + State::AfterLifetime => { + if c == '\'' { + state = State::InLifetime; + } else if !c.is_whitespace() { + without_lifetimes.push(c); + state = State::OutOfLifetime; + } + } + } + } + without_lifetimes +} + /// Heper function to get the name of the type of `self` without generic arguments fn self_partial_type(ast_func: &ast::Fn) -> Option { let mut self_type = self_type(ast_func)?; @@ -991,6 +1036,84 @@ impl MyGenericStruct { ); } + #[test] + fn removes_one_lifetime_from_description() { + check_assist( + generate_documentation_template, + r#" +#[derive(Debug, PartialEq)] +pub struct MyGenericStruct<'a, T> { + pub x: &'a T, +} +impl<'a, T> MyGenericStruct<'a, T> { + pub fn new$0(x: &'a T) -> Self { + MyGenericStruct { x } + } +} +"#, + r#" +#[derive(Debug, PartialEq)] +pub struct MyGenericStruct<'a, T> { + pub x: &'a T, +} +impl<'a, T> MyGenericStruct<'a, T> { + /// Creates a new [`MyGenericStruct`]. + /// + /// # Examples + /// + /// ``` + /// use test::MyGenericStruct; + /// + /// assert_eq!(MyGenericStruct::new(x), ); + /// ``` + pub fn new(x: &'a T) -> Self { + MyGenericStruct { x } + } +} +"#, + ); + } + + #[test] + fn removes_all_lifetimes_from_description() { + check_assist( + generate_documentation_template, + r#" +#[derive(Debug, PartialEq)] +pub struct MyGenericStruct<'a, 'b, T> { + pub x: &'a T, + pub y: &'b T, +} +impl<'a, 'b, T> MyGenericStruct<'a, 'b, T> { + pub fn new$0(x: &'a T, y: &'b T) -> Self { + MyGenericStruct { x, y } + } +} +"#, + r#" +#[derive(Debug, PartialEq)] +pub struct MyGenericStruct<'a, 'b, T> { + pub x: &'a T, + pub y: &'b T, +} +impl<'a, 'b, T> MyGenericStruct<'a, 'b, T> { + /// Creates a new [`MyGenericStruct`]. + /// + /// # Examples + /// + /// ``` + /// use test::MyGenericStruct; + /// + /// assert_eq!(MyGenericStruct::new(x, y), ); + /// ``` + pub fn new(x: &'a T, y: &'b T) -> Self { + MyGenericStruct { x, y } + } +} +"#, + ); + } + #[test] fn detects_new_with_self() { check_assist( From c2d3f908867335e393ac549a9e9c18d6c75f2996 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20ALLART?= Date: Thu, 6 Jan 2022 01:51:04 +0100 Subject: [PATCH 2/3] fix: remove brackets if no generic types --- .../generate_documentation_template.rs | 109 ++++++++++-------- 1 file changed, 60 insertions(+), 49 deletions(-) diff --git a/crates/ide_assists/src/handlers/generate_documentation_template.rs b/crates/ide_assists/src/handlers/generate_documentation_template.rs index 4558f06fb51..97d1c2bd6d8 100644 --- a/crates/ide_assists/src/handlers/generate_documentation_template.rs +++ b/crates/ide_assists/src/handlers/generate_documentation_template.rs @@ -89,7 +89,7 @@ fn introduction_builder(ast_func: &ast::Fn, ctx: &AssistContext) -> String { let is_new = ast_func.name()?.to_string() == "new"; match is_new && ret_ty == self_ty { true => { - Some(format!("Creates a new [`{}`].", lifetimes_removed(&self_type(ast_func)?))) + Some(format!("Creates a new [`{}`].", self_type_without_lifetimes(ast_func)?)) } false => None, } @@ -223,61 +223,32 @@ fn self_name(ast_func: &ast::Fn) -> Option { } /// Heper function to get the name of the type of `self` -fn self_type(ast_func: &ast::Fn) -> Option { - ast_func - .syntax() - .ancestors() - .find_map(ast::Impl::cast) - .and_then(|i| i.self_ty()) - .map(|t| (t.to_string())) +fn self_type(ast_func: &ast::Fn) -> Option { + ast_func.syntax().ancestors().find_map(ast::Impl::cast).and_then(|i| i.self_ty()) } -/// Output the same string as the input, removing lifetimes. -/// -/// Lifetimes are detected as starting with a `'` and ending with `,\s*` or before a `>`. -fn lifetimes_removed(with_lifetimes: &str) -> String { - #[derive(Debug)] - enum State { - OutOfLifetime, - AfterLifetime, - InLifetime, +/// Output the real name of `Self` like `MyType`, without the lifetimes. +fn self_type_without_lifetimes(ast_func: &ast::Fn) -> Option { + let path_segment = + ast::PathType::cast(self_type(ast_func)?.syntax().clone())?.path()?.segment()?; + let mut name = path_segment.name_ref()?.to_string(); + let generics = path_segment + .generic_arg_list()? + .generic_args() + .filter(|generic| matches!(generic, ast::GenericArg::TypeArg(_))) + .map(|generic| generic.to_string()); + let generics: String = Itertools::intersperse(generics, ", ".to_string()).collect(); + if !generics.is_empty() { + name.push('<'); + name.push_str(&generics); + name.push('>'); } - - let mut state = State::OutOfLifetime; - let mut without_lifetimes = String::new(); - for c in with_lifetimes.chars() { - match state { - State::OutOfLifetime => { - if c == '\'' { - state = State::InLifetime; - } else { - without_lifetimes.push(c); - } - } - State::InLifetime => { - if c == ',' { - state = State::AfterLifetime; - } else if c == '>' { - without_lifetimes.push(c); - state = State::OutOfLifetime; - } - } - State::AfterLifetime => { - if c == '\'' { - state = State::InLifetime; - } else if !c.is_whitespace() { - without_lifetimes.push(c); - state = State::OutOfLifetime; - } - } - } - } - without_lifetimes + Some(name) } /// Heper function to get the name of the type of `self` without generic arguments fn self_partial_type(ast_func: &ast::Fn) -> Option { - let mut self_type = self_type(ast_func)?; + let mut self_type = self_type(ast_func)?.to_string(); if let Some(idx) = self_type.find(|c| ['<', ' '].contains(&c)) { self_type.truncate(idx); } @@ -1114,6 +1085,46 @@ impl<'a, 'b, T> MyGenericStruct<'a, 'b, T> { ); } + #[test] + fn removes_all_lifetimes_and_brackets_from_description() { + check_assist( + generate_documentation_template, + r#" +#[derive(Debug, PartialEq)] +pub struct MyGenericStruct<'a, 'b> { + pub x: &'a usize, + pub y: &'b usize, +} +impl<'a, 'b> MyGenericStruct<'a, 'b> { + pub fn new$0(x: &'a usize, y: &'b usize) -> Self { + MyGenericStruct { x, y } + } +} +"#, + r#" +#[derive(Debug, PartialEq)] +pub struct MyGenericStruct<'a, 'b> { + pub x: &'a usize, + pub y: &'b usize, +} +impl<'a, 'b> MyGenericStruct<'a, 'b> { + /// Creates a new [`MyGenericStruct`]. + /// + /// # Examples + /// + /// ``` + /// use test::MyGenericStruct; + /// + /// assert_eq!(MyGenericStruct::new(x, y), ); + /// ``` + pub fn new(x: &'a usize, y: &'b usize) -> Self { + MyGenericStruct { x, y } + } +} +"#, + ); + } + #[test] fn detects_new_with_self() { check_assist( From 1b5c60f5491d8f992d89734e3027d5ba01fabfab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20ALLART?= Date: Fri, 7 Jan 2022 14:04:03 +0100 Subject: [PATCH 3/3] refactor: apply suggestions See PR #11194 --- .../src/handlers/generate_documentation_template.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/crates/ide_assists/src/handlers/generate_documentation_template.rs b/crates/ide_assists/src/handlers/generate_documentation_template.rs index 97d1c2bd6d8..bb7c8b10102 100644 --- a/crates/ide_assists/src/handlers/generate_documentation_template.rs +++ b/crates/ide_assists/src/handlers/generate_documentation_template.rs @@ -229,15 +229,17 @@ fn self_type(ast_func: &ast::Fn) -> Option { /// Output the real name of `Self` like `MyType`, without the lifetimes. fn self_type_without_lifetimes(ast_func: &ast::Fn) -> Option { - let path_segment = - ast::PathType::cast(self_type(ast_func)?.syntax().clone())?.path()?.segment()?; + let path_segment = match self_type(ast_func)? { + ast::Type::PathType(path_type) => path_type.path()?.segment()?, + _ => return None, + }; let mut name = path_segment.name_ref()?.to_string(); let generics = path_segment .generic_arg_list()? .generic_args() .filter(|generic| matches!(generic, ast::GenericArg::TypeArg(_))) .map(|generic| generic.to_string()); - let generics: String = Itertools::intersperse(generics, ", ".to_string()).collect(); + let generics: String = generics.format(", ").to_string(); if !generics.is_empty() { name.push('<'); name.push_str(&generics); @@ -325,7 +327,7 @@ fn arguments_from_params(param_list: &ast::ParamList) -> String { }, _ => "_".to_string(), }); - Itertools::intersperse(args_iter, ", ".to_string()).collect() + args_iter.format(", ").to_string() } /// Helper function to build a function call. `None` if expected `self_name` was not provided