Unnamed repository; edit this file 'description' to name the repository.
| -rw-r--r-- | crates/ide-assists/src/handlers/add_missing_match_arms.rs | 130 | ||||
| -rw-r--r-- | crates/ide-assists/src/handlers/generate_delegate_trait.rs | 35 | ||||
| -rw-r--r-- | crates/ide-assists/src/handlers/introduce_named_generic.rs | 21 | ||||
| -rw-r--r-- | crates/ide-completion/src/completions/pattern.rs | 3 | ||||
| -rw-r--r-- | crates/ide-db/src/syntax_helpers/suggest_name.rs | 212 | ||||
| -rw-r--r-- | crates/syntax/src/lib.rs | 2 |
6 files changed, 298 insertions, 105 deletions
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 b6abb06a2a..a211ca8f2d 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::{sym, Adt, Crate, HasAttrs, HasSource, ImportPathConfig, ModuleDef, Semantics}; +use hir::{sym, Adt, Crate, HasAttrs, ImportPathConfig, ModuleDef, Semantics}; +use ide_db::syntax_helpers::suggest_name; use ide_db::RootDatabase; use ide_db::{famous_defs::FamousDefs, helpers::mod_path_to_ast}; use itertools::Itertools; use syntax::ast::edit_in_place::Removable; -use syntax::ast::{self, make, AstNode, HasName, MatchArmList, MatchExpr, Pat}; +use syntax::ast::{self, make, AstNode, MatchArmList, MatchExpr, Pat}; use crate::{utils, AssistContext, AssistId, AssistKind, Assists}; @@ -90,7 +91,7 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>) .into_iter() .filter_map(|variant| { Some(( - build_pat(ctx.db(), module, variant, cfg)?, + build_pat(ctx, module, variant, cfg)?, variant.should_be_hidden(ctx.db(), module.krate()), )) }) @@ -141,9 +142,8 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>) 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, cfg)); + let patterns = + variants.into_iter().filter_map(|variant| build_pat(ctx, module, variant, cfg)); (ast::Pat::from(make::tuple_pat(patterns)), is_hidden) }) @@ -174,9 +174,8 @@ pub(crate) fn add_missing_match_arms(acc: &mut Assists, ctx: &AssistContext<'_>) 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, cfg)); + let patterns = + variants.into_iter().filter_map(|variant| build_pat(ctx, module, variant, cfg)); (ast::Pat::from(make::slice_pat(patterns)), is_hidden) }) .filter(|(variant_pat, _)| is_variant_missing(&top_lvl_pats, variant_pat)); @@ -438,33 +437,39 @@ fn resolve_array_of_enum_def( } fn build_pat( - db: &RootDatabase, + ctx: &AssistContext<'_>, module: hir::Module, var: ExtendedVariant, cfg: ImportPathConfig, ) -> Option<ast::Pat> { + let db = ctx.db(); match var { ExtendedVariant::Variant(var) => { let edition = module.krate().edition(db); let path = mod_path_to_ast(&module.find_path(db, ModuleDef::from(var), cfg)?, edition); - // FIXME: use HIR for this; it doesn't currently expose struct vs. tuple vs. unit variants though - Some(match var.source(db)?.value.kind() { - ast::StructKind::Tuple(field_list) => { - let pats = - iter::repeat(make::wildcard_pat().into()).take(field_list.fields().count()); + let fields = var.fields(db); + let pat = match var.kind(db) { + hir::StructKind::Tuple => { + let mut name_generator = suggest_name::NameGenerator::new(); + let pats = fields.into_iter().map(|f| { + let name = name_generator.for_type(&f.ty(db), db, edition); + match name { + Some(name) => make::ext::simple_ident_pat(make::name(&name)).into(), + None => make::wildcard_pat().into(), + } + }); make::tuple_struct_pat(path, pats).into() } - ast::StructKind::Record(field_list) => { - let pats = field_list.fields().map(|f| { - make::ext::simple_ident_pat( - f.name().expect("Record field must have a name"), - ) - .into() - }); + hir::StructKind::Record => { + let pats = fields + .into_iter() + .map(|f| make::name(f.name(db).as_str())) + .map(|name| make::ext::simple_ident_pat(name).into()); make::record_pat(path, pats).into() } - ast::StructKind::Unit => make::path_pat(path), - }) + hir::StructKind::Unit => make::path_pat(path), + }; + Some(pat) } ExtendedVariant::True => Some(ast::Pat::from(make::literal_pat("true"))), ExtendedVariant::False => Some(ast::Pat::from(make::literal_pat("false"))), @@ -1976,4 +1981,81 @@ fn a() { }"#, ) } + + #[test] + fn suggest_name_for_tuple_struct_patterns() { + // single tuple struct + check_assist( + add_missing_match_arms, + r#" +struct S; + +pub enum E { + A + B(S), +} + +fn f() { + let value = E::A; + match value { + $0 + } +} +"#, + r#" +struct S; + +pub enum E { + A + B(S), +} + +fn f() { + let value = E::A; + match value { + $0E::A => todo!(), + E::B(s) => todo!(), + } +} +"#, + ); + + // multiple tuple struct patterns + check_assist( + add_missing_match_arms, + r#" +struct S1; +struct S2; + +pub enum E { + A + B(S1, S2), +} + +fn f() { + let value = E::A; + match value { + $0 + } +} +"#, + r#" +struct S1; +struct S2; + +pub enum E { + A + B(S1, S2), +} + +fn f() { + let value = E::A; + match value { + $0E::A => todo!(), + E::B(s1, s2) => todo!(), + } +} +"#, + ); + } } diff --git a/crates/ide-assists/src/handlers/generate_delegate_trait.rs b/crates/ide-assists/src/handlers/generate_delegate_trait.rs index c22d19574f..a55323eb59 100644 --- a/crates/ide-assists/src/handlers/generate_delegate_trait.rs +++ b/crates/ide-assists/src/handlers/generate_delegate_trait.rs @@ -584,7 +584,7 @@ fn resolve_name_conflicts( for old_strukt_param in old_strukt_params.generic_params() { // Get old name from `strukt` - let mut name = SmolStr::from(match &old_strukt_param { + let name = SmolStr::from(match &old_strukt_param { ast::GenericParam::ConstParam(c) => c.name()?.to_string(), ast::GenericParam::LifetimeParam(l) => { l.lifetime()?.lifetime_ident_token()?.to_string() @@ -593,8 +593,19 @@ fn resolve_name_conflicts( }); // The new name cannot be conflicted with generics in trait, and the renamed names. - name = suggest_name::for_unique_generic_name(&name, old_impl_params); - name = suggest_name::for_unique_generic_name(&name, ¶ms); + let param_list_to_names = |param_list: &GenericParamList| { + param_list.generic_params().flat_map(|param| match param { + ast::GenericParam::TypeParam(t) => t.name().map(|name| name.to_string()), + p => Some(p.to_string()), + }) + }; + let existing_names = param_list_to_names(old_impl_params) + .chain(param_list_to_names(¶ms)) + .collect_vec(); + let mut name_generator = suggest_name::NameGenerator::new_with_names( + existing_names.iter().map(|s| s.as_str()), + ); + let name = name_generator.suggest_name(&name); match old_strukt_param { ast::GenericParam::ConstParam(c) => { if let Some(const_ty) = c.ty() { @@ -1213,9 +1224,9 @@ struct S<T> { b : B<T>, } -impl<T0> Trait<T0> for S<T0> { - fn f(&self, a: T0) -> T0 { - <B<T0> as Trait<T0>>::f(&self.b, a) +impl<T1> Trait<T1> for S<T1> { + fn f(&self, a: T1) -> T1 { + <B<T1> as Trait<T1>>::f(&self.b, a) } } "#, @@ -1527,12 +1538,12 @@ where b : B<T, T1>, } -impl<T, T2, T10> Trait<T> for S<T2, T10> +impl<T, T2, T3> Trait<T> for S<T2, T3> where - T10: AnotherTrait + T3: AnotherTrait { fn f(&self, a: T) -> T { - <B<T2, T10> as Trait<T>>::f(&self.b, a) + <B<T2, T3> as Trait<T>>::f(&self.b, a) } }"#, ); @@ -1589,12 +1600,12 @@ where b : B<T>, } -impl<T, T0> Trait<T> for S<T0> +impl<T, T2> Trait<T> for S<T2> where - T0: AnotherTrait + T2: AnotherTrait { fn f(&self, a: T) -> T { - <B<T0> as Trait<T>>::f(&self.b, a) + <B<T2> as Trait<T>>::f(&self.b, a) } }"#, ); diff --git a/crates/ide-assists/src/handlers/introduce_named_generic.rs b/crates/ide-assists/src/handlers/introduce_named_generic.rs index a734a6cc2b..bf6ac1719f 100644 --- a/crates/ide-assists/src/handlers/introduce_named_generic.rs +++ b/crates/ide-assists/src/handlers/introduce_named_generic.rs @@ -1,6 +1,7 @@ use ide_db::syntax_helpers::suggest_name; +use itertools::Itertools; use syntax::{ - ast::{self, edit_in_place::GenericParamsOwnerEdit, make, AstNode, HasGenericParams}, + ast::{self, edit_in_place::GenericParamsOwnerEdit, make, AstNode, HasGenericParams, HasName}, ted, }; @@ -33,8 +34,18 @@ pub(crate) fn introduce_named_generic(acc: &mut Assists, ctx: &AssistContext<'_> let impl_trait_type = edit.make_mut(impl_trait_type); let fn_ = edit.make_mut(fn_); let fn_generic_param_list = fn_.get_or_create_generic_param_list(); - let type_param_name = - suggest_name::for_impl_trait_as_generic(&impl_trait_type, &fn_generic_param_list); + + let existing_names = fn_generic_param_list + .generic_params() + .flat_map(|param| match param { + ast::GenericParam::TypeParam(t) => t.name().map(|name| name.to_string()), + p => Some(p.to_string()), + }) + .collect_vec(); + let type_param_name = suggest_name::NameGenerator::new_with_names( + existing_names.iter().map(|s| s.as_str()), + ) + .for_impl_trait_as_generic(&impl_trait_type); let type_param = make::type_param(make::name(&type_param_name), Some(type_bound_list)) .clone_for_update(); @@ -116,7 +127,7 @@ fn foo<$0B: Bar check_assist( introduce_named_generic, r#"fn foo<B>(bar: $0impl Bar) {}"#, - r#"fn foo<B, $0B0: Bar>(bar: B0) {}"#, + r#"fn foo<B, $0B1: Bar>(bar: B1) {}"#, ); } @@ -125,7 +136,7 @@ fn foo<$0B: Bar check_assist( introduce_named_generic, r#"fn foo<B, B0, B1, B3>(bar: $0impl Bar) {}"#, - r#"fn foo<B, B0, B1, B3, $0B2: Bar>(bar: B2) {}"#, + r#"fn foo<B, B0, B1, B3, $0B4: Bar>(bar: B4) {}"#, ); } diff --git a/crates/ide-completion/src/completions/pattern.rs b/crates/ide-completion/src/completions/pattern.rs index 2a06fc4017..8f38e02ed7 100644 --- a/crates/ide-completion/src/completions/pattern.rs +++ b/crates/ide-completion/src/completions/pattern.rs @@ -48,11 +48,12 @@ pub(crate) fn complete_pattern( // Suggest name only in let-stmt and fn param if pattern_ctx.should_suggest_name { + let mut name_generator = suggest_name::NameGenerator::new(); if let Some(suggested) = ctx .expected_type .as_ref() .map(|ty| ty.strip_references()) - .and_then(|ty| suggest_name::for_type(&ty, ctx.db, ctx.edition)) + .and_then(|ty| name_generator.for_type(&ty, ctx.db, ctx.edition)) { acc.suggest_name(ctx, &suggested); } diff --git a/crates/ide-db/src/syntax_helpers/suggest_name.rs b/crates/ide-db/src/syntax_helpers/suggest_name.rs index e60deb3bf5..2679cbef61 100644 --- a/crates/ide-db/src/syntax_helpers/suggest_name.rs +++ b/crates/ide-db/src/syntax_helpers/suggest_name.rs @@ -1,12 +1,14 @@ //! This module contains functions to suggest names for expressions, functions and other items +use std::{collections::hash_map::Entry, str::FromStr}; + use hir::Semantics; use itertools::Itertools; -use rustc_hash::FxHashSet; +use rustc_hash::FxHashMap; use stdx::to_lower_snake_case; use syntax::{ ast::{self, HasName}, - match_ast, AstNode, Edition, SmolStr, + match_ast, AstNode, Edition, SmolStr, SmolStrBuilder, }; use crate::RootDatabase; @@ -62,71 +64,131 @@ const USELESS_METHODS: &[&str] = &[ "into_future", ]; -/// Suggest a name for given type. +/// Generator for new names /// -/// The function will strip references first, and suggest name from the inner type. +/// The generator keeps track of existing names and suggests new names that do +/// not conflict with existing names. /// -/// - If `ty` is an ADT, it will suggest the name of the ADT. -/// + If `ty` is wrapped in `Box`, `Option` or `Result`, it will suggest the name from the inner type. -/// - If `ty` is a trait, it will suggest the name of the trait. -/// - If `ty` is an `impl Trait`, it will suggest the name of the first trait. +/// The generator will try to resolve conflicts by adding a numeric suffix to +/// the name, e.g. `a`, `a1`, `a2`, ... /// -/// If the suggested name conflicts with reserved keywords, it will return `None`. -pub fn for_type(ty: &hir::Type, db: &RootDatabase, edition: Edition) -> Option<String> { - let ty = ty.strip_references(); - name_of_type(&ty, db, edition) -} - -/// Suggest a unique name for generic parameter. -/// -/// `existing_params` is used to check if the name conflicts with existing -/// generic parameters. +/// # Examples +/// ```rust +/// let mut generator = NameGenerator::new(); +/// assert_eq!(generator.suggest_name("a"), "a"); +/// assert_eq!(generator.suggest_name("a"), "a1"); /// -/// The function checks if the name conflicts with existing generic parameters. -/// If so, it will try to resolve the conflict by adding a number suffix, e.g. -/// `T`, `T0`, `T1`, ... -pub fn for_unique_generic_name(name: &str, existing_params: &ast::GenericParamList) -> SmolStr { - let param_names = existing_params - .generic_params() - .map(|param| match param { - ast::GenericParam::TypeParam(t) => t.name().unwrap().to_string(), - p => p.to_string(), - }) - .collect::<FxHashSet<_>>(); - let mut name = name.to_owned(); - let base_len = name.len(); - let mut count = 0; - while param_names.contains(&name) { - name.truncate(base_len); - name.push_str(&count.to_string()); - count += 1; - } - - name.into() +/// assert_eq!(generator.suggest_name("b2"), "b2"); +/// assert_eq!(generator.suggest_name("b"), "b3"); +/// ``` +#[derive(Debug, Default)] +pub struct NameGenerator { + pool: FxHashMap<SmolStr, usize>, } -/// Suggest name of impl trait type -/// -/// `existing_params` is used to check if the name conflicts with existing -/// generic parameters. -/// -/// # Current implementation -/// -/// In current implementation, the function tries to get the name from the first -/// character of the name for the first type bound. -/// -/// If the name conflicts with existing generic parameters, it will try to -/// resolve the conflict with `for_unique_generic_name`. -pub fn for_impl_trait_as_generic( - ty: &ast::ImplTraitType, - existing_params: &ast::GenericParamList, -) -> SmolStr { - let c = ty - .type_bound_list() - .and_then(|bounds| bounds.syntax().text().char_at(0.into())) - .unwrap_or('T'); - - for_unique_generic_name(c.encode_utf8(&mut [0; 4]), existing_params) +impl NameGenerator { + /// Create a new empty generator + pub fn new() -> Self { + Self { pool: FxHashMap::default() } + } + + /// Create a new generator with existing names. When suggesting a name, it will + /// avoid conflicts with existing names. + pub fn new_with_names<'a>(existing_names: impl Iterator<Item = &'a str>) -> Self { + let mut generator = Self::new(); + existing_names.for_each(|name| generator.insert(name)); + generator + } + + /// Suggest a name without conflicts. If the name conflicts with existing names, + /// it will try to resolve the conflict by adding a numeric suffix. + pub fn suggest_name(&mut self, name: &str) -> SmolStr { + let (prefix, suffix) = Self::split_numeric_suffix(name); + let prefix = SmolStr::new(prefix); + let suffix = suffix.unwrap_or(0); + + match self.pool.entry(prefix.clone()) { + Entry::Vacant(entry) => { + entry.insert(suffix); + SmolStr::from_str(name).unwrap() + } + Entry::Occupied(mut entry) => { + let count = entry.get_mut(); + *count = (*count + 1).max(suffix); + + let mut new_name = SmolStrBuilder::new(); + new_name.push_str(&prefix); + new_name.push_str(count.to_string().as_str()); + new_name.finish() + } + } + } + + /// Suggest a name for given type. + /// + /// The function will strip references first, and suggest name from the inner type. + /// + /// - If `ty` is an ADT, it will suggest the name of the ADT. + /// + If `ty` is wrapped in `Box`, `Option` or `Result`, it will suggest the name from the inner type. + /// - If `ty` is a trait, it will suggest the name of the trait. + /// - If `ty` is an `impl Trait`, it will suggest the name of the first trait. + /// + /// If the suggested name conflicts with reserved keywords, it will return `None`. + pub fn for_type( + &mut self, + ty: &hir::Type, + db: &RootDatabase, + edition: Edition, + ) -> Option<SmolStr> { + let name = name_of_type(ty, db, edition)?; + Some(self.suggest_name(&name)) + } + + /// Suggest name of impl trait type + /// + /// # Current implementation + /// + /// In current implementation, the function tries to get the name from the first + /// character of the name for the first type bound. + /// + /// If the name conflicts with existing generic parameters, it will try to + /// resolve the conflict with `for_unique_generic_name`. + pub fn for_impl_trait_as_generic(&mut self, ty: &ast::ImplTraitType) -> SmolStr { + let c = ty + .type_bound_list() + .and_then(|bounds| bounds.syntax().text().char_at(0.into())) + .unwrap_or('T'); + + self.suggest_name(&c.to_string()) + } + + /// Insert a name into the pool + fn insert(&mut self, name: &str) { + let (prefix, suffix) = Self::split_numeric_suffix(name); + let prefix = SmolStr::new(prefix); + let suffix = suffix.unwrap_or(0); + + match self.pool.entry(prefix) { + Entry::Vacant(entry) => { + entry.insert(suffix); + } + Entry::Occupied(mut entry) => { + let count = entry.get_mut(); + *count = (*count).max(suffix); + } + } + } + + /// Remove the numeric suffix from the name + /// + /// # Examples + /// `a1b2c3` -> `a1b2c` + fn split_numeric_suffix(name: &str) -> (&str, Option<usize>) { + let pos = + name.rfind(|c: char| !c.is_numeric()).expect("Name cannot be empty or all-numeric"); + let (prefix, suffix) = name.split_at(pos + 1); + (prefix, suffix.parse().ok()) + } } /// Suggest name of variable for given expression @@ -290,9 +352,10 @@ fn var_name_from_pat(pat: &ast::Pat) -> Option<ast::Name> { fn from_type(expr: &ast::Expr, sema: &Semantics<'_, RootDatabase>) -> Option<String> { let ty = sema.type_of_expr(expr)?.adjusted(); + let ty = ty.remove_ref().unwrap_or(ty); let edition = sema.scope(expr.syntax())?.krate().edition(sema.db); - for_type(&ty, sema.db, edition) + name_of_type(&ty, sema.db, edition) } fn name_of_type(ty: &hir::Type, db: &RootDatabase, edition: Edition) -> Option<String> { @@ -925,4 +988,29 @@ fn main() { "bar", ); } + + #[test] + fn conflicts_with_existing_names() { + let mut generator = NameGenerator::new(); + assert_eq!(generator.suggest_name("a"), "a"); + assert_eq!(generator.suggest_name("a"), "a1"); + assert_eq!(generator.suggest_name("a"), "a2"); + assert_eq!(generator.suggest_name("a"), "a3"); + + assert_eq!(generator.suggest_name("b"), "b"); + assert_eq!(generator.suggest_name("b2"), "b2"); + assert_eq!(generator.suggest_name("b"), "b3"); + assert_eq!(generator.suggest_name("b"), "b4"); + assert_eq!(generator.suggest_name("b3"), "b5"); + + // --------- + let mut generator = NameGenerator::new_with_names(["a", "b", "b2", "c4"].into_iter()); + assert_eq!(generator.suggest_name("a"), "a1"); + assert_eq!(generator.suggest_name("a"), "a2"); + + assert_eq!(generator.suggest_name("b"), "b3"); + assert_eq!(generator.suggest_name("b2"), "b4"); + + assert_eq!(generator.suggest_name("c"), "c5"); + } } diff --git a/crates/syntax/src/lib.rs b/crates/syntax/src/lib.rs index dbbb290b4f..c1554c4b29 100644 --- a/crates/syntax/src/lib.rs +++ b/crates/syntax/src/lib.rs @@ -66,7 +66,7 @@ pub use rowan::{ TokenAtOffset, WalkEvent, }; pub use rustc_lexer::unescape; -pub use smol_str::{format_smolstr, SmolStr, ToSmolStr}; +pub use smol_str::{format_smolstr, SmolStr, SmolStrBuilder, ToSmolStr}; /// `Parse` is the result of the parsing: a syntax tree and a collection of /// errors. |