Skip to content

Commit 32d9597

Browse files
committed
refactor: introduce NameGenerator in suggest_name
1 parent 600f7cf commit 32d9597

File tree

5 files changed

+192
-81
lines changed

5 files changed

+192
-81
lines changed

src/tools/rust-analyzer/crates/ide-assists/src/handlers/generate_delegate_trait.rs

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -584,7 +584,7 @@ fn resolve_name_conflicts(
584584

585585
for old_strukt_param in old_strukt_params.generic_params() {
586586
// Get old name from `strukt`
587-
let mut name = SmolStr::from(match &old_strukt_param {
587+
let name = SmolStr::from(match &old_strukt_param {
588588
ast::GenericParam::ConstParam(c) => c.name()?.to_string(),
589589
ast::GenericParam::LifetimeParam(l) => {
590590
l.lifetime()?.lifetime_ident_token()?.to_string()
@@ -593,8 +593,19 @@ fn resolve_name_conflicts(
593593
});
594594

595595
// The new name cannot be conflicted with generics in trait, and the renamed names.
596-
name = suggest_name::for_unique_generic_name(&name, old_impl_params);
597-
name = suggest_name::for_unique_generic_name(&name, &params);
596+
let param_list_to_names = |param_list: &GenericParamList| {
597+
param_list.generic_params().flat_map(|param| match param {
598+
ast::GenericParam::TypeParam(t) => t.name().map(|name| name.to_string()),
599+
p => Some(p.to_string()),
600+
})
601+
};
602+
let existing_names = param_list_to_names(old_impl_params)
603+
.chain(param_list_to_names(&params))
604+
.collect_vec();
605+
let mut name_generator = suggest_name::NameGenerator::new_with_names(
606+
existing_names.iter().map(|s| s.as_str()),
607+
);
608+
let name = name_generator.suggest_name(&name);
598609
match old_strukt_param {
599610
ast::GenericParam::ConstParam(c) => {
600611
if let Some(const_ty) = c.ty() {
@@ -1213,9 +1224,9 @@ struct S<T> {
12131224
b : B<T>,
12141225
}
12151226
1216-
impl<T0> Trait<T0> for S<T0> {
1217-
fn f(&self, a: T0) -> T0 {
1218-
<B<T0> as Trait<T0>>::f(&self.b, a)
1227+
impl<T1> Trait<T1> for S<T1> {
1228+
fn f(&self, a: T1) -> T1 {
1229+
<B<T1> as Trait<T1>>::f(&self.b, a)
12191230
}
12201231
}
12211232
"#,
@@ -1527,12 +1538,12 @@ where
15271538
b : B<T, T1>,
15281539
}
15291540
1530-
impl<T, T2, T10> Trait<T> for S<T2, T10>
1541+
impl<T, T2, T3> Trait<T> for S<T2, T3>
15311542
where
1532-
T10: AnotherTrait
1543+
T3: AnotherTrait
15331544
{
15341545
fn f(&self, a: T) -> T {
1535-
<B<T2, T10> as Trait<T>>::f(&self.b, a)
1546+
<B<T2, T3> as Trait<T>>::f(&self.b, a)
15361547
}
15371548
}"#,
15381549
);
@@ -1589,12 +1600,12 @@ where
15891600
b : B<T>,
15901601
}
15911602
1592-
impl<T, T0> Trait<T> for S<T0>
1603+
impl<T, T2> Trait<T> for S<T2>
15931604
where
1594-
T0: AnotherTrait
1605+
T2: AnotherTrait
15951606
{
15961607
fn f(&self, a: T) -> T {
1597-
<B<T0> as Trait<T>>::f(&self.b, a)
1608+
<B<T2> as Trait<T>>::f(&self.b, a)
15981609
}
15991610
}"#,
16001611
);

src/tools/rust-analyzer/crates/ide-assists/src/handlers/introduce_named_generic.rs

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use ide_db::syntax_helpers::suggest_name;
2+
use itertools::Itertools;
23
use syntax::{
3-
ast::{self, edit_in_place::GenericParamsOwnerEdit, make, AstNode, HasGenericParams},
4+
ast::{self, edit_in_place::GenericParamsOwnerEdit, make, AstNode, HasGenericParams, HasName},
45
ted,
56
};
67

@@ -33,8 +34,18 @@ pub(crate) fn introduce_named_generic(acc: &mut Assists, ctx: &AssistContext<'_>
3334
let impl_trait_type = edit.make_mut(impl_trait_type);
3435
let fn_ = edit.make_mut(fn_);
3536
let fn_generic_param_list = fn_.get_or_create_generic_param_list();
36-
let type_param_name =
37-
suggest_name::for_impl_trait_as_generic(&impl_trait_type, &fn_generic_param_list);
37+
38+
let existing_names = fn_generic_param_list
39+
.generic_params()
40+
.flat_map(|param| match param {
41+
ast::GenericParam::TypeParam(t) => t.name().map(|name| name.to_string()),
42+
p => Some(p.to_string()),
43+
})
44+
.collect_vec();
45+
let type_param_name = suggest_name::NameGenerator::new_with_names(
46+
existing_names.iter().map(|s| s.as_str()),
47+
)
48+
.for_impl_trait_as_generic(&impl_trait_type);
3849

3950
let type_param = make::type_param(make::name(&type_param_name), Some(type_bound_list))
4051
.clone_for_update();
@@ -116,7 +127,7 @@ fn foo<$0B: Bar
116127
check_assist(
117128
introduce_named_generic,
118129
r#"fn foo<B>(bar: $0impl Bar) {}"#,
119-
r#"fn foo<B, $0B0: Bar>(bar: B0) {}"#,
130+
r#"fn foo<B, $0B1: Bar>(bar: B1) {}"#,
120131
);
121132
}
122133

@@ -125,7 +136,7 @@ fn foo<$0B: Bar
125136
check_assist(
126137
introduce_named_generic,
127138
r#"fn foo<B, B0, B1, B3>(bar: $0impl Bar) {}"#,
128-
r#"fn foo<B, B0, B1, B3, $0B2: Bar>(bar: B2) {}"#,
139+
r#"fn foo<B, B0, B1, B3, $0B4: Bar>(bar: B4) {}"#,
129140
);
130141
}
131142

src/tools/rust-analyzer/crates/ide-completion/src/completions/pattern.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,12 @@ pub(crate) fn complete_pattern(
4848

4949
// Suggest name only in let-stmt and fn param
5050
if pattern_ctx.should_suggest_name {
51+
let mut name_generator = suggest_name::NameGenerator::new();
5152
if let Some(suggested) = ctx
5253
.expected_type
5354
.as_ref()
5455
.map(|ty| ty.strip_references())
55-
.and_then(|ty| suggest_name::for_type(&ty, ctx.db, ctx.edition))
56+
.and_then(|ty| name_generator.for_type(&ty, ctx.db, ctx.edition))
5657
{
5758
acc.suggest_name(ctx, &suggested);
5859
}

src/tools/rust-analyzer/crates/ide-db/src/syntax_helpers/suggest_name.rs

Lines changed: 150 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
//! This module contains functions to suggest names for expressions, functions and other items
22
3+
use std::{collections::hash_map::Entry, str::FromStr};
4+
35
use hir::Semantics;
46
use itertools::Itertools;
5-
use rustc_hash::FxHashSet;
7+
use rustc_hash::FxHashMap;
68
use stdx::to_lower_snake_case;
79
use syntax::{
810
ast::{self, HasName},
9-
match_ast, AstNode, Edition, SmolStr,
11+
match_ast, AstNode, Edition, SmolStr, SmolStrBuilder,
1012
};
1113

1214
use crate::RootDatabase;
@@ -62,71 +64,131 @@ const USELESS_METHODS: &[&str] = &[
6264
"into_future",
6365
];
6466

65-
/// Suggest a name for given type.
67+
/// Generator for new names
6668
///
67-
/// The function will strip references first, and suggest name from the inner type.
69+
/// The generator keeps track of existing names and suggests new names that do
70+
/// not conflict with existing names.
6871
///
69-
/// - If `ty` is an ADT, it will suggest the name of the ADT.
70-
/// + If `ty` is wrapped in `Box`, `Option` or `Result`, it will suggest the name from the inner type.
71-
/// - If `ty` is a trait, it will suggest the name of the trait.
72-
/// - If `ty` is an `impl Trait`, it will suggest the name of the first trait.
72+
/// The generator will try to resolve conflicts by adding a numeric suffix to
73+
/// the name, e.g. `a`, `a1`, `a2`, ...
7374
///
74-
/// If the suggested name conflicts with reserved keywords, it will return `None`.
75-
pub fn for_type(ty: &hir::Type, db: &RootDatabase, edition: Edition) -> Option<String> {
76-
let ty = ty.strip_references();
77-
name_of_type(&ty, db, edition)
78-
}
79-
80-
/// Suggest a unique name for generic parameter.
81-
///
82-
/// `existing_params` is used to check if the name conflicts with existing
83-
/// generic parameters.
75+
/// # Examples
76+
/// ```rust
77+
/// let mut generator = NameGenerator::new();
78+
/// assert_eq!(generator.suggest_name("a"), "a");
79+
/// assert_eq!(generator.suggest_name("a"), "a1");
8480
///
85-
/// The function checks if the name conflicts with existing generic parameters.
86-
/// If so, it will try to resolve the conflict by adding a number suffix, e.g.
87-
/// `T`, `T0`, `T1`, ...
88-
pub fn for_unique_generic_name(name: &str, existing_params: &ast::GenericParamList) -> SmolStr {
89-
let param_names = existing_params
90-
.generic_params()
91-
.map(|param| match param {
92-
ast::GenericParam::TypeParam(t) => t.name().unwrap().to_string(),
93-
p => p.to_string(),
94-
})
95-
.collect::<FxHashSet<_>>();
96-
let mut name = name.to_owned();
97-
let base_len = name.len();
98-
let mut count = 0;
99-
while param_names.contains(&name) {
100-
name.truncate(base_len);
101-
name.push_str(&count.to_string());
102-
count += 1;
103-
}
104-
105-
name.into()
81+
/// assert_eq!(generator.suggest_name("b2"), "b2");
82+
/// assert_eq!(generator.suggest_name("b"), "b3");
83+
/// ```
84+
#[derive(Debug, Default)]
85+
pub struct NameGenerator {
86+
pool: FxHashMap<SmolStr, usize>,
10687
}
10788

108-
/// Suggest name of impl trait type
109-
///
110-
/// `existing_params` is used to check if the name conflicts with existing
111-
/// generic parameters.
112-
///
113-
/// # Current implementation
114-
///
115-
/// In current implementation, the function tries to get the name from the first
116-
/// character of the name for the first type bound.
117-
///
118-
/// If the name conflicts with existing generic parameters, it will try to
119-
/// resolve the conflict with `for_unique_generic_name`.
120-
pub fn for_impl_trait_as_generic(
121-
ty: &ast::ImplTraitType,
122-
existing_params: &ast::GenericParamList,
123-
) -> SmolStr {
124-
let c = ty
125-
.type_bound_list()
126-
.and_then(|bounds| bounds.syntax().text().char_at(0.into()))
127-
.unwrap_or('T');
128-
129-
for_unique_generic_name(c.encode_utf8(&mut [0; 4]), existing_params)
89+
impl NameGenerator {
90+
/// Create a new empty generator
91+
pub fn new() -> Self {
92+
Self { pool: FxHashMap::default() }
93+
}
94+
95+
/// Create a new generator with existing names. When suggesting a name, it will
96+
/// avoid conflicts with existing names.
97+
pub fn new_with_names<'a>(existing_names: impl Iterator<Item = &'a str>) -> Self {
98+
let mut generator = Self::new();
99+
existing_names.for_each(|name| generator.insert(name));
100+
generator
101+
}
102+
103+
/// Suggest a name without conflicts. If the name conflicts with existing names,
104+
/// it will try to resolve the conflict by adding a numeric suffix.
105+
pub fn suggest_name(&mut self, name: &str) -> SmolStr {
106+
let (prefix, suffix) = Self::split_numeric_suffix(name);
107+
let prefix = SmolStr::new(prefix);
108+
let suffix = suffix.unwrap_or(0);
109+
110+
match self.pool.entry(prefix.clone()) {
111+
Entry::Vacant(entry) => {
112+
entry.insert(suffix);
113+
SmolStr::from_str(name).unwrap()
114+
}
115+
Entry::Occupied(mut entry) => {
116+
let count = entry.get_mut();
117+
*count = (*count + 1).max(suffix);
118+
119+
let mut new_name = SmolStrBuilder::new();
120+
new_name.push_str(&prefix);
121+
new_name.push_str(count.to_string().as_str());
122+
new_name.finish()
123+
}
124+
}
125+
}
126+
127+
/// Suggest a name for given type.
128+
///
129+
/// The function will strip references first, and suggest name from the inner type.
130+
///
131+
/// - If `ty` is an ADT, it will suggest the name of the ADT.
132+
/// + If `ty` is wrapped in `Box`, `Option` or `Result`, it will suggest the name from the inner type.
133+
/// - If `ty` is a trait, it will suggest the name of the trait.
134+
/// - If `ty` is an `impl Trait`, it will suggest the name of the first trait.
135+
///
136+
/// If the suggested name conflicts with reserved keywords, it will return `None`.
137+
pub fn for_type(
138+
&mut self,
139+
ty: &hir::Type,
140+
db: &RootDatabase,
141+
edition: Edition,
142+
) -> Option<SmolStr> {
143+
let name = name_of_type(ty, db, edition)?;
144+
Some(self.suggest_name(&name))
145+
}
146+
147+
/// Suggest name of impl trait type
148+
///
149+
/// # Current implementation
150+
///
151+
/// In current implementation, the function tries to get the name from the first
152+
/// character of the name for the first type bound.
153+
///
154+
/// If the name conflicts with existing generic parameters, it will try to
155+
/// resolve the conflict with `for_unique_generic_name`.
156+
pub fn for_impl_trait_as_generic(&mut self, ty: &ast::ImplTraitType) -> SmolStr {
157+
let c = ty
158+
.type_bound_list()
159+
.and_then(|bounds| bounds.syntax().text().char_at(0.into()))
160+
.unwrap_or('T');
161+
162+
self.suggest_name(&c.to_string())
163+
}
164+
165+
/// Insert a name into the pool
166+
fn insert(&mut self, name: &str) {
167+
let (prefix, suffix) = Self::split_numeric_suffix(name);
168+
let prefix = SmolStr::new(prefix);
169+
let suffix = suffix.unwrap_or(0);
170+
171+
match self.pool.entry(prefix) {
172+
Entry::Vacant(entry) => {
173+
entry.insert(suffix);
174+
}
175+
Entry::Occupied(mut entry) => {
176+
let count = entry.get_mut();
177+
*count = (*count).max(suffix);
178+
}
179+
}
180+
}
181+
182+
/// Remove the numeric suffix from the name
183+
///
184+
/// # Examples
185+
/// `a1b2c3` -> `a1b2c`
186+
fn split_numeric_suffix(name: &str) -> (&str, Option<usize>) {
187+
let pos =
188+
name.rfind(|c: char| !c.is_numeric()).expect("Name cannot be empty or all-numeric");
189+
let (prefix, suffix) = name.split_at(pos + 1);
190+
(prefix, suffix.parse().ok())
191+
}
130192
}
131193

132194
/// Suggest name of variable for given expression
@@ -290,9 +352,10 @@ fn var_name_from_pat(pat: &ast::Pat) -> Option<ast::Name> {
290352

291353
fn from_type(expr: &ast::Expr, sema: &Semantics<'_, RootDatabase>) -> Option<String> {
292354
let ty = sema.type_of_expr(expr)?.adjusted();
355+
let ty = ty.remove_ref().unwrap_or(ty);
293356
let edition = sema.scope(expr.syntax())?.krate().edition(sema.db);
294357

295-
for_type(&ty, sema.db, edition)
358+
name_of_type(&ty, sema.db, edition)
296359
}
297360

298361
fn name_of_type(ty: &hir::Type, db: &RootDatabase, edition: Edition) -> Option<String> {
@@ -925,4 +988,29 @@ fn main() {
925988
"bar",
926989
);
927990
}
991+
992+
#[test]
993+
fn conflicts_with_existing_names() {
994+
let mut generator = NameGenerator::new();
995+
assert_eq!(generator.suggest_name("a"), "a");
996+
assert_eq!(generator.suggest_name("a"), "a1");
997+
assert_eq!(generator.suggest_name("a"), "a2");
998+
assert_eq!(generator.suggest_name("a"), "a3");
999+
1000+
assert_eq!(generator.suggest_name("b"), "b");
1001+
assert_eq!(generator.suggest_name("b2"), "b2");
1002+
assert_eq!(generator.suggest_name("b"), "b3");
1003+
assert_eq!(generator.suggest_name("b"), "b4");
1004+
assert_eq!(generator.suggest_name("b3"), "b5");
1005+
1006+
// ---------
1007+
let mut generator = NameGenerator::new_with_names(["a", "b", "b2", "c4"].into_iter());
1008+
assert_eq!(generator.suggest_name("a"), "a1");
1009+
assert_eq!(generator.suggest_name("a"), "a2");
1010+
1011+
assert_eq!(generator.suggest_name("b"), "b3");
1012+
assert_eq!(generator.suggest_name("b2"), "b4");
1013+
1014+
assert_eq!(generator.suggest_name("c"), "c5");
1015+
}
9281016
}

src/tools/rust-analyzer/crates/syntax/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ pub use rowan::{
6565
TokenAtOffset, WalkEvent,
6666
};
6767
pub use rustc_lexer::unescape;
68-
pub use smol_str::{format_smolstr, SmolStr, ToSmolStr};
68+
pub use smol_str::{format_smolstr, SmolStr, SmolStrBuilder, ToSmolStr};
6969

7070
/// `Parse` is the result of the parsing: a syntax tree and a collection of
7171
/// errors.

0 commit comments

Comments
 (0)