Auto merge of #16112 - roife:rewrite-generate-delete-trait, r=Veykril

fix: rewrite code_action `generate_delegate_trait`

I've made substantial enhancements to the "generate delegate trait" code action in rust-analyzer. Here's a summary of the changes:

#### Resolved the "Can’t find CONST_ARG@158..159 in AstIdMap" error

Fix #15804, fix #15968, fix #15108

The issue stemmed from an incorrect application of PathTransform in the original code. Previously, a new 'impl' was generated first and then transformed, causing PathTransform to fail in locating the correct AST node, resulting in an error. I rectified this by performing the transformation before generating the new 'impl' (using make::impl_trait), ensuring a step-by-step transformation of associated items.

#### Rectified generation of `Self` type

`generate_delegate_trait` is unable to properly handle trait with `Self` type.

Let's take the following code as an example:

```rust
trait Trait {
    fn f() -> Self;
}

struct B {}
impl Trait for B {
    fn f() -> B { B{} }
}

struct S {
    b: B,
}
```

Here, if we implement `Trait` for `S`, the type of `f` should be `() -> Self`, i.e. `() -> S`. However we cannot automatically generate a function that constructs `S`.

To ensure that the code action doesn't generate delegate traits for traits with Self types, I add a function named `has_self_type` to handle it.

#### Extended support for generics in structs and fields within this code action

The former version of `generate_delegate_trait` cannot handle structs with generics properly. Here's an example:

```rust
struct B<T> {
    a: T
}

trait Trait<T> {
    fn f(a: T);
}

impl<T1, T2> Trait<T1> for B<T2> {
    fn f(a: T1) -> T2 { self.a }
}

struct A {}
struct S {
    b$0 : B<A>,
}
```

The former version  will generates improper code:

```rust
impl<T1, T2> Trait<T1, T2> for S {
    fn f(&self, a: T1) -> T1 {
        <B as Trait<T1, T2>>::f( &self.b , a)
    }
}
```

The rewritten version can handle generics properly:

```rust
impl<T1> Trait<T1> for S {
    fn f(&self, a: T1) -> T1 {
        <B<A> as Trait<T1>>::f(&self.b, a)
    }
}
```

See more examples in added unit tests.

I enabled support for generic structs in `generate_delegate_trait` through the following steps (using the code example provided):

1. Initially, to prevent conflicts between the generic parameters in struct `S` and the ones in the impl of `B`, I renamed the generic parameters of `S`.
2. Then, since `B`'s parameters are instantiated within `S`, the original generic parameters of `B` needed removal within `S` (to avoid errors from redundant parameters). An important consideration here arises when Trait and B share parameters in `B`'s impl. In such cases, these shared generic parameters cannot be removed.
3. Next, I addressed the matching of types between `B`'s type in `S` and its type in the impl. Given that some generic parameters in the impl are instantiated in `B`, I replaced these parameters with their instantiated results using PathTransform. For instance, in the example provided, matching `B<A>` and `B<T2>`, where `T2` is instantiated as `A`, I replaced all occurrences of `T2` in the impl with `A` (i.e. apply the instantiated generic arguments to the params).
4. Finally, I performed transformations on each assoc item (also to prevent the initial issue) and handled redundant where clauses.

For a more detailed explanation, please refer to the code and comments. I welcome suggestions and any further questions!
This commit is contained in:
bors 2024-01-02 12:30:19 +00:00
commit 34df29620a
8 changed files with 993 additions and 158 deletions

View file

@ -13,7 +13,7 @@ use crate::{
SyntaxNode, SyntaxToken,
};
use super::{HasArgList, HasName};
use super::{GenericParam, HasArgList, HasName};
pub trait GenericParamsOwnerEdit: ast::HasGenericParams {
fn get_or_create_generic_param_list(&self) -> ast::GenericParamList;
@ -272,6 +272,36 @@ impl ast::GenericParamList {
}
}
/// Find the params corresponded to generic arg
pub fn find_generic_arg(&self, generic_arg: &ast::GenericArg) -> Option<GenericParam> {
self.generic_params().find_map(move |param| match (&param, &generic_arg) {
(ast::GenericParam::LifetimeParam(a), ast::GenericArg::LifetimeArg(b)) => {
(a.lifetime()?.lifetime_ident_token()?.text()
== b.lifetime()?.lifetime_ident_token()?.text())
.then_some(param)
}
(ast::GenericParam::TypeParam(a), ast::GenericArg::TypeArg(b)) => {
debug_assert_eq!(b.syntax().first_token(), b.syntax().last_token());
(a.name()?.text() == b.syntax().first_token()?.text()).then_some(param)
}
(ast::GenericParam::ConstParam(a), ast::GenericArg::TypeArg(b)) => {
debug_assert_eq!(b.syntax().first_token(), b.syntax().last_token());
(a.name()?.text() == b.syntax().first_token()?.text()).then_some(param)
}
_ => None,
})
}
/// Removes the corresponding generic arg
pub fn remove_generic_arg(&self, generic_arg: &ast::GenericArg) -> Option<GenericParam> {
let param_to_remove = self.find_generic_arg(generic_arg);
if let Some(param) = &param_to_remove {
self.remove_generic_param(param.clone());
}
param_to_remove
}
/// Constructs a matching [`ast::GenericArgList`]
pub fn to_generic_args(&self) -> ast::GenericArgList {
let args = self.generic_params().filter_map(|param| match param {
@ -300,6 +330,20 @@ impl ast::WhereClause {
}
ted::append_child(self.syntax(), predicate.syntax());
}
pub fn remove_predicate(&self, predicate: ast::WherePred) {
if let Some(previous) = predicate.syntax().prev_sibling() {
if let Some(next_token) = previous.next_sibling_or_token() {
ted::remove_all(next_token..=predicate.syntax().clone().into());
}
} else if let Some(next) = predicate.syntax().next_sibling() {
if let Some(next_token) = next.prev_sibling_or_token() {
ted::remove_all(predicate.syntax().clone().into()..=next_token);
}
} else {
ted::remove(predicate.syntax());
}
}
}
impl ast::TypeParam {

View file

@ -207,10 +207,28 @@ fn merge_gen_params(
(None, Some(bs)) => Some(bs),
(Some(ps), None) => Some(ps),
(Some(ps), Some(bs)) => {
for b in bs.generic_params() {
ps.add_generic_param(b);
}
Some(ps)
// make sure lifetime is placed before other generic params
let generic_params = ps.generic_params().merge_by(bs.generic_params(), |_, b| {
!matches!(b, ast::GenericParam::LifetimeParam(_))
});
Some(generic_param_list(generic_params))
}
}
}
fn merge_where_clause(
ps: Option<ast::WhereClause>,
bs: Option<ast::WhereClause>,
) -> Option<ast::WhereClause> {
match (ps, bs) {
(None, None) => None,
(None, Some(bs)) => Some(bs),
(Some(ps), None) => Some(ps),
(Some(ps), Some(bs)) => {
let preds = where_clause(std::iter::empty()).clone_for_update();
ps.predicates().for_each(|p| preds.add_predicate(p));
bs.predicates().for_each(|p| preds.add_predicate(p));
Some(preds)
}
}
}
@ -251,9 +269,9 @@ pub fn impl_(
pub fn impl_trait(
is_unsafe: bool,
trait_gen_params: Option<ast::GenericParamList>,
trait_gen_args: Option<ast::GenericParamList>,
trait_gen_args: Option<ast::GenericArgList>,
type_gen_params: Option<ast::GenericParamList>,
type_gen_args: Option<ast::GenericParamList>,
type_gen_args: Option<ast::GenericArgList>,
is_negative: bool,
path_type: ast::Type,
ty: ast::Type,
@ -262,15 +280,9 @@ pub fn impl_trait(
body: Option<Vec<either::Either<ast::Attr, ast::AssocItem>>>,
) -> ast::Impl {
let is_unsafe = if is_unsafe { "unsafe " } else { "" };
let ty_gen_args = match merge_gen_params(type_gen_params.clone(), type_gen_args) {
Some(pars) => pars.to_generic_args().to_string(),
None => String::new(),
};
let tr_gen_args = match merge_gen_params(trait_gen_params.clone(), trait_gen_args) {
Some(pars) => pars.to_generic_args().to_string(),
None => String::new(),
};
let trait_gen_args = trait_gen_args.map(|args| args.to_string()).unwrap_or_default();
let type_gen_args = type_gen_args.map(|args| args.to_string()).unwrap_or_default();
let gen_params = match merge_gen_params(trait_gen_params, type_gen_params) {
Some(pars) => pars.to_string(),
@ -279,25 +291,15 @@ pub fn impl_trait(
let is_negative = if is_negative { "! " } else { "" };
let where_clause = match (ty_where_clause, trait_where_clause) {
(None, None) => " ".to_string(),
(None, Some(tr)) => format!("\n{}\n", tr).to_string(),
(Some(ty), None) => format!("\n{}\n", ty).to_string(),
(Some(ty), Some(tr)) => {
let updated = ty.clone_for_update();
tr.predicates().for_each(|p| {
ty.add_predicate(p);
});
format!("\n{}\n", updated).to_string()
}
};
let where_clause = merge_where_clause(ty_where_clause, trait_where_clause)
.map_or_else(|| " ".to_string(), |wc| format!("\n{}\n", wc));
let body = match body {
Some(bd) => bd.iter().map(|elem| elem.to_string()).join(""),
None => String::new(),
};
ast_from_text(&format!("{is_unsafe}impl{gen_params} {is_negative}{path_type}{tr_gen_args} for {ty}{ty_gen_args}{where_clause}{{{}}}" , body))
ast_from_text(&format!("{is_unsafe}impl{gen_params} {is_negative}{path_type}{trait_gen_args} for {ty}{type_gen_args}{where_clause}{{{}}}" , body))
}
pub fn impl_trait_type(bounds: ast::TypeBoundList) -> ast::ImplTraitType {
@ -922,6 +924,10 @@ pub fn type_param(name: ast::Name, bounds: Option<ast::TypeBoundList>) -> ast::T
ast_from_text(&format!("fn f<{name}{bounds}>() {{ }}"))
}
pub fn const_param(name: ast::Name, ty: ast::Type) -> ast::ConstParam {
ast_from_text(&format!("fn f<const {name}: {ty}>() {{ }}"))
}
pub fn lifetime_param(lifetime: ast::Lifetime) -> ast::LifetimeParam {
ast_from_text(&format!("fn f<{lifetime}>() {{ }}"))
}
@ -948,9 +954,7 @@ pub fn turbofish_generic_arg_list(
ast_from_text(&format!("const S: T::<{args}> = ();"))
}
pub(crate) fn generic_arg_list(
args: impl IntoIterator<Item = ast::GenericArg>,
) -> ast::GenericArgList {
pub fn generic_arg_list(args: impl IntoIterator<Item = ast::GenericArg>) -> ast::GenericArgList {
let args = args.into_iter().join(", ");
ast_from_text(&format!("const S: T<{args}> = ();"))
}

View file

@ -617,6 +617,16 @@ impl ast::Item {
}
}
impl ast::Type {
pub fn generic_arg_list(&self) -> Option<ast::GenericArgList> {
if let ast::Type::PathType(path_type) = self {
path_type.path()?.segment()?.generic_arg_list()
} else {
None
}
}
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum FieldKind {
Name(ast::NameRef),