Complete assoc. types on paths without a trait #4149
Conversation
| @@ -532,7 +532,10 @@ fn is_valid_candidate( | |||
| let data = db.const_data(c); | |||
| name.map_or(true, |name| data.name.as_ref() == Some(name)) && receiver_ty.is_none() | |||
| } | |||
| _ => false, | |||
| AssocItemId::TypeAliasId(t) => { | |||
There was a problem hiding this comment.
r? @flodiebold
From the look of it, the old code looks correct, as it only uses the values namespace. I would expect two similar fuctions, one for consttants & methods, and one for types. The caller can then select the appropriate namespace
There was a problem hiding this comment.
Yeah, method_resolution is for values only, and associated types resolution has different rules. (You can't just write S::Item if S is a struct implementing Iterator, the only situation where you can write T::AssociatedType where T is a type is when T is a type parameter. I thought you could do this as well before I actually implemented it!)
There was a problem hiding this comment.
Might still be a good idea to replace _ with explicit match & comment
| @r###" | ||
| [ | ||
| CompletionItem { | ||
| label: "Assoc", |
There was a problem hiding this comment.
... so this completion is actually wrong, rustc will complain about this.
There was a problem hiding this comment.
Oh great, not sure how I missed that :/
I think this is actually a bug in rustc, namely rust-lang/rust#22519. But yeah, rust-analyzer shouldn't suggest something that doesn't work.
There was a problem hiding this comment.
As far as I can see, that issue doesn't propose allowing foo::S::AssocTy. It's rather about the fact that even if T is a type parameter, <T>::AssocTy doesn't work.
(Edit: Ok, eddyb's last comment does refer to allowing this, but still it doesn't seem like this will change anytime soon.)
|
The code that resolves this associated type shorthand is currently here; the proper way to fix this would probably to extract that logic so it can be used to list all possible associated types, and then expose that through the |
There might be a reason for why this was not done before, so maybe this broke something subtle?