diff options
| author | bors <bors@rust-lang.org> | 2022-07-24 12:46:08 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2022-07-24 12:46:08 +0000 |
| commit | 894d6a232c1cf69f7050a33223f687c4623e4d16 (patch) | |
| tree | 147a43e3e764c162139c42bd4935ad819754165c | |
| parent | 977e12a0bdc3e329af179ef3a9d466af9eb613bb (diff) | |
| parent | 7e0bd377c2ffe55a1f014ce932a7218a9b4dc639 (diff) | |
| download | rust-894d6a232c1cf69f7050a33223f687c4623e4d16.tar.gz rust-894d6a232c1cf69f7050a33223f687c4623e4d16.zip | |
Auto merge of #12832 - lowr:fix/impl-default-members-no-codegen, r=Veykril
fix: don't replace default members' body cc #12779, #12821 addresses https://github.com/rust-lang/rust-analyzer/pull/12821#issuecomment-1190157506 `gen_trait_fn_body()` only attempts to implement required trait member functions, so we shouldn't call it for `Implement default members` assist. This patch also documents the precondition of `gen_trait_fn_body()` and inserts `debug_assert!`, but I'm not entirely sure if the assertions are appropriate.
| -rw-r--r-- | crates/ide-assists/src/handlers/add_missing_impl_members.rs | 17 | ||||
| -rw-r--r-- | crates/ide-assists/src/utils/gen_trait_fn_body.rs | 11 |
2 files changed, 16 insertions, 12 deletions
diff --git a/crates/ide-assists/src/handlers/add_missing_impl_members.rs b/crates/ide-assists/src/handlers/add_missing_impl_members.rs index 7f2a26ad067..c808c010c67 100644 --- a/crates/ide-assists/src/handlers/add_missing_impl_members.rs +++ b/crates/ide-assists/src/handlers/add_missing_impl_members.rs @@ -145,13 +145,16 @@ fn add_missing_impl_members_inner( Some(cap) => { let mut cursor = Cursor::Before(first_new_item.syntax()); let placeholder; - if let ast::AssocItem::Fn(func) = &first_new_item { - if try_gen_trait_body(ctx, func, &trait_, &impl_def).is_none() { - if let Some(m) = func.syntax().descendants().find_map(ast::MacroCall::cast) - { - if m.syntax().text() == "todo!()" { - placeholder = m; - cursor = Cursor::Replace(placeholder.syntax()); + if let DefaultMethods::No = mode { + if let ast::AssocItem::Fn(func) = &first_new_item { + if try_gen_trait_body(ctx, func, &trait_, &impl_def).is_none() { + if let Some(m) = + func.syntax().descendants().find_map(ast::MacroCall::cast) + { + if m.syntax().text() == "todo!()" { + placeholder = m; + cursor = Cursor::Replace(placeholder.syntax()); + } } } } diff --git a/crates/ide-assists/src/utils/gen_trait_fn_body.rs b/crates/ide-assists/src/utils/gen_trait_fn_body.rs index ec4835969f8..43aaad3150c 100644 --- a/crates/ide-assists/src/utils/gen_trait_fn_body.rs +++ b/crates/ide-assists/src/utils/gen_trait_fn_body.rs @@ -5,7 +5,7 @@ use syntax::{ ted, }; -/// Generate custom trait bodies where possible. +/// Generate custom trait bodies without default implementation where possible. /// /// Returns `Option` so that we can use `?` rather than `if let Some`. Returning /// `None` means that generating a custom trait body failed, and the body will remain @@ -28,6 +28,7 @@ pub(crate) fn gen_trait_fn_body( /// Generate a `Clone` impl based on the fields and members of the target type. fn gen_clone_impl(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { + debug_assert!(func.name().map_or(false, |name| name.text() == "clone")); fn gen_clone_call(target: ast::Expr) -> ast::Expr { let method = make::name_ref("clone"); make::expr_method_call(target, method, make::arg_list(None)) @@ -339,6 +340,7 @@ fn gen_default_impl(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { /// Generate a `Hash` impl based on the fields and members of the target type. fn gen_hash_impl(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { + debug_assert!(func.name().map_or(false, |name| name.text() == "hash")); fn gen_hash_call(target: ast::Expr) -> ast::Stmt { let method = make::name_ref("hash"); let arg = make::expr_path(make::ext::ident_path("state")); @@ -394,9 +396,7 @@ fn gen_hash_impl(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { /// Generate a `PartialEq` impl based on the fields and members of the target type. fn gen_partial_eq(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { - if func.name().map_or(false, |name| name.text() == "ne") { - return None; - } + debug_assert!(func.name().map_or(false, |name| name.text() == "eq")); fn gen_eq_chain(expr: Option<ast::Expr>, cmp: ast::Expr) -> Option<ast::Expr> { match expr { Some(expr) => Some(make::expr_bin_op(expr, BinaryOp::LogicOp(LogicOp::And), cmp)), @@ -573,6 +573,7 @@ fn gen_partial_eq(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { } fn gen_partial_ord(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { + debug_assert!(func.name().map_or(false, |name| name.text() == "partial_cmp")); fn gen_partial_eq_match(match_target: ast::Expr) -> Option<ast::Stmt> { let mut arms = vec![]; @@ -643,7 +644,7 @@ fn gen_partial_ord(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { make::block_expr(stmts.into_iter(), tail).indent(ast::edit::IndentLevel(1)) } - // No fields in the body means there's nothing to hash. + // No fields in the body means there's nothing to compare. None => { let expr = make::expr_literal("true").into(); make::block_expr(None, Some(expr)).indent(ast::edit::IndentLevel(1)) |
