Skip to content

Commit

Permalink
Auto merge of rust-lang#18038 - roife:fix-issue-18034, r=Veykril
Browse files Browse the repository at this point in the history
feat: generate names for tuple-struct in add-missing-match-arms

fix rust-lang#18034.

This PR includes the following enhancement:

- Introduced a `NameGenerator` in `suggest_name`, which implements an automatic renaming algorithm to avoid name conflicts. Here are a few examples:

```rust
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("b"), "b");
assert_eq!(generator.suggest_name("b"), "b1");
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");
```

- Updated existing testcases in ide-assists for the new `NameGenerator` (only modified generated names).
- Generate names for tuple structs instead of using wildcard patterns in `add-missing-match-arms`.
  • Loading branch information
bors committed Sep 12, 2024
2 parents 4fcad5a + b304701 commit 3831b72
Show file tree
Hide file tree
Showing 6 changed files with 298 additions and 105 deletions.
Original file line number Diff line number Diff line change
@@ -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};

Expand Down Expand Up @@ -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()),
))
})
Expand Down Expand Up @@ -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)
})
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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"))),
Expand Down Expand Up @@ -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!(),
}
}
"#,
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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, &params);
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(&params))
.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() {
Expand Down Expand Up @@ -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)
}
}
"#,
Expand Down Expand Up @@ -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)
}
}"#,
);
Expand Down Expand Up @@ -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)
}
}"#,
);
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
};

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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) {}"#,
);
}

Expand All @@ -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) {}"#,
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Loading

0 comments on commit 3831b72

Please sign in to comment.