Skip to content

Commit df19895

Browse files
yoshuawuytsyoshuawuyts
committed
implement feedback from review
1 parent 8ed2402 commit df19895

File tree

1 file changed

+24
-42
lines changed

1 file changed

+24
-42
lines changed

‎crates/ide_assists/src/utils/gen_trait_fn_body.rs

Lines changed: 24 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,7 @@ fn gen_clone_impl(adt: &ast::Adt, func: &ast::Fn) -> Option<()> {
3838
let list = enum_.variant_list()?;
3939
let mut arms = vec![];
4040
for variant in list.variants() {
41-
let name = variant.name()?;
42-
let left = make::ext::ident_path("Self");
43-
let right = make::ext::ident_path(&format!("{}", name));
44-
let variant_name = make::path_concat(left, right);
41+
let variant_name = make_variant_path(&variant)?;
4542

4643
match variant.field_list() {
4744
// => match self { Self::Name { x } => Self::Name { x: x.clone() } }
@@ -151,9 +148,7 @@ fn gen_debug_impl(adt: &ast::Adt, func: &ast::Fn) -> Option<()> {
151148
let mut arms = vec![];
152149
for variant in list.variants() {
153150
let name = variant.name()?;
154-
let left = make::ext::ident_path("Self");
155-
let right = make::ext::ident_path(&format!("{}", name));
156-
let variant_name = make::path_pat(make::path_concat(left, right));
151+
let variant_name = make::path_pat(make::path_from_text(&format!("Self::{}", name)));
157152

158153
let target = make::expr_path(make::ext::ident_path("f").into());
159154
let fmt_string = make::expr_literal(&(format!("\"{}\"", name))).into();
@@ -226,10 +221,8 @@ fn gen_debug_impl(adt: &ast::Adt, func: &ast::Fn) -> Option<()> {
226221
/// Generate a `Debug` impl based on the fields and members of the target type.
227222
fn gen_default_impl(adt: &ast::Adt, func: &ast::Fn) -> Option<()> {
228223
fn gen_default_call() -> ast::Expr {
229-
let trait_name = make::ext::ident_path("Default");
230-
let method_name = make::ext::ident_path("default");
231-
let fn_name = make::expr_path(make::path_concat(trait_name, method_name));
232-
make::expr_call(fn_name, make::arg_list(None))
224+
let fn_name = make::path_from_text(&"Default::default");
225+
make::expr_call(make::expr_path(fn_name), make::arg_list(None))
233226
}
234227
match adt {
235228
// `Debug` cannot be derived for unions, so no default impl can be provided.
@@ -283,11 +276,7 @@ fn gen_hash_impl(adt: &ast::Adt, func: &ast::Fn) -> Option<()> {
283276

284277
// => std::mem::discriminant(self).hash(state);
285278
ast::Adt::Enum(_) => {
286-
let root = make::ext::ident_path("core");
287-
let submodule = make::ext::ident_path("mem");
288-
let fn_name = make::ext::ident_path("discriminant");
289-
let fn_name = make::path_concat(submodule, fn_name);
290-
let fn_name = make::expr_path(make::path_concat(root, fn_name));
279+
let fn_name = make_discriminant();
291280

292281
let arg = make::expr_path(make::ext::ident_path("self"));
293282
let fn_call = make::expr_call(fn_name, make::arg_list(Some(arg)));
@@ -329,14 +318,6 @@ fn gen_hash_impl(adt: &ast::Adt, func: &ast::Fn) -> Option<()> {
329318

330319
/// Generate a `PartialEq` impl based on the fields and members of the target type.
331320
fn gen_partial_eq(adt: &ast::Adt, func: &ast::Fn) -> Option<()> {
332-
fn gen_discriminant() -> ast::Expr {
333-
let root = make::ext::ident_path("core");
334-
let submodule = make::ext::ident_path("mem");
335-
let fn_name = make::ext::ident_path("discriminant");
336-
let fn_name = make::path_concat(submodule, fn_name);
337-
make::expr_path(make::path_concat(root, fn_name))
338-
}
339-
340321
fn gen_eq_chain(expr: Option<ast::Expr>, cmp: ast::Expr) -> Option<ast::Expr> {
341322
match expr {
342323
Some(expr) => Some(make::expr_op(ast::BinOp::BooleanAnd, expr, cmp)),
@@ -355,13 +336,6 @@ fn gen_partial_eq(adt: &ast::Adt, func: &ast::Fn) -> Option<()> {
355336
make::record_pat_with_fields(record_name, list)
356337
}
357338

358-
fn gen_variant_path(variant: &ast::Variant) -> Option<ast::Path> {
359-
let first = make::ext::ident_path("Self");
360-
let second = make::path_from_text(&variant.name()?.to_string());
361-
let record_name = make::path_concat(first, second);
362-
Some(record_name)
363-
}
364-
365339
fn gen_tuple_field(field_name: &String) -> ast::Pat {
366340
ast::Pat::IdentPat(make::ident_pat(false, false, make::name(field_name)))
367341
}
@@ -370,15 +344,15 @@ fn gen_partial_eq(adt: &ast::Adt, func: &ast::Fn) -> Option<()> {
370344
// generate this code `Self` for the time being.
371345

372346
let body = match adt {
373-
// `Hash` cannot be derived for unions, so no default impl can be provided.
347+
// `PartialEq` cannot be derived for unions, so no default impl can be provided.
374348
ast::Adt::Union(_) => return None,
375349

376350
ast::Adt::Enum(enum_) => {
377351
// => std::mem::discriminant(self) == std::mem::discriminant(other)
378-
let self_name = make::expr_path(make::ext::ident_path("self"));
379-
let lhs = make::expr_call(gen_discriminant(), make::arg_list(Some(self_name.clone())));
380-
let other_name = make::expr_path(make::ext::ident_path("other"));
381-
let rhs = make::expr_call(gen_discriminant(), make::arg_list(Some(other_name.clone())));
352+
let lhs_name = make::expr_path(make::ext::ident_path("self"));
353+
let lhs = make::expr_call(make_discriminant(), make::arg_list(Some(lhs_name.clone())));
354+
let rhs_name = make::expr_path(make::ext::ident_path("other"));
355+
let rhs = make::expr_call(make_discriminant(), make::arg_list(Some(rhs_name.clone())));
382356
let eq_check = make::expr_op(ast::BinOp::EqualityTest, lhs, rhs);
383357

384358
let mut case_count = 0;
@@ -407,8 +381,8 @@ fn gen_partial_eq(adt: &ast::Adt, func: &ast::Fn) -> Option<()> {
407381
expr = gen_eq_chain(expr, cmp);
408382
}
409383

410-
let left = gen_record_pat(gen_variant_path(&variant)?, l_fields);
411-
let right = gen_record_pat(gen_variant_path(&variant)?, r_fields);
384+
let left = gen_record_pat(make_variant_path(&variant)?, l_fields);
385+
let right = gen_record_pat(make_variant_path(&variant)?, r_fields);
412386
let tuple = make::tuple_pat(vec![left.into(), right.into()]);
413387

414388
if let Some(expr) = expr {
@@ -436,8 +410,8 @@ fn gen_partial_eq(adt: &ast::Adt, func: &ast::Fn) -> Option<()> {
436410
expr = gen_eq_chain(expr, cmp);
437411
}
438412

439-
let left = make::tuple_struct_pat(gen_variant_path(&variant)?, l_fields);
440-
let right = make::tuple_struct_pat(gen_variant_path(&variant)?, r_fields);
413+
let left = make::tuple_struct_pat(make_variant_path(&variant)?, l_fields);
414+
let right = make::tuple_struct_pat(make_variant_path(&variant)?, r_fields);
441415
let tuple = make::tuple_pat(vec![left.into(), right.into()]);
442416

443417
if let Some(expr) = expr {
@@ -456,7 +430,7 @@ fn gen_partial_eq(adt: &ast::Adt, func: &ast::Fn) -> Option<()> {
456430
arms.push(make::match_arm(Some(lhs), None, eq_check));
457431
}
458432

459-
let match_target = make::expr_tuple(vec![self_name, other_name]);
433+
let match_target = make::expr_tuple(vec![lhs_name, rhs_name]);
460434
let list = make::match_arm_list(arms).indent(ast::edit::IndentLevel(1));
461435
make::expr_match(match_target, list)
462436
}
@@ -492,7 +466,7 @@ fn gen_partial_eq(adt: &ast::Adt, func: &ast::Fn) -> Option<()> {
492466
make::block_expr(None, expr).indent(ast::edit::IndentLevel(1))
493467
}
494468

495-
// No fields in the body means there's nothing to hash.
469+
// No fields in the body means there's nothing to compare.
496470
None => {
497471
let expr = make::expr_literal("true").into();
498472
make::block_expr(None, Some(expr)).indent(ast::edit::IndentLevel(1))
@@ -503,3 +477,11 @@ fn gen_partial_eq(adt: &ast::Adt, func: &ast::Fn) -> Option<()> {
503477
ted::replace(func.body()?.syntax(), body.clone_for_update().syntax());
504478
Some(())
505479
}
480+
481+
fn make_discriminant() -> ast::Expr {
482+
make::expr_path(make::path_from_text("core::mem::discriminant"))
483+
}
484+
485+
fn make_variant_path(variant: &ast::Variant) -> Option<ast::Path> {
486+
Some(make::path_from_text(&format!("Self::{}", &variant.name()?)))
487+
}

0 commit comments

Comments
 (0)