Magic Completion for impl Trait for Associated Items

[kdelorey]

Summary

This PR adds a set of magic completions to auto complete associated trait items (functions/consts/types).

Notes

Since the assist and completion share the same logic when figuring out the associated items that are missing, a shared utility was created in the ra_assists::utils module.

Resolves #1046

As this is my first PR to the rust-analyzer project, I'm new to the codebase, feedback welcomed!

[kiljacken]

This an amazing addition, thanks so much for working on it!

[matklad]

This looks good to me!

I think the test fail primaraly due to rustfmt? You can run cargo xtask format to format things, or cargo xtask install pre-commit-hook to install the hook.

I think there's a bunch of things we can further tweak here, but it's also good to land as is.

[kdelorey]

I think the test fail primaraly due to rustfmt? You can run cargo xtask format to format things, or cargo xtask install pre-commit-hook to install the hook.

Ah, there was formatting issues. I was sure I had the pre-commit-hook on my environment; I'll have to check that when I get home.

I think there's a bunch of things we can further tweak here, but it's also good to land as is.

I'm happy to further refine this if you had thoughts. I can also refine it in a separate PR if your intention was to accept this as is.

[kdelorey]

Whoops, spoke too soon, test still failed. Let me try removing the doc comments as it seems to be bringing up docs::no_docs_comments.

[kdelorey]

Oh, @matklad, maybe the CI is complaining about needing module level documentation? That would make sense 🤔

Edit: Yup that was it 🤦‍♂

[matklad]

I'd lean towards checking this in this specific completion, and not in the general completion context. But it doesn't matter that much, moving between the two should be easy.

…

On Tue, 11 Feb 2020 at 17:15, Kevin DeLorey ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In crates/ra_ide/src/completion/complete_trait_impl.rs <#3108 (comment)> : > +/// ```ignore +/// trait SomeTrait { +/// const SOME_CONST: u16; +/// } +/// +/// impl SomeTrait for () { +/// const <|> +/// } +/// ``` +pub(crate) fn complete_trait_impl(acc: &mut Completions, ctx: &CompletionContext) { + + // it is possible to have a parent `fn` and `impl` block. Ignore completion + // attempts from within a `fn` block. + if ctx.function_syntax.is_some() { + return; + } Ah, makes sense. Would there any need for the CompletionContext to provide the impl_block as this behavior seems pretty specific to this completion type? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3108>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AANB3M7GX6LTCFMMTEJ4SUTRCLFILANCNFSM4KTAQCCA> .

[kdelorey]

I agree, when I get home from my day job I'll work on that change. Thanks for taking a look.

[kdelorey]

@matklad one thing I've discovered while working on the suggested change was the CONST_DEF syntax node is made a little differently than its counterparts (fn/type). Consder the following:

impl SomeTrait for () {
   fn<|>
}

will produce something like the following syntax tree:

ITEM_LIST@[85; 95)
  L_CURLY@[85; 86) "{"
  WHITESPACE@[86; 91) "\n    "
  FN_DEF@[91; 93)  <-- incomplete `fn` syntrax makes `FN_DEF`
    FN_KW@[91; 93) "fn"
  WHITESPACE@[93; 94) "\n"
  R_CURLY@[94; 95) "}"

But with a const it does something a little different:

impl SomeTrait for () {
   const<|>
}

will produce something like the following

ITEM_LIST@[85; 98)
  L_CURLY@[85; 86) "{"
  WHITESPACE@[86; 91) "\n    "
  ERROR@[91; 96)   <-- Expected `CONST_DEF`
    CONST_KW@[91; 96) "const"
  WHITESPACE@[96; 97) "\n"
  R_CURLY@[97; 98) "}"

It won't generate a CONST_DEF node until a user starts typing a IDENT. Anyways, with my limited experince, I wasn't sure if I stumbled upon a bug or something intentional? It currently makes the handling of the const case a little different than fn and type associated items.

Edit: I pushed my WIP changes so you can see how I started going about your suggestion.

[matklad]

Anyways, with my limited experince, I wasn't sure if I stumbled upon a bug or something intentional

That's semi-intentional, in that const is not necessary a const FOO, it could be a const fn as well, so we can't relibely decide, by just seening a const, what's the end result would be.

[matklad]

bors r+

Thanks!

[bors]

Build succeeded

• Rust (macos-latest)
• Rust (ubuntu-latest)
• Rust (windows-latest)
• TypeScript

[matklad]

Opened a sequel in #3183

[kdelorey]

@matklad, thanks for working with me on this. It was pretty rewarding to open reddit this morning and see a gif of my contribution 🎉