Skip to content
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

No completion for impl blocks on dyn traits #6777

Closed
Tracked by #86
MaikKlein opened this issue Dec 9, 2020 · 2 comments · Fixed by #7602
Closed
Tracked by #86

No completion for impl blocks on dyn traits #6777

MaikKlein opened this issue Dec 9, 2020 · 2 comments · Fixed by #7602
Labels
A-ty type system / type inference / traits / method resolution E-has-instructions Issue has some instructions and pointers to code to get started E-medium S-actionable Someone could pick this issue up and work on it right now

Comments

@MaikKlein
Copy link

trait Foo {
    fn foo_dyn(&self, s: &str);
}

impl dyn Foo {
    pub fn foo<T: AsRef<str>>(&self, s: T) {
        self.foo_dyn(s.as_ref())
    }
}

impl Foo for u32 {
    fn foo_dyn(&self, s: &str) {
        println!("{} {}", self, s);
    }
}

fn main() {

    let f: Box<dyn Foo> = Box::new(42);
    // No completion for `f.foo`
    f.foo("Hello");
}

There are no completions for functions inside an impl dyn Trait { } block. It is often nice to add generics on top of your trait objects.

Tested in vim and vscode

Language server version 0.2.408 for rust-analyzer
@lnicola lnicola added A-ty type system / type inference / traits / method resolution S-actionable Someone could pick this issue up and work on it right now and removed S-actionable Someone could pick this issue up and work on it right now labels Dec 9, 2020
@flodiebold
Copy link
Member

flodiebold commented Dec 9, 2020

I believe the problem may be in this code:

https://github.com/rust-analyzer/rust-analyzer/blob/ef989880fff36f10b7e166647497779bacc1c47f/crates/hir_ty/src/method_resolution.rs#L248-L276

which determines the crates in which inherent impls for a type may be found. Note that it currently doesn't handle Ty::Dyn at all; it probably needs to look at the first trait and take its definition crate.

We probably also need to add a variant to TyFingerprint:

https://github.com/rust-analyzer/rust-analyzer/blob/ef989880fff36f10b7e166647497779bacc1c47f/crates/hir_ty/src/method_resolution.rs#L30-L32

It would probably look something like Dyn(TraitId).

The TraitId can be retrieved using the dyn_trait method on Ty.

@flodiebold flodiebold added E-has-instructions Issue has some instructions and pointers to code to get started E-medium S-actionable Someone could pick this issue up and work on it right now labels Dec 9, 2020
@fisherdarling
Copy link
Contributor

I'll try and take this one on!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ty type system / type inference / traits / method resolution E-has-instructions Issue has some instructions and pointers to code to get started E-medium S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants