-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FIX: generic parameters handling by the generate_delegate_trait
assist
#15520
Conversation
d42e56d
to
001952d
Compare
@Veykril, hello! It seems natural to request your review here |
crates/hir-def/src/item_tree.rs
Outdated
#[derive(Debug, Clone, Eq, PartialEq, Hash)] | ||
pub struct GenericParamGroups { | ||
// pub target_trait: Option<Vec<String>>, | ||
pub self_ty: Option<Vec<String>>, | ||
} | ||
|
||
impl GenericParamGroups { | ||
fn from_impl(impl_: &ast::Impl) -> Self { | ||
Self { | ||
// target_trait: Self::get_arg_list(impl_.trait_()), | ||
self_ty: Self::get_arg_list(impl_.self_ty()), | ||
} | ||
} | ||
|
||
fn get_arg_list(ty: Option<ast::Type>) -> Option<Vec<String>> { | ||
let list = match ty? { | ||
ast::Type::PathType(p) => p.path()?.segment()?.generic_arg_list(), | ||
_ => None, | ||
}; | ||
Some(list?.generic_args().map(|arg| arg.syntax().text().to_string()).collect()) | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the wrong way too approach this. The fix to the issue here should not involve touching item tree whatsoever.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have to touch it, I'm afraid.
Let's have a look on few relevant lines from the test:
struct S(Foo<i32, bool>);
...
impl<A, B, C> Trait<A, B> for Foo<C, B> { ... }
In this case the assist should turn C
and B
into i32
and bool
repectively. In general, it should get Foo
-related generic arguments from the trait implementation and correlate them Foo
's parameters within the structure. Consequently, we have to memorize the "distribution" of the generic arguments in impl
lines; that's exactly what happens in this file - the names of args pertaining to self_ty
are being saved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That data is already encoded in the sekf_ty
TypeRef
in the item tree. So you just need to fetch that from there instead, that is check if the TypeRef
is a TypeRef::Path
and take the generic args from that path instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Now these data are fetched from Type
instead of TypeRef
, but anyway the item tree is unchanged
☔ The latest upstream changes (presumably #15891) made this pull request unmergeable. Please resolve the merge conflicts. |
The linked issue has been fixed by a heavy rewrite of the assist in #16112, so sorry for not merging this now, but thanks anyways for your contribution! |
A part of #15108
I'm going to write a separate PR to solve the problem completely; currently only
impl
bofy generation is fixed, but generation of theimpl ...
line itself is to be overhauled.The code might look somewhat complicated, but basically the changes are as follows:
item_tree.rs
);generate_delegate_trait.rs
)