Unnamed repository; edit this file 'description' to name the repository.
In "Fill match arms", allow users to prefer `Self` to the enum name when possible
But default to not to.
I chose to have a more generic config name because maybe other assists could also use the same approach.
| -rw-r--r-- | crates/ide-assists/src/assist_config.rs | 1 | ||||
| -rw-r--r-- | crates/ide-assists/src/handlers/add_missing_match_arms.rs | 207 | ||||
| -rw-r--r-- | crates/ide-assists/src/tests.rs | 21 | ||||
| -rw-r--r-- | crates/rust-analyzer/src/config.rs | 3 | ||||
| -rw-r--r-- | crates/syntax/src/ast/make.rs | 7 | ||||
| -rw-r--r-- | docs/book/src/configuration_generated.md | 7 | ||||
| -rw-r--r-- | editors/code/package.json | 10 |
7 files changed, 227 insertions, 29 deletions
diff --git a/crates/ide-assists/src/assist_config.rs b/crates/ide-assists/src/assist_config.rs index fb569f8cda..57ced8d853 100644 --- a/crates/ide-assists/src/assist_config.rs +++ b/crates/ide-assists/src/assist_config.rs @@ -22,6 +22,7 @@ pub struct AssistConfig { pub term_search_borrowck: bool, pub code_action_grouping: bool, pub expr_fill_default: ExprFillDefaultMode, + pub prefer_self_ty: bool, } impl AssistConfig { diff --git a/crates/ide-assists/src/handlers/add_missing_match_arms.rs b/crates/ide-assists/src/handlers/add_missing_match_arms.rs index 858d436991..682a8e6c78 100644 --- a/crates/ide-assists/src/handlers/add_missing_match_arms.rs +++ b/crates/ide-assists/src/handlers/add_missing_match_arms.rs @@ -1,12 +1,13 @@ use std::iter::{self, Peekable}; use either::Either; -use hir::{Adt, Crate, HasAttrs, ImportPathConfig, ModuleDef, Semantics, sym}; +use hir::{Adt, AsAssocItem, Crate, HasAttrs, ImportPathConfig, ModuleDef, Semantics, sym}; use ide_db::RootDatabase; use ide_db::assists::ExprFillDefaultMode; use ide_db::syntax_helpers::suggest_name; use ide_db::{famous_defs::FamousDefs, helpers::mod_path_to_ast}; use itertools::Itertools; +use syntax::ToSmolStr; use syntax::ast::edit::IndentLevel; use syntax::ast::edit_in_place::Indent; use syntax::ast::syntax_factory::SyntaxFactory; @@ -79,12 +80,20 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>) let make = SyntaxFactory::with_mappings(); - let module = ctx.sema.scope(expr.syntax())?.module(); + let scope = ctx.sema.scope(expr.syntax())?; + let module = scope.module(); + let self_ty = if ctx.config.prefer_self_ty { + scope + .containing_function() + .and_then(|function| function.as_assoc_item(ctx.db())?.implementing_ty(ctx.db())) + } else { + None + }; let (mut missing_pats, is_non_exhaustive, has_hidden_variants): ( Peekable<Box<dyn Iterator<Item = (ast::Pat, bool)>>>, bool, bool, - ) = if let Some(enum_def) = resolve_enum_def(&ctx.sema, &expr) { + ) = if let Some(enum_def) = resolve_enum_def(&ctx.sema, &expr, self_ty.as_ref()) { let is_non_exhaustive = enum_def.is_non_exhaustive(ctx.db(), module.krate()); let variants = enum_def.variants(ctx.db()); @@ -102,8 +111,9 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>) }) .filter(|(variant_pat, _)| is_variant_missing(&top_lvl_pats, variant_pat)); - let option_enum = FamousDefs(&ctx.sema, module.krate()).core_option_Option().map(lift_enum); - let missing_pats: Box<dyn Iterator<Item = _>> = if Some(enum_def) == option_enum { + let option_enum = FamousDefs(&ctx.sema, module.krate()).core_option_Option(); + let missing_pats: Box<dyn Iterator<Item = _>> = if matches!(enum_def, ExtendedEnum::Enum { enum_: e, .. } if Some(e) == option_enum) + { // Match `Some` variant first. cov_mark::hit!(option_order); Box::new(missing_pats.rev()) @@ -111,7 +121,7 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>) Box::new(missing_pats) }; (missing_pats.peekable(), is_non_exhaustive, has_hidden_variants) - } else if let Some(enum_defs) = resolve_tuple_of_enum_def(&ctx.sema, &expr) { + } else if let Some(enum_defs) = resolve_tuple_of_enum_def(&ctx.sema, &expr, self_ty.as_ref()) { let is_non_exhaustive = enum_defs.iter().any(|enum_def| enum_def.is_non_exhaustive(ctx.db(), module.krate())); @@ -159,7 +169,9 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>) is_non_exhaustive, has_hidden_variants, ) - } else if let Some((enum_def, len)) = resolve_array_of_enum_def(&ctx.sema, &expr) { + } else if let Some((enum_def, len)) = + resolve_array_of_enum_def(&ctx.sema, &expr, self_ty.as_ref()) + { let is_non_exhaustive = enum_def.is_non_exhaustive(ctx.db(), module.krate()); let variants = enum_def.variants(ctx.db()); @@ -373,23 +385,23 @@ fn does_pat_match_variant(pat: &Pat, var: &Pat) -> bool { } } -#[derive(Eq, PartialEq, Clone, Copy)] +#[derive(Eq, PartialEq, Clone)] enum ExtendedEnum { Bool, - Enum(hir::Enum), + Enum { enum_: hir::Enum, use_self: bool }, } #[derive(Eq, PartialEq, Clone, Copy, Debug)] enum ExtendedVariant { True, False, - Variant(hir::Variant), + Variant { variant: hir::Variant, use_self: bool }, } impl ExtendedVariant { fn should_be_hidden(self, db: &RootDatabase, krate: Crate) -> bool { match self { - ExtendedVariant::Variant(var) => { + ExtendedVariant::Variant { variant: var, .. } => { var.attrs(db).has_doc_hidden() && var.module(db).krate() != krate } _ => false, @@ -397,25 +409,35 @@ impl ExtendedVariant { } } -fn lift_enum(e: hir::Enum) -> ExtendedEnum { - ExtendedEnum::Enum(e) -} - impl ExtendedEnum { - fn is_non_exhaustive(self, db: &RootDatabase, krate: Crate) -> bool { + fn enum_( + db: &RootDatabase, + enum_: hir::Enum, + enum_ty: &hir::Type, + self_ty: Option<&hir::Type>, + ) -> Self { + ExtendedEnum::Enum { + enum_, + use_self: self_ty.is_some_and(|self_ty| self_ty.could_unify_with_deeply(db, enum_ty)), + } + } + + fn is_non_exhaustive(&self, db: &RootDatabase, krate: Crate) -> bool { match self { - ExtendedEnum::Enum(e) => { + ExtendedEnum::Enum { enum_: e, .. } => { e.attrs(db).by_key(sym::non_exhaustive).exists() && e.module(db).krate() != krate } _ => false, } } - fn variants(self, db: &RootDatabase) -> Vec<ExtendedVariant> { - match self { - ExtendedEnum::Enum(e) => { - e.variants(db).into_iter().map(ExtendedVariant::Variant).collect::<Vec<_>>() - } + fn variants(&self, db: &RootDatabase) -> Vec<ExtendedVariant> { + match *self { + ExtendedEnum::Enum { enum_: e, use_self } => e + .variants(db) + .into_iter() + .map(|variant| ExtendedVariant::Variant { variant, use_self }) + .collect::<Vec<_>>(), ExtendedEnum::Bool => { Vec::<ExtendedVariant>::from([ExtendedVariant::True, ExtendedVariant::False]) } @@ -423,9 +445,13 @@ impl ExtendedEnum { } } -fn resolve_enum_def(sema: &Semantics<'_, RootDatabase>, expr: &ast::Expr) -> Option<ExtendedEnum> { +fn resolve_enum_def( + sema: &Semantics<'_, RootDatabase>, + expr: &ast::Expr, + self_ty: Option<&hir::Type>, +) -> Option<ExtendedEnum> { sema.type_of_expr(expr)?.adjusted().autoderef(sema.db).find_map(|ty| match ty.as_adt() { - Some(Adt::Enum(e)) => Some(ExtendedEnum::Enum(e)), + Some(Adt::Enum(e)) => Some(ExtendedEnum::enum_(sema.db, e, &ty, self_ty)), _ => ty.is_bool().then_some(ExtendedEnum::Bool), }) } @@ -433,6 +459,7 @@ fn resolve_enum_def(sema: &Semantics<'_, RootDatabase>, expr: &ast::Expr) -> Opt fn resolve_tuple_of_enum_def( sema: &Semantics<'_, RootDatabase>, expr: &ast::Expr, + self_ty: Option<&hir::Type>, ) -> Option<Vec<ExtendedEnum>> { sema.type_of_expr(expr)? .adjusted() @@ -441,7 +468,7 @@ fn resolve_tuple_of_enum_def( .map(|ty| { ty.autoderef(sema.db).find_map(|ty| { match ty.as_adt() { - Some(Adt::Enum(e)) => Some(lift_enum(e)), + Some(Adt::Enum(e)) => Some(ExtendedEnum::enum_(sema.db, e, &ty, self_ty)), // 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>>. @@ -456,10 +483,11 @@ fn resolve_tuple_of_enum_def( fn resolve_array_of_enum_def( sema: &Semantics<'_, RootDatabase>, expr: &ast::Expr, + self_ty: Option<&hir::Type>, ) -> 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)), + Some(Adt::Enum(e)) => Some((ExtendedEnum::enum_(sema.db, e, &ty, self_ty), len)), _ => ty.is_bool().then_some((ExtendedEnum::Bool, len)), }) }) @@ -474,9 +502,21 @@ fn build_pat( ) -> Option<ast::Pat> { let db = ctx.db(); match var { - ExtendedVariant::Variant(var) => { + ExtendedVariant::Variant { variant: var, use_self } => { let edition = module.krate().edition(db); - let path = mod_path_to_ast(&module.find_path(db, ModuleDef::from(var), cfg)?, edition); + let path = if use_self { + make::path_from_segments( + [ + make::path_segment(make::name_ref_self_ty()), + make::path_segment(make::name_ref( + &var.name(db).display(db, edition).to_smolstr(), + )), + ], + false, + ) + } else { + mod_path_to_ast(&module.find_path(db, ModuleDef::from(var), cfg)?, edition) + }; let fields = var.fields(db); let pat: ast::Pat = match var.kind(db) { hir::StructKind::Tuple => { @@ -509,8 +549,10 @@ fn build_pat( #[cfg(test)] mod tests { + use crate::AssistConfig; use crate::tests::{ - check_assist, check_assist_not_applicable, check_assist_target, check_assist_unresolved, + TEST_CONFIG, check_assist, check_assist_not_applicable, check_assist_target, + check_assist_unresolved, check_assist_with_config, }; use super::add_missing_match_arms; @@ -2095,4 +2137,111 @@ fn f() { "#, ); } + + #[test] + fn prefer_self() { + check_assist_with_config( + add_missing_match_arms, + AssistConfig { prefer_self_ty: true, ..TEST_CONFIG }, + r#" +enum Foo { + Bar, + Baz, +} + +impl Foo { + fn qux(&self) { + match self { + $0_ => {} + } + } +} + "#, + r#" +enum Foo { + Bar, + Baz, +} + +impl Foo { + fn qux(&self) { + match self { + Self::Bar => ${1:todo!()}, + Self::Baz => ${2:todo!()},$0 + } + } +} + "#, + ); + } + + #[test] + fn prefer_self_with_generics() { + check_assist_with_config( + add_missing_match_arms, + AssistConfig { prefer_self_ty: true, ..TEST_CONFIG }, + r#" +enum Foo<T> { + Bar(T), + Baz, +} + +impl<T> Foo<T> { + fn qux(&self) { + match self { + $0_ => {} + } + } +} + "#, + r#" +enum Foo<T> { + Bar(T), + Baz, +} + +impl<T> Foo<T> { + fn qux(&self) { + match self { + Self::Bar(${1:_}) => ${2:todo!()}, + Self::Baz => ${3:todo!()},$0 + } + } +} + "#, + ); + check_assist_with_config( + add_missing_match_arms, + AssistConfig { prefer_self_ty: true, ..TEST_CONFIG }, + r#" +enum Foo<T> { + Bar(T), + Baz, +} + +impl<T> Foo<T> { + fn qux(v: Foo<i32>) { + match v { + $0_ => {} + } + } +} + "#, + r#" +enum Foo<T> { + Bar(T), + Baz, +} + +impl<T> Foo<T> { + fn qux(v: Foo<i32>) { + match v { + Foo::Bar(${1:_}) => ${2:todo!()}, + Foo::Baz => ${3:todo!()},$0 + } + } +} + "#, + ); + } } diff --git a/crates/ide-assists/src/tests.rs b/crates/ide-assists/src/tests.rs index 5e6889792d..cda2ad4327 100644 --- a/crates/ide-assists/src/tests.rs +++ b/crates/ide-assists/src/tests.rs @@ -37,6 +37,7 @@ pub(crate) const TEST_CONFIG: AssistConfig = AssistConfig { term_search_borrowck: true, code_action_grouping: true, expr_fill_default: ExprFillDefaultMode::Todo, + prefer_self_ty: false, }; pub(crate) const TEST_CONFIG_NO_GROUPING: AssistConfig = AssistConfig { @@ -57,6 +58,7 @@ pub(crate) const TEST_CONFIG_NO_GROUPING: AssistConfig = AssistConfig { term_search_borrowck: true, code_action_grouping: false, expr_fill_default: ExprFillDefaultMode::Todo, + prefer_self_ty: false, }; pub(crate) const TEST_CONFIG_NO_SNIPPET_CAP: AssistConfig = AssistConfig { @@ -77,6 +79,7 @@ pub(crate) const TEST_CONFIG_NO_SNIPPET_CAP: AssistConfig = AssistConfig { term_search_borrowck: true, code_action_grouping: true, expr_fill_default: ExprFillDefaultMode::Todo, + prefer_self_ty: false, }; pub(crate) const TEST_CONFIG_IMPORT_ONE: AssistConfig = AssistConfig { @@ -97,6 +100,7 @@ pub(crate) const TEST_CONFIG_IMPORT_ONE: AssistConfig = AssistConfig { term_search_borrowck: true, code_action_grouping: true, expr_fill_default: ExprFillDefaultMode::Todo, + prefer_self_ty: false, }; pub(crate) fn with_single_file(text: &str) -> (RootDatabase, EditionedFileId) { @@ -114,6 +118,23 @@ pub(crate) fn check_assist( } #[track_caller] +pub(crate) fn check_assist_with_config( + assist: Handler, + config: AssistConfig, + #[rust_analyzer::rust_fixture] ra_fixture_before: &str, + #[rust_analyzer::rust_fixture] ra_fixture_after: &str, +) { + let ra_fixture_after = trim_indent(ra_fixture_after); + check_with_config( + config, + assist, + ra_fixture_before, + ExpectedResult::After(&ra_fixture_after), + None, + ); +} + +#[track_caller] pub(crate) fn check_assist_no_snippet_cap( assist: Handler, #[rust_analyzer::rust_fixture] ra_fixture_before: &str, diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 5cbea9c2b3..ba5142845d 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -452,6 +452,8 @@ config_data! { assist_emitMustUse: bool = false, /// Placeholder expression to use for missing expressions in assists. assist_expressionFillDefault: ExprFillDefaultDef = ExprFillDefaultDef::Todo, + /// When inserting a type (e.g. in "fill match arms" assist), prefer to use `Self` over the type name where possible. + assist_preferSelf: bool = false, /// Enable borrow checking for term search code assists. If set to false, also there will be more suggestions, but some of them may not borrow-check. assist_termSearch_borrowcheck: bool = true, /// Term search fuel in "units of work" for assists (Defaults to 1800). @@ -1501,6 +1503,7 @@ impl Config { ExprFillDefaultDef::Default => ExprFillDefaultMode::Default, ExprFillDefaultDef::Underscore => ExprFillDefaultMode::Underscore, }, + prefer_self_ty: *self.assist_preferSelf(source_root), } } diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index fab4cb287c..955aadaa25 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -134,6 +134,13 @@ pub fn name_ref(name_ref: &str) -> ast::NameRef { } } } +pub fn name_ref_self_ty() -> ast::NameRef { + quote! { + NameRef { + [Self] + } + } +} fn raw_ident_esc(ident: &str) -> &'static str { if is_raw_identifier(ident, Edition::CURRENT) { "r#" } else { "" } } diff --git a/docs/book/src/configuration_generated.md b/docs/book/src/configuration_generated.md index 0e07dadfb7..7c1aafcdfa 100644 --- a/docs/book/src/configuration_generated.md +++ b/docs/book/src/configuration_generated.md @@ -13,6 +13,13 @@ Default: `"todo"` Placeholder expression to use for missing expressions in assists. +## rust-analyzer.assist.preferSelf {#assist.preferSelf} + +Default: `false` + +When inserting a type (e.g. in "fill match arms" assist), prefer to use `Self` over the type name where possible. + + ## rust-analyzer.assist.termSearch.borrowcheck {#assist.termSearch.borrowcheck} Default: `true` diff --git a/editors/code/package.json b/editors/code/package.json index c8c36cd85c..398d0c51e9 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -683,6 +683,16 @@ { "title": "assist", "properties": { + "rust-analyzer.assist.preferSelf": { + "markdownDescription": "When inserting a type (e.g. in \"fill match arms\" assist), prefer to use `Self` over the type name where possible.", + "default": false, + "type": "boolean" + } + } + }, + { + "title": "assist", + "properties": { "rust-analyzer.assist.termSearch.borrowcheck": { "markdownDescription": "Enable borrow checking for term search code assists. If set to false, also there will be more suggestions, but some of them may not borrow-check.", "default": true, |