diff options
Diffstat (limited to 'src/tools/rust-analyzer/crates/ide-assists/src')
28 files changed, 2227 insertions, 300 deletions
diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/add_braces.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/add_braces.rs new file mode 100644 index 000000000..2f4a263ee --- /dev/null +++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/add_braces.rs @@ -0,0 +1,155 @@ +use syntax::{ + ast::{self, edit::AstNodeEdit, make}, + AstNode, +}; + +use crate::{AssistContext, AssistId, AssistKind, Assists}; + +// Assist: add_braces +// +// Adds braces to lambda and match arm expressions. +// +// ``` +// fn foo(n: i32) -> i32 { +// match n { +// 1 =>$0 n + 1, +// _ => 0 +// } +// } +// ``` +// -> +// ``` +// fn foo(n: i32) -> i32 { +// match n { +// 1 => { +// n + 1 +// }, +// _ => 0 +// } +// } +// ``` +pub(crate) fn add_braces(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { + let (expr_type, expr) = get_replacement_node(ctx)?; + + acc.add( + AssistId("add_braces", AssistKind::RefactorRewrite), + match expr_type { + ParentType::ClosureExpr => "Add braces to closure body", + ParentType::MatchArmExpr => "Add braces to arm expression", + }, + expr.syntax().text_range(), + |builder| { + let block_expr = AstNodeEdit::indent( + &make::block_expr(None, Some(expr.clone())), + AstNodeEdit::indent_level(&expr), + ); + + builder.replace(expr.syntax().text_range(), block_expr.syntax().text()); + }, + ) +} + +enum ParentType { + MatchArmExpr, + ClosureExpr, +} + +fn get_replacement_node(ctx: &AssistContext<'_>) -> Option<(ParentType, ast::Expr)> { + if let Some(match_arm) = ctx.find_node_at_offset::<ast::MatchArm>() { + let match_arm_expr = match_arm.expr()?; + + if matches!(match_arm_expr, ast::Expr::BlockExpr(_)) { + return None; + } + + return Some((ParentType::MatchArmExpr, match_arm_expr)); + } else if let Some(closure_expr) = ctx.find_node_at_offset::<ast::ClosureExpr>() { + let body = closure_expr.body()?; + + if matches!(body, ast::Expr::BlockExpr(_)) { + return None; + } + + return Some((ParentType::ClosureExpr, body)); + } + + None +} + +#[cfg(test)] +mod tests { + use crate::tests::{check_assist, check_assist_not_applicable}; + + use super::*; + + #[test] + fn suggest_add_braces_for_closure() { + check_assist( + add_braces, + r#" +fn foo() { + t(|n|$0 n + 100); +} +"#, + r#" +fn foo() { + t(|n| { + n + 100 + }); +} +"#, + ); + } + + #[test] + fn no_assist_for_closures_with_braces() { + check_assist_not_applicable( + add_braces, + r#" +fn foo() { + t(|n|$0 { n + 100 }); +} +"#, + ); + } + + #[test] + fn suggest_add_braces_for_match() { + check_assist( + add_braces, + r#" +fn foo() { + match n { + Some(n) $0=> 29, + _ => () + }; +} +"#, + r#" +fn foo() { + match n { + Some(n) => { + 29 + }, + _ => () + }; +} +"#, + ); + } + + #[test] + fn no_assist_for_match_with_braces() { + check_assist_not_applicable( + add_braces, + r#" +fn foo() { + match n { + Some(n) $0=> { return 29; }, + _ => () + }; +} +"#, + ); + } +} diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/add_missing_impl_members.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/add_missing_impl_members.rs index 161bcc5c8..4e11b31de 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/add_missing_impl_members.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/add_missing_impl_members.rs @@ -1,7 +1,5 @@ use hir::HasSource; -use ide_db::{ - syntax_helpers::insert_whitespace_into_node::insert_ws_into, traits::resolve_target_trait, -}; +use ide_db::syntax_helpers::insert_whitespace_into_node::insert_ws_into; use syntax::ast::{self, make, AstNode}; use crate::{ @@ -107,16 +105,19 @@ fn add_missing_impl_members_inner( ) -> Option<()> { let _p = profile::span("add_missing_impl_members_inner"); let impl_def = ctx.find_node_at_offset::<ast::Impl>()?; + let impl_ = ctx.sema.to_def(&impl_def)?; if ctx.token_at_offset().all(|t| { t.parent_ancestors() + .take_while(|node| node != impl_def.syntax()) .any(|s| ast::BlockExpr::can_cast(s.kind()) || ast::ParamList::can_cast(s.kind())) }) { return None; } let target_scope = ctx.sema.scope(impl_def.syntax())?; - let trait_ = resolve_target_trait(&ctx.sema, &impl_def)?; + let trait_ref = impl_.trait_ref(ctx.db())?; + let trait_ = trait_ref.trait_(); let missing_items = filter_assoc_items( &ctx.sema, @@ -155,7 +156,7 @@ fn add_missing_impl_members_inner( let placeholder; if let DefaultMethods::No = mode { if let ast::AssocItem::Fn(func) = &first_new_item { - if try_gen_trait_body(ctx, func, &trait_, &impl_def).is_none() { + if try_gen_trait_body(ctx, func, trait_ref, &impl_def).is_none() { if let Some(m) = func.syntax().descendants().find_map(ast::MacroCall::cast) { @@ -180,13 +181,13 @@ fn add_missing_impl_members_inner( fn try_gen_trait_body( ctx: &AssistContext<'_>, func: &ast::Fn, - trait_: &hir::Trait, + trait_ref: hir::TraitRef, impl_def: &ast::Impl, ) -> Option<()> { - let trait_path = make::ext::ident_path(&trait_.name(ctx.db()).to_string()); + let trait_path = make::ext::ident_path(&trait_ref.trait_().name(ctx.db()).to_string()); let hir_ty = ctx.sema.resolve_type(&impl_def.self_ty()?)?; let adt = hir_ty.as_adt()?.source(ctx.db())?; - gen_trait_fn_body(func, &trait_path, &adt.value) + gen_trait_fn_body(func, &trait_path, &adt.value, Some(trait_ref)) } #[cfg(test)] @@ -1353,6 +1354,50 @@ impl PartialEq for SomeStruct { } #[test] + fn test_partial_eq_body_when_types_semantically_match() { + check_assist( + add_missing_impl_members, + r#" +//- minicore: eq +struct S<T, U>(T, U); +type Alias<T> = S<T, T>; +impl<T> PartialEq<Alias<T>> for S<T, T> {$0} +"#, + r#" +struct S<T, U>(T, U); +type Alias<T> = S<T, T>; +impl<T> PartialEq<Alias<T>> for S<T, T> { + $0fn eq(&self, other: &Alias<T>) -> bool { + self.0 == other.0 && self.1 == other.1 + } +} +"#, + ); + } + + #[test] + fn test_partial_eq_body_when_types_dont_match() { + check_assist( + add_missing_impl_members, + r#" +//- minicore: eq +struct S<T, U>(T, U); +type Alias<T> = S<T, T>; +impl<T> PartialEq<Alias<T>> for S<T, i32> {$0} +"#, + r#" +struct S<T, U>(T, U); +type Alias<T> = S<T, T>; +impl<T> PartialEq<Alias<T>> for S<T, i32> { + fn eq(&self, other: &Alias<T>) -> bool { + ${0:todo!()} + } +} +"#, + ); + } + + #[test] fn test_ignore_function_body() { check_assist_not_applicable( add_missing_default_members, @@ -1442,4 +1487,35 @@ impl Trait for () { }"#, ) } + + #[test] + fn test_works_inside_function() { + check_assist( + add_missing_impl_members, + r#" +trait Tr { + fn method(); +} +fn main() { + struct S; + impl Tr for S { + $0 + } +} +"#, + r#" +trait Tr { + fn method(); +} +fn main() { + struct S; + impl Tr for S { + fn method() { + ${0:todo!()} + } + } +} +"#, + ); + } } diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/add_missing_match_arms.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/add_missing_match_arms.rs index 8e4ac69ae..5d81e8cfe 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/add_missing_match_arms.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/add_missing_match_arms.rs @@ -140,6 +140,31 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>) }) .filter(|(variant_pat, _)| is_variant_missing(&top_lvl_pats, variant_pat)); ((Box::new(missing_pats) as Box<dyn Iterator<Item = _>>).peekable(), is_non_exhaustive) + } else if let Some((enum_def, len)) = resolve_array_of_enum_def(&ctx.sema, &expr) { + let is_non_exhaustive = enum_def.is_non_exhaustive(ctx.db(), module.krate()); + let variants = enum_def.variants(ctx.db()); + + if len.pow(variants.len() as u32) > 256 { + return None; + } + + let variants_of_enums = vec![variants.clone(); len]; + + let missing_pats = variants_of_enums + .into_iter() + .multi_cartesian_product() + .inspect(|_| cov_mark::hit!(add_missing_match_arms_lazy_computation)) + .map(|variants| { + let is_hidden = variants + .iter() + .any(|variant| variant.should_be_hidden(ctx.db(), module.krate())); + let patterns = variants.into_iter().filter_map(|variant| { + build_pat(ctx.db(), module, variant.clone(), ctx.config.prefer_no_std) + }); + (ast::Pat::from(make::slice_pat(patterns)), is_hidden) + }) + .filter(|(variant_pat, _)| is_variant_missing(&top_lvl_pats, variant_pat)); + ((Box::new(missing_pats) as Box<dyn Iterator<Item = _>>).peekable(), is_non_exhaustive) } else { return None; }; @@ -266,9 +291,13 @@ fn is_variant_missing(existing_pats: &[Pat], var: &Pat) -> bool { fn does_pat_match_variant(pat: &Pat, var: &Pat) -> bool { match (pat, var) { (Pat::WildcardPat(_), _) => true, + (Pat::SlicePat(spat), Pat::SlicePat(svar)) => { + spat.pats().zip(svar.pats()).all(|(p, v)| does_pat_match_variant(&p, &v)) + } (Pat::TuplePat(tpat), Pat::TuplePat(tvar)) => { tpat.fields().zip(tvar.fields()).all(|(p, v)| does_pat_match_variant(&p, &v)) } + (Pat::OrPat(opat), _) => opat.pats().any(|p| does_pat_match_variant(&p, var)), _ => utils::does_pat_match_variant(pat, var), } } @@ -279,7 +308,7 @@ enum ExtendedEnum { Enum(hir::Enum), } -#[derive(Eq, PartialEq, Clone, Copy)] +#[derive(Eq, PartialEq, Clone, Copy, Debug)] enum ExtendedVariant { True, False, @@ -339,15 +368,30 @@ fn resolve_tuple_of_enum_def( .tuple_fields(sema.db) .iter() .map(|ty| { - ty.autoderef(sema.db).find_map(|ty| match ty.as_adt() { - Some(Adt::Enum(e)) => Some(lift_enum(e)), - // For now we only handle expansion for a tuple of enums. Here - // we map non-enum items to None and rely on `collect` to - // convert Vec<Option<hir::Enum>> into Option<Vec<hir::Enum>>. - _ => ty.is_bool().then_some(ExtendedEnum::Bool), + ty.autoderef(sema.db).find_map(|ty| { + match ty.as_adt() { + Some(Adt::Enum(e)) => Some(lift_enum(e)), + // For now we only handle expansion for a tuple of enums. Here + // we map non-enum items to None and rely on `collect` to + // convert Vec<Option<hir::Enum>> into Option<Vec<hir::Enum>>. + _ => ty.is_bool().then_some(ExtendedEnum::Bool), + } }) }) - .collect() + .collect::<Option<Vec<ExtendedEnum>>>() + .and_then(|list| if list.is_empty() { None } else { Some(list) }) +} + +fn resolve_array_of_enum_def( + sema: &Semantics<'_, RootDatabase>, + expr: &ast::Expr, +) -> Option<(ExtendedEnum, usize)> { + sema.type_of_expr(expr)?.adjusted().as_array(sema.db).and_then(|(ty, len)| { + ty.autoderef(sema.db).find_map(|ty| match ty.as_adt() { + Some(Adt::Enum(e)) => Some((lift_enum(e), len)), + _ => ty.is_bool().then_some((ExtendedEnum::Bool, len)), + }) + }) } fn build_pat( @@ -376,7 +420,6 @@ fn build_pat( } ast::StructKind::Unit => make::path_pat(path), }; - Some(pat) } ExtendedVariant::True => Some(ast::Pat::from(make::literal_pat("true"))), @@ -526,6 +569,19 @@ fn foo(a: bool) { r#" fn foo(a: bool) { match (a, a)$0 { + (true | false, true) => {} + (true, false) => {} + (false, false) => {} + } +} +"#, + ); + + check_assist_not_applicable( + add_missing_match_arms, + r#" +fn foo(a: bool) { + match (a, a)$0 { (true, true) => {} (true, false) => {} (false, true) => {} @@ -560,12 +616,112 @@ fn foo(a: bool) { } #[test] + fn fill_boolean_array() { + check_assist( + add_missing_match_arms, + r#" +fn foo(a: bool) { + match [a]$0 { + } +} +"#, + r#" +fn foo(a: bool) { + match [a] { + $0[true] => todo!(), + [false] => todo!(), + } +} +"#, + ); + + check_assist( + add_missing_match_arms, + r#" +fn foo(a: bool) { + match [a,]$0 { + } +} +"#, + r#" +fn foo(a: bool) { + match [a,] { + $0[true] => todo!(), + [false] => todo!(), + } +} +"#, + ); + + check_assist( + add_missing_match_arms, + r#" +fn foo(a: bool) { + match [a, a]$0 { + [true, true] => todo!(), + } +} +"#, + r#" +fn foo(a: bool) { + match [a, a] { + [true, true] => todo!(), + $0[true, false] => todo!(), + [false, true] => todo!(), + [false, false] => todo!(), + } +} +"#, + ); + + check_assist( + add_missing_match_arms, + r#" +fn foo(a: bool) { + match [a, a]$0 { + } +} +"#, + r#" +fn foo(a: bool) { + match [a, a] { + $0[true, true] => todo!(), + [true, false] => todo!(), + [false, true] => todo!(), + [false, false] => todo!(), + } +} +"#, + ) + } + + #[test] fn partial_fill_boolean_tuple() { check_assist( add_missing_match_arms, r#" fn foo(a: bool) { match (a, a)$0 { + (true | false, true) => {} + } +} +"#, + r#" +fn foo(a: bool) { + match (a, a) { + (true | false, true) => {} + $0(true, false) => todo!(), + (false, false) => todo!(), + } +} +"#, + ); + + check_assist( + add_missing_match_arms, + r#" +fn foo(a: bool) { + match (a, a)$0 { (false, true) => {} } } @@ -882,6 +1038,33 @@ fn main() { } "#, ); + + check_assist( + add_missing_match_arms, + r#" +enum E { A, B, C } +fn main() { + use E::*; + match (A, B, C)$0 { + (A | B , A, A | B | C) => (), + (A | B | C , B | C, A | B | C) => (), + } +} +"#, + r#" +enum E { A, B, C } +fn main() { + use E::*; + match (A, B, C) { + (A | B , A, A | B | C) => (), + (A | B | C , B | C, A | B | C) => (), + $0(C, A, A) => todo!(), + (C, A, B) => todo!(), + (C, A, C) => todo!(), + } +} +"#, + ) } #[test] diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/convert_comment_block.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/convert_comment_block.rs index 312cb65ab..1acd5ee97 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/convert_comment_block.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/convert_comment_block.rs @@ -107,7 +107,7 @@ fn line_to_block(acc: &mut Assists, comment: ast::Comment) -> Option<()> { /// The line -> block assist can be invoked from anywhere within a sequence of line comments. /// relevant_line_comments crawls backwards and forwards finding the complete sequence of comments that will /// be joined. -fn relevant_line_comments(comment: &ast::Comment) -> Vec<Comment> { +pub(crate) fn relevant_line_comments(comment: &ast::Comment) -> Vec<Comment> { // The prefix identifies the kind of comment we're dealing with let prefix = comment.prefix(); let same_prefix = |c: &ast::Comment| c.prefix() == prefix; @@ -159,7 +159,7 @@ fn relevant_line_comments(comment: &ast::Comment) -> Vec<Comment> { // */ // // But since such comments aren't idiomatic we're okay with this. -fn line_comment_text(indentation: IndentLevel, comm: ast::Comment) -> String { +pub(crate) fn line_comment_text(indentation: IndentLevel, comm: ast::Comment) -> String { let contents_without_prefix = comm.text().strip_prefix(comm.prefix()).unwrap(); let contents = contents_without_prefix.strip_prefix(' ').unwrap_or(contents_without_prefix); diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/convert_match_to_let_else.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/convert_match_to_let_else.rs index 5bf04a3ad..65c2479e9 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/convert_match_to_let_else.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/convert_match_to_let_else.rs @@ -30,24 +30,23 @@ use crate::{ // ``` pub(crate) fn convert_match_to_let_else(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { let let_stmt: ast::LetStmt = ctx.find_node_at_offset()?; - let binding = find_binding(let_stmt.pat()?)?; + let binding = let_stmt.pat()?; - let initializer = match let_stmt.initializer() { - Some(ast::Expr::MatchExpr(it)) => it, - _ => return None, - }; + let Some(ast::Expr::MatchExpr(initializer)) = let_stmt.initializer() else { return None }; let initializer_expr = initializer.expr()?; - let (extracting_arm, diverging_arm) = match find_arms(ctx, &initializer) { - Some(it) => it, - None => return None, - }; + let Some((extracting_arm, diverging_arm)) = find_arms(ctx, &initializer) else { return None }; if extracting_arm.guard().is_some() { cov_mark::hit!(extracting_arm_has_guard); return None; } - let diverging_arm_expr = diverging_arm.expr()?; + let diverging_arm_expr = match diverging_arm.expr()? { + ast::Expr::BlockExpr(block) if block.modifier().is_none() && block.label().is_none() => { + block.to_string() + } + other => format!("{{ {other} }}"), + }; let extracting_arm_pat = extracting_arm.pat()?; let extracted_variable = find_extracted_variable(ctx, &extracting_arm)?; @@ -56,24 +55,16 @@ pub(crate) fn convert_match_to_let_else(acc: &mut Assists, ctx: &AssistContext<' "Convert match to let-else", let_stmt.syntax().text_range(), |builder| { - let extracting_arm_pat = rename_variable(&extracting_arm_pat, extracted_variable, binding); + let extracting_arm_pat = + rename_variable(&extracting_arm_pat, extracted_variable, binding); builder.replace( let_stmt.syntax().text_range(), - format!("let {extracting_arm_pat} = {initializer_expr} else {{ {diverging_arm_expr} }};") + format!("let {extracting_arm_pat} = {initializer_expr} else {diverging_arm_expr};"), ) }, ) } -// Given a pattern, find the name introduced to the surrounding scope. -fn find_binding(pat: ast::Pat) -> Option<ast::IdentPat> { - if let ast::Pat::IdentPat(ident) = pat { - Some(ident) - } else { - None - } -} - // Given a match expression, find extracting and diverging arms. fn find_arms( ctx: &AssistContext<'_>, @@ -87,7 +78,7 @@ fn find_arms( let mut extracting = None; let mut diverging = None; for arm in arms { - if ctx.sema.type_of_expr(&arm.expr().unwrap()).unwrap().original().is_never() { + if ctx.sema.type_of_expr(&arm.expr()?)?.original().is_never() { diverging = Some(arm); } else { extracting = Some(arm); @@ -124,7 +115,7 @@ fn find_extracted_variable(ctx: &AssistContext<'_>, arm: &ast::MatchArm) -> Opti } // Rename `extracted` with `binding` in `pat`. -fn rename_variable(pat: &ast::Pat, extracted: ast::Name, binding: ast::IdentPat) -> SyntaxNode { +fn rename_variable(pat: &ast::Pat, extracted: ast::Name, binding: ast::Pat) -> SyntaxNode { let syntax = pat.syntax().clone_for_update(); let extracted_syntax = syntax.covering_element(extracted.syntax().text_range()); @@ -136,7 +127,7 @@ fn rename_variable(pat: &ast::Pat, extracted: ast::Name, binding: ast::IdentPat) if let Some(name_ref) = record_pat_field.field_name() { ted::replace( record_pat_field.syntax(), - ast::make::record_pat_field(ast::make::name_ref(&name_ref.text()), binding.into()) + ast::make::record_pat_field(ast::make::name_ref(&name_ref.text()), binding) .syntax() .clone_for_update(), ); @@ -410,4 +401,52 @@ fn foo(opt: Option<i32>) -> Option<i32> { "#, ); } + + #[test] + fn complex_pattern() { + check_assist( + convert_match_to_let_else, + r#" +//- minicore: option +fn f() { + let (x, y) = $0match Some((0, 1)) { + Some(it) => it, + None => return, + }; +} +"#, + r#" +fn f() { + let Some((x, y)) = Some((0, 1)) else { return }; +} +"#, + ); + } + + #[test] + fn diverging_block() { + check_assist( + convert_match_to_let_else, + r#" +//- minicore: option +fn f() { + let x = $0match Some(()) { + Some(it) => it, + None => {//comment + println!("nope"); + return + }, + }; +} +"#, + r#" +fn f() { + let Some(x) = Some(()) else {//comment + println!("nope"); + return + }; +} +"#, + ); + } } diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/convert_named_struct_to_tuple_struct.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/convert_named_struct_to_tuple_struct.rs index 8d11e0bac..9dc1da246 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/convert_named_struct_to_tuple_struct.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/convert_named_struct_to_tuple_struct.rs @@ -52,10 +52,7 @@ pub(crate) fn convert_named_struct_to_tuple_struct( acc: &mut Assists, ctx: &AssistContext<'_>, ) -> Option<()> { - let strukt = ctx - .find_node_at_offset::<ast::Struct>() - .map(Either::Left) - .or_else(|| ctx.find_node_at_offset::<ast::Variant>().map(Either::Right))?; + let strukt = ctx.find_node_at_offset::<Either<ast::Struct, ast::Variant>>()?; let field_list = strukt.as_ref().either(|s| s.field_list(), |v| v.field_list())?; let record_fields = match field_list { ast::FieldList::RecordFieldList(it) => it, diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/convert_tuple_struct_to_named_struct.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/convert_tuple_struct_to_named_struct.rs index b0383291e..772e032fb 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/convert_tuple_struct_to_named_struct.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/convert_tuple_struct_to_named_struct.rs @@ -50,10 +50,7 @@ pub(crate) fn convert_tuple_struct_to_named_struct( acc: &mut Assists, ctx: &AssistContext<'_>, ) -> Option<()> { - let strukt = ctx - .find_node_at_offset::<ast::Struct>() - .map(Either::Left) - .or_else(|| ctx.find_node_at_offset::<ast::Variant>().map(Either::Right))?; + let strukt = ctx.find_node_at_offset::<Either<ast::Struct, ast::Variant>>()?; let field_list = strukt.as_ref().either(|s| s.field_list(), |v| v.field_list())?; let tuple_fields = match field_list { ast::FieldList::TupleFieldList(it) => it, diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/desugar_doc_comment.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/desugar_doc_comment.rs new file mode 100644 index 000000000..226a5dd9f --- /dev/null +++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/desugar_doc_comment.rs @@ -0,0 +1,312 @@ +use either::Either; +use itertools::Itertools; +use syntax::{ + ast::{self, edit::IndentLevel, CommentPlacement, Whitespace}, + AstToken, TextRange, +}; + +use crate::{ + handlers::convert_comment_block::{line_comment_text, relevant_line_comments}, + utils::required_hashes, + AssistContext, AssistId, AssistKind, Assists, +}; + +// Assist: desugar_doc_comment +// +// Desugars doc-comments to the attribute form. +// +// ``` +// /// Multi-line$0 +// /// comment +// ``` +// -> +// ``` +// #[doc = r"Multi-line +// comment"] +// ``` +pub(crate) fn desugar_doc_comment(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { + let comment = ctx.find_token_at_offset::<ast::Comment>()?; + // Only allow doc comments + let Some(placement) = comment.kind().doc else { return None; }; + + // Only allow comments which are alone on their line + if let Some(prev) = comment.syntax().prev_token() { + if Whitespace::cast(prev).filter(|w| w.text().contains('\n')).is_none() { + return None; + } + } + + let indentation = IndentLevel::from_token(comment.syntax()).to_string(); + + let (target, comments) = match comment.kind().shape { + ast::CommentShape::Block => (comment.syntax().text_range(), Either::Left(comment)), + ast::CommentShape::Line => { + // Find all the comments we'll be desugaring + let comments = relevant_line_comments(&comment); + + // Establish the target of our edit based on the comments we found + ( + TextRange::new( + comments[0].syntax().text_range().start(), + comments.last().unwrap().syntax().text_range().end(), + ), + Either::Right(comments), + ) + } + }; + + acc.add( + AssistId("desugar_doc_comment", AssistKind::RefactorRewrite), + "Desugar doc-comment to attribute macro", + target, + |edit| { + let text = match comments { + Either::Left(comment) => { + let text = comment.text(); + text[comment.prefix().len()..(text.len() - "*/".len())] + .trim() + .lines() + .map(|l| l.strip_prefix(&indentation).unwrap_or(l)) + .join("\n") + } + Either::Right(comments) => { + comments.into_iter().map(|c| line_comment_text(IndentLevel(0), c)).join("\n") + } + }; + + let hashes = "#".repeat(required_hashes(&text)); + + let prefix = match placement { + CommentPlacement::Inner => "#!", + CommentPlacement::Outer => "#", + }; + + let output = format!(r#"{prefix}[doc = r{hashes}"{text}"{hashes}]"#); + + edit.replace(target, output) + }, + ) +} + +#[cfg(test)] +mod tests { + use crate::tests::{check_assist, check_assist_not_applicable}; + + use super::*; + + #[test] + fn single_line() { + check_assist( + desugar_doc_comment, + r#" +/// line$0 comment +fn main() { + foo(); +} +"#, + r#" +#[doc = r"line comment"] +fn main() { + foo(); +} +"#, + ); + check_assist( + desugar_doc_comment, + r#" +//! line$0 comment +fn main() { + foo(); +} +"#, + r#" +#![doc = r"line comment"] +fn main() { + foo(); +} +"#, + ); + } + + #[test] + fn single_line_indented() { + check_assist( + desugar_doc_comment, + r#" +fn main() { + /// line$0 comment + struct Foo; +} +"#, + r#" +fn main() { + #[doc = r"line comment"] + struct Foo; +} +"#, + ); + } + + #[test] + fn multiline() { + check_assist( + desugar_doc_comment, + r#" +fn main() { + /// above + /// line$0 comment + /// + /// below + struct Foo; +} +"#, + r#" +fn main() { + #[doc = r"above +line comment + +below"] + struct Foo; +} +"#, + ); + } + + #[test] + fn end_of_line() { + check_assist_not_applicable( + desugar_doc_comment, + r#" +fn main() { /// end-of-line$0 comment + struct Foo; +} +"#, + ); + } + + #[test] + fn single_line_different_kinds() { + check_assist( + desugar_doc_comment, + r#" +fn main() { + //! different prefix + /// line$0 comment + /// below + struct Foo; +} +"#, + r#" +fn main() { + //! different prefix + #[doc = r"line comment +below"] + struct Foo; +} +"#, + ); + } + + #[test] + fn single_line_separate_chunks() { + check_assist( + desugar_doc_comment, + r#" +/// different chunk + +/// line$0 comment +/// below +"#, + r#" +/// different chunk + +#[doc = r"line comment +below"] +"#, + ); + } + + #[test] + fn block_comment() { + check_assist( + desugar_doc_comment, + r#" +/** + hi$0 there +*/ +"#, + r#" +#[doc = r"hi there"] +"#, + ); + } + + #[test] + fn inner_doc_block() { + check_assist( + desugar_doc_comment, + r#" +/*! + hi$0 there +*/ +"#, + r#" +#![doc = r"hi there"] +"#, + ); + } + + #[test] + fn block_indent() { + check_assist( + desugar_doc_comment, + r#" +fn main() { + /*! + hi$0 there + + ``` + code_sample + ``` + */ +} +"#, + r#" +fn main() { + #![doc = r"hi there + +``` + code_sample +```"] +} +"#, + ); + } + + #[test] + fn end_of_line_block() { + check_assist_not_applicable( + desugar_doc_comment, + r#" +fn main() { + foo(); /** end-of-line$0 comment */ +} +"#, + ); + } + + #[test] + fn regular_comment() { + check_assist_not_applicable(desugar_doc_comment, r#"// some$0 comment"#); + check_assist_not_applicable(desugar_doc_comment, r#"/* some$0 comment*/"#); + } + + #[test] + fn quotes_and_escapes() { + check_assist( + desugar_doc_comment, + r###"/// some$0 "\ "## comment"###, + r####"#[doc = r###"some "\ "## comment"###]"####, + ); + } +} diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/extract_type_alias.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/extract_type_alias.rs index 0505f5784..b310c2db9 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/extract_type_alias.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/extract_type_alias.rs @@ -1,9 +1,6 @@ use either::Either; use ide_db::syntax_helpers::node_ext::walk_ty; -use syntax::{ - ast::{self, edit::IndentLevel, make, AstNode, HasGenericParams, HasName}, - match_ast, -}; +use syntax::ast::{self, edit::IndentLevel, make, AstNode, HasGenericParams, HasName}; use crate::{AssistContext, AssistId, AssistKind, Assists}; @@ -31,15 +28,8 @@ pub(crate) fn extract_type_alias(acc: &mut Assists, ctx: &AssistContext<'_>) -> let ty = ctx.find_node_at_range::<ast::Type>()?; let item = ty.syntax().ancestors().find_map(ast::Item::cast)?; - let assoc_owner = item.syntax().ancestors().nth(2).and_then(|it| { - match_ast! { - match it { - ast::Trait(tr) => Some(Either::Left(tr)), - ast::Impl(impl_) => Some(Either::Right(impl_)), - _ => None, - } - } - }); + let assoc_owner = + item.syntax().ancestors().nth(2).and_then(Either::<ast::Trait, ast::Impl>::cast); let node = assoc_owner.as_ref().map_or_else( || item.syntax(), |impl_| impl_.as_ref().either(AstNode::syntax, AstNode::syntax), @@ -161,19 +151,17 @@ fn collect_used_generics<'gp>( .and_then(|lt| known_generics.iter().find(find_lifetime(<.text()))), ), ast::Type::ArrayType(ar) => { - if let Some(expr) = ar.expr() { - if let ast::Expr::PathExpr(p) = expr { - if let Some(path) = p.path() { - if let Some(name_ref) = path.as_single_name_ref() { - if let Some(param) = known_generics.iter().find(|gp| { - if let ast::GenericParam::ConstParam(cp) = gp { - cp.name().map_or(false, |n| n.text() == name_ref.text()) - } else { - false - } - }) { - generics.push(param); + if let Some(ast::Expr::PathExpr(p)) = ar.expr() { + if let Some(path) = p.path() { + if let Some(name_ref) = path.as_single_name_ref() { + if let Some(param) = known_generics.iter().find(|gp| { + if let ast::GenericParam::ConstParam(cp) = gp { + cp.name().map_or(false, |n| n.text() == name_ref.text()) + } else { + false } + }) { + generics.push(param); } } } diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/generate_default_from_new.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/generate_default_from_new.rs index 2d074a33e..860372941 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/generate_default_from_new.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/generate_default_from_new.rs @@ -82,18 +82,18 @@ fn generate_trait_impl_text_from_impl(impl_: &ast::Impl, trait_text: &str, code: let generic_params = impl_.generic_param_list().map(|generic_params| { let lifetime_params = generic_params.lifetime_params().map(ast::GenericParam::LifetimeParam); - let ty_or_const_params = generic_params.type_or_const_params().filter_map(|param| { + let ty_or_const_params = generic_params.type_or_const_params().map(|param| { // remove defaults since they can't be specified in impls match param { ast::TypeOrConstParam::Type(param) => { let param = param.clone_for_update(); param.remove_default(); - Some(ast::GenericParam::TypeParam(param)) + ast::GenericParam::TypeParam(param) } ast::TypeOrConstParam::Const(param) => { let param = param.clone_for_update(); param.remove_default(); - Some(ast::GenericParam::ConstParam(param)) + ast::GenericParam::ConstParam(param) } } }); diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/generate_delegate_methods.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/generate_delegate_methods.rs index c8d0493d0..ed1b8f4e2 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/generate_delegate_methods.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/generate_delegate_methods.rs @@ -109,7 +109,7 @@ pub(crate) fn generate_delegate_methods(acc: &mut Assists, ctx: &AssistContext<' let tail_expr_finished = if is_async { make::expr_await(tail_expr) } else { tail_expr }; let body = make::block_expr([], Some(tail_expr_finished)); - let f = make::fn_(vis, name, type_params, params, body, ret_type, is_async) + let f = make::fn_(vis, name, type_params, None, params, body, ret_type, is_async) .indent(ast::edit::IndentLevel(1)) .clone_for_update(); diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/generate_function.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/generate_function.rs index da9b0cda5..45b27a63c 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/generate_function.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/generate_function.rs @@ -1,8 +1,11 @@ -use hir::{Adt, HasSource, HirDisplay, Module, Semantics, TypeInfo}; +use hir::{ + Adt, AsAssocItem, HasSource, HirDisplay, Module, PathResolution, Semantics, Type, TypeInfo, +}; use ide_db::{ base_db::FileId, defs::{Definition, NameRefClass}, famous_defs::FamousDefs, + path_transform::PathTransform, FxHashMap, FxHashSet, RootDatabase, SnippetCap, }; use stdx::to_lower_snake_case; @@ -10,14 +13,13 @@ use syntax::{ ast::{ self, edit::{AstNodeEdit, IndentLevel}, - make, AstNode, CallExpr, HasArgList, HasModuleItem, + make, AstNode, CallExpr, HasArgList, HasGenericParams, HasModuleItem, HasTypeBounds, }, SyntaxKind, SyntaxNode, TextRange, TextSize, }; use crate::{ - utils::convert_reference_type, - utils::{find_struct_impl, render_snippet, Cursor}, + utils::{convert_reference_type, find_struct_impl, render_snippet, Cursor}, AssistContext, AssistId, AssistKind, Assists, }; @@ -107,7 +109,7 @@ fn fn_target_info( match path.qualifier() { Some(qualifier) => match ctx.sema.resolve_path(&qualifier) { Some(hir::PathResolution::Def(hir::ModuleDef::Module(module))) => { - get_fn_target_info(ctx, &Some(module), call.clone()) + get_fn_target_info(ctx, Some(module), call.clone()) } Some(hir::PathResolution::Def(hir::ModuleDef::Adt(adt))) => { if let hir::Adt::Enum(_) = adt { @@ -125,7 +127,7 @@ fn fn_target_info( } _ => None, }, - _ => get_fn_target_info(ctx, &None, call.clone()), + _ => get_fn_target_info(ctx, None, call.clone()), } } @@ -136,7 +138,8 @@ fn gen_method(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { } let fn_name = call.name_ref()?; - let adt = ctx.sema.type_of_expr(&call.receiver()?)?.original().strip_references().as_adt()?; + let receiver_ty = ctx.sema.type_of_expr(&call.receiver()?)?.original().strip_references(); + let adt = receiver_ty.as_adt()?; let current_module = ctx.sema.scope(call.syntax())?.module(); let target_module = adt.module(ctx.sema.db); @@ -147,8 +150,14 @@ fn gen_method(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { let (impl_, file) = get_adt_source(ctx, &adt, fn_name.text().as_str())?; let (target, insert_offset) = get_method_target(ctx, &impl_, &adt)?; - let function_builder = - FunctionBuilder::from_method_call(ctx, &call, &fn_name, target_module, target)?; + let function_builder = FunctionBuilder::from_method_call( + ctx, + &call, + &fn_name, + receiver_ty, + target_module, + target, + )?; let text_range = call.syntax().text_range(); let adt_name = if impl_.is_none() { Some(adt.name(ctx.sema.db)) } else { None }; let label = format!("Generate {} method", function_builder.fn_name); @@ -179,6 +188,7 @@ fn add_func_to_accumulator( let function_template = function_builder.render(adt_name.is_some()); let mut func = function_template.to_string(ctx.config.snippet_cap); if let Some(name) = adt_name { + // FIXME: adt may have generic params. func = format!("\n{indent}impl {name} {{\n{func}\n{indent}}}"); } builder.edit_file(file); @@ -238,7 +248,8 @@ impl FunctionTemplate { struct FunctionBuilder { target: GeneratedFunctionTarget, fn_name: ast::Name, - type_params: Option<ast::GenericParamList>, + generic_param_list: Option<ast::GenericParamList>, + where_clause: Option<ast::WhereClause>, params: ast::ParamList, ret_type: Option<ast::RetType>, should_focus_return_type: bool, @@ -260,19 +271,32 @@ impl FunctionBuilder { let target_module = target_module.or_else(|| ctx.sema.scope(target.syntax()).map(|it| it.module()))?; let fn_name = make::name(fn_name); - let (type_params, params) = - fn_args(ctx, target_module, ast::CallableExpr::Call(call.clone()))?; + let mut necessary_generic_params = FxHashSet::default(); + let params = fn_args( + ctx, + target_module, + ast::CallableExpr::Call(call.clone()), + &mut necessary_generic_params, + )?; let await_expr = call.syntax().parent().and_then(ast::AwaitExpr::cast); let is_async = await_expr.is_some(); - let (ret_type, should_focus_return_type) = - make_return_type(ctx, &ast::Expr::CallExpr(call.clone()), target_module); + let (ret_type, should_focus_return_type) = make_return_type( + ctx, + &ast::Expr::CallExpr(call.clone()), + target_module, + &mut necessary_generic_params, + ); + + let (generic_param_list, where_clause) = + fn_generic_params(ctx, necessary_generic_params, &target)?; Some(Self { target, fn_name, - type_params, + generic_param_list, + where_clause, params, ret_type, should_focus_return_type, @@ -285,25 +309,40 @@ impl FunctionBuilder { ctx: &AssistContext<'_>, call: &ast::MethodCallExpr, name: &ast::NameRef, + receiver_ty: Type, target_module: Module, target: GeneratedFunctionTarget, ) -> Option<Self> { let needs_pub = !module_is_descendant(&ctx.sema.scope(call.syntax())?.module(), &target_module, ctx); let fn_name = make::name(&name.text()); - let (type_params, params) = - fn_args(ctx, target_module, ast::CallableExpr::MethodCall(call.clone()))?; + let mut necessary_generic_params = FxHashSet::default(); + necessary_generic_params.extend(receiver_ty.generic_params(ctx.db())); + let params = fn_args( + ctx, + target_module, + ast::CallableExpr::MethodCall(call.clone()), + &mut necessary_generic_params, + )?; let await_expr = call.syntax().parent().and_then(ast::AwaitExpr::cast); let is_async = await_expr.is_some(); - let (ret_type, should_focus_return_type) = - make_return_type(ctx, &ast::Expr::MethodCallExpr(call.clone()), target_module); + let (ret_type, should_focus_return_type) = make_return_type( + ctx, + &ast::Expr::MethodCallExpr(call.clone()), + target_module, + &mut necessary_generic_params, + ); + + let (generic_param_list, where_clause) = + fn_generic_params(ctx, necessary_generic_params, &target)?; Some(Self { target, fn_name, - type_params, + generic_param_list, + where_clause, params, ret_type, should_focus_return_type, @@ -319,7 +358,8 @@ impl FunctionBuilder { let mut fn_def = make::fn_( visibility, self.fn_name, - self.type_params, + self.generic_param_list, + self.where_clause, self.params, fn_body, self.ret_type, @@ -375,6 +415,7 @@ fn make_return_type( ctx: &AssistContext<'_>, call: &ast::Expr, target_module: Module, + necessary_generic_params: &mut FxHashSet<hir::GenericParam>, ) -> (Option<ast::RetType>, bool) { let (ret_ty, should_focus_return_type) = { match ctx.sema.type_of_expr(call).map(TypeInfo::original) { @@ -382,6 +423,7 @@ fn make_return_type( None => (Some(make::ty_placeholder()), true), Some(ty) if ty.is_unit() => (None, false), Some(ty) => { + necessary_generic_params.extend(ty.generic_params(ctx.db())); let rendered = ty.display_source_code(ctx.db(), target_module.into()); match rendered { Ok(rendered) => (Some(make::ty(&rendered)), false), @@ -396,16 +438,16 @@ fn make_return_type( fn get_fn_target_info( ctx: &AssistContext<'_>, - target_module: &Option<Module>, + target_module: Option<Module>, call: CallExpr, ) -> Option<TargetInfo> { let (target, file, insert_offset) = get_fn_target(ctx, target_module, call)?; - Some(TargetInfo::new(*target_module, None, target, file, insert_offset)) + Some(TargetInfo::new(target_module, None, target, file, insert_offset)) } fn get_fn_target( ctx: &AssistContext<'_>, - target_module: &Option<Module>, + target_module: Option<Module>, call: CallExpr, ) -> Option<(GeneratedFunctionTarget, FileId, TextSize)> { let mut file = ctx.file_id(); @@ -473,37 +515,386 @@ impl GeneratedFunctionTarget { GeneratedFunctionTarget::InEmptyItemList(it) => it, } } + + fn parent(&self) -> SyntaxNode { + match self { + GeneratedFunctionTarget::BehindItem(it) => it.parent().expect("item without parent"), + GeneratedFunctionTarget::InEmptyItemList(it) => it.clone(), + } + } } -/// Computes the type variables and arguments required for the generated function +/// Computes parameter list for the generated function. fn fn_args( ctx: &AssistContext<'_>, target_module: hir::Module, call: ast::CallableExpr, -) -> Option<(Option<ast::GenericParamList>, ast::ParamList)> { + necessary_generic_params: &mut FxHashSet<hir::GenericParam>, +) -> Option<ast::ParamList> { let mut arg_names = Vec::new(); let mut arg_types = Vec::new(); for arg in call.arg_list()?.args() { arg_names.push(fn_arg_name(&ctx.sema, &arg)); - arg_types.push(fn_arg_type(ctx, target_module, &arg)); + arg_types.push(fn_arg_type(ctx, target_module, &arg, necessary_generic_params)); } deduplicate_arg_names(&mut arg_names); let params = arg_names.into_iter().zip(arg_types).map(|(name, ty)| { make::param(make::ext::simple_ident_pat(make::name(&name)).into(), make::ty(&ty)) }); - Some(( - None, - make::param_list( - match call { - ast::CallableExpr::Call(_) => None, - ast::CallableExpr::MethodCall(_) => Some(make::self_param()), - }, - params, - ), + Some(make::param_list( + match call { + ast::CallableExpr::Call(_) => None, + ast::CallableExpr::MethodCall(_) => Some(make::self_param()), + }, + params, )) } +/// Gets parameter bounds and where predicates in scope and filters out irrelevant ones. Returns +/// `None` when it fails to get scope information. +/// +/// See comment on `filter_unnecessary_bounds()` for what bounds we consider relevant. +/// +/// NOTE: Generic parameters returned from this function may cause name clash at `target`. We don't +/// currently do anything about it because it's actually easy to resolve it after the assist: just +/// use the Rename functionality. +fn fn_generic_params( + ctx: &AssistContext<'_>, + necessary_params: FxHashSet<hir::GenericParam>, + target: &GeneratedFunctionTarget, +) -> Option<(Option<ast::GenericParamList>, Option<ast::WhereClause>)> { + if necessary_params.is_empty() { + // Not really needed but fast path. + return Some((None, None)); + } + + // 1. Get generic parameters (with bounds) and where predicates in scope. + let (generic_params, where_preds) = params_and_where_preds_in_scope(ctx); + + // 2. Extract type parameters included in each bound. + let mut generic_params = generic_params + .into_iter() + .filter_map(|it| compute_contained_params_in_generic_param(ctx, it)) + .collect(); + let mut where_preds = where_preds + .into_iter() + .filter_map(|it| compute_contained_params_in_where_pred(ctx, it)) + .collect(); + + // 3. Filter out unnecessary bounds. + filter_unnecessary_bounds(&mut generic_params, &mut where_preds, necessary_params); + filter_bounds_in_scope(&mut generic_params, &mut where_preds, ctx, target); + + let generic_params: Vec<_> = + generic_params.into_iter().map(|it| it.node.clone_for_update()).collect(); + let where_preds: Vec<_> = + where_preds.into_iter().map(|it| it.node.clone_for_update()).collect(); + + // 4. Rewrite paths + if let Some(param) = generic_params.first() { + let source_scope = ctx.sema.scope(param.syntax())?; + let target_scope = ctx.sema.scope(&target.parent())?; + if source_scope.module() != target_scope.module() { + let transform = PathTransform::generic_transformation(&target_scope, &source_scope); + let generic_params = generic_params.iter().map(|it| it.syntax()); + let where_preds = where_preds.iter().map(|it| it.syntax()); + transform.apply_all(generic_params.chain(where_preds)); + } + } + + let generic_param_list = make::generic_param_list(generic_params); + let where_clause = + if where_preds.is_empty() { None } else { Some(make::where_clause(where_preds)) }; + + Some((Some(generic_param_list), where_clause)) +} + +fn params_and_where_preds_in_scope( + ctx: &AssistContext<'_>, +) -> (Vec<ast::GenericParam>, Vec<ast::WherePred>) { + let Some(body) = containing_body(ctx) else { return Default::default(); }; + + let mut generic_params = Vec::new(); + let mut where_clauses = Vec::new(); + + // There are two items where generic parameters currently in scope may be declared: the item + // the cursor is at, and its parent (if any). + // + // We handle parent first so that their generic parameters appear first in the generic + // parameter list of the function we're generating. + let db = ctx.db(); + if let Some(parent) = body.as_assoc_item(db).map(|it| it.container(db)) { + match parent { + hir::AssocItemContainer::Impl(it) => { + let (params, clauses) = get_bounds_in_scope(ctx, it); + generic_params.extend(params); + where_clauses.extend(clauses); + } + hir::AssocItemContainer::Trait(it) => { + let (params, clauses) = get_bounds_in_scope(ctx, it); + generic_params.extend(params); + where_clauses.extend(clauses); + } + } + } + + // Other defs with body may inherit generic parameters from its parent, but never have their + // own generic parameters. + if let hir::DefWithBody::Function(it) = body { + let (params, clauses) = get_bounds_in_scope(ctx, it); + generic_params.extend(params); + where_clauses.extend(clauses); + } + + (generic_params, where_clauses) +} + +fn containing_body(ctx: &AssistContext<'_>) -> Option<hir::DefWithBody> { + let item: ast::Item = ctx.find_node_at_offset()?; + let def = match item { + ast::Item::Fn(it) => ctx.sema.to_def(&it)?.into(), + ast::Item::Const(it) => ctx.sema.to_def(&it)?.into(), + ast::Item::Static(it) => ctx.sema.to_def(&it)?.into(), + _ => return None, + }; + Some(def) +} + +fn get_bounds_in_scope<D>( + ctx: &AssistContext<'_>, + def: D, +) -> (impl Iterator<Item = ast::GenericParam>, impl Iterator<Item = ast::WherePred>) +where + D: HasSource, + D::Ast: HasGenericParams, +{ + // This function should be only called with `Impl`, `Trait`, or `Function`, for which it's + // infallible to get source ast. + let node = ctx.sema.source(def).unwrap().value; + let generic_params = node.generic_param_list().into_iter().flat_map(|it| it.generic_params()); + let where_clauses = node.where_clause().into_iter().flat_map(|it| it.predicates()); + (generic_params, where_clauses) +} + +#[derive(Debug)] +struct ParamBoundWithParams { + node: ast::GenericParam, + /// Generic parameter `node` introduces. + /// + /// ```text + /// impl<T> S<T> { + /// fn f<U: Trait<T>>() {} + /// ^ this + /// } + /// ``` + /// + /// `U` in this example. + self_ty_param: hir::GenericParam, + /// Generic parameters contained in the trait reference of this bound. + /// + /// ```text + /// impl<T> S<T> { + /// fn f<U: Trait<T>>() {} + /// ^^^^^^^^ params in this part + /// } + /// ``` + /// + /// `T` in this example. + other_params: FxHashSet<hir::GenericParam>, +} + +#[derive(Debug)] +struct WherePredWithParams { + node: ast::WherePred, + /// Generic parameters contained in the "self type" of this where predicate. + /// + /// ```text + /// Struct<T, U>: Trait<T, Assoc = V>, + /// ^^^^^^^^^^^^ params in this part + /// ``` + /// + /// `T` and `U` in this example. + self_ty_params: FxHashSet<hir::GenericParam>, + /// Generic parameters contained in the trait reference of this where predicate. + /// + /// ```text + /// Struct<T, U>: Trait<T, Assoc = V>, + /// ^^^^^^^^^^^^^^^^^^^ params in this part + /// ``` + /// + /// `T` and `V` in this example. + other_params: FxHashSet<hir::GenericParam>, +} + +fn compute_contained_params_in_generic_param( + ctx: &AssistContext<'_>, + node: ast::GenericParam, +) -> Option<ParamBoundWithParams> { + match &node { + ast::GenericParam::TypeParam(ty) => { + let self_ty_param = ctx.sema.to_def(ty)?.into(); + + let other_params = ty + .type_bound_list() + .into_iter() + .flat_map(|it| it.bounds()) + .flat_map(|bound| bound.syntax().descendants()) + .filter_map(|node| filter_generic_params(ctx, node)) + .collect(); + + Some(ParamBoundWithParams { node, self_ty_param, other_params }) + } + ast::GenericParam::ConstParam(ct) => { + let self_ty_param = ctx.sema.to_def(ct)?.into(); + Some(ParamBoundWithParams { node, self_ty_param, other_params: FxHashSet::default() }) + } + ast::GenericParam::LifetimeParam(_) => { + // FIXME: It might be a good idea to handle lifetime parameters too. + None + } + } +} + +fn compute_contained_params_in_where_pred( + ctx: &AssistContext<'_>, + node: ast::WherePred, +) -> Option<WherePredWithParams> { + let self_ty = node.ty()?; + let bound_list = node.type_bound_list()?; + + let self_ty_params = self_ty + .syntax() + .descendants() + .filter_map(|node| filter_generic_params(ctx, node)) + .collect(); + + let other_params = bound_list + .bounds() + .flat_map(|bound| bound.syntax().descendants()) + .filter_map(|node| filter_generic_params(ctx, node)) + .collect(); + + Some(WherePredWithParams { node, self_ty_params, other_params }) +} + +fn filter_generic_params(ctx: &AssistContext<'_>, node: SyntaxNode) -> Option<hir::GenericParam> { + let path = ast::Path::cast(node)?; + match ctx.sema.resolve_path(&path)? { + PathResolution::TypeParam(it) => Some(it.into()), + PathResolution::ConstParam(it) => Some(it.into()), + _ => None, + } +} + +/// Filters out irrelevant bounds from `generic_params` and `where_preds`. +/// +/// Say we have a trait bound `Struct<T>: Trait<U>`. Given `necessary_params`, when is it relevant +/// and when not? Some observations: +/// - When `necessary_params` contains `T`, it's likely that we want this bound, but now we have +/// an extra param to consider: `U`. +/// - On the other hand, when `necessary_params` contains `U` (but not `T`), then it's unlikely +/// that we want this bound because it doesn't really constrain `U`. +/// +/// (FIXME?: The latter clause might be overstating. We may want to include the bound if the self +/// type does *not* include generic params at all - like `Option<i32>: From<U>`) +/// +/// Can we make this a bit more formal? Let's define "dependency" between generic parameters and +/// trait bounds: +/// - A generic parameter `T` depends on a trait bound if `T` appears in the self type (i.e. left +/// part) of the bound. +/// - A trait bound depends on a generic parameter `T` if `T` appears in the bound. +/// +/// Using the notion, what we want is all the bounds that params in `necessary_params` +/// *transitively* depend on! +/// +/// Now it's not hard to solve: we build a dependency graph and compute all reachable nodes from +/// nodes that represent params in `necessary_params` by usual and boring DFS. +/// +/// The time complexity is O(|generic_params| + |where_preds| + |necessary_params|). +fn filter_unnecessary_bounds( + generic_params: &mut Vec<ParamBoundWithParams>, + where_preds: &mut Vec<WherePredWithParams>, + necessary_params: FxHashSet<hir::GenericParam>, +) { + // All `self_ty_param` should be unique as they were collected from `ast::GenericParamList`s. + let param_map: FxHashMap<hir::GenericParam, usize> = + generic_params.iter().map(|it| it.self_ty_param).zip(0..).collect(); + let param_count = param_map.len(); + let generic_params_upper_bound = param_count + generic_params.len(); + let node_count = generic_params_upper_bound + where_preds.len(); + + // | node index range | what the node represents | + // |-----------------------------------------|--------------------------| + // | 0..param_count | generic parameter | + // | param_count..generic_params_upper_bound | `ast::GenericParam` | + // | generic_params_upper_bound..node_count | `ast::WherePred` | + let mut graph = Graph::new(node_count); + for (pred, pred_idx) in generic_params.iter().zip(param_count..) { + let param_idx = param_map[&pred.self_ty_param]; + graph.add_edge(param_idx, pred_idx); + graph.add_edge(pred_idx, param_idx); + + for param in &pred.other_params { + let param_idx = param_map[param]; + graph.add_edge(pred_idx, param_idx); + } + } + for (pred, pred_idx) in where_preds.iter().zip(generic_params_upper_bound..) { + for param in &pred.self_ty_params { + let param_idx = param_map[param]; + graph.add_edge(param_idx, pred_idx); + graph.add_edge(pred_idx, param_idx); + } + for param in &pred.other_params { + let param_idx = param_map[param]; + graph.add_edge(pred_idx, param_idx); + } + } + + let starting_nodes = necessary_params.iter().map(|param| param_map[param]); + let reachable = graph.compute_reachable_nodes(starting_nodes); + + // Not pretty, but effective. If only there were `Vec::retain_index()`... + let mut idx = param_count; + generic_params.retain(|_| { + idx += 1; + reachable[idx - 1] + }); + stdx::always!(idx == generic_params_upper_bound, "inconsistent index"); + where_preds.retain(|_| { + idx += 1; + reachable[idx - 1] + }); +} + +/// Filters out bounds from impl if we're generating the function into the same impl we're +/// generating from. +fn filter_bounds_in_scope( + generic_params: &mut Vec<ParamBoundWithParams>, + where_preds: &mut Vec<WherePredWithParams>, + ctx: &AssistContext<'_>, + target: &GeneratedFunctionTarget, +) -> Option<()> { + let target_impl = target.parent().ancestors().find_map(ast::Impl::cast)?; + let target_impl = ctx.sema.to_def(&target_impl)?; + // It's sufficient to test only the first element of `generic_params` because of the order of + // insertion (see `relevant_parmas_and_where_clauses()`). + let def = generic_params.first()?.self_ty_param.parent(); + if def != hir::GenericDef::Impl(target_impl) { + return None; + } + + // Now we know every element that belongs to an impl would be in scope at `target`, we can + // filter them out just by lookint at their parent. + generic_params.retain(|it| !matches!(it.self_ty_param.parent(), hir::GenericDef::Impl(_))); + where_preds.retain(|it| { + it.node.syntax().parent().and_then(|it| it.parent()).and_then(ast::Impl::cast).is_none() + }); + + Some(()) +} + /// Makes duplicate argument names unique by appending incrementing numbers. /// /// ``` @@ -564,17 +955,25 @@ fn fn_arg_name(sema: &Semantics<'_, RootDatabase>, arg_expr: &ast::Expr) -> Stri } } -fn fn_arg_type(ctx: &AssistContext<'_>, target_module: hir::Module, fn_arg: &ast::Expr) -> String { +fn fn_arg_type( + ctx: &AssistContext<'_>, + target_module: hir::Module, + fn_arg: &ast::Expr, + generic_params: &mut FxHashSet<hir::GenericParam>, +) -> String { fn maybe_displayed_type( ctx: &AssistContext<'_>, target_module: hir::Module, fn_arg: &ast::Expr, + generic_params: &mut FxHashSet<hir::GenericParam>, ) -> Option<String> { let ty = ctx.sema.type_of_expr(fn_arg)?.adjusted(); if ty.is_unknown() { return None; } + generic_params.extend(ty.generic_params(ctx.db())); + if ty.is_reference() || ty.is_mutable_reference() { let famous_defs = &FamousDefs(&ctx.sema, ctx.sema.scope(fn_arg.syntax())?.krate()); convert_reference_type(ty.strip_references(), ctx.db(), famous_defs) @@ -585,7 +984,8 @@ fn fn_arg_type(ctx: &AssistContext<'_>, target_module: hir::Module, fn_arg: &ast } } - maybe_displayed_type(ctx, target_module, fn_arg).unwrap_or_else(|| String::from("_")) + maybe_displayed_type(ctx, target_module, fn_arg, generic_params) + .unwrap_or_else(|| String::from("_")) } /// Returns the position inside the current mod or file @@ -640,10 +1040,11 @@ fn next_space_for_fn_in_module( } fn next_space_for_fn_in_impl(impl_: &ast::Impl) -> Option<GeneratedFunctionTarget> { - if let Some(last_item) = impl_.assoc_item_list().and_then(|it| it.assoc_items().last()) { + let assoc_item_list = impl_.assoc_item_list()?; + if let Some(last_item) = assoc_item_list.assoc_items().last() { Some(GeneratedFunctionTarget::BehindItem(last_item.syntax().clone())) } else { - Some(GeneratedFunctionTarget::InEmptyItemList(impl_.assoc_item_list()?.syntax().clone())) + Some(GeneratedFunctionTarget::InEmptyItemList(assoc_item_list.syntax().clone())) } } @@ -659,6 +1060,73 @@ fn module_is_descendant(module: &hir::Module, ans: &hir::Module, ctx: &AssistCon false } +// This is never intended to be used as a generic graph strucuture. If there's ever another need of +// graph algorithm, consider adding a library for that (and replace the following). +/// Minimally implemented directed graph structure represented by adjacency list. +struct Graph { + edges: Vec<Vec<usize>>, +} + +impl Graph { + fn new(node_count: usize) -> Self { + Self { edges: vec![Vec::new(); node_count] } + } + + fn add_edge(&mut self, from: usize, to: usize) { + self.edges[from].push(to); + } + + fn edges_for(&self, node_idx: usize) -> &[usize] { + &self.edges[node_idx] + } + + fn len(&self) -> usize { + self.edges.len() + } + + fn compute_reachable_nodes( + &self, + starting_nodes: impl IntoIterator<Item = usize>, + ) -> Vec<bool> { + let mut visitor = Visitor::new(self); + for idx in starting_nodes { + visitor.mark_reachable(idx); + } + visitor.visited + } +} + +struct Visitor<'g> { + graph: &'g Graph, + visited: Vec<bool>, + // Stack is held in this struct so we can reuse its buffer. + stack: Vec<usize>, +} + +impl<'g> Visitor<'g> { + fn new(graph: &'g Graph) -> Self { + let visited = vec![false; graph.len()]; + Self { graph, visited, stack: Vec::new() } + } + + fn mark_reachable(&mut self, start_idx: usize) { + // non-recursive DFS + stdx::always!(self.stack.is_empty()); + + self.stack.push(start_idx); + while let Some(idx) = self.stack.pop() { + if !self.visited[idx] { + self.visited[idx] = true; + for &neighbor in self.graph.edges_for(idx) { + if !self.visited[neighbor] { + self.stack.push(neighbor); + } + } + } + } + } +} + #[cfg(test)] mod tests { use crate::tests::{check_assist, check_assist_not_applicable}; @@ -1087,21 +1555,167 @@ fn bar(baz: Baz::Bof) { } #[test] - fn add_function_with_generic_arg() { - // FIXME: This is wrong, generated `bar` should include generic parameter. + fn generate_function_with_generic_param() { + check_assist( + generate_function, + r" +fn foo<T, const N: usize>(t: [T; N]) { $0bar(t) } +", + r" +fn foo<T, const N: usize>(t: [T; N]) { bar(t) } + +fn bar<T, const N: usize>(t: [T; N]) { + ${0:todo!()} +} +", + ) + } + + #[test] + fn generate_function_with_parent_generic_param() { + check_assist( + generate_function, + r" +struct S<T>(T); +impl<T> S<T> { + fn foo<U>(t: T, u: U) { $0bar(t, u) } +} +", + r" +struct S<T>(T); +impl<T> S<T> { + fn foo<U>(t: T, u: U) { bar(t, u) } +} + +fn bar<T, U>(t: T, u: U) { + ${0:todo!()} +} +", + ) + } + + #[test] + fn generic_param_in_receiver_type() { + // FIXME: Generic parameter `T` should be part of impl, not method. + check_assist( + generate_function, + r" +struct S<T>(T); +fn foo<T, U>(s: S<T>, u: U) { s.$0foo(u) } +", + r" +struct S<T>(T); +impl S { + fn foo<T, U>(&self, u: U) { + ${0:todo!()} + } +} +fn foo<T, U>(s: S<T>, u: U) { s.foo(u) } +", + ) + } + + #[test] + fn generic_param_in_return_type() { + check_assist( + generate_function, + r" +fn foo<T, const N: usize>() -> [T; N] { $0bar() } +", + r" +fn foo<T, const N: usize>() -> [T; N] { bar() } + +fn bar<T, const N: usize>() -> [T; N] { + ${0:todo!()} +} +", + ) + } + + #[test] + fn generate_fn_with_bounds() { + // FIXME: where predicates should be on next lines. + check_assist( + generate_function, + r" +trait A<T> {} +struct S<T>(T); +impl<T: A<i32>> S<T> +where + T: A<i64>, +{ + fn foo<U>(t: T, u: U) + where + T: A<()>, + U: A<i32> + A<i64>, + { + $0bar(t, u) + } +} +", + r" +trait A<T> {} +struct S<T>(T); +impl<T: A<i32>> S<T> +where + T: A<i64>, +{ + fn foo<U>(t: T, u: U) + where + T: A<()>, + U: A<i32> + A<i64>, + { + bar(t, u) + } +} + +fn bar<T: A<i32>, U>(t: T, u: U) where T: A<i64>, T: A<()>, U: A<i32> + A<i64> { + ${0:todo!()} +} +", + ) + } + + #[test] + fn include_transitive_param_dependency() { + // FIXME: where predicates should be on next lines. check_assist( generate_function, r" -fn foo<T>(t: T) { - $0bar(t) +trait A<T> { type Assoc; } +trait B { type Item; } +struct S<T>(T); +impl<T, U, V: B, W> S<(T, U, V, W)> +where + T: A<U, Assoc = V>, + S<V::Item>: A<U, Assoc = W>, +{ + fn foo<I>(t: T, u: U) + where + U: A<T, Assoc = I>, + { + $0bar(u) + } } ", r" -fn foo<T>(t: T) { - bar(t) +trait A<T> { type Assoc; } +trait B { type Item; } +struct S<T>(T); +impl<T, U, V: B, W> S<(T, U, V, W)> +where + T: A<U, Assoc = V>, + S<V::Item>: A<U, Assoc = W>, +{ + fn foo<I>(t: T, u: U) + where + U: A<T, Assoc = I>, + { + bar(u) + } } -fn bar(t: T) { +fn bar<T, U, V: B, W, I>(u: U) where T: A<U, Assoc = V>, S<V::Item>: A<U, Assoc = W>, U: A<T, Assoc = I> { ${0:todo!()} } ", @@ -1109,6 +1723,135 @@ fn bar(t: T) { } #[test] + fn irrelevant_bounds_are_filtered_out() { + check_assist( + generate_function, + r" +trait A<T> {} +struct S<T>(T); +impl<T, U, V, W> S<(T, U, V, W)> +where + T: A<U>, + V: A<W>, +{ + fn foo<I>(t: T, u: U) + where + U: A<T> + A<I>, + { + $0bar(u) + } +} +", + r" +trait A<T> {} +struct S<T>(T); +impl<T, U, V, W> S<(T, U, V, W)> +where + T: A<U>, + V: A<W>, +{ + fn foo<I>(t: T, u: U) + where + U: A<T> + A<I>, + { + bar(u) + } +} + +fn bar<T, U, I>(u: U) where T: A<U>, U: A<T> + A<I> { + ${0:todo!()} +} +", + ) + } + + #[test] + fn params_in_trait_arg_are_not_dependency() { + // Even though `bar` depends on `U` and `I`, we don't have to copy these bounds: + // `T: A<I>` and `T: A<U>`. + check_assist( + generate_function, + r" +trait A<T> {} +struct S<T>(T); +impl<T, U> S<(T, U)> +where + T: A<U>, +{ + fn foo<I>(t: T, u: U) + where + T: A<I>, + U: A<I>, + { + $0bar(u) + } +} +", + r" +trait A<T> {} +struct S<T>(T); +impl<T, U> S<(T, U)> +where + T: A<U>, +{ + fn foo<I>(t: T, u: U) + where + T: A<I>, + U: A<I>, + { + bar(u) + } +} + +fn bar<U, I>(u: U) where U: A<I> { + ${0:todo!()} +} +", + ) + } + + #[test] + fn dont_copy_bounds_already_in_scope() { + check_assist( + generate_function, + r" +trait A<T> {} +struct S<T>(T); +impl<T: A<i32>> S<T> +where + T: A<usize>, +{ + fn foo<U: A<()>>(t: T, u: U) + where + T: A<S<i32>>, + { + Self::$0bar(t, u); + } +} +", + r" +trait A<T> {} +struct S<T>(T); +impl<T: A<i32>> S<T> +where + T: A<usize>, +{ + fn foo<U: A<()>>(t: T, u: U) + where + T: A<S<i32>>, + { + Self::bar(t, u); + } + + fn bar<U: A<()>>(t: T, u: U) ${0:-> _} where T: A<S<i32>> { + todo!() + } +} +", + ) + } + + #[test] fn add_function_with_fn_arg() { // FIXME: The argument in `bar` is wrong. check_assist( @@ -1290,6 +2033,50 @@ fn baz(foo: foo::Foo) { } #[test] + fn qualified_path_in_generic_bounds_uses_correct_scope() { + check_assist( + generate_function, + r" +mod a { + pub trait A {}; +} +pub mod b { + pub struct S<T>(T); +} +struct S<T>(T); +impl<T> S<T> +where + T: a::A, +{ + fn foo<U: a::A>(t: b::S<T>, u: S<U>) { + a::$0bar(t, u); + } +} +", + r" +mod a { + pub trait A {} + + pub(crate) fn bar<T, U: self::A>(t: crate::b::S<T>, u: crate::S<U>) ${0:-> _} where T: self::A { + todo!() + }; +} +pub mod b { + pub struct S<T>(T); +} +struct S<T>(T); +impl<T> S<T> +where + T: a::A, +{ + fn foo<U: a::A>(t: b::S<T>, u: S<U>) { + a::bar(t, u); + } +} +", + ) + } + #[test] fn add_function_in_module_containing_other_items() { check_assist( generate_function, @@ -1607,6 +2394,26 @@ fn foo() {S::bar();} } #[test] + fn create_generic_static_method() { + check_assist( + generate_function, + r" +struct S; +fn foo<T, const N: usize>(t: [T; N]) { S::bar$0(t); } +", + r" +struct S; +impl S { + fn bar<T, const N: usize>(t: [T; N]) ${0:-> _} { + todo!() + } +} +fn foo<T, const N: usize>(t: [T; N]) { S::bar(t); } +", + ) + } + + #[test] fn create_static_method_within_an_impl() { check_assist( generate_function, diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/generate_getter.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/generate_getter.rs index 15641b448..4595cfe29 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/generate_getter.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/generate_getter.rs @@ -180,7 +180,9 @@ pub(crate) fn generate_getter_impl( // Insert `$0` only for last getter we generate if i == record_fields_count - 1 { - getter_buf = getter_buf.replacen("fn ", "fn $0", 1); + if ctx.config.snippet_cap.is_some() { + getter_buf = getter_buf.replacen("fn ", "fn $0", 1); + } } // For first element we do not merge with '\n', as @@ -330,7 +332,7 @@ fn parse_record_field(record_field: ast::RecordField, mutable: bool) -> Option<R #[cfg(test)] mod tests { - use crate::tests::{check_assist, check_assist_not_applicable}; + use crate::tests::{check_assist, check_assist_no_snippet_cap, check_assist_not_applicable}; use super::*; @@ -378,6 +380,49 @@ impl Context { } #[test] + fn test_generate_getter_from_field_no_snippet_cap() { + check_assist_no_snippet_cap( + generate_getter, + r#" +struct Context { + dat$0a: Data, +} +"#, + r#" +struct Context { + data: Data, +} + +impl Context { + fn data(&self) -> &Data { + &self.data + } +} +"#, + ); + + check_assist_no_snippet_cap( + generate_getter_mut, + r#" +struct Context { + dat$0a: Data, +} +"#, + r#" +struct Context { + data: Data, +} + +impl Context { + fn data_mut(&mut self) -> &mut Data { + &mut self.data + } +} +"#, + ); + } + + #[test] fn test_generate_getter_already_implemented() { check_assist_not_applicable( generate_getter, @@ -434,6 +479,29 @@ impl Context { } #[test] + fn test_generate_getter_from_field_with_visibility_marker_no_snippet_cap() { + check_assist_no_snippet_cap( + generate_getter, + r#" +pub(crate) struct Context { + dat$0a: Data, +} +"#, + r#" +pub(crate) struct Context { + data: Data, +} + +impl Context { + pub(crate) fn data(&self) -> &Data { + &self.data + } +} +"#, + ); + } + + #[test] fn test_multiple_generate_getter() { check_assist( generate_getter, @@ -469,6 +537,41 @@ impl Context { } #[test] + fn test_multiple_generate_getter_no_snippet_cap() { + check_assist_no_snippet_cap( + generate_getter, + r#" +struct Context { + data: Data, + cou$0nt: usize, +} + +impl Context { + fn data(&self) -> &Data { + &self.data + } +} +"#, + r#" +struct Context { + data: Data, + count: usize, +} + +impl Context { + fn data(&self) -> &Data { + &self.data + } + + fn count(&self) -> &usize { + &self.count + } +} +"#, + ); + } + + #[test] fn test_not_a_special_case() { cov_mark::check_count!(convert_reference_type, 0); // Fake string which doesn't implement AsRef<str> diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/inline_macro.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/inline_macro.rs index 9d03f03d2..3fc552306 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/inline_macro.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/inline_macro.rs @@ -1,3 +1,4 @@ +use ide_db::syntax_helpers::insert_whitespace_into_node::insert_ws_into; use syntax::ast::{self, AstNode}; use crate::{AssistContext, AssistId, AssistKind, Assists}; @@ -35,7 +36,7 @@ use crate::{AssistContext, AssistId, AssistKind, Assists}; // ``` pub(crate) fn inline_macro(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { let unexpanded = ctx.find_node_at_offset::<ast::MacroCall>()?; - let expanded = ctx.sema.expand(&unexpanded)?.clone_for_update(); + let expanded = insert_ws_into(ctx.sema.expand(&unexpanded)?.clone_for_update()); let text_range = unexpanded.syntax().text_range(); @@ -230,4 +231,27 @@ fn f() { let result = foo$0(); } "#, ); } + + #[test] + fn inline_macro_with_whitespace() { + check_assist( + inline_macro, + r#" +macro_rules! whitespace { + () => { + if true {} + }; +} +fn f() { whitespace$0!(); } +"#, + r#" +macro_rules! whitespace { + () => { + if true {} + }; +} +fn f() { if true{}; } +"#, + ) + } } diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/merge_imports.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/merge_imports.rs index 2bdbec93b..d7ddc5f23 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/merge_imports.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/merge_imports.rs @@ -92,7 +92,7 @@ trait Merge: AstNode + Clone { fn try_merge_from(self, items: &mut dyn Iterator<Item = Self>) -> Option<Vec<Edit>> { let mut edits = Vec::new(); let mut merged = self.clone(); - while let Some(item) = items.next() { + for item in items { merged = merged.try_merge(&item)?; edits.push(Edit::Remove(item.into_either())); } diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/move_const_to_impl.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/move_const_to_impl.rs index 0e3a1e652..d848fce4b 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/move_const_to_impl.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/move_const_to_impl.rs @@ -5,10 +5,7 @@ use syntax::{ SyntaxKind, }; -use crate::{ - assist_context::{AssistContext, Assists}, - utils, -}; +use crate::assist_context::{AssistContext, Assists}; // NOTE: Code may break if the self type implements a trait that has associated const with the same // name, but it's pretty expensive to check that (`hir::Impl::all_for_type()`) and we assume that's @@ -130,9 +127,7 @@ pub(crate) fn move_const_to_impl(acc: &mut Assists, ctx: &AssistContext<'_>) -> let const_ = const_.clone_for_update(); const_.reindent_to(indent); - let mut const_text = format!("\n{indent}{const_}{fixup}"); - utils::escape_non_snippet(&mut const_text); - builder.insert(insert_offset, const_text); + builder.insert(insert_offset, format!("\n{indent}{const_}{fixup}")); }, ) } @@ -443,39 +438,4 @@ impl S { "#, ); } - - #[test] - fn moved_const_body_is_escaped() { - // Note that the last argument is what *lsp clients would see* rather than - // what users would see. Unescaping happens thereafter. - check_assist( - move_const_to_impl, - r#" -struct S; -impl S { - fn f() -> usize { - /// doc comment - /// \\ - /// ${snippet} - const C$0: &str = "\ and $1"; - - C.len() - } -} -"#, - r#" -struct S; -impl S { - /// doc comment - /// \\\\ - /// \${snippet} - const C: &str = "\\ and \$1"; - - fn f() -> usize { - Self::C.len() - } -} -"#, - ) - } } diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/raw_string.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/raw_string.rs index c9bc25b27..01420430b 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/raw_string.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/raw_string.rs @@ -2,7 +2,7 @@ use std::borrow::Cow; use syntax::{ast, ast::IsString, AstToken, TextRange, TextSize}; -use crate::{AssistContext, AssistId, AssistKind, Assists}; +use crate::{utils::required_hashes, AssistContext, AssistId, AssistKind, Assists}; // Assist: make_raw_string // @@ -155,16 +155,6 @@ pub(crate) fn remove_hash(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option< }) } -fn required_hashes(s: &str) -> usize { - let mut res = 0usize; - for idx in s.match_indices('"').map(|(i, _)| i) { - let (_, sub) = s.split_at(idx + 1); - let n_hashes = sub.chars().take_while(|c| *c == '#').count(); - res = res.max(n_hashes + 1) - } - res -} - #[cfg(test)] mod tests { use crate::tests::{check_assist, check_assist_not_applicable, check_assist_target}; @@ -172,17 +162,6 @@ mod tests { use super::*; #[test] - fn test_required_hashes() { - assert_eq!(0, required_hashes("abc")); - assert_eq!(0, required_hashes("###")); - assert_eq!(1, required_hashes("\"")); - assert_eq!(2, required_hashes("\"#abc")); - assert_eq!(0, required_hashes("#abc")); - assert_eq!(3, required_hashes("#ab\"##c")); - assert_eq!(5, required_hashes("#ab\"##\"####c")); - } - - #[test] fn make_raw_string_target() { check_assist_target( make_raw_string, diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/reorder_fields.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/reorder_fields.rs index a899c7a64..58dcaf9a2 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/reorder_fields.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/reorder_fields.rs @@ -20,10 +20,7 @@ use crate::{AssistContext, AssistId, AssistKind, Assists}; // const test: Foo = Foo {foo: 1, bar: 0} // ``` pub(crate) fn reorder_fields(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { - let record = ctx - .find_node_at_offset::<ast::RecordExpr>() - .map(Either::Left) - .or_else(|| ctx.find_node_at_offset::<ast::RecordPat>().map(Either::Right))?; + let record = ctx.find_node_at_offset::<Either<ast::RecordExpr, ast::RecordPat>>()?; let path = record.as_ref().either(|it| it.path(), |it| it.path())?; let ranks = compute_fields_ranks(&path, ctx)?; diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/replace_arith_op.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/replace_arith_op.rs index f1ca35caf..4b20b35c4 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/replace_arith_op.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/replace_arith_op.rs @@ -81,7 +81,7 @@ fn replace_arith(acc: &mut Assists, ctx: &AssistContext<'_>, kind: ArithKind) -> let range = TextRange::new(start, end); acc.add_group( - &GroupLabel("replace_arith".into()), + &GroupLabel("Replace arithmetic...".into()), kind.assist_id(), kind.label(), range, diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs index a6693d7d7..4cfae0c72 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs @@ -214,7 +214,7 @@ fn impl_def_from_trait( // Generate a default `impl` function body for the derived trait. if let ast::AssocItem::Fn(ref func) = first_assoc_item { - let _ = gen_trait_fn_body(func, trait_path, adt); + let _ = gen_trait_fn_body(func, trait_path, adt, None); }; Some((impl_def, first_assoc_item)) diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/replace_if_let_with_match.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/replace_if_let_with_match.rs index 484c27387..457559656 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/replace_if_let_with_match.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/replace_if_let_with_match.rs @@ -13,7 +13,7 @@ use syntax::{ edit::{AstNodeEdit, IndentLevel}, make, HasName, }, - AstNode, TextRange, + AstNode, TextRange, T, }; use crate::{ @@ -96,8 +96,9 @@ pub(crate) fn replace_if_let_with_match(acc: &mut Assists, ctx: &AssistContext<' cond_bodies.push((cond, body)); } - if !pat_seen { - // Don't offer turning an if (chain) without patterns into a match + if !pat_seen && cond_bodies.len() != 1 { + // Don't offer turning an if (chain) without patterns into a match, + // unless its a simple `if cond { .. } (else { .. })` return None; } @@ -114,6 +115,11 @@ pub(crate) fn replace_if_let_with_match(acc: &mut Assists, ctx: &AssistContext<' Either::Left(pat) => { make::match_arm(iter::once(pat), None, unwrap_trivial_block(body)) } + Either::Right(_) if !pat_seen => make::match_arm( + iter::once(make::literal_pat("true").into()), + None, + unwrap_trivial_block(body), + ), Either::Right(expr) => make::match_arm( iter::once(make::wildcard_pat().into()), Some(expr), @@ -144,31 +150,36 @@ fn make_else_arm( else_block: Option<ast::BlockExpr>, conditionals: &[(Either<ast::Pat, ast::Expr>, ast::BlockExpr)], ) -> ast::MatchArm { - if let Some(else_block) = else_block { - let pattern = if let [(Either::Left(pat), _)] = conditionals { - ctx.sema + let (pattern, expr) = if let Some(else_block) = else_block { + let pattern = match conditionals { + [(Either::Right(_), _)] => make::literal_pat("false").into(), + [(Either::Left(pat), _)] => match ctx + .sema .type_of_pat(pat) .and_then(|ty| TryEnum::from_ty(&ctx.sema, &ty.adjusted())) - .zip(Some(pat)) - } else { - None - }; - let pattern = match pattern { - Some((it, pat)) => { - if does_pat_match_variant(pat, &it.sad_pattern()) { - it.happy_pattern_wildcard() - } else if does_nested_pattern(pat) { - make::wildcard_pat().into() - } else { - it.sad_pattern() + { + Some(it) => { + if does_pat_match_variant(pat, &it.sad_pattern()) { + it.happy_pattern_wildcard() + } else if does_nested_pattern(pat) { + make::wildcard_pat().into() + } else { + it.sad_pattern() + } } - } - None => make::wildcard_pat().into(), + None => make::wildcard_pat().into(), + }, + _ => make::wildcard_pat().into(), }; - make::match_arm(iter::once(pattern), None, unwrap_trivial_block(else_block)) + (pattern, unwrap_trivial_block(else_block)) } else { - make::match_arm(iter::once(make::wildcard_pat().into()), None, make::expr_unit()) - } + let pattern = match conditionals { + [(Either::Right(_), _)] => make::literal_pat("false").into(), + _ => make::wildcard_pat().into(), + }; + (pattern, make::expr_unit()) + }; + make::match_arm(iter::once(pattern), None, expr) } // Assist: replace_match_with_if_let @@ -231,7 +242,19 @@ pub(crate) fn replace_match_with_if_let(acc: &mut Assists, ctx: &AssistContext<' } } - let condition = make::expr_let(if_let_pat, scrutinee); + let condition = match if_let_pat { + ast::Pat::LiteralPat(p) + if p.literal().map_or(false, |it| it.token().kind() == T![true]) => + { + scrutinee + } + ast::Pat::LiteralPat(p) + if p.literal().map_or(false, |it| it.token().kind() == T![false]) => + { + make::expr_prefix(T![!], scrutinee) + } + _ => make::expr_let(if_let_pat, scrutinee).into(), + }; let then_block = make_block_expr(then_expr.reset_indent()); let else_expr = if is_empty_expr(&else_expr) { None } else { Some(else_expr) }; let if_let_expr = make::expr_if( @@ -328,6 +351,58 @@ fn main() { } #[test] + fn test_if_with_match_no_else() { + check_assist( + replace_if_let_with_match, + r#" +pub fn foo(foo: bool) { + if foo$0 { + self.foo(); + } +} +"#, + r#" +pub fn foo(foo: bool) { + match foo { + true => { + self.foo(); + } + false => (), + } +} +"#, + ) + } + + #[test] + fn test_if_with_match_with_else() { + check_assist( + replace_if_let_with_match, + r#" +pub fn foo(foo: bool) { + if foo$0 { + self.foo(); + } else { + self.bar(); + } +} +"#, + r#" +pub fn foo(foo: bool) { + match foo { + true => { + self.foo(); + } + false => { + self.bar(); + } + } +} +"#, + ) + } + + #[test] fn test_if_let_with_match_no_else() { check_assist( replace_if_let_with_match, @@ -996,4 +1071,64 @@ fn main() { "#, ) } + + #[test] + fn test_replace_match_with_if_bool() { + check_assist( + replace_match_with_if_let, + r#" +fn main() { + match$0 b { + true => (), + _ => code(), + } +} +"#, + r#" +fn main() { + if b { + () + } else { + code() + } +} +"#, + ); + check_assist( + replace_match_with_if_let, + r#" +fn main() { + match$0 b { + false => code(), + true => (), + } +} +"#, + r#" +fn main() { + if !b { + code() + } +} +"#, + ); + check_assist( + replace_match_with_if_let, + r#" +fn main() { + match$0 b { + false => (), + true => code(), + } +} +"#, + r#" +fn main() { + if b { + code() + } +} +"#, + ) + } } diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/unmerge_match_arm.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/unmerge_match_arm.rs index 9565f0ee6..db789cfa3 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/unmerge_match_arm.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/unmerge_match_arm.rs @@ -86,8 +86,7 @@ pub(crate) fn unmerge_match_arm(acc: &mut Assists, ctx: &AssistContext<'_>) -> O it.prev_sibling_or_token() }) .map(|it| it.kind()) - .skip_while(|it| it.is_trivia()) - .next() + .find(|it| !it.is_trivia()) == Some(T![,]); let has_arms_after = neighbor(&match_arm, Direction::Next).is_some(); if !has_comma_after && !has_arms_after { diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/unwrap_block.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/unwrap_block.rs index 53cdac03a..33b19a354 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/unwrap_block.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/unwrap_block.rs @@ -2,6 +2,7 @@ use syntax::{ ast::{ self, edit::{AstNodeEdit, IndentLevel}, + make, }, AstNode, SyntaxKind, TextRange, T, }; @@ -37,61 +38,89 @@ pub(crate) fn unwrap_block(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option parent = parent.ancestors().find(|it| ast::MatchExpr::can_cast(it.kind()))? } - if matches!(parent.kind(), SyntaxKind::STMT_LIST | SyntaxKind::EXPR_STMT | SyntaxKind::LET_STMT) - { - return acc.add(assist_id, assist_label, target, |builder| { + let kind = parent.kind(); + if matches!(kind, SyntaxKind::STMT_LIST | SyntaxKind::EXPR_STMT) { + acc.add(assist_id, assist_label, target, |builder| { builder.replace(block.syntax().text_range(), update_expr_string(block.to_string())); - }); - } - - let parent = ast::Expr::cast(parent)?; - - match parent.clone() { - ast::Expr::ForExpr(_) | ast::Expr::WhileExpr(_) | ast::Expr::LoopExpr(_) => (), - ast::Expr::MatchExpr(_) => block = block.dedent(IndentLevel(1)), - ast::Expr::IfExpr(if_expr) => { - let then_branch = if_expr.then_branch()?; - if then_branch == block { - if let Some(ancestor) = if_expr.syntax().parent().and_then(ast::IfExpr::cast) { - // For `else if` blocks - let ancestor_then_branch = ancestor.then_branch()?; - + }) + } else if matches!(kind, SyntaxKind::LET_STMT) { + let parent = ast::LetStmt::cast(parent)?; + let pattern = ast::Pat::cast(parent.syntax().first_child()?)?; + let ty = parent.ty(); + let list = block.stmt_list()?; + let replaced = match list.syntax().last_child() { + Some(last) => { + let stmts: Vec<ast::Stmt> = list.statements().collect(); + let initializer = ast::Expr::cast(last.clone())?; + let let_stmt = make::let_stmt(pattern, ty, Some(initializer)); + if stmts.len() > 0 { + let block = make::block_expr(stmts, None); + format!( + "{}\n {}", + update_expr_string(block.to_string()), + let_stmt.to_string() + ) + } else { + let_stmt.to_string() + } + } + None => { + let empty_tuple = make::expr_tuple([]); + make::let_stmt(pattern, ty, Some(empty_tuple)).to_string() + } + }; + acc.add(assist_id, assist_label, target, |builder| { + builder.replace(parent.syntax().text_range(), replaced); + }) + } else { + let parent = ast::Expr::cast(parent)?; + match parent.clone() { + ast::Expr::ForExpr(_) | ast::Expr::WhileExpr(_) | ast::Expr::LoopExpr(_) => (), + ast::Expr::MatchExpr(_) => block = block.dedent(IndentLevel(1)), + ast::Expr::IfExpr(if_expr) => { + let then_branch = if_expr.then_branch()?; + if then_branch == block { + if let Some(ancestor) = if_expr.syntax().parent().and_then(ast::IfExpr::cast) { + // For `else if` blocks + let ancestor_then_branch = ancestor.then_branch()?; + + return acc.add(assist_id, assist_label, target, |edit| { + let range_to_del_else_if = TextRange::new( + ancestor_then_branch.syntax().text_range().end(), + l_curly_token.text_range().start(), + ); + let range_to_del_rest = TextRange::new( + then_branch.syntax().text_range().end(), + if_expr.syntax().text_range().end(), + ); + + edit.delete(range_to_del_rest); + edit.delete(range_to_del_else_if); + edit.replace( + target, + update_expr_string_without_newline(then_branch.to_string()), + ); + }); + } + } else { return acc.add(assist_id, assist_label, target, |edit| { - let range_to_del_else_if = TextRange::new( - ancestor_then_branch.syntax().text_range().end(), - l_curly_token.text_range().start(), - ); - let range_to_del_rest = TextRange::new( + let range_to_del = TextRange::new( then_branch.syntax().text_range().end(), - if_expr.syntax().text_range().end(), + l_curly_token.text_range().start(), ); - edit.delete(range_to_del_rest); - edit.delete(range_to_del_else_if); - edit.replace( - target, - update_expr_string_without_newline(then_branch.to_string()), - ); + edit.delete(range_to_del); + edit.replace(target, update_expr_string_without_newline(block.to_string())); }); } - } else { - return acc.add(assist_id, assist_label, target, |edit| { - let range_to_del = TextRange::new( - then_branch.syntax().text_range().end(), - l_curly_token.text_range().start(), - ); - - edit.delete(range_to_del); - edit.replace(target, update_expr_string_without_newline(block.to_string())); - }); } - } - _ => return None, - }; + _ => return None, + }; - acc.add(assist_id, assist_label, target, |builder| { - builder.replace(parent.syntax().text_range(), update_expr_string(block.to_string())); - }) + acc.add(assist_id, assist_label, target, |builder| { + builder.replace(parent.syntax().text_range(), update_expr_string(block.to_string())); + }) + } } fn update_expr_string(expr_string: String) -> String { @@ -725,6 +754,19 @@ fn main() -> i32 { unwrap_block, r#" fn main() { + let x = {$0}; +} +"#, + r#" +fn main() { + let x = (); +} +"#, + ); + check_assist( + unwrap_block, + r#" +fn main() { let x = {$0 bar }; @@ -736,6 +778,34 @@ fn main() { } "#, ); + check_assist( + unwrap_block, + r#" +fn main() -> i32 { + let _ = {$01; 2}; +} +"#, + r#" +fn main() -> i32 { + 1; + let _ = 2; +} +"#, + ); + check_assist( + unwrap_block, + r#" +fn main() -> i32 { + let mut a = {$01; 2}; +} +"#, + r#" +fn main() -> i32 { + 1; + let mut a = 2; +} +"#, + ); } #[test] diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/lib.rs b/src/tools/rust-analyzer/crates/ide-assists/src/lib.rs index 7813c9f9c..276cf5f5d 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/lib.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/lib.rs @@ -106,6 +106,7 @@ mod handlers { pub(crate) type Handler = fn(&mut Assists, &AssistContext<'_>) -> Option<()>; + mod add_braces; mod add_explicit_type; mod add_label_to_loop; mod add_lifetime_to_type; @@ -126,6 +127,7 @@ mod handlers { mod convert_to_guarded_return; mod convert_two_arm_bool_match_to_matches_macro; mod convert_while_to_loop; + mod desugar_doc_comment; mod destructure_tuple_binding; mod expand_glob_import; mod extract_expressions_from_format_string; @@ -208,6 +210,7 @@ mod handlers { pub(crate) fn all() -> &'static [Handler] { &[ // These are alphabetic for the foolish consistency + add_braces::add_braces, add_explicit_type::add_explicit_type, add_label_to_loop::add_label_to_loop, add_missing_match_arms::add_missing_match_arms, @@ -231,6 +234,7 @@ mod handlers { convert_tuple_struct_to_named_struct::convert_tuple_struct_to_named_struct, convert_two_arm_bool_match_to_matches_macro::convert_two_arm_bool_match_to_matches_macro, convert_while_to_loop::convert_while_to_loop, + desugar_doc_comment::desugar_doc_comment, destructure_tuple_binding::destructure_tuple_binding, expand_glob_import::expand_glob_import, extract_expressions_from_format_string::extract_expressions_from_format_string, diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/tests.rs b/src/tools/rust-analyzer/crates/ide-assists/src/tests.rs index fca268a1f..94be99fd7 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/tests.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/tests.rs @@ -33,6 +33,20 @@ pub(crate) const TEST_CONFIG: AssistConfig = AssistConfig { assist_emit_must_use: false, }; +pub(crate) const TEST_CONFIG_NO_SNIPPET_CAP: AssistConfig = AssistConfig { + snippet_cap: None, + allowed: None, + insert_use: InsertUseConfig { + granularity: ImportGranularity::Crate, + prefix_kind: hir::PrefixKind::Plain, + enforce_granularity: true, + group: true, + skip_glob_imports: true, + }, + prefer_no_std: false, + assist_emit_must_use: false, +}; + pub(crate) fn with_single_file(text: &str) -> (RootDatabase, FileId) { RootDatabase::with_single_file(text) } @@ -43,6 +57,22 @@ pub(crate) fn check_assist(assist: Handler, ra_fixture_before: &str, ra_fixture_ check(assist, ra_fixture_before, ExpectedResult::After(&ra_fixture_after), None); } +#[track_caller] +pub(crate) fn check_assist_no_snippet_cap( + assist: Handler, + ra_fixture_before: &str, + ra_fixture_after: &str, +) { + let ra_fixture_after = trim_indent(ra_fixture_after); + check_with_config( + TEST_CONFIG_NO_SNIPPET_CAP, + assist, + ra_fixture_before, + ExpectedResult::After(&ra_fixture_after), + None, + ); +} + // There is no way to choose what assist within a group you want to test against, // so this is here to allow you choose. pub(crate) fn check_assist_by_label( @@ -119,6 +149,17 @@ enum ExpectedResult<'a> { #[track_caller] fn check(handler: Handler, before: &str, expected: ExpectedResult<'_>, assist_label: Option<&str>) { + check_with_config(TEST_CONFIG, handler, before, expected, assist_label); +} + +#[track_caller] +fn check_with_config( + config: AssistConfig, + handler: Handler, + before: &str, + expected: ExpectedResult<'_>, + assist_label: Option<&str>, +) { let (mut db, file_with_caret_id, range_or_offset) = RootDatabase::with_range_or_offset(before); db.set_enable_proc_attr_macros(true); let text_without_caret = db.file_text(file_with_caret_id).to_string(); @@ -126,7 +167,6 @@ fn check(handler: Handler, before: &str, expected: ExpectedResult<'_>, assist_la let frange = FileRange { file_id: file_with_caret_id, range: range_or_offset.into() }; let sema = Semantics::new(&db); - let config = TEST_CONFIG; let ctx = AssistContext::new(sema, &config, frange); let resolve = match expected { ExpectedResult::Unresolved => AssistResolveStrategy::None, diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/tests/generated.rs b/src/tools/rust-analyzer/crates/ide-assists/src/tests/generated.rs index 006ae4b30..8a25e1f64 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/tests/generated.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/tests/generated.rs @@ -3,6 +3,31 @@ use super::check_doc_test; #[test] +fn doctest_add_braces() { + check_doc_test( + "add_braces", + r#####" +fn foo(n: i32) -> i32 { + match n { + 1 =>$0 n + 1, + _ => 0 + } +} +"#####, + r#####" +fn foo(n: i32) -> i32 { + match n { + 1 => { + n + 1 + }, + _ => 0 + } +} +"#####, + ) +} + +#[test] fn doctest_add_explicit_type() { check_doc_test( "add_explicit_type", @@ -598,6 +623,21 @@ fn main() { } #[test] +fn doctest_desugar_doc_comment() { + check_doc_test( + "desugar_doc_comment", + r#####" +/// Multi-line$0 +/// comment +"#####, + r#####" +#[doc = r"Multi-line +comment"] +"#####, + ) +} + +#[test] fn doctest_expand_glob_import() { check_doc_test( "expand_glob_import", diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/utils.rs b/src/tools/rust-analyzer/crates/ide-assists/src/utils.rs index 7add66064..f323ebcf7 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/utils.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/utils.rs @@ -208,23 +208,6 @@ pub(crate) fn render_snippet(_cap: SnippetCap, node: &SyntaxNode, cursor: Cursor } } -/// Escapes text that should be rendered as-is, typically those that we're copy-pasting what the -/// users wrote. -/// -/// This function should only be used when the text doesn't contain snippet **AND** the text -/// wouldn't be included in a snippet. -pub(crate) fn escape_non_snippet(text: &mut String) { - // While we *can* escape `}`, we don't really have to in this specific case. We only need to - // escape it inside `${}` to disambiguate it from the ending token of the syntax, but after we - // escape every occurrence of `$`, we wouldn't have `${}` in the first place. - // - // This will break if the text contains snippet or it will be included in a snippet (hence doc - // comment). Compare `fn escape(buf)` in `render_snippet()` above, where the escaped text is - // included in a snippet. - stdx::replace(text, '\\', r"\\"); - stdx::replace(text, '$', r"\$"); -} - pub(crate) fn vis_offset(node: &SyntaxNode) -> TextSize { node.children_with_tokens() .find(|it| !matches!(it.kind(), WHITESPACE | COMMENT | ATTR)) @@ -758,3 +741,24 @@ pub(crate) fn convert_param_list_to_arg_list(list: ast::ParamList) -> ast::ArgLi } make::arg_list(args) } + +/// Calculate the number of hashes required for a raw string containing `s` +pub(crate) fn required_hashes(s: &str) -> usize { + let mut res = 0usize; + for idx in s.match_indices('"').map(|(i, _)| i) { + let (_, sub) = s.split_at(idx + 1); + let n_hashes = sub.chars().take_while(|c| *c == '#').count(); + res = res.max(n_hashes + 1) + } + res +} +#[test] +fn test_required_hashes() { + assert_eq!(0, required_hashes("abc")); + assert_eq!(0, required_hashes("###")); + assert_eq!(1, required_hashes("\"")); + assert_eq!(2, required_hashes("\"#abc")); + assert_eq!(0, required_hashes("#abc")); + assert_eq!(3, required_hashes("#ab\"##c")); + assert_eq!(5, required_hashes("#ab\"##\"####c")); +} diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/utils/gen_trait_fn_body.rs b/src/tools/rust-analyzer/crates/ide-assists/src/utils/gen_trait_fn_body.rs index d4abb5125..808b23405 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/utils/gen_trait_fn_body.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/utils/gen_trait_fn_body.rs @@ -1,5 +1,6 @@ //! This module contains functions to generate default trait impl function bodies where possible. +use hir::TraitRef; use syntax::{ ast::{self, edit::AstNodeEdit, make, AstNode, BinaryOp, CmpOp, HasName, LogicOp}, ted, @@ -7,6 +8,8 @@ use syntax::{ /// Generate custom trait bodies without default implementation where possible. /// +/// If `func` is defined within an existing impl block, pass [`TraitRef`]. Otherwise pass `None`. +/// /// Returns `Option` so that we can use `?` rather than `if let Some`. Returning /// `None` means that generating a custom trait body failed, and the body will remain /// as `todo!` instead. @@ -14,14 +17,15 @@ pub(crate) fn gen_trait_fn_body( func: &ast::Fn, trait_path: &ast::Path, adt: &ast::Adt, + trait_ref: Option<TraitRef>, ) -> Option<()> { match trait_path.segment()?.name_ref()?.text().as_str() { "Clone" => gen_clone_impl(adt, func), "Debug" => gen_debug_impl(adt, func), "Default" => gen_default_impl(adt, func), "Hash" => gen_hash_impl(adt, func), - "PartialEq" => gen_partial_eq(adt, func), - "PartialOrd" => gen_partial_ord(adt, func), + "PartialEq" => gen_partial_eq(adt, func, trait_ref), + "PartialOrd" => gen_partial_ord(adt, func, trait_ref), _ => None, } } @@ -395,7 +399,7 @@ fn gen_hash_impl(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { } /// Generate a `PartialEq` impl based on the fields and members of the target type. -fn gen_partial_eq(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { +fn gen_partial_eq(adt: &ast::Adt, func: &ast::Fn, trait_ref: Option<TraitRef>) -> Option<()> { stdx::always!(func.name().map_or(false, |name| name.text() == "eq")); fn gen_eq_chain(expr: Option<ast::Expr>, cmp: ast::Expr) -> Option<ast::Expr> { match expr { @@ -423,8 +427,15 @@ fn gen_partial_eq(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { ast::Pat::IdentPat(make::ident_pat(false, false, make::name(field_name))) } - // FIXME: return `None` if the trait carries a generic type; we can only - // generate this code `Self` for the time being. + // Check that self type and rhs type match. We don't know how to implement the method + // automatically otherwise. + if let Some(trait_ref) = trait_ref { + let self_ty = trait_ref.self_ty(); + let rhs_ty = trait_ref.get_type_argument(1)?; + if self_ty != rhs_ty { + return None; + } + } let body = match adt { // `PartialEq` cannot be derived for unions, so no default impl can be provided. @@ -568,7 +579,7 @@ fn gen_partial_eq(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { make::block_expr(None, expr).indent(ast::edit::IndentLevel(1)) } - // No fields in the body means there's nothing to hash. + // No fields in the body means there's nothing to compare. None => { let expr = make::expr_literal("true").into(); make::block_expr(None, Some(expr)).indent(ast::edit::IndentLevel(1)) @@ -580,7 +591,7 @@ fn gen_partial_eq(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { Some(()) } -fn gen_partial_ord(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { +fn gen_partial_ord(adt: &ast::Adt, func: &ast::Fn, trait_ref: Option<TraitRef>) -> Option<()> { stdx::always!(func.name().map_or(false, |name| name.text() == "partial_cmp")); fn gen_partial_eq_match(match_target: ast::Expr) -> Option<ast::Stmt> { let mut arms = vec![]; @@ -605,8 +616,15 @@ fn gen_partial_ord(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { make::expr_method_call(lhs, method, make::arg_list(Some(rhs))) } - // FIXME: return `None` if the trait carries a generic type; we can only - // generate this code `Self` for the time being. + // Check that self type and rhs type match. We don't know how to implement the method + // automatically otherwise. + if let Some(trait_ref) = trait_ref { + let self_ty = trait_ref.self_ty(); + let rhs_ty = trait_ref.get_type_argument(1)?; + if self_ty != rhs_ty { + return None; + } + } let body = match adt { // `PartialOrd` cannot be derived for unions, so no default impl can be provided. |