From 998b91af9c5bc4d1515f4a219cbcb8e941ab6eef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20ALLART?= Date: Mon, 6 Dec 2021 18:32:25 +0100 Subject: [PATCH 01/12] feat: assist to generate documentation templates --- .../generate_documentation_template.rs | 970 ++++++++++++++++++ crates/ide_assists/src/lib.rs | 2 + 2 files changed, 972 insertions(+) create mode 100644 crates/ide_assists/src/handlers/generate_documentation_template.rs diff --git a/crates/ide_assists/src/handlers/generate_documentation_template.rs b/crates/ide_assists/src/handlers/generate_documentation_template.rs new file mode 100644 index 0000000000..3abdddbb8c --- /dev/null +++ b/crates/ide_assists/src/handlers/generate_documentation_template.rs @@ -0,0 +1,970 @@ +use ide_db::assists::{AssistId, AssistKind}; +use stdx::to_lower_snake_case; +use syntax::{ + ast::{self, edit::IndentLevel, HasDocComments, HasName}, + AstNode, +}; + +use crate::assist_context::{AssistContext, Assists}; + +/// Assist: generate_documentation_template +/// +/// Adds a documentation template above a function definition / declaration +/// +/// ``` +/// fn my_func(a: i32, b: i32) -> Result<(), std::io::Error> { +/// unimplemented!() +/// } +/// ``` +/// -> +/// ``` +/// /// . +/// /// +/// /// # Examples +/// /// +/// /// ```rust +/// /// use my_crate::my_func; +/// /// +/// /// let result = my_func(a, b); +/// /// assert_eq!(result, ); +/// /// ``` +/// /// +/// /// # Errors +/// /// +/// /// This function will return an error if . +/// fn my_func(a: i32, b: i32) -> Result<(), std::io::Error> { +/// unimplemented!() +/// } +/// ``` +pub(crate) fn generate_documentation_template( + acc: &mut Assists, + ctx: &AssistContext, +) -> Option<()> { + let name = ctx.find_node_at_offset::()?; + let ast_func = ast::Fn::cast(name.syntax().parent()?)?; + if is_in_trait_impl(&ast_func) { + return None; + } + // TODO disable at least examples if function not public, as the example will fail to build on + // `cargo test`. What is the exact criteria of `pub`ness? All parent modules must be `pub`, for + // `impl { fn }` both `fn` and `struct`* must be public. + // + // What about `pub(crate)`? + // + // *: Seems complex but maybe ignoring this criteria can be ignored. + + let parent_syntax = ast_func.syntax(); + let text_range = parent_syntax.text_range(); + let indent_level = IndentLevel::from_node(&parent_syntax); + let krate_name = + ctx.sema.scope(&parent_syntax).module()?.krate().display_name(ctx.db())?.to_string(); + + acc.add( + AssistId("generate_documentation_template", AssistKind::Generate), + "Generate a documentation template", + text_range, + |builder| { + let mut doc_lines = Vec::new(); + // Introduction / short function description before the sections + doc_lines.push(introduction_builder(&ast_func)); + // Then come the sections + if let Some(mut lines) = examples_builder(&ast_func, krate_name) { + doc_lines.push("".into()); + doc_lines.append(&mut lines); + } + for section_builder in [panics_builder, errors_builder, safety_builder] { + if let Some(mut lines) = section_builder(&ast_func) { + doc_lines.push("".into()); + doc_lines.append(&mut lines); + } + } + if ast_func.doc_comments().next().is_some() { + doc_lines.push("--- OLD VERSION BELOW ---".into()); + } + builder.insert(text_range.start(), documentation_from_lines(doc_lines, indent_level)); + }, + ) +} + +/// Builds an introduction, trying to be smart if the function is `::new()` +fn introduction_builder(ast_func: &ast::Fn) -> String { + let is_new = ast_func.name().map(|name| &name.to_string() == "new").unwrap_or(false); + if is_new { + let ret_type = return_type(ast_func).map(|ret_type| ret_type.to_string()); + let self_type = self_type(ast_func); + if ret_type.as_deref() == Some("Self") || ret_type == self_type { + if let Some(self_type) = self_type { + return format!("Creates a new [`{}`].", self_type); + } + } + } + ".".into() +} + +/// Builds an `# Examples` section. An option is returned to be able to manage an error in the AST. +fn examples_builder(ast_func: &ast::Fn, krate_name: String) -> Option> { + let (no_panic_ex, panic_ex) = if is_in_trait_def(ast_func) { + let message = "// Example template not implemented for trait functions"; + (Some(vec![message.into()]), Some(vec![message.into()])) + } else { + let panic_ex = match can_panic(ast_func) { + Some(true) => gen_panic_ex_template(ast_func, krate_name.clone()), + _ => None, + }; + let no_panic_ex = gen_ex_template(ast_func, krate_name); + (no_panic_ex, panic_ex) + }; + + let mut lines = string_vec_from(&["# Examples", "", "```"]); + lines.append(&mut no_panic_ex?); + lines.push("```".into()); + if let Some(mut ex) = panic_ex { + lines.push("".into()); + lines.push("```should_panic".into()); + lines.append(&mut ex); + lines.push("```".into()); + } + Some(lines) +} + +/// Builds an optional `# Panics` section +fn panics_builder(ast_func: &ast::Fn) -> Option> { + match can_panic(ast_func) { + Some(true) => Some(string_vec_from(&["# Panics", "", "Panics if ."])), + _ => None, + } +} + +/// Builds an optional `# Errors` section +fn errors_builder(ast_func: &ast::Fn) -> Option> { + match return_type(ast_func)?.to_string().contains("Result") { + true => Some(string_vec_from(&["# Errors", "", "This function will return an error if ."])), + false => None, + } +} + +/// Builds an optional `# Safety` section +fn safety_builder(ast_func: &ast::Fn) -> Option> { + let is_unsafe = ast_func.unsafe_token().is_some(); + match is_unsafe { + true => Some(string_vec_from(&["# Safety", "", "."])), + false => None, + } +} + +/// Generate an example template which should not panic +/// `None` if the function has a `self` parameter but is not in an `impl`. +fn gen_ex_template(ast_func: &ast::Fn, krate_name: String) -> Option> { + let (mut lines, ex_helper) = gen_ex_start_helper(ast_func, krate_name)?; + // Call the function, check result + if returns_a_value(ast_func) { + if count_parameters(&ex_helper.param_list) < 3 { + lines.push(format!("assert_eq!({}, );", ex_helper.function_call)); + } else { + lines.push(format!("let result = {};", ex_helper.function_call)); + lines.push("assert_eq!(result, );".into()); + } + } else { + lines.push(format!("{};", ex_helper.function_call)); + } + // Check the mutated values + if is_ref_mut_self(ast_func) == Some(true) { + lines.push(format!("assert_eq!({}, );", ex_helper.self_name?)); + } + for param_name in &ex_helper.ref_mut_params { + lines.push(format!("assert_eq!({}, );", param_name)); + } + Some(lines) +} + +/// Generate an example template which should panic +/// `None` if the function has a `self` parameter but is not in an `impl`. +fn gen_panic_ex_template(ast_func: &ast::Fn, krate_name: String) -> Option> { + let (mut lines, ex_helper) = gen_ex_start_helper(ast_func, krate_name)?; + match returns_a_value(ast_func) { + true => lines.push(format!("let _ = {}; // panics", ex_helper.function_call)), + false => lines.push(format!("{}; // panics", ex_helper.function_call)), + } + Some(lines) +} + +/// Intermediary results of the start of example generation +struct ExHelper { + function_call: String, + param_list: ast::ParamList, + ref_mut_params: Vec, + self_name: Option, +} + +/// Build the start of the example and transmit the useful intermediary results. +/// `None` if the function has a `self` parameter but is not in an `impl`. +fn gen_ex_start_helper(ast_func: &ast::Fn, krate_name: String) -> Option<(Vec, ExHelper)> { + let mut lines = Vec::new(); + let is_unsafe = ast_func.unsafe_token().is_some(); + let param_list = ast_func.param_list()?; + let ref_mut_params = ref_mut_params(¶m_list); + let self_name: Option = self_name(ast_func); + + lines.push(format!("use {};", build_path(ast_func, krate_name))); + lines.push("".into()); + if let Some(self_definition) = self_definition(ast_func, self_name.as_deref()) { + lines.push(self_definition); + } + for param_name in &ref_mut_params { + lines.push(format!("let mut {} = ;", param_name)) + } + let function_call = function_call(ast_func, ¶m_list, self_name.as_deref(), is_unsafe)?; + let ex_helper = ExHelper { function_call, param_list, ref_mut_params, self_name }; + Some((lines, ex_helper)) +} + +/// `None` if function without a body; some bool to guess if function can panic +fn can_panic(ast_func: &ast::Fn) -> Option { + let body = ast_func.body()?.to_string(); + let can_panic = body.contains("panic!(") + || body.contains("assert!(") + || body.contains(".unwrap()") + || body.contains(".expect("); + Some(can_panic) +} + +/// Helper function to get the name that should be given to `self` arguments +fn self_name(ast_func: &ast::Fn) -> Option { + self_partial_type(ast_func).map(|name| to_lower_snake_case(&name)) +} + +/// 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())) +} + +/// 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)?; + if let Some(idx) = self_type.find(|c| ['<', ' '].contains(&c)) { + self_type.truncate(idx); + } + Some(self_type) +} + +/// Helper function to determine if the function is in a trait implementation +fn is_in_trait_impl(ast_func: &ast::Fn) -> bool { + ast_func + .syntax() + .ancestors() + .find_map(ast::Impl::cast) + .and_then(|impl_| impl_.trait_()) + .is_some() +} + +/// Helper function to determine if the function definition is in a trait definition +fn is_in_trait_def(ast_func: &ast::Fn) -> bool { + ast_func.syntax().ancestors().find_map(ast::Trait::cast).is_some() +} + +/// Returns `None` if no `self` at all, `Some(true)` if there is `&mut self` else `Some(false)` +fn is_ref_mut_self(ast_func: &ast::Fn) -> Option { + let self_param = ast_func.param_list()?.self_param()?; + Some(self_param.mut_token().is_some() && self_param.amp_token().is_some()) +} + +/// Helper function to define an variable to be the `self` argument +fn self_definition(ast_func: &ast::Fn, self_name: Option<&str>) -> Option { + let definition = match is_ref_mut_self(ast_func)? { + true => format!("let mut {} = ;", self_name?), + false => format!("let {} = ;", self_name?), + }; + Some(definition) +} + +/// Helper function to determine if a parameter is `&mut` +fn is_a_ref_mut_param(param: &ast::Param) -> bool { + match param.ty() { + Some(ast::Type::RefType(param_ref)) => param_ref.mut_token().is_some(), + _ => false, + } +} + +/// Helper function to build the list of `&mut` parameters +fn ref_mut_params(param_list: &ast::ParamList) -> Vec { + param_list + .params() + .filter_map(|param| match is_a_ref_mut_param(¶m) { + // Maybe better filter the param name (to do this maybe extract a function from + // `arguments_from_params`?) in case of a `mut a: &mut T`. Anyway managing most (not + // all) cases might be enough, the goal is just to produce a template. + true => Some(param.pat()?.to_string()), + false => None, + }) + .collect() +} + +/// Helper function to build the comma-separated list of arguments of the function +fn arguments_from_params(param_list: &ast::ParamList) -> String { + let args_iter = param_list.params().map(|param| match param.pat() { + // To avoid `mut` in the function call (which would be a nonsense), `Pat` should not be + // written as is so its variants must be managed independently. Other variants (for + // instance `TuplePat`) could be managed later. + Some(ast::Pat::IdentPat(ident_pat)) => match ident_pat.name() { + Some(name) => match is_a_ref_mut_param(¶m) { + true => format!("&mut {}", name.to_string()), + false => name.to_string(), + }, + None => "_".to_string(), + }, + _ => "_".to_string(), + }); + intersperse_string(args_iter, ", ") +} + +/// Helper function to build a function call. `None` if expected `self_name` was not provided +fn function_call( + ast_func: &ast::Fn, + param_list: &ast::ParamList, + self_name: Option<&str>, + is_unsafe: bool, +) -> Option { + let name = ast_func.name()?; + let arguments = arguments_from_params(¶m_list); + let function_call = if param_list.self_param().is_some() { + format!("{}.{}({})", self_name?, name, arguments) + } else if let Some(implementation) = self_partial_type(ast_func) { + format!("{}::{}({})", implementation, name, arguments) + } else { + format!("{}({})", name, arguments) + }; + match is_unsafe { + true => Some(format!("unsafe {{ {} }}", function_call)), + false => Some(function_call), + } +} + +/// Helper function to count the parameters including `self` +fn count_parameters(param_list: &ast::ParamList) -> usize { + param_list.params().count() + if param_list.self_param().is_some() { 1 } else { 0 } +} + +/// Helper function to transform lines of documentation into a Rust code documentation +fn documentation_from_lines(doc_lines: Vec, indent_level: IndentLevel) -> String { + let mut result = String::new(); + for doc_line in doc_lines { + result.push_str("///"); + if !doc_line.is_empty() { + result.push(' '); + result.push_str(&doc_line); + } + result.push('\n'); + result.push_str(&indent_level.to_string()); + } + result +} + +/// Helper function to transform an array of borrowed strings to an owned `Vec` +fn string_vec_from(string_array: &[&str]) -> Vec { + string_array.iter().map(|&s| s.to_owned()).collect() +} + +/// Helper function to build the path of the module in the which is the node +fn build_path(ast_func: &ast::Fn, krate_name: String) -> String { + let mut path: Vec = ast_func + .syntax() + .ancestors() + .filter_map(|m| ast::Module::cast(m).and_then(|m| m.name())) + .map(|m| m.to_string()) + .collect(); + path.push(krate_name); + path.reverse(); + path.push( + self_partial_type(ast_func) + .or_else(|| ast_func.name().map(|n| n.to_string())) + .unwrap_or_else(|| "*".into()), + ); + intersperse_string(path.into_iter(), "::") +} + +/// Helper function to get the return type of a function +fn return_type(ast_func: &ast::Fn) -> Option { + ast_func.ret_type()?.ty() +} + +/// Helper function to determine if the function returns some data +fn returns_a_value(ast_func: &ast::Fn) -> bool { + match return_type(ast_func) { + Some(ret_type) => !["()", "!"].contains(&ret_type.to_string().as_str()), + None => false, + } +} + +/// Helper function to concatenate string with a separator between them +fn intersperse_string(mut iter: impl Iterator, separator: &str) -> String { + let mut result = String::new(); + if let Some(first) = iter.next() { + result.push_str(&first); + } + for string in iter { + result.push_str(separator); + result.push_str(&string); + } + result +} + +#[cfg(test)] +mod tests { + use crate::tests::{check_assist, check_assist_not_applicable}; + + use super::*; + + #[test] + fn not_applicable_on_function_calls() { + check_assist_not_applicable( + generate_documentation_template, + r#" +fn hello_world() {} +fn calls_hello_world() { + hello_world$0(); +} +"#, + ) + } + + #[test] + fn not_applicable_in_trait_impl() { + check_assist_not_applicable( + generate_documentation_template, + r#" +trait MyTrait {} +struct MyStruct; +impl MyTrait for MyStruct { + fn hello_world$0(); +} +"#, + ) + } + + #[test] + fn supports_noop_function() { + check_assist( + generate_documentation_template, + r#" +fn no$0op() {} +"#, + r#" +/// . +/// +/// # Examples +/// +/// ``` +/// use test::noop; +/// +/// noop(); +/// ``` +fn noop() {} +"#, + ); + } + + #[test] + fn supports_a_parameter() { + check_assist( + generate_documentation_template, + r#" +fn no$0op_with_param(_a: i32) {} +"#, + r#" +/// . +/// +/// # Examples +/// +/// ``` +/// use test::noop_with_param; +/// +/// noop_with_param(_a); +/// ``` +fn noop_with_param(_a: i32) {} +"#, + ); + } + + #[test] + fn detects_unsafe_function() { + check_assist( + generate_documentation_template, + r#" +unsafe fn no$0op_unsafe() {} +"#, + r#" +/// . +/// +/// # Examples +/// +/// ``` +/// use test::noop_unsafe; +/// +/// unsafe { noop_unsafe() }; +/// ``` +/// +/// # Safety +/// +/// . +unsafe fn noop_unsafe() {} +"#, + ); + } + + #[test] + fn guesses_panic_macro_can_panic() { + check_assist( + generate_documentation_template, + r#" +fn panic$0s_if(a: bool) { + if a { + panic!(); + } +} +"#, + r#" +/// . +/// +/// # Examples +/// +/// ``` +/// use test::panics_if; +/// +/// panics_if(a); +/// ``` +/// +/// ```should_panic +/// use test::panics_if; +/// +/// panics_if(a); // panics +/// ``` +/// +/// # Panics +/// +/// Panics if . +fn panics_if(a: bool) { + if a { + panic!(); + } +} +"#, + ); + } + + #[test] + fn guesses_assert_macro_can_panic() { + check_assist( + generate_documentation_template, + r#" +fn $0panics_if_not(a: bool) { + assert!(a == true); +} +"#, + r#" +/// . +/// +/// # Examples +/// +/// ``` +/// use test::panics_if_not; +/// +/// panics_if_not(a); +/// ``` +/// +/// ```should_panic +/// use test::panics_if_not; +/// +/// panics_if_not(a); // panics +/// ``` +/// +/// # Panics +/// +/// Panics if . +fn panics_if_not(a: bool) { + assert!(a == true); +} +"#, + ); + } + + #[test] + fn guesses_unwrap_can_panic() { + check_assist( + generate_documentation_template, + r#" +fn $0panics_if_none(a: Option<()>) { + a.unwrap(); +} +"#, + r#" +/// . +/// +/// # Examples +/// +/// ``` +/// use test::panics_if_none; +/// +/// panics_if_none(a); +/// ``` +/// +/// ```should_panic +/// use test::panics_if_none; +/// +/// panics_if_none(a); // panics +/// ``` +/// +/// # Panics +/// +/// Panics if . +fn panics_if_none(a: Option<()>) { + a.unwrap(); +} +"#, + ); + } + + #[test] + fn guesses_expect_can_panic() { + check_assist( + generate_documentation_template, + r#" +fn $0panics_if_none2(a: Option<()>) { + a.expect("Bouh!"); +} +"#, + r#" +/// . +/// +/// # Examples +/// +/// ``` +/// use test::panics_if_none2; +/// +/// panics_if_none2(a); +/// ``` +/// +/// ```should_panic +/// use test::panics_if_none2; +/// +/// panics_if_none2(a); // panics +/// ``` +/// +/// # Panics +/// +/// Panics if . +fn panics_if_none2(a: Option<()>) { + a.expect("Bouh!"); +} +"#, + ); + } + + #[test] + fn checks_output_in_example() { + check_assist( + generate_documentation_template, + r#" +fn returns_a_value$0() -> i32 { + 0 +} +"#, + r#" +/// . +/// +/// # Examples +/// +/// ``` +/// use test::returns_a_value; +/// +/// assert_eq!(returns_a_value(), ); +/// ``` +fn returns_a_value() -> i32 { + 0 +} +"#, + ); + } + + #[test] + fn detects_result_output() { + check_assist( + generate_documentation_template, + r#" +fn returns_a_result$0() -> Result { + Ok(0) +} +"#, + r#" +/// . +/// +/// # Examples +/// +/// ``` +/// use test::returns_a_result; +/// +/// assert_eq!(returns_a_result(), ); +/// ``` +/// +/// # Errors +/// +/// This function will return an error if . +fn returns_a_result() -> Result { + Ok(0) +} +"#, + ); + } + + #[test] + fn checks_ref_mut_in_example() { + check_assist( + generate_documentation_template, + r#" +fn modifies_a_value$0(a: &mut i32) { + *a = 0; +} +"#, + r#" +/// . +/// +/// # Examples +/// +/// ``` +/// use test::modifies_a_value; +/// +/// let mut a = ; +/// modifies_a_value(&mut a); +/// assert_eq!(a, ); +/// ``` +fn modifies_a_value(a: &mut i32) { + *a = 0; +} +"#, + ); + } + + #[test] + fn stores_result_if_at_least_3_params() { + check_assist( + generate_documentation_template, + r#" +fn sum3$0(a: i32, b: i32, c: i32) -> i32 { + a + b + c +} +"#, + r#" +/// . +/// +/// # Examples +/// +/// ``` +/// use test::sum3; +/// +/// let result = sum3(a, b, c); +/// assert_eq!(result, ); +/// ``` +fn sum3(a: i32, b: i32, c: i32) -> i32 { + a + b + c +} +"#, + ); + } + + #[test] + fn supports_fn_in_mods() { + check_assist( + generate_documentation_template, + r#" +mod a { + mod b { + fn no$0op() {} + } +} +"#, + r#" +mod a { + mod b { + /// . + /// + /// # Examples + /// + /// ``` + /// use test::a::b::noop; + /// + /// noop(); + /// ``` + fn noop() {} + } +} +"#, + ); + } + + #[test] + fn supports_fn_in_impl() { + check_assist( + generate_documentation_template, + r#" +struct MyStruct; +impl MyStruct { + fn no$0op() {} +} +"#, + r#" +struct MyStruct; +impl MyStruct { + /// . + /// + /// # Examples + /// + /// ``` + /// use test::MyStruct; + /// + /// MyStruct::noop(); + /// ``` + fn noop() {} +} +"#, + ); + } + + #[test] + fn detects_new() { + check_assist( + generate_documentation_template, + r#" +#[derive(Debug, PartialEq)] +pub struct MyGenericStruct { + pub x: T, +} +impl MyGenericStruct { + pub fn new$0(x: T) -> MyGenericStruct { + MyGenericStruct { x } + } +} +"#, + r#" +#[derive(Debug, PartialEq)] +pub struct MyGenericStruct { + pub x: T, +} +impl MyGenericStruct { + /// Creates a new [`MyGenericStruct`]. + /// + /// # Examples + /// + /// ``` + /// use test::MyGenericStruct; + /// + /// assert_eq!(MyGenericStruct::new(x), ); + /// ``` + pub fn new(x: T) -> MyGenericStruct { + MyGenericStruct { x } + } +} +"#, + ); + } + + #[test] + fn detects_new_with_self() { + check_assist( + generate_documentation_template, + r#" +#[derive(Debug, PartialEq)] +pub struct MyGenericStruct2 { + pub x: T, +} +impl MyGenericStruct2 { + pub fn new$0(x: T) -> Self { + MyGenericStruct2 { x } + } +} +"#, + r#" +#[derive(Debug, PartialEq)] +pub struct MyGenericStruct2 { + pub x: T, +} +impl MyGenericStruct2 { + /// Creates a new [`MyGenericStruct2`]. + /// + /// # Examples + /// + /// ``` + /// use test::MyGenericStruct2; + /// + /// assert_eq!(MyGenericStruct2::new(x), ); + /// ``` + pub fn new(x: T) -> Self { + MyGenericStruct2 { x } + } +} +"#, + ); + } + + #[test] + fn supports_method_call() { + check_assist( + generate_documentation_template, + r#" +impl MyGenericStruct { + pub fn co$0nsume(self) {} +} +"#, + r#" +impl MyGenericStruct { + /// . + /// + /// # Examples + /// + /// ``` + /// use test::MyGenericStruct; + /// + /// let my_generic_struct = ; + /// my_generic_struct.consume(); + /// ``` + pub fn consume(self) {} +} +"#, + ); + } + + #[test] + fn checks_modified_self_param() { + check_assist( + generate_documentation_template, + r#" +impl MyGenericStruct { + pub fn modi$0fy(&mut self, new_value: T) { + self.x = new_value; + } +} +"#, + r#" +impl MyGenericStruct { + /// . + /// + /// # Examples + /// + /// ``` + /// use test::MyGenericStruct; + /// + /// let mut my_generic_struct = ; + /// my_generic_struct.modify(new_value); + /// assert_eq!(my_generic_struct, ); + /// ``` + pub fn modify(&mut self, new_value: T) { + self.x = new_value; + } +} +"#, + ); + } +} diff --git a/crates/ide_assists/src/lib.rs b/crates/ide_assists/src/lib.rs index 5d4c1532db..61ac45637b 100644 --- a/crates/ide_assists/src/lib.rs +++ b/crates/ide_assists/src/lib.rs @@ -136,6 +136,7 @@ mod handlers { mod generate_default_from_new; mod generate_deref; mod generate_derive; + mod generate_documentation_template; mod generate_enum_is_method; mod generate_enum_projection_method; mod generate_from_impl_for_enum; @@ -219,6 +220,7 @@ mod handlers { generate_delegate_methods::generate_delegate_methods, generate_deref::generate_deref, generate_derive::generate_derive, + generate_documentation_template::generate_documentation_template, generate_enum_is_method::generate_enum_is_method, generate_enum_projection_method::generate_enum_as_method, generate_enum_projection_method::generate_enum_try_into_method, From d55d3b63cb5276e3aa4681ca256565b64037d80d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20ALLART?= Date: Mon, 6 Dec 2021 19:04:44 +0100 Subject: [PATCH 02/12] fix: format assist doc for sourcegen_assists_docs --- .../generate_documentation_template.rs | 57 +++++++++---------- crates/ide_assists/src/tests/generated.rs | 30 ++++++++++ 2 files changed, 58 insertions(+), 29 deletions(-) diff --git a/crates/ide_assists/src/handlers/generate_documentation_template.rs b/crates/ide_assists/src/handlers/generate_documentation_template.rs index 3abdddbb8c..8500a4d254 100644 --- a/crates/ide_assists/src/handlers/generate_documentation_template.rs +++ b/crates/ide_assists/src/handlers/generate_documentation_template.rs @@ -7,35 +7,34 @@ use syntax::{ use crate::assist_context::{AssistContext, Assists}; -/// Assist: generate_documentation_template -/// -/// Adds a documentation template above a function definition / declaration -/// -/// ``` -/// fn my_func(a: i32, b: i32) -> Result<(), std::io::Error> { -/// unimplemented!() -/// } -/// ``` -/// -> -/// ``` -/// /// . -/// /// -/// /// # Examples -/// /// -/// /// ```rust -/// /// use my_crate::my_func; -/// /// -/// /// let result = my_func(a, b); -/// /// assert_eq!(result, ); -/// /// ``` -/// /// -/// /// # Errors -/// /// -/// /// This function will return an error if . -/// fn my_func(a: i32, b: i32) -> Result<(), std::io::Error> { -/// unimplemented!() -/// } -/// ``` +// Assist: generate_documentation_template +// +// Adds a documentation template above a function definition / declaration. +// +// ``` +// fn my_$0func(a: i32, b: i32) -> Result<(), std::io::Error> { +// unimplemented!() +// } +// ``` +// -> +// ``` +// /// . +// /// +// /// # Examples +// /// +// /// ``` +// /// use test::my_func; +// /// +// /// assert_eq!(my_func(a, b), ); +// /// ``` +// /// +// /// # Errors +// /// +// /// This function will return an error if . +// fn my_func(a: i32, b: i32) -> Result<(), std::io::Error> { +// unimplemented!() +// } +// ``` pub(crate) fn generate_documentation_template( acc: &mut Assists, ctx: &AssistContext, diff --git a/crates/ide_assists/src/tests/generated.rs b/crates/ide_assists/src/tests/generated.rs index e30f98bcd1..653e51c837 100644 --- a/crates/ide_assists/src/tests/generated.rs +++ b/crates/ide_assists/src/tests/generated.rs @@ -839,6 +839,36 @@ struct Point { ) } +#[test] +fn doctest_generate_documentation_template() { + check_doc_test( + "generate_documentation_template", + r#####" +fn my_$0func(a: i32, b: i32) -> Result<(), std::io::Error> { + unimplemented!() +} +"#####, + r#####" +/// . +/// +/// # Examples +/// +/// ``` +/// use test::my_func; +/// +/// assert_eq!(my_func(a, b), ); +/// ``` +/// +/// # Errors +/// +/// This function will return an error if . +fn my_func(a: i32, b: i32) -> Result<(), std::io::Error> { + unimplemented!() +} +"#####, + ) +} + #[test] fn doctest_generate_enum_as_method() { check_doc_test( From 3a82548c5e04395ba63add086461671e89d80262 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20ALLART?= Date: Mon, 6 Dec 2021 23:33:24 +0100 Subject: [PATCH 03/12] fix: reduce assist scope: pub fn's in pub modules --- .../generate_documentation_template.rs | 144 ++++++++++++------ crates/ide_assists/src/tests/generated.rs | 4 +- 2 files changed, 102 insertions(+), 46 deletions(-) diff --git a/crates/ide_assists/src/handlers/generate_documentation_template.rs b/crates/ide_assists/src/handlers/generate_documentation_template.rs index 8500a4d254..09179af613 100644 --- a/crates/ide_assists/src/handlers/generate_documentation_template.rs +++ b/crates/ide_assists/src/handlers/generate_documentation_template.rs @@ -1,7 +1,7 @@ use ide_db::assists::{AssistId, AssistKind}; use stdx::to_lower_snake_case; use syntax::{ - ast::{self, edit::IndentLevel, HasDocComments, HasName}, + ast::{self, edit::IndentLevel, HasDocComments, HasName, HasVisibility}, AstNode, }; @@ -12,7 +12,7 @@ use crate::assist_context::{AssistContext, Assists}; // Adds a documentation template above a function definition / declaration. // // ``` -// fn my_$0func(a: i32, b: i32) -> Result<(), std::io::Error> { +// pub fn my_$0func(a: i32, b: i32) -> Result<(), std::io::Error> { // unimplemented!() // } // ``` @@ -31,7 +31,7 @@ use crate::assist_context::{AssistContext, Assists}; // /// # Errors // /// // /// This function will return an error if . -// fn my_func(a: i32, b: i32) -> Result<(), std::io::Error> { +// pub fn my_func(a: i32, b: i32) -> Result<(), std::io::Error> { // unimplemented!() // } // ``` @@ -40,17 +40,10 @@ pub(crate) fn generate_documentation_template( ctx: &AssistContext, ) -> Option<()> { let name = ctx.find_node_at_offset::()?; - let ast_func = ast::Fn::cast(name.syntax().parent()?)?; - if is_in_trait_impl(&ast_func) { + let ast_func = name.syntax().parent().and_then(ast::Fn::cast)?; + if is_in_trait_impl(&ast_func) || !is_public(&ast_func) { return None; } - // TODO disable at least examples if function not public, as the example will fail to build on - // `cargo test`. What is the exact criteria of `pub`ness? All parent modules must be `pub`, for - // `impl { fn }` both `fn` and `struct`* must be public. - // - // What about `pub(crate)`? - // - // *: Seems complex but maybe ignoring this criteria can be ignored. let parent_syntax = ast_func.syntax(); let text_range = parent_syntax.text_range(); @@ -217,6 +210,21 @@ fn gen_ex_start_helper(ast_func: &ast::Fn, krate_name: String) -> Option<(Vec bool { + has_pub(ast_func) + && ast_func + .syntax() + .ancestors() + .filter_map(ast::Module::cast) + .all(|module| has_pub(&module)) +} + +/// Check if visibility is exactly `pub` (not `pub(crate)` for instance) +fn has_pub(item: &T) -> bool { + item.visibility().map(|v| v.path().is_none()).unwrap_or(false) +} + /// `None` if function without a body; some bool to guess if function can panic fn can_panic(ast_func: &ast::Fn) -> Option { let body = ast_func.body()?.to_string(); @@ -445,12 +453,60 @@ impl MyTrait for MyStruct { ) } + #[test] + fn not_applicable_if_function_is_private() { + check_assist_not_applicable(generate_documentation_template, r#"fn priv$0ate() {}"#); + } + + #[test] + fn not_applicable_if_function_is_pub_crate() { + check_assist_not_applicable( + generate_documentation_template, + r#"pub(crate) fn pri$0vate() {}"#, + ); + } + + #[test] + fn not_applicable_if_function_is_in_private_mod() { + check_assist_not_applicable( + generate_documentation_template, + r#" +mod PrivateModule { + pub fn pri$0vate() {} +}"#, + ); + } + + #[test] + fn not_applicable_if_function_is_in_pub_crate_mod() { + check_assist_not_applicable( + generate_documentation_template, + r#" +pub(crate) mod PrivateModule { + pub fn pr$0ivate() {} +}"#, + ); + } + + #[test] + fn not_applicable_if_function_is_in_non_public_mod_is_recursive() { + check_assist_not_applicable( + generate_documentation_template, + r#" +mod ParentPrivateModule { + pub mod PrivateModule { + pub fn pr$0ivate() {} + } +}"#, + ); + } + #[test] fn supports_noop_function() { check_assist( generate_documentation_template, r#" -fn no$0op() {} +pub fn no$0op() {} "#, r#" /// . @@ -462,7 +518,7 @@ fn no$0op() {} /// /// noop(); /// ``` -fn noop() {} +pub fn noop() {} "#, ); } @@ -472,7 +528,7 @@ fn noop() {} check_assist( generate_documentation_template, r#" -fn no$0op_with_param(_a: i32) {} +pub fn no$0op_with_param(_a: i32) {} "#, r#" /// . @@ -484,7 +540,7 @@ fn no$0op_with_param(_a: i32) {} /// /// noop_with_param(_a); /// ``` -fn noop_with_param(_a: i32) {} +pub fn noop_with_param(_a: i32) {} "#, ); } @@ -494,7 +550,7 @@ fn noop_with_param(_a: i32) {} check_assist( generate_documentation_template, r#" -unsafe fn no$0op_unsafe() {} +pub unsafe fn no$0op_unsafe() {} "#, r#" /// . @@ -510,7 +566,7 @@ unsafe fn no$0op_unsafe() {} /// # Safety /// /// . -unsafe fn noop_unsafe() {} +pub unsafe fn noop_unsafe() {} "#, ); } @@ -520,7 +576,7 @@ unsafe fn noop_unsafe() {} check_assist( generate_documentation_template, r#" -fn panic$0s_if(a: bool) { +pub fn panic$0s_if(a: bool) { if a { panic!(); } @@ -546,7 +602,7 @@ fn panic$0s_if(a: bool) { /// # Panics /// /// Panics if . -fn panics_if(a: bool) { +pub fn panics_if(a: bool) { if a { panic!(); } @@ -560,7 +616,7 @@ fn panics_if(a: bool) { check_assist( generate_documentation_template, r#" -fn $0panics_if_not(a: bool) { +pub fn $0panics_if_not(a: bool) { assert!(a == true); } "#, @@ -584,7 +640,7 @@ fn $0panics_if_not(a: bool) { /// # Panics /// /// Panics if . -fn panics_if_not(a: bool) { +pub fn panics_if_not(a: bool) { assert!(a == true); } "#, @@ -596,7 +652,7 @@ fn panics_if_not(a: bool) { check_assist( generate_documentation_template, r#" -fn $0panics_if_none(a: Option<()>) { +pub fn $0panics_if_none(a: Option<()>) { a.unwrap(); } "#, @@ -620,7 +676,7 @@ fn $0panics_if_none(a: Option<()>) { /// # Panics /// /// Panics if . -fn panics_if_none(a: Option<()>) { +pub fn panics_if_none(a: Option<()>) { a.unwrap(); } "#, @@ -632,7 +688,7 @@ fn panics_if_none(a: Option<()>) { check_assist( generate_documentation_template, r#" -fn $0panics_if_none2(a: Option<()>) { +pub fn $0panics_if_none2(a: Option<()>) { a.expect("Bouh!"); } "#, @@ -656,7 +712,7 @@ fn $0panics_if_none2(a: Option<()>) { /// # Panics /// /// Panics if . -fn panics_if_none2(a: Option<()>) { +pub fn panics_if_none2(a: Option<()>) { a.expect("Bouh!"); } "#, @@ -668,7 +724,7 @@ fn panics_if_none2(a: Option<()>) { check_assist( generate_documentation_template, r#" -fn returns_a_value$0() -> i32 { +pub fn returns_a_value$0() -> i32 { 0 } "#, @@ -682,7 +738,7 @@ fn returns_a_value$0() -> i32 { /// /// assert_eq!(returns_a_value(), ); /// ``` -fn returns_a_value() -> i32 { +pub fn returns_a_value() -> i32 { 0 } "#, @@ -694,7 +750,7 @@ fn returns_a_value() -> i32 { check_assist( generate_documentation_template, r#" -fn returns_a_result$0() -> Result { +pub fn returns_a_result$0() -> Result { Ok(0) } "#, @@ -712,7 +768,7 @@ fn returns_a_result$0() -> Result { /// # Errors /// /// This function will return an error if . -fn returns_a_result() -> Result { +pub fn returns_a_result() -> Result { Ok(0) } "#, @@ -724,7 +780,7 @@ fn returns_a_result() -> Result { check_assist( generate_documentation_template, r#" -fn modifies_a_value$0(a: &mut i32) { +pub fn modifies_a_value$0(a: &mut i32) { *a = 0; } "#, @@ -740,7 +796,7 @@ fn modifies_a_value$0(a: &mut i32) { /// modifies_a_value(&mut a); /// assert_eq!(a, ); /// ``` -fn modifies_a_value(a: &mut i32) { +pub fn modifies_a_value(a: &mut i32) { *a = 0; } "#, @@ -752,7 +808,7 @@ fn modifies_a_value(a: &mut i32) { check_assist( generate_documentation_template, r#" -fn sum3$0(a: i32, b: i32, c: i32) -> i32 { +pub fn sum3$0(a: i32, b: i32, c: i32) -> i32 { a + b + c } "#, @@ -767,7 +823,7 @@ fn sum3$0(a: i32, b: i32, c: i32) -> i32 { /// let result = sum3(a, b, c); /// assert_eq!(result, ); /// ``` -fn sum3(a: i32, b: i32, c: i32) -> i32 { +pub fn sum3(a: i32, b: i32, c: i32) -> i32 { a + b + c } "#, @@ -779,15 +835,15 @@ fn sum3(a: i32, b: i32, c: i32) -> i32 { check_assist( generate_documentation_template, r#" -mod a { - mod b { - fn no$0op() {} +pub mod a { + pub mod b { + pub fn no$0op() {} } } "#, r#" -mod a { - mod b { +pub mod a { + pub mod b { /// . /// /// # Examples @@ -797,7 +853,7 @@ mod a { /// /// noop(); /// ``` - fn noop() {} + pub fn noop() {} } } "#, @@ -809,13 +865,13 @@ mod a { check_assist( generate_documentation_template, r#" -struct MyStruct; +pub struct MyStruct; impl MyStruct { - fn no$0op() {} + pub fn no$0op() {} } "#, r#" -struct MyStruct; +pub struct MyStruct; impl MyStruct { /// . /// @@ -826,7 +882,7 @@ impl MyStruct { /// /// MyStruct::noop(); /// ``` - fn noop() {} + pub fn noop() {} } "#, ); diff --git a/crates/ide_assists/src/tests/generated.rs b/crates/ide_assists/src/tests/generated.rs index 653e51c837..c67e15b2ce 100644 --- a/crates/ide_assists/src/tests/generated.rs +++ b/crates/ide_assists/src/tests/generated.rs @@ -844,7 +844,7 @@ fn doctest_generate_documentation_template() { check_doc_test( "generate_documentation_template", r#####" -fn my_$0func(a: i32, b: i32) -> Result<(), std::io::Error> { +pub fn my_$0func(a: i32, b: i32) -> Result<(), std::io::Error> { unimplemented!() } "#####, @@ -862,7 +862,7 @@ fn my_$0func(a: i32, b: i32) -> Result<(), std::io::Error> { /// # Errors /// /// This function will return an error if . -fn my_func(a: i32, b: i32) -> Result<(), std::io::Error> { +pub fn my_func(a: i32, b: i32) -> Result<(), std::io::Error> { unimplemented!() } "#####, From 220137f1cfd8ec1bf1f4d4e755a6ce4ea1421032 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20ALLART?= Date: Tue, 7 Dec 2021 18:02:18 +0100 Subject: [PATCH 04/12] fix: disable assist for documented functions --- .../generate_documentation_template.rs | 19 +++++++++++++++---- 1 file changed, 15 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 09179af613..b69fc1332b 100644 --- a/crates/ide_assists/src/handlers/generate_documentation_template.rs +++ b/crates/ide_assists/src/handlers/generate_documentation_template.rs @@ -41,7 +41,10 @@ pub(crate) fn generate_documentation_template( ) -> Option<()> { let name = ctx.find_node_at_offset::()?; let ast_func = name.syntax().parent().and_then(ast::Fn::cast)?; - if is_in_trait_impl(&ast_func) || !is_public(&ast_func) { + if is_in_trait_impl(&ast_func) + || !is_public(&ast_func) + || ast_func.doc_comments().next().is_some() + { return None; } @@ -70,9 +73,6 @@ pub(crate) fn generate_documentation_template( doc_lines.append(&mut lines); } } - if ast_func.doc_comments().next().is_some() { - doc_lines.push("--- OLD VERSION BELOW ---".into()); - } builder.insert(text_range.start(), documentation_from_lines(doc_lines, indent_level)); }, ) @@ -501,6 +501,17 @@ mod ParentPrivateModule { ); } + #[test] + fn not_applicable_if_function_already_documented() { + check_assist_not_applicable( + generate_documentation_template, + r#" +/// Some documentation here +pub fn $0documented_function() {} +"#, + ); + } + #[test] fn supports_noop_function() { check_assist( From dc4e4c7daab74612cf730028790a8ccd564a0168 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20ALLART?= Date: Tue, 7 Dec 2021 23:34:53 +0100 Subject: [PATCH 05/12] fix: add mod files in path in generated examples --- .../generate_documentation_template.rs | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/crates/ide_assists/src/handlers/generate_documentation_template.rs b/crates/ide_assists/src/handlers/generate_documentation_template.rs index b69fc1332b..f7b68e398a 100644 --- a/crates/ide_assists/src/handlers/generate_documentation_template.rs +++ b/crates/ide_assists/src/handlers/generate_documentation_template.rs @@ -1,3 +1,4 @@ +use hir::ModuleDef; use ide_db::assists::{AssistId, AssistKind}; use stdx::to_lower_snake_case; use syntax::{ @@ -51,8 +52,6 @@ pub(crate) fn generate_documentation_template( let parent_syntax = ast_func.syntax(); let text_range = parent_syntax.text_range(); let indent_level = IndentLevel::from_node(&parent_syntax); - let krate_name = - ctx.sema.scope(&parent_syntax).module()?.krate().display_name(ctx.db())?.to_string(); acc.add( AssistId("generate_documentation_template", AssistKind::Generate), @@ -63,7 +62,7 @@ pub(crate) fn generate_documentation_template( // Introduction / short function description before the sections doc_lines.push(introduction_builder(&ast_func)); // Then come the sections - if let Some(mut lines) = examples_builder(&ast_func, krate_name) { + if let Some(mut lines) = examples_builder(&ast_func, ctx) { doc_lines.push("".into()); doc_lines.append(&mut lines); } @@ -94,16 +93,16 @@ fn introduction_builder(ast_func: &ast::Fn) -> String { } /// Builds an `# Examples` section. An option is returned to be able to manage an error in the AST. -fn examples_builder(ast_func: &ast::Fn, krate_name: String) -> Option> { +fn examples_builder(ast_func: &ast::Fn, ctx: &AssistContext) -> Option> { let (no_panic_ex, panic_ex) = if is_in_trait_def(ast_func) { let message = "// Example template not implemented for trait functions"; (Some(vec![message.into()]), Some(vec![message.into()])) } else { let panic_ex = match can_panic(ast_func) { - Some(true) => gen_panic_ex_template(ast_func, krate_name.clone()), + Some(true) => gen_panic_ex_template(ast_func, ctx), _ => None, }; - let no_panic_ex = gen_ex_template(ast_func, krate_name); + let no_panic_ex = gen_ex_template(ast_func, ctx); (no_panic_ex, panic_ex) }; @@ -146,8 +145,8 @@ fn safety_builder(ast_func: &ast::Fn) -> Option> { /// Generate an example template which should not panic /// `None` if the function has a `self` parameter but is not in an `impl`. -fn gen_ex_template(ast_func: &ast::Fn, krate_name: String) -> Option> { - let (mut lines, ex_helper) = gen_ex_start_helper(ast_func, krate_name)?; +fn gen_ex_template(ast_func: &ast::Fn, ctx: &AssistContext) -> Option> { + let (mut lines, ex_helper) = gen_ex_start_helper(ast_func, ctx)?; // Call the function, check result if returns_a_value(ast_func) { if count_parameters(&ex_helper.param_list) < 3 { @@ -171,8 +170,8 @@ fn gen_ex_template(ast_func: &ast::Fn, krate_name: String) -> Option /// Generate an example template which should panic /// `None` if the function has a `self` parameter but is not in an `impl`. -fn gen_panic_ex_template(ast_func: &ast::Fn, krate_name: String) -> Option> { - let (mut lines, ex_helper) = gen_ex_start_helper(ast_func, krate_name)?; +fn gen_panic_ex_template(ast_func: &ast::Fn, ctx: &AssistContext) -> Option> { + let (mut lines, ex_helper) = gen_ex_start_helper(ast_func, ctx)?; match returns_a_value(ast_func) { true => lines.push(format!("let _ = {}; // panics", ex_helper.function_call)), false => lines.push(format!("{}; // panics", ex_helper.function_call)), @@ -190,14 +189,14 @@ struct ExHelper { /// Build the start of the example and transmit the useful intermediary results. /// `None` if the function has a `self` parameter but is not in an `impl`. -fn gen_ex_start_helper(ast_func: &ast::Fn, krate_name: String) -> Option<(Vec, ExHelper)> { +fn gen_ex_start_helper(ast_func: &ast::Fn, ctx: &AssistContext) -> Option<(Vec, ExHelper)> { let mut lines = Vec::new(); let is_unsafe = ast_func.unsafe_token().is_some(); let param_list = ast_func.param_list()?; let ref_mut_params = ref_mut_params(¶m_list); let self_name: Option = self_name(ast_func); - lines.push(format!("use {};", build_path(ast_func, krate_name))); + lines.push(format!("use {};", build_path(ast_func, ctx)?)); lines.push("".into()); if let Some(self_definition) = self_definition(ast_func, self_name.as_deref()) { lines.push(self_definition); @@ -220,6 +219,12 @@ fn is_public(ast_func: &ast::Fn) -> bool { .all(|module| has_pub(&module)) } +/// Get the name of the current crate +fn crate_name(ast_func: &ast::Fn, ctx: &AssistContext) -> Option { + let krate = ctx.sema.scope(&ast_func.syntax()).module()?.krate(); + Some(krate.display_name(ctx.db())?.to_string()) +} + /// Check if visibility is exactly `pub` (not `pub(crate)` for instance) fn has_pub(item: &T) -> bool { item.visibility().map(|v| v.path().is_none()).unwrap_or(false) @@ -377,21 +382,16 @@ fn string_vec_from(string_array: &[&str]) -> Vec { } /// Helper function to build the path of the module in the which is the node -fn build_path(ast_func: &ast::Fn, krate_name: String) -> String { - let mut path: Vec = ast_func - .syntax() - .ancestors() - .filter_map(|m| ast::Module::cast(m).and_then(|m| m.name())) - .map(|m| m.to_string()) - .collect(); - path.push(krate_name); - path.reverse(); - path.push( - self_partial_type(ast_func) - .or_else(|| ast_func.name().map(|n| n.to_string())) - .unwrap_or_else(|| "*".into()), - ); - intersperse_string(path.into_iter(), "::") +fn build_path(ast_func: &ast::Fn, ctx: &AssistContext) -> Option { + let crate_name = crate_name(ast_func, ctx)?; + let leaf = self_partial_type(ast_func) + .or_else(|| ast_func.name().map(|n| n.to_string())) + .unwrap_or_else(|| "*".into()); + let func_module_def: ModuleDef = ctx.sema.to_def(ast_func)?.module(ctx.db()).into(); + match func_module_def.canonical_path(ctx.db()) { + Some(path) => Some(format!("{}::{}::{}", crate_name, path, leaf)), + None => Some(format!("{}::{}", crate_name, leaf)), + } } /// Helper function to get the return type of a function From c3d151ada6d18a7fb378d883d5abb0fb29f190ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20ALLART?= Date: Fri, 10 Dec 2021 14:16:07 +0100 Subject: [PATCH 06/12] fix: check correctly if function is exported --- .../generate_documentation_template.rs | 50 +++++++++++-------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/crates/ide_assists/src/handlers/generate_documentation_template.rs b/crates/ide_assists/src/handlers/generate_documentation_template.rs index f7b68e398a..f65175dffe 100644 --- a/crates/ide_assists/src/handlers/generate_documentation_template.rs +++ b/crates/ide_assists/src/handlers/generate_documentation_template.rs @@ -1,8 +1,8 @@ -use hir::ModuleDef; +use hir::{HasVisibility, ModuleDef, Visibility}; use ide_db::assists::{AssistId, AssistKind}; use stdx::to_lower_snake_case; use syntax::{ - ast::{self, edit::IndentLevel, HasDocComments, HasName, HasVisibility}, + ast::{self, edit::IndentLevel, HasDocComments, HasName}, AstNode, }; @@ -43,7 +43,7 @@ pub(crate) fn generate_documentation_template( let name = ctx.find_node_at_offset::()?; let ast_func = name.syntax().parent().and_then(ast::Fn::cast)?; if is_in_trait_impl(&ast_func) - || !is_public(&ast_func) + || !is_public(&ast_func, ctx)? || ast_func.doc_comments().next().is_some() { return None; @@ -187,7 +187,7 @@ struct ExHelper { self_name: Option, } -/// Build the start of the example and transmit the useful intermediary results. +/// Builds the start of the example and transmit the useful intermediary results. /// `None` if the function has a `self` parameter but is not in an `impl`. fn gen_ex_start_helper(ast_func: &ast::Fn, ctx: &AssistContext) -> Option<(Vec, ExHelper)> { let mut lines = Vec::new(); @@ -209,31 +209,41 @@ fn gen_ex_start_helper(ast_func: &ast::Fn, ctx: &AssistContext) -> Option<(Vec bool { - has_pub(ast_func) - && ast_func - .syntax() - .ancestors() - .filter_map(ast::Module::cast) - .all(|module| has_pub(&module)) +/// Checks if the function is public / exported +fn is_public(ast_func: &ast::Fn, ctx: &AssistContext) -> Option { + let hir_func = ctx.sema.to_def(ast_func)?; + Some( + hir_func.visibility(ctx.db()) == Visibility::Public + && all_parent_mods_public(&hir_func, ctx), + ) } -/// Get the name of the current crate +/// Checks that all parent modules of the function are public / exported +fn all_parent_mods_public(hir_func: &hir::Function, ctx: &AssistContext) -> bool { + let mut module = hir_func.module(ctx.db()); + loop { + if let Some(parent) = module.parent(ctx.db()) { + match ModuleDef::from(module).visibility(ctx.db()) { + Visibility::Public => module = parent, + _ => break false, + } + } else { + break true; + } + } +} + +/// Returns the name of the current crate fn crate_name(ast_func: &ast::Fn, ctx: &AssistContext) -> Option { let krate = ctx.sema.scope(&ast_func.syntax()).module()?.krate(); Some(krate.display_name(ctx.db())?.to_string()) } -/// Check if visibility is exactly `pub` (not `pub(crate)` for instance) -fn has_pub(item: &T) -> bool { - item.visibility().map(|v| v.path().is_none()).unwrap_or(false) -} - /// `None` if function without a body; some bool to guess if function can panic fn can_panic(ast_func: &ast::Fn) -> Option { let body = ast_func.body()?.to_string(); let can_panic = body.contains("panic!(") + // FIXME it would be better to not match `debug_assert*!` macro invocations || body.contains("assert!(") || body.contains(".unwrap()") || body.contains(".expect("); @@ -387,8 +397,8 @@ fn build_path(ast_func: &ast::Fn, ctx: &AssistContext) -> Option { let leaf = self_partial_type(ast_func) .or_else(|| ast_func.name().map(|n| n.to_string())) .unwrap_or_else(|| "*".into()); - let func_module_def: ModuleDef = ctx.sema.to_def(ast_func)?.module(ctx.db()).into(); - match func_module_def.canonical_path(ctx.db()) { + let module_def: ModuleDef = ctx.sema.to_def(ast_func)?.module(ctx.db()).into(); + match module_def.canonical_path(ctx.db()) { Some(path) => Some(format!("{}::{}::{}", crate_name, path, leaf)), None => Some(format!("{}::{}", crate_name, leaf)), } From f5e0998402c4087613992092f34f96f0c90b2a86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20ALLART?= Date: Fri, 10 Dec 2021 15:16:04 +0100 Subject: [PATCH 07/12] refactor: use hir to check if fn in trait/impl --- .../generate_documentation_template.rs | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/crates/ide_assists/src/handlers/generate_documentation_template.rs b/crates/ide_assists/src/handlers/generate_documentation_template.rs index f65175dffe..6c69f227f7 100644 --- a/crates/ide_assists/src/handlers/generate_documentation_template.rs +++ b/crates/ide_assists/src/handlers/generate_documentation_template.rs @@ -1,4 +1,4 @@ -use hir::{HasVisibility, ModuleDef, Visibility}; +use hir::{AsAssocItem, HasVisibility, ModuleDef, Visibility}; use ide_db::assists::{AssistId, AssistKind}; use stdx::to_lower_snake_case; use syntax::{ @@ -42,7 +42,7 @@ pub(crate) fn generate_documentation_template( ) -> Option<()> { let name = ctx.find_node_at_offset::()?; let ast_func = name.syntax().parent().and_then(ast::Fn::cast)?; - if is_in_trait_impl(&ast_func) + if is_in_trait_impl(&ast_func, ctx) || !is_public(&ast_func, ctx)? || ast_func.doc_comments().next().is_some() { @@ -94,7 +94,7 @@ fn introduction_builder(ast_func: &ast::Fn) -> String { /// Builds an `# Examples` section. An option is returned to be able to manage an error in the AST. fn examples_builder(ast_func: &ast::Fn, ctx: &AssistContext) -> Option> { - let (no_panic_ex, panic_ex) = if is_in_trait_def(ast_func) { + let (no_panic_ex, panic_ex) = if is_in_trait_def(ast_func, ctx) { let message = "// Example template not implemented for trait functions"; (Some(vec![message.into()]), Some(vec![message.into()])) } else { @@ -275,18 +275,21 @@ fn self_partial_type(ast_func: &ast::Fn) -> Option { } /// Helper function to determine if the function is in a trait implementation -fn is_in_trait_impl(ast_func: &ast::Fn) -> bool { - ast_func - .syntax() - .ancestors() - .find_map(ast::Impl::cast) - .and_then(|impl_| impl_.trait_()) +fn is_in_trait_impl(ast_func: &ast::Fn, ctx: &AssistContext) -> bool { + ctx.sema + .to_def(ast_func) + .and_then(|hir_func| hir_func.as_assoc_item(ctx.db())) + .and_then(|assoc_item| assoc_item.containing_trait_impl(ctx.db())) .is_some() } /// Helper function to determine if the function definition is in a trait definition -fn is_in_trait_def(ast_func: &ast::Fn) -> bool { - ast_func.syntax().ancestors().find_map(ast::Trait::cast).is_some() +fn is_in_trait_def(ast_func: &ast::Fn, ctx: &AssistContext) -> bool { + ctx.sema + .to_def(ast_func) + .and_then(|hir_func| hir_func.as_assoc_item(ctx.db())) + .and_then(|assoc_item| assoc_item.containing_trait(ctx.db())) + .is_some() } /// Returns `None` if no `self` at all, `Some(true)` if there is `&mut self` else `Some(false)` From 9c0f9d02bf9c38e774a2560e4f4f98d433e9dbb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20ALLART?= Date: Fri, 10 Dec 2021 15:53:43 +0100 Subject: [PATCH 08/12] feat: trait fn: add panicking example only if default panicks --- .../generate_documentation_template.rs | 122 +++++++++++++++++- 1 file changed, 121 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 6c69f227f7..113fe7c92c 100644 --- a/crates/ide_assists/src/handlers/generate_documentation_template.rs +++ b/crates/ide_assists/src/handlers/generate_documentation_template.rs @@ -96,7 +96,11 @@ fn introduction_builder(ast_func: &ast::Fn) -> String { fn examples_builder(ast_func: &ast::Fn, ctx: &AssistContext) -> Option> { let (no_panic_ex, panic_ex) = if is_in_trait_def(ast_func, ctx) { let message = "// Example template not implemented for trait functions"; - (Some(vec![message.into()]), Some(vec![message.into()])) + let panic_ex = match can_panic(ast_func) { + Some(true) => Some(vec![message.into()]), + _ => None, + }; + (Some(vec![message.into()]), panic_ex) } else { let panic_ex = match can_panic(ast_func) { Some(true) => gen_panic_ex_template(ast_func, ctx), @@ -912,6 +916,122 @@ impl MyStruct { ); } + #[test] + fn supports_fn_in_trait() { + check_assist( + generate_documentation_template, + r#" +pub trait MyTrait { + fn fun$0ction_trait(); +} +"#, + r#" +pub trait MyTrait { + /// . + /// + /// # Examples + /// + /// ``` + /// // Example template not implemented for trait functions + /// ``` + fn function_trait(); +} +"#, + ); + } + + #[test] + fn supports_unsafe_fn_in_trait() { + check_assist( + generate_documentation_template, + r#" +pub trait MyTrait { + unsafe fn unsafe_funct$0ion_trait(); +} +"#, + r#" +pub trait MyTrait { + /// . + /// + /// # Examples + /// + /// ``` + /// // Example template not implemented for trait functions + /// ``` + /// + /// # Safety + /// + /// . + unsafe fn unsafe_function_trait(); +} +"#, + ); + } + + #[test] + fn supports_fn_in_trait_with_default_panicking() { + check_assist( + generate_documentation_template, + r#" +pub trait MyTrait { + fn function_trait_with_$0default_panicking() { + panic!() + } +} +"#, + r#" +pub trait MyTrait { + /// . + /// + /// # Examples + /// + /// ``` + /// // Example template not implemented for trait functions + /// ``` + /// + /// ```should_panic + /// // Example template not implemented for trait functions + /// ``` + /// + /// # Panics + /// + /// Panics if . + fn function_trait_with_default_panicking() { + panic!() + } +} +"#, + ); + } + + #[test] + fn supports_fn_in_trait_returning_result() { + check_assist( + generate_documentation_template, + r#" +pub trait MyTrait { + fn function_tr$0ait_returning_result() -> Result<(), std::io::Error>; +} +"#, + r#" +pub trait MyTrait { + /// . + /// + /// # Examples + /// + /// ``` + /// // Example template not implemented for trait functions + /// ``` + /// + /// # Errors + /// + /// This function will return an error if . + fn function_trait_returning_result() -> Result<(), std::io::Error>; +} +"#, + ); + } + #[test] fn detects_new() { check_assist( From 7266fdb5a400170563622da48ec1b2d1b524c7c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20ALLART?= Date: Sat, 11 Dec 2021 20:33:08 +0100 Subject: [PATCH 09/12] refactor: use hir to compare returned and self types --- .../generate_documentation_template.rs | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/crates/ide_assists/src/handlers/generate_documentation_template.rs b/crates/ide_assists/src/handlers/generate_documentation_template.rs index 113fe7c92c..5205a8ac7e 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( |builder| { let mut doc_lines = Vec::new(); // Introduction / short function description before the sections - doc_lines.push(introduction_builder(&ast_func)); + doc_lines.push(introduction_builder(&ast_func, ctx)); // Then come the sections if let Some(mut lines) = examples_builder(&ast_func, ctx) { doc_lines.push("".into()); @@ -78,18 +78,24 @@ 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) -> String { - let is_new = ast_func.name().map(|name| &name.to_string() == "new").unwrap_or(false); - if is_new { - let ret_type = return_type(ast_func).map(|ret_type| ret_type.to_string()); - let self_type = self_type(ast_func); - if ret_type.as_deref() == Some("Self") || ret_type == self_type { - if let Some(self_type) = self_type { - return format!("Creates a new [`{}`].", self_type); +fn introduction_builder(ast_func: &ast::Fn, ctx: &AssistContext) -> String { + || -> Option { + 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(ast_func)?)), + false => None, } + } else { + None } - } - ".".into() + }() + .unwrap_or_else(|| ".".into()) } /// Builds an `# Examples` section. An option is returned to be able to manage an error in the AST. From 80a68685db6914d05cda7c7ae05a87721c4d467d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20ALLART?= Date: Sat, 11 Dec 2021 20:41:23 +0100 Subject: [PATCH 10/12] refactor: use Itertools::intersperse --- .../handlers/generate_documentation_template.rs | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/crates/ide_assists/src/handlers/generate_documentation_template.rs b/crates/ide_assists/src/handlers/generate_documentation_template.rs index 5205a8ac7e..a2fe3463b6 100644 --- a/crates/ide_assists/src/handlers/generate_documentation_template.rs +++ b/crates/ide_assists/src/handlers/generate_documentation_template.rs @@ -1,5 +1,6 @@ use hir::{AsAssocItem, HasVisibility, ModuleDef, Visibility}; use ide_db::assists::{AssistId, AssistKind}; +use itertools::Itertools; use stdx::to_lower_snake_case; use syntax::{ ast::{self, edit::IndentLevel, HasDocComments, HasName}, @@ -354,7 +355,7 @@ fn arguments_from_params(param_list: &ast::ParamList) -> String { }, _ => "_".to_string(), }); - intersperse_string(args_iter, ", ") + Itertools::intersperse(args_iter, ", ".to_string()).collect() } /// Helper function to build a function call. `None` if expected `self_name` was not provided @@ -430,19 +431,6 @@ fn returns_a_value(ast_func: &ast::Fn) -> bool { } } -/// Helper function to concatenate string with a separator between them -fn intersperse_string(mut iter: impl Iterator, separator: &str) -> String { - let mut result = String::new(); - if let Some(first) = iter.next() { - result.push_str(&first); - } - for string in iter { - result.push_str(separator); - result.push_str(&string); - } - result -} - #[cfg(test)] mod tests { use crate::tests::{check_assist, check_assist_not_applicable}; From 9e53db274b146e3cba5ebaaa3588f6a26fe3aba1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20ALLART?= Date: Sat, 11 Dec 2021 20:52:14 +0100 Subject: [PATCH 11/12] refactor: use hir to test if a value is returned --- .../handlers/generate_documentation_template.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/crates/ide_assists/src/handlers/generate_documentation_template.rs b/crates/ide_assists/src/handlers/generate_documentation_template.rs index a2fe3463b6..36b3321f91 100644 --- a/crates/ide_assists/src/handlers/generate_documentation_template.rs +++ b/crates/ide_assists/src/handlers/generate_documentation_template.rs @@ -159,7 +159,7 @@ fn safety_builder(ast_func: &ast::Fn) -> Option> { fn gen_ex_template(ast_func: &ast::Fn, ctx: &AssistContext) -> Option> { let (mut lines, ex_helper) = gen_ex_start_helper(ast_func, ctx)?; // Call the function, check result - if returns_a_value(ast_func) { + if returns_a_value(ast_func, ctx) { if count_parameters(&ex_helper.param_list) < 3 { lines.push(format!("assert_eq!({}, );", ex_helper.function_call)); } else { @@ -183,7 +183,7 @@ fn gen_ex_template(ast_func: &ast::Fn, ctx: &AssistContext) -> Option Option> { let (mut lines, ex_helper) = gen_ex_start_helper(ast_func, ctx)?; - match returns_a_value(ast_func) { + match returns_a_value(ast_func, ctx) { true => lines.push(format!("let _ = {}; // panics", ex_helper.function_call)), false => lines.push(format!("{}; // panics", ex_helper.function_call)), } @@ -424,11 +424,12 @@ fn return_type(ast_func: &ast::Fn) -> Option { } /// Helper function to determine if the function returns some data -fn returns_a_value(ast_func: &ast::Fn) -> bool { - match return_type(ast_func) { - Some(ret_type) => !["()", "!"].contains(&ret_type.to_string().as_str()), - None => false, - } +fn returns_a_value(ast_func: &ast::Fn, ctx: &AssistContext) -> bool { + ctx.sema + .to_def(ast_func) + .map(|hir_func| hir_func.ret_type(ctx.db())) + .map(|ret_ty| !ret_ty.is_unit() && !ret_ty.is_never()) + .unwrap_or(false) } #[cfg(test)] From 0e89f2f346ff00f5411a238fb85e48ede50e35cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20ALLART?= Date: Sat, 11 Dec 2021 22:46:54 +0100 Subject: [PATCH 12/12] feat: remove should_panic example generation --- .../generate_documentation_template.rs | 121 ++++-------------- 1 file changed, 24 insertions(+), 97 deletions(-) diff --git a/crates/ide_assists/src/handlers/generate_documentation_template.rs b/crates/ide_assists/src/handlers/generate_documentation_template.rs index 36b3321f91..cfc7e9d043 100644 --- a/crates/ide_assists/src/handlers/generate_documentation_template.rs +++ b/crates/ide_assists/src/handlers/generate_documentation_template.rs @@ -101,31 +101,14 @@ fn introduction_builder(ast_func: &ast::Fn, ctx: &AssistContext) -> String { /// Builds an `# Examples` section. An option is returned to be able to manage an error in the AST. fn examples_builder(ast_func: &ast::Fn, ctx: &AssistContext) -> Option> { - let (no_panic_ex, panic_ex) = if is_in_trait_def(ast_func, ctx) { - let message = "// Example template not implemented for trait functions"; - let panic_ex = match can_panic(ast_func) { - Some(true) => Some(vec![message.into()]), - _ => None, - }; - (Some(vec![message.into()]), panic_ex) + let mut lines = string_vec_from(&["# Examples", "", "```"]); + if is_in_trait_def(ast_func, ctx) { + lines.push("// Example template not implemented for trait functions".into()); } else { - let panic_ex = match can_panic(ast_func) { - Some(true) => gen_panic_ex_template(ast_func, ctx), - _ => None, - }; - let no_panic_ex = gen_ex_template(ast_func, ctx); - (no_panic_ex, panic_ex) + lines.append(&mut gen_ex_template(ast_func, ctx)?) }; - let mut lines = string_vec_from(&["# Examples", "", "```"]); - lines.append(&mut no_panic_ex?); lines.push("```".into()); - if let Some(mut ex) = panic_ex { - lines.push("".into()); - lines.push("```should_panic".into()); - lines.append(&mut ex); - lines.push("```".into()); - } Some(lines) } @@ -154,53 +137,8 @@ fn safety_builder(ast_func: &ast::Fn) -> Option> { } } -/// Generate an example template which should not panic -/// `None` if the function has a `self` parameter but is not in an `impl`. +/// Generates an example template fn gen_ex_template(ast_func: &ast::Fn, ctx: &AssistContext) -> Option> { - let (mut lines, ex_helper) = gen_ex_start_helper(ast_func, ctx)?; - // Call the function, check result - if returns_a_value(ast_func, ctx) { - if count_parameters(&ex_helper.param_list) < 3 { - lines.push(format!("assert_eq!({}, );", ex_helper.function_call)); - } else { - lines.push(format!("let result = {};", ex_helper.function_call)); - lines.push("assert_eq!(result, );".into()); - } - } else { - lines.push(format!("{};", ex_helper.function_call)); - } - // Check the mutated values - if is_ref_mut_self(ast_func) == Some(true) { - lines.push(format!("assert_eq!({}, );", ex_helper.self_name?)); - } - for param_name in &ex_helper.ref_mut_params { - lines.push(format!("assert_eq!({}, );", param_name)); - } - Some(lines) -} - -/// Generate an example template which should panic -/// `None` if the function has a `self` parameter but is not in an `impl`. -fn gen_panic_ex_template(ast_func: &ast::Fn, ctx: &AssistContext) -> Option> { - let (mut lines, ex_helper) = gen_ex_start_helper(ast_func, ctx)?; - match returns_a_value(ast_func, ctx) { - true => lines.push(format!("let _ = {}; // panics", ex_helper.function_call)), - false => lines.push(format!("{}; // panics", ex_helper.function_call)), - } - Some(lines) -} - -/// Intermediary results of the start of example generation -struct ExHelper { - function_call: String, - param_list: ast::ParamList, - ref_mut_params: Vec, - self_name: Option, -} - -/// Builds the start of the example and transmit the useful intermediary results. -/// `None` if the function has a `self` parameter but is not in an `impl`. -fn gen_ex_start_helper(ast_func: &ast::Fn, ctx: &AssistContext) -> Option<(Vec, ExHelper)> { let mut lines = Vec::new(); let is_unsafe = ast_func.unsafe_token().is_some(); let param_list = ast_func.param_list()?; @@ -215,9 +153,26 @@ fn gen_ex_start_helper(ast_func: &ast::Fn, ctx: &AssistContext) -> Option<(Vec) { /// panics_if_none(a); /// ``` /// -/// ```should_panic -/// use test::panics_if_none; -/// -/// panics_if_none(a); // panics -/// ``` -/// /// # Panics /// /// Panics if . @@ -726,12 +663,6 @@ pub fn $0panics_if_none2(a: Option<()>) { /// panics_if_none2(a); /// ``` /// -/// ```should_panic -/// use test::panics_if_none2; -/// -/// panics_if_none2(a); // panics -/// ``` -/// /// # Panics /// /// Panics if . @@ -984,10 +915,6 @@ pub trait MyTrait { /// // Example template not implemented for trait functions /// ``` /// - /// ```should_panic - /// // Example template not implemented for trait functions - /// ``` - /// /// # Panics /// /// Panics if .